Auto merge of rust-lang#13599 - RuairidhWilliamson:proc_macro_attr, r=blyxyas

Fix allow_attributes when expanded from some macros

fixes rust-lang#13349

The issue here was that the start pattern being matched on the original source code was not specific enough. When using derive macros or in the issue case a `#[repr(C)]` the `#` would match the start pattern meaning that the expanded macro appeared to be unchanged and clippy would lint it.

The change I made was to make the matching more specific by matching `#[ident` at the start. We still need the second string to match just the ident on its own because of things like `#[cfg_attr(panic = "unwind", allow(unused))]`.

I also noticed some typos with start and end, these code paths weren't being reached so this doesn't fix anything.

changelog: FP: [`allow_attributes`]: don't trigger when expanded from some macros

Author: bors

clippy_utils/src/check_proc_macro.rs

@@ -63,8 +63,8 @@ fn span_matches_pat(sess: &Session, span: Span, start_pat: Pat, end_pat: Pat) ->
             Pat::Num => start_str.as_bytes().first().map_or(false, u8::is_ascii_digit),
         } && match end_pat {
             Pat::Str(text) => end_str.ends_with(text),
-            Pat::MultiStr(texts) => texts.iter().any(|s| start_str.ends_with(s)),
-            Pat::OwnedMultiStr(texts) => texts.iter().any(|s| start_str.starts_with(s)),
+            Pat::MultiStr(texts) => texts.iter().any(|s| end_str.ends_with(s)),
+            Pat::OwnedMultiStr(texts) => texts.iter().any(|s| end_str.ends_with(s)),
             Pat::Sym(sym) => end_str.ends_with(sym.as_str()),
             Pat::Num => end_str.as_bytes().last().map_or(false, u8::is_ascii_hexdigit),
         })
