fix [large_stack_arrays] linting in vec macro & add matching_root_macro_call function in clippy_utils

J-ZhengLi · J-ZhengLi · commit 228f432b0e5f · 2024-04-16T15:43:44.000+08:00
diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs
@@ -4,7 +4,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::is_diag_trait_item;
 use clippy_utils::macros::{
     find_format_arg_expr, find_format_args, format_arg_removal_span, format_placeholder_format_span, is_assert_macro,
-    is_format_macro, is_panic, root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall,
+    is_format_macro, is_panic, matching_root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall,
 };
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::{implements_trait, is_type_lang_item};
@@ -271,9 +271,7 @@ impl<'a, 'tcx> FormatArgsExpr<'a, 'tcx> {
                     let mut suggest_format = |spec| {
                         let message = format!("for the {spec} to apply consider using `format!()`");
 
-                        if let Some(mac_call) = root_macro_call(arg_span)
-                            && self.cx.tcx.is_diagnostic_item(sym::format_args_macro, mac_call.def_id)
-                        {
+                        if let Some(mac_call) = matching_root_macro_call(self.cx, arg_span, sym::format_args_macro) {
                             diag.span_suggestion(
                                 self.cx.sess().source_map().span_until_char(mac_call.span, '!'),
                                 message,
diff --git a/clippy_lints/src/large_stack_arrays.rs b/clippy_lints/src/large_stack_arrays.rs
@@ -1,10 +1,12 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::macros::macro_backtrace;
 use clippy_utils::source::snippet;
 use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::{self, ConstKind};
 use rustc_session::impl_lint_pass;
+use rustc_span::{sym, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -39,6 +41,7 @@ impl_lint_pass!(LargeStackArrays => [LARGE_STACK_ARRAYS]);
 impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
         if let ExprKind::Repeat(_, _) | ExprKind::Array(_) = expr.kind
+            && !is_from_vec_macro(cx, expr.span)
             && let ty::Array(element_type, cst) = cx.typeck_results().expr_ty(expr).kind()
             && let ConstKind::Value(ty::ValTree::Leaf(element_count)) = cst.kind()
             && let Ok(element_count) = element_count.try_to_target_usize(cx.tcx)
@@ -54,20 +57,28 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackArrays {
             })
             && self.maximum_allowed_size < u128::from(element_count) * u128::from(element_size)
         {
-            span_lint_and_help(
+            span_lint_and_then(
                 cx,
                 LARGE_STACK_ARRAYS,
                 expr.span,
                 format!(
                     "allocating a local array larger than {} bytes",
                     self.maximum_allowed_size
                 ),
-                None,
-                format!(
-                    "consider allocating on the heap with `vec!{}.into_boxed_slice()`",
-                    snippet(cx, expr.span, "[...]")
-                ),
+                |diag| {
+                    if !expr.span.from_expansion() {
+                        diag.help(format!(
+                            "consider allocating on the heap with `vec!{}.into_boxed_slice()`",
+                            snippet(cx, expr.span, "[...]")
+                        ));
+                    }
+                },
             );
         }
     }
 }
+
+/// We shouldn't lint messages if the expr is already in a `vec!` call
+fn is_from_vec_macro(cx: &LateContext<'_>, expr_span: Span) -> bool {
+    macro_backtrace(expr_span).any(|mac| cx.tcx.is_diagnostic_item(sym::vec_macro, mac.def_id))
+}
diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs
@@ -1,12 +1,11 @@
 use crate::rustc_lint::LintContext;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::macros::root_macro_call;
+use clippy_utils::macros::{is_panic, root_macro_call};
 use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
-use rustc_span::sym;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -42,7 +41,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
             && !expr.span.from_expansion()
             && let then = peel_blocks_with_stmt(then)
             && let Some(macro_call) = root_macro_call(then.span)
-            && cx.tcx.item_name(macro_call.def_id) == sym::panic
+            && is_panic(cx, macro_call.def_id)
             && !cx.tcx.sess.source_map().is_multiline(cond.span)
             && let Ok(panic_snippet) = cx.sess().source_map().span_to_snippet(macro_call.span)
             && let Some(panic_snippet) = panic_snippet.strip_suffix(')')
diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs
@@ -1,6 +1,6 @@
 use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::macros::root_macro_call;
+use clippy_utils::macros::matching_root_macro_call;
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{higher, in_constant};
 use rustc_ast::ast::RangeLimits;
@@ -9,7 +9,6 @@ use rustc_errors::Applicability;
 use rustc_hir::{BorrowKind, Expr, ExprKind, PatKind, RangeEnd};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::impl_lint_pass;
-use rustc_span::def_id::DefId;
 use rustc_span::{sym, Span};
 
 declare_clippy_lint! {
@@ -97,9 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
             return;
         }
 
-        if let Some(macro_call) = root_macro_call(expr.span)
-            && is_matches_macro(cx, macro_call.def_id)
-        {
+        if let Some(macro_call) = matching_root_macro_call(cx, expr.span, sym::matches_macro) {
             if let ExprKind::Match(recv, [arm, ..], _) = expr.kind {
                 let range = check_pat(&arm.pat.kind);
                 check_is_ascii(cx, macro_call.span, recv, &range);
@@ -187,11 +184,3 @@ fn check_range(start: &Expr<'_>, end: &Expr<'_>) -> CharRange {
         CharRange::Otherwise
     }
 }
-
-fn is_matches_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
-    if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
-        return sym::matches_macro == name;
-    }
-
-    false
-}
diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs
@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::macros::{is_panic, root_macro_call};
+use clippy_utils::macros::{is_panic, matching_root_macro_call, root_macro_call};
 use clippy_utils::source::{indent_of, reindent_multiline, snippet};
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::{higher, is_trait_method, path_to_local_id, peel_blocks, SpanlessEq};
@@ -247,8 +247,7 @@ impl<'tcx> OffendingFilterExpr<'tcx> {
             } else {
                 None
             }
-        } else if let Some(macro_call) = root_macro_call(expr.span)
-            && cx.tcx.get_diagnostic_name(macro_call.def_id) == Some(sym::matches_macro)
+        } else if matching_root_macro_call(cx, expr.span, sym::matches_macro).is_some()
             // we know for a fact that the wildcard pattern is the second arm
             && let ExprKind::Match(scrutinee, [arm, _], _) = expr.kind
             && path_to_local_id(scrutinee, filter_param_id)
diff --git a/clippy_lints/src/repeat_vec_with_capacity.rs b/clippy_lints/src/repeat_vec_with_capacity.rs
@@ -1,7 +1,7 @@
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::higher::VecArgs;
-use clippy_utils::macros::root_macro_call;
+use clippy_utils::macros::matching_root_macro_call;
 use clippy_utils::source::snippet;
 use clippy_utils::{expr_or_init, fn_def_id, match_def_path, paths};
 use rustc_errors::Applicability;
@@ -65,8 +65,7 @@ fn emit_lint(cx: &LateContext<'_>, span: Span, kind: &str, note: &'static str, s
 
 /// Checks `vec![Vec::with_capacity(x); n]`
 fn check_vec_macro(cx: &LateContext<'_>, expr: &Expr<'_>) {
-    if let Some(mac_call) = root_macro_call(expr.span)
-        && cx.tcx.is_diagnostic_item(sym::vec_macro, mac_call.def_id)
+    if matching_root_macro_call(cx, expr.span, sym::vec_macro).is_some()
         && let Some(VecArgs::Repeat(repeat_expr, len_expr)) = VecArgs::hir(cx, expr)
         && fn_def_id(cx, repeat_expr).is_some_and(|did| match_def_path(cx, did, &paths::VEC_WITH_CAPACITY))
         && !len_expr.span.from_expansion()
diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs
@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::macros::root_macro_call;
+use clippy_utils::macros::matching_root_macro_call;
 use clippy_utils::sugg::Sugg;
 use clippy_utils::{
     get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local,
@@ -145,9 +145,7 @@ impl SlowVectorInit {
         // Generally don't warn if the vec initializer comes from an expansion, except for the vec! macro.
         // This lets us still warn on `vec![]`, while ignoring other kinds of macros that may output an
         // empty vec
-        if expr.span.from_expansion()
-            && root_macro_call(expr.span).map(|m| m.def_id) != cx.tcx.get_diagnostic_item(sym::vec_macro)
-        {
+        if expr.span.from_expansion() && matching_root_macro_call(cx, expr.span, sym::vec_macro).is_none() {
             return None;
         }
 
diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs
@@ -119,10 +119,20 @@ pub fn macro_backtrace(span: Span) -> impl Iterator<Item = MacroCall> {
 
 /// If the macro backtrace of `span` has a macro call at the root expansion
 /// (i.e. not a nested macro call), returns `Some` with the `MacroCall`
+///
+/// If you only want to check whether the root macro has a specific name,
+/// consider using [`matching_root_macro_call`] instead.
 pub fn root_macro_call(span: Span) -> Option<MacroCall> {
     macro_backtrace(span).last()
 }
 
+/// A combination of [`root_macro_call`] and
+/// [`is_diagnostic_item`](rustc_middle::ty::TyCtxt::is_diagnostic_item) that returns a `MacroCall`
+/// at the root expansion if only it matches the given name.
+pub fn matching_root_macro_call(cx: &LateContext<'_>, span: Span, name: Symbol) -> Option<MacroCall> {
+    root_macro_call(span).filter(|mc| cx.tcx.is_diagnostic_item(name, mc.def_id))
+}
+
 /// Like [`root_macro_call`], but only returns `Some` if `node` is the "first node"
 /// produced by the macro call, as in [`first_node_in_macro`].
 pub fn root_macro_call_first_node(cx: &LateContext<'_>, node: &impl HirNode) -> Option<MacroCall> {
diff --git a/tests/ui/large_stack_arrays.rs b/tests/ui/large_stack_arrays.rs
@@ -55,3 +55,37 @@ fn main() {
         [(); 20_000_000],
     );
 }
+
+#[allow(clippy::useless_vec)]
+fn issue_12586() {
+    macro_rules! dummy {
+        ($n:expr) => {
+            $n
+        };
+        // Weird rule to test help messages.
+        ($a:expr => $b:expr) => {
+            [$a, $b, $a, $b]
+            //~^ ERROR: allocating a local array larger than 512000 bytes
+        };
+        ($id:ident; $n:literal) => {
+            dummy!(::std::vec![$id;$n])
+        };
+        ($($id:expr),+ $(,)?) => {
+            ::std::vec![$($id),*]
+        }
+    }
+
+    let x = [0u32; 50_000];
+    let y = vec![x, x, x, x, x];
+    let y = vec![dummy![x, x, x, x, x]];
+    let y = vec![dummy![[x, x, x, x, x]]];
+    //~^ ERROR: allocating a local array larger than 512000 bytes
+    let y = dummy![x, x, x, x, x];
+    let y = [x, x, dummy!(x), x, x];
+    //~^ ERROR: allocating a local array larger than 512000 bytes
+    let y = dummy![x => x];
+    let y = dummy![x;5];
+    let y = dummy!(vec![dummy![x, x, x, x, x]]);
+    let y = dummy![[x, x, x, x, x]];
+    //~^ ERROR: allocating a local array larger than 512000 bytes
+}
diff --git a/tests/ui/large_stack_arrays.stderr b/tests/ui/large_stack_arrays.stderr
@@ -56,5 +56,40 @@ LL |         [0u8; usize::MAX],
    |
    = help: consider allocating on the heap with `vec![0u8; usize::MAX].into_boxed_slice()`
 
-error: aborting due to 7 previous errors
+error: allocating a local array larger than 512000 bytes
+  --> tests/ui/large_stack_arrays.rs:81:25
+   |
+LL |     let y = vec![dummy![[x, x, x, x, x]]];
+   |                         ^^^^^^^^^^^^^^^
+   |
+   = help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
+
+error: allocating a local array larger than 512000 bytes
+  --> tests/ui/large_stack_arrays.rs:84:13
+   |
+LL |     let y = [x, x, dummy!(x), x, x];
+   |             ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider allocating on the heap with `vec![x, x, dummy!(x), x, x].into_boxed_slice()`
+
+error: allocating a local array larger than 512000 bytes
+  --> tests/ui/large_stack_arrays.rs:67:13
+   |
+LL |             [$a, $b, $a, $b]
+   |             ^^^^^^^^^^^^^^^^
+...
+LL |     let y = dummy![x => x];
+   |             -------------- in this macro invocation
+   |
+   = note: this error originates in the macro `dummy` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error: allocating a local array larger than 512000 bytes
+  --> tests/ui/large_stack_arrays.rs:89:20
+   |
+LL |     let y = dummy![[x, x, x, x, x]];
+   |                    ^^^^^^^^^^^^^^^
+   |
+   = help: consider allocating on the heap with `vec![x, x, x, x, x].into_boxed_slice()`
+
+error: aborting due to 11 previous errors
 
