emit warning for every reference of the same metavariable and refactor tests

Author: y21

clippy_lints/src/expr_metavars_in_unsafe.rs

@@ -151,18 +151,24 @@ fn extract_metavariable<'a>(
     }
 }
 
+#[derive(Debug)]
+struct ReferencedMetavar {
+    /// unsafe { $v; };
+    ///          ^^
+    span: Span,
+    /// unsafe { $v; };
+    /// ^^^^^^^^
+    unsafe_block_span: Span,
+}
+
+#[derive(Debug)]
 enum MetavarState {
-    /// The metavariable was referenced inside of an unsafe block
+    /// The metavariable was referenced inside of an unsafe block.
     Referenced {
-        /// unsafe { $v }
-        ///          ^^
-        reference_span: Span,
-        /// macro_rules! x { (... $v:expr ...) => {} }
+        occurences: Vec<ReferencedMetavar>,
+        /// macro_rules! x { (... $x:expr ...) => {} }
         ///                       ^^^^^^^
         decl_span: Span,
-        /// unsafe { $v }
-        /// ^^^^^^^^
-        unsafe_bl_span: Span,
     },
 
     /// The metavariable was not found yet, but we're still looking for it
@@ -209,6 +215,29 @@ impl UnsafeMetavariableFinder {
         self.enclosing.clear();
     }
 
+    fn report_warnings(&self, cx: &LateContext<'_>) {
+        for metavar in self.expr_variables.values() {
+            if let MetavarState::Referenced { decl_span, occurences } = metavar {
+                for occurence in occurences {
+                    span_lint_and_then(
+                        cx,
+                        EXPR_METAVARS_IN_UNSAFE,
+                        occurence.span,
+                        "expanding an expression metavariable in an unsafe block",
+                        |diag| {
+                            diag.span_note(*decl_span, "metavariable declared here");
+                            diag.span_note(occurence.unsafe_block_span, "unsafe block entered here");
+                            diag.help("consider expanding it outside of this block, e.g. by storing it in a variable");
+                            diag.note(
+                                "this allows the user of the macro to write unsafe code without an unsafe block (at the callsite)",
+                            );
+                        },
+                    );
+                }
+            }
+        }
+    }
+
     /// Given the following delimited token stream:
     /// ```ignore
     /// macro_rules! x { (... $var:expr ...) => { ... } }
@@ -258,41 +287,47 @@ impl UnsafeMetavariableFinder {
     /// }
     /// ```
     /// (statics don't have any extra rules so they are also handled here)
-    fn find_metavariables_in_const_static<'b>(&mut self, tts: &mut Peekable<impl Iterator<Item = &'b TokenTree>>) {
+    ///
+    /// Also note that this might be a const generic parameter and not a const item, which only
+    /// spans until the next comma, or right angle bracket:
+    /// ```ignore
+    /// struct T<const N: usize = { $v }, const O: usize = 1>;
+    ///                ^^^^^^^^^^^^^^^^^ `,` ends the const generic
+    /// struct T<const N: usize = { $v }>;
+    ///                ^^^^^^^^^^^^^^^^^ `>` ends the const generic
+    /// ```
+    /// Returns the terminating token. This is either `,` `;` or `>` as explained above
+    fn find_metavariables_in_const_static<'b>(
+        &mut self,
+        tts: &mut Peekable<impl Iterator<Item = &'b TokenTree>>,
+    ) -> Option<TokenKind> {
         let mut angle_bracket_depth = 0u32;
 
+        let mut last_token = None;
         self.enclosing.push(EnclosingNode::Body);
         for tt in tts {
             match tt {
                 TokenTree::Delimited(.., body) => self.find_metavariables_in_body(body),
-                TokenTree::Token(
-                    Token {
-                        kind: TokenKind::Semi, ..
-                    },
-                    _,
-                ) => {
-                    // We might be in a const generic position: don't break early for the first semicolon in
-                    // `const _: Ty<{ 2; 1 }> = ...;`
-                    if angle_bracket_depth == 0 {
-                        break;
+                TokenTree::Token(token, _) => {
+                    last_token = Some(token.kind.clone());
+                    match token.kind {
+                        // We might be in a const generic position: don't break early for the first comma in
+                        // `const _: Ty<1, 2> = ...,`
+                        TokenKind::Semi | TokenKind::Comma if angle_bracket_depth == 0 => {
+                            break;
+                        },
+                        TokenKind::Lt => angle_bracket_depth += 1,
+                        // If we see a `>` without having seen any `<`, it must mean that we are in a const
+                        // generic position and this ends it.
+                        TokenKind::Gt if angle_bracket_depth == 0 => break,
+                        TokenKind::Gt => angle_bracket_depth -= 1,
+                        _ => {},
                     }
                 },
-                TokenTree::Token(
-                    Token {
-                        kind: TokenKind::Lt, ..
-                    },
-                    _,
-                ) => angle_bracket_depth += 1,
-                TokenTree::Token(
-                    Token {
-                        kind: TokenKind::Gt, ..
-                    },
-                    _,
-                ) => angle_bracket_depth = angle_bracket_depth.saturating_sub(1),
-                TokenTree::Token(..) => {},
             }
         }
         self.enclosing.pop();
+        last_token
     }
 
     /// ```ignore
@@ -312,11 +347,11 @@ impl UnsafeMetavariableFinder {
         let mut angle_bracket_depth = 0u32;
 
         self.enclosing.push(EnclosingNode::Body);
-        for tt in tts {
+        while let Some(tt) = tts.next() {
             match tt {
                 TokenTree::Delimited(.., Delimiter::Brace, body) => {
                     self.find_metavariables_in_body(body);
-                    // We might be in a const generic position:
+                    // We might be in a const generic argument position:
                     // fn() -> Ty<{ ... }> {}
                     //            ^^^^^^^
                     // which we know if angle_bracket_depth > 0,
@@ -326,19 +361,21 @@ impl UnsafeMetavariableFinder {
                         break;
                     }
                 },
-                TokenTree::Token(
-                    Token {
-                        kind: TokenKind::Lt, ..
-                    },
-                    _,
-                ) => angle_bracket_depth += 1,
-                TokenTree::Token(
-                    Token {
-                        kind: TokenKind::Gt, ..
-                    },
-                    _,
-                ) => angle_bracket_depth = angle_bracket_depth.saturating_sub(1),
-                _ => {},
+                TokenTree::Token(token, _) => {
+                    match token.kind {
+                        TokenKind::Ident(kw::Const, false) => {
+                            // fn ...<const
+                            if let Some(TokenKind::Gt) = self.find_metavariables_in_const_static(tts) {
+                                // should not underflow, but macros don't have to be valid syntax if unexpanded
+                                angle_bracket_depth = angle_bracket_depth.saturating_sub(1);
+                            }
+                        },
+                        TokenKind::Lt => angle_bracket_depth += 1,
+                        TokenKind::Gt => angle_bracket_depth = angle_bracket_depth.saturating_sub(1),
+                        _ => {},
+                    }
+                },
+                TokenTree::Delimited(..) => {},
             }
         }
         self.enclosing.pop();
@@ -377,18 +414,25 @@ impl UnsafeMetavariableFinder {
                             EnclosingNode::UnsafeBlock(span) => Some(*span),
                         });
 
-                        match (&*state, unsafe_block_span) {
-                            (MetavarState::Referenced { .. }, Some(_)) => {},
+                        match (&mut *state, unsafe_block_span) {
+                            (MetavarState::Referenced { occurences, .. }, Some(unsafe_block_span)) => {
+                                occurences.push(ReferencedMetavar {
+                                    span,
+                                    unsafe_block_span,
+                                });
+                            },
                             (_, None) => {
                                 // Metavar was referenced outside of an unsafe block. Set to
                                 // suppressed because we don't want to lint.
                                 *state = MetavarState::Suppressed;
                             },
-                            (MetavarState::Unreferenced { decl_span }, Some(unsafe_bl_span)) => {
+                            (MetavarState::Unreferenced { decl_span }, Some(unsafe_block_span)) => {
                                 *state = MetavarState::Referenced {
-                                    reference_span: span,
+                                    occurences: vec![ReferencedMetavar {
+                                        span,
+                                        unsafe_block_span,
+                                    }],
                                     decl_span: *decl_span,
-                                    unsafe_bl_span,
                                 };
                             },
                             (MetavarState::Suppressed, _) => {},
@@ -433,6 +477,8 @@ impl<'tcx> LateLintPass<'tcx> for ExprMetavarsInUnsafe {
                 //    ^^^^^^^ look in here for references to one of the collected metavariables
                 with_token_stream(tts.next(), |body| finder.find_metavariables_in_body(body));
 
+                finder.report_warnings(cx);
+
                 // skip any trailing semicolons
                 // (...) => {};;;
                 //            ^^^
@@ -452,30 +498,6 @@ impl<'tcx> LateLintPass<'tcx> for ExprMetavarsInUnsafe {
                     .is_some()
                 {}
             }
-
-            for metavar in finder.expr_variables.values() {
-                if let &MetavarState::Referenced {
-                    decl_span,
-                    reference_span,
-                    unsafe_bl_span,
-                } = metavar
-                {
-                    span_lint_and_then(
-                        cx,
-                        EXPR_METAVARS_IN_UNSAFE,
-                        reference_span,
-                        "expanding an expression metavariable in an unsafe block",
-                        |diag| {
-                            diag.span_note(decl_span, "metavariable declared here");
-                            diag.span_note(unsafe_bl_span, "unsafe block entered here");
-                            diag.help("consider expanding it outside of this block, e.g. by storing it in a variable");
-                            diag.note(
-                                "this allows the user of the macro to write unsafe code without an unsafe block (at the callsite)",
-                            );
-                        },
-                    );
-                }
-            }
         }
     }
 }