@@ -333,26 +333,32 @@ fn attr_search_pat(attr: &Attribute) -> (Pat, Pat) {
     match attr.kind {
         AttrKind::Normal(..) => {
             if let Some(ident) = attr.ident() {
-                // TODO: I feel like it's likely we can use `Cow` instead but this will require quite a bit of
-                // refactoring
                 // NOTE: This will likely have false positives, like `allow = 1`
-                (
-                    Pat::OwnedMultiStr(vec![ident.to_string(), "#".to_owned()]),
-                    Pat::Str(""),
-                )
+                let ident_string = ident.to_string();
+                if attr.style == AttrStyle::Outer {
+                    (
+                        Pat::OwnedMultiStr(vec!["#[".to_owned() + &ident_string, ident_string]),
+                        Pat::Str(""),
+                    )
+                } else {
+                    (
+                        Pat::OwnedMultiStr(vec!["#![".to_owned() + &ident_string, ident_string]),
+                        Pat::Str(""),
+                    )
+                }
             } else {
                 (Pat::Str("#"), Pat::Str("]"))
             }
         },
         AttrKind::DocComment(_kind @ CommentKind::Line, ..) => {
-            if matches!(attr.style, AttrStyle::Outer) {
+            if attr.style == AttrStyle::Outer {
                 (Pat::Str("///"), Pat::Str(""))
             } else {
                 (Pat::Str("//!"), Pat::Str(""))
             }
         },
         AttrKind::DocComment(_kind @ CommentKind::Block, ..) => {
-            if matches!(attr.style, AttrStyle::Outer) {
+            if attr.style == AttrStyle::Outer {
                 (Pat::Str("/**"), Pat::Str("*/"))
             } else {
                 (Pat::Str("/*!"), Pat::Str("*/"))

tests/ui/allow_attributes.fixed

@@ -1,4 +1,5 @@
 //@aux-build:proc_macros.rs
+//@aux-build:proc_macro_derive.rs
 #![allow(unused)]
 #![warn(clippy::allow_attributes)]
 #![no_main]
@@ -65,3 +66,9 @@ fn deny_allow_attributes() -> Option<u8> {
     allow?;
     Some(42)
 }
+
+// Edge case where the generated tokens spans match on #[repr(transparent)] which tricks the proc
+// macro check
+#[repr(transparent)]
+#[derive(proc_macro_derive::AllowLintSameSpan)] // This macro generates tokens with the same span as the whole struct and repr
+struct IgnoreDerived;

tests/ui/allow_attributes.rs

@@ -1,4 +1,5 @@
 //@aux-build:proc_macros.rs
+//@aux-build:proc_macro_derive.rs
 #![allow(unused)]
 #![warn(clippy::allow_attributes)]
 #![no_main]
@@ -65,3 +66,9 @@ fn deny_allow_attributes() -> Option<u8> {
     allow?;
     Some(42)
 }
+
+// Edge case where the generated tokens spans match on #[repr(transparent)] which tricks the proc
+// macro check
+#[repr(transparent)]
+#[derive(proc_macro_derive::AllowLintSameSpan)] // This macro generates tokens with the same span as the whole struct and repr
+struct IgnoreDerived;

tests/ui/allow_attributes.stderr

@@ -1,5 +1,5 @@
 error: #[allow] attribute found
-  --> tests/ui/allow_attributes.rs:12:3
+  --> tests/ui/allow_attributes.rs:13:3
    |
 LL | #[allow(dead_code)]
    |   ^^^^^ help: replace it with: `expect`
@@ -8,19 +8,19 @@ LL | #[allow(dead_code)]
    = help: to override `-D warnings` add `#[allow(clippy::allow_attributes)]`
 
 error: #[allow] attribute found
-  --> tests/ui/allow_attributes.rs:21:30
+  --> tests/ui/allow_attributes.rs:22:30
    |
 LL | #[cfg_attr(panic = "unwind", allow(dead_code))]
    |                              ^^^^^ help: replace it with: `expect`
 
 error: #[allow] attribute found
-  --> tests/ui/allow_attributes.rs:52:7
+  --> tests/ui/allow_attributes.rs:53:7
    |
 LL |     #[allow(unused)]
    |       ^^^^^ help: replace it with: `expect`
 
 error: #[allow] attribute found
-  --> tests/ui/allow_attributes.rs:52:7
+  --> tests/ui/allow_attributes.rs:53:7
    |
 LL |     #[allow(unused)]
    |       ^^^^^ help: replace it with: `expect`

tests/ui/auxiliary/proc_macro_derive.rs

@@ -1,4 +1,4 @@
-#![feature(repr128, proc_macro_quote)]
+#![feature(repr128, proc_macro_quote, proc_macro_span)]
 #![allow(incomplete_features)]
 #![allow(clippy::field_reassign_with_default)]
 #![allow(clippy::eq_op)]
@@ -182,3 +182,51 @@ pub fn non_canonical_clone_derive(_: TokenStream) -> TokenStream {
         impl Copy for NonCanonicalClone {}
     }
 }
+
+// Derive macro that generates the following but where all generated spans are set to the entire
+// input span.
+//
+// ```
+// #[allow(clippy::missing_const_for_fn)]
+// fn check() {}
+// ```
+#[proc_macro_derive(AllowLintSameSpan)]
+pub fn allow_lint_same_span_derive(input: TokenStream) -> TokenStream {
+    let mut iter = input.into_iter();
+    let first = iter.next().unwrap();
+    let last = iter.last().unwrap();
+    let span = first.span().join(last.span()).unwrap();
+    let span_help = |mut t: TokenTree| -> TokenTree {
+        t.set_span(span);
+        t
+    };
+    // Generate the TokenStream but setting all the spans to the entire input span
+    <TokenStream as FromIterator<TokenTree>>::from_iter([
+        span_help(Punct::new('#', Spacing::Alone).into()),
+        span_help(
+            Group::new(
+                Delimiter::Bracket,
+                <TokenStream as FromIterator<TokenTree>>::from_iter([
+                    Ident::new("allow", span).into(),
+                    span_help(
+                        Group::new(
+                            Delimiter::Parenthesis,
+                            <TokenStream as FromIterator<TokenTree>>::from_iter([
+                                Ident::new("clippy", span).into(),
+                                span_help(Punct::new(':', Spacing::Joint).into()),
+                                span_help(Punct::new(':', Spacing::Alone).into()),
+                                Ident::new("missing_const_for_fn", span).into(),
+                            ]),
+                        )
+                        .into(),
+                    ),
+                ]),
+            )
+            .into(),
+        ),
+        Ident::new("fn", span).into(),
+        Ident::new("check", span).into(),
+        span_help(Group::new(Delimiter::Parenthesis, TokenStream::new()).into()),
+        span_help(Group::new(Delimiter::Brace, TokenStream::new()).into()),
+    ])
+}