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.24k stars 1.51k forks source link

skip needless_lifetimes inside macros (needs test!) #5283

Closed matthiaskrgr closed 1 year ago

matthiaskrgr commented 4 years ago

I came across this example

warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration)
    --> src/librustc/ty/query/plumbing.rs:1156:19
     |
817  | /        macro_rules! define_queries {
818  | |            (<$tcx:tt> $($category:tt {
819  | |                $($(#[$attr:meta])* [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*
820  | |            },)*) => {
821  | /                define_queries_inner! { <$tcx>
822  |                      $($( $(#[$attr])* category<$category> [$($modifiers)*] fn $name: $node($K) -> $V,)*)*
823  | |                }
     | |________________- in this macro invocation (#3)
824  | |            }
825  | |        }
     | |________- in this expansion of `define_queries!` (#2)
826  | 
827  | /        macro_rules! define_queries_inner {
828  | |            (<$tcx:tt>
829  | |             $($(#[$attr:meta])* category<$category:tt>
830  | |                [$($modifiers:tt)*] fn $name:ident: $node:ident($K:ty) -> $V:ty,)*) => {
...    |
1118 | /                define_provider_struct! {
1119 |                      tcx: $tcx,
1120 |                      input: ($(([$($modifiers)*] [$name] [$K] [$V]))*)
1121 | |                }
     | |________________- in this macro invocation (#4)
...
1127 | |            }
1128 | |        }
     | |________- in this expansion of `define_queries_inner!` (#3)
...
1147 | /        macro_rules! define_provider_struct {
1148 | |            (tcx: $tcx:tt,
1149 | |             input: ($(([$($modifiers:tt)*] [$name:ident] [$K:ty] [$R:ty]))*)) => {
1150 | |                pub struct Providers<$tcx> {
...    |
1156 |                          $(fn $name<$tcx>(_: TyCtxt<$tcx>, key: $K) -> $R {
     |  __________________________^
1157 |                              bug!("`tcx.{}({:?})` unsupported by its crate",
1158 |                                   stringify!($name), key);
1159 | |                        })*
     | |________________________^
...
1163 | |            };
1164 | |        }
     | |________- in this expansion of `define_provider_struct!` (#4)
     | 
    ::: src/librustc/ty/query/mod.rs:107:1
     |
107  | /        rustc_query_append! { [define_queries!][ <'tcx>
108  | |            Other {
109  | |                /// Runs analysis passes on the crate.
110  | |                [eval_always] fn analysis: Analysis(CrateNum) -> Result<(), ErrorReported>,
111  | |            },
112  | |        ]}
     | |_________- in this macro invocation (#1)
     | 
    ::: src/librustc/query/mod.rs:38:1
     |
38   |        / rustc_queries! {
39   |        |     Other {
40   |        |         query trigger_delay_span_bug(key: DefId) -> () {
41   |        |             desc { "trigger a delay span bug" }
...           |
1254 |        |     }
1255 |        | }
     |        | -
     |        | |
     |        |_in this expansion of `rustc_query_append!` (#1)
     |          in this macro invocation (#2)
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes

and I'm wondering if the lint should skip macros.

matthiaskrgr commented 4 years ago

Hmm, actually the lint should be skipped in external macros but not in local ones...?

    if in_external_macro(cx.sess(), span) || has_where_lifetimes(cx, &generics.where_clause) {
        return;
    }

https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/lifetimes.rs#L129

matthiaskrgr commented 4 years ago

Reopened as this still needs a testcase (see https://github.com/rust-lang/rust-clippy/pull/5293#issuecomment-606089488)

tylerjw commented 1 year ago

@rustbot claim

@matthiaskrgr I created a PR to add a test for this. I see your comment above about external macros, but I need to figure out how to test that. Do you mean a macro defined in a different crate?

Similarly, I have this PR, which could use something defined in a separate crate for testing, and I am unsure how to achieve that. Are there any example tests in Clippy that define symbols in external crates?

tylerjw commented 1 year ago

@matthiaskrgr I updated the PR to lint local macros and test for local and external macro expansion with needless lifetimes.