search

rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.27k stars 1.52k forks source link

Custom `dbg!` macros for `dbg_macro` lint #11303

Open ojeda opened 1 year ago

ojeda commented 1 year ago

Back in Rust 1.60.0 we added -Wclippy::dbg_macro to the Linux kernel compilation flags, which worked great with our custom dbg! macro (essentially the std one but calling the kernel printing facilities).

However, in the next version (1.61.0), as well as the current stable (1.80.0) and the current nightly (1.83.0-nightly (bd53aa3bf 2024-09-02)), it does not work. Thus only the first of the following lines emit a warning:

echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.60.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.61.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run  1.80.0 clippy-driver -Wclippy::dbg_macro -
echo 'macro_rules! dbg { () => {} } fn main() { dbg!(); }' | rustup run nightly clippy-driver -Wclippy::dbg_macro -

Is the lint intended to work only with the standard library dbg macro? I imagine that is the case, given the move to diagnostic items in https://github.com/rust-lang/rust-clippy/pull/7466, but it is not entirely clear from the lint description.

A workaround is to use disallowed_macros, but that does not have a specialized diagnostic message nor has the nice help: suggestion. Another is to use rustc_attrs, but I imagine that is not going to be stable.

Thus, instead, could the dbg_macro lint take a configuration with paths to dbg! macros (like disallowed_macros) or, even better, could there be an attribute that users could apply to their dbg! macro (like rustc_diagnostic_item)?

Cc @xFrednet and @Alexendoo who both seemed to work on this in the past.

Centri3 commented 1 year ago

I'd assume it was this PR which changed it from a simple path check to a diagnostic item: #8411

The lint's purpose is only the dbg! macro, nothing more. Instead of making it work on arbitrary macros, we should probably enhance the diagnostics from disallowed_macros. Maybe we can add an option for "this macro invocation just repeats its input, so suggest to remove it entirely"? As that's what dbg! does already. We can probably also add an option to not show the "use of a disallowed macro" part and allow the user to set the message arbitrarily.

ojeda commented 1 year ago

If one gets that level of customization, then it would be fine. Though if we wanted it to be as flexible, we would also need to be able to customize the lint level per path (which may be desirable anyway -- filled https://github.com/rust-lang/rust-clippy/issues/11307), since right now a project may put a different level to dbg_macro vs. disallowed_macros.