fix [dbg_macro] FN when dbg is inside some complex macros

Author: J-ZhengLi

clippy_lints/src/dbg_macro.rs

@@ -1,12 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::macros::root_macro_call_first_node;
+use clippy_utils::macros::root_macro_call;
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{is_in_cfg_test, is_in_test_function};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, Node};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::impl_lint_pass;
-use rustc_span::sym;
+use rustc_span::{sym, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -34,82 +34,93 @@ declare_clippy_lint! {
 #[derive(Copy, Clone)]
 pub struct DbgMacro {
     allow_dbg_in_tests: bool,
+    /// Keep tracking of the previous `dbg!` macro call site in order to
+    /// skip any other expressions from the same expansion, including nested macro calls.
+    prev_dbg_call_site: Span,
 }
 
 impl_lint_pass!(DbgMacro => [DBG_MACRO]);
 
 impl DbgMacro {
     pub fn new(allow_dbg_in_tests: bool) -> Self {
-        DbgMacro { allow_dbg_in_tests }
+        DbgMacro {
+            allow_dbg_in_tests,
+            prev_dbg_call_site: Span::default(),
+        }
     }
 }
 
 impl LateLintPass<'_> for DbgMacro {
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
-        let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
+        let Some(macro_call) =
+            root_macro_call(expr.span).filter(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
+        else {
             return;
         };
-        if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) {
-            // allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
-            if self.allow_dbg_in_tests
-                && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id))
-            {
-                return;
-            }
-            let mut applicability = Applicability::MachineApplicable;
-
-            let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
-                // dbg!()
-                ExprKind::Block(..) => {
-                    // If the `dbg!` macro is a "free" statement and not contained within other expressions,
-                    // remove the whole statement.
-                    if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
-                        && let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
-                    {
-                        (macro_call.span.to(semi_span), String::new())
-                    } else {
-                        (macro_call.span, String::from("()"))
-                    }
-                },
-                // dbg!(1)
-                ExprKind::Match(val, ..) => (
-                    macro_call.span,
-                    snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
-                ),
-                // dbg!(2, 3)
-                ExprKind::Tup(
-                    [
-                        Expr {
-                            kind: ExprKind::Match(first, ..),
-                            ..
-                        },
-                        ..,
-                        Expr {
-                            kind: ExprKind::Match(last, ..),
-                            ..
-                        },
-                    ],
-                ) => {
-                    let snippet = snippet_with_applicability(
-                        cx,
-                        first.span.source_callsite().to(last.span.source_callsite()),
-                        "..",
-                        &mut applicability,
-                    );
-                    (macro_call.span, format!("({snippet})"))
-                },
-                _ => return,
-            };
+        // skip previous checked exprs
+        if self.prev_dbg_call_site.contains(macro_call.span) {
+            return;
+        }
+        self.prev_dbg_call_site = macro_call.span;
 
-            span_lint_and_sugg(
-                cx,
-                DBG_MACRO,
-                sugg_span,
-                "the `dbg!` macro is intended as a debugging tool",
-                "remove the invocation before committing it to a version control system",
-                suggestion,
-                applicability,
-            );
+        // allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
+        if self.allow_dbg_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id))
+        {
+            return;
         }
+        let mut applicability = Applicability::MachineApplicable;
+
+        let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
+            // dbg!()
+            ExprKind::Block(..) => {
+                // If the `dbg!` macro is a "free" statement and not contained within other expressions,
+                // remove the whole statement.
+                if let Some(Node::Stmt(_)) = cx.tcx.hir().find_parent(expr.hir_id)
+                    && let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
+                {
+                    (macro_call.span.to(semi_span), String::new())
+                } else {
+                    (macro_call.span, String::from("()"))
+                }
+            },
+            // dbg!(1)
+            ExprKind::Match(val, ..) => (
+                macro_call.span,
+                snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
+            ),
+            // dbg!(2, 3)
+            ExprKind::Tup(
+                [
+                    Expr {
+                        kind: ExprKind::Match(first, ..),
+                        ..
+                    },
+                    ..,
+                    Expr {
+                        kind: ExprKind::Match(last, ..),
+                        ..
+                    },
+                ],
+            ) => {
+                let snippet = snippet_with_applicability(
+                    cx,
+                    first.span.source_callsite().to(last.span.source_callsite()),
+                    "..",
+                    &mut applicability,
+                );
+                (macro_call.span, format!("({snippet})"))
+            },
+            _ => return,
+        };
+
+        span_lint_and_sugg(
+            cx,
+            DBG_MACRO,
+            sugg_span,
+            "the `dbg!` macro is intended as a debugging tool",
+            "remove the invocation before committing it to a version control system",
+            suggestion,
+            applicability,
+        );
     }
 }

tests/ui/dbg_macro/dbg_macro.rs

@@ -107,3 +107,12 @@ mod mod1 {
         //~^ ERROR: the `dbg!` macro is intended as a debugging tool
     }
 }
+
+mod issue12131 {
+    fn dbg_in_print(s: &str) {
+        println!("dbg: {:?}", dbg!(s));
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+        print!("{}", dbg!(s));
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    }
+}

tests/ui/dbg_macro/dbg_macro.stderr

@@ -211,5 +211,27 @@ help: remove the invocation before committing it to a version control system
 LL |         1;
    |         ~
 
-error: aborting due to 19 previous errors
+error: the `dbg!` macro is intended as a debugging tool
+  --> $DIR/dbg_macro.rs:113:31
+   |
+LL |         println!("dbg: {:?}", dbg!(s));
+   |                               ^^^^^^^
+   |
+help: remove the invocation before committing it to a version control system
+   |
+LL |         println!("dbg: {:?}", s);
+   |                               ~
+
+error: the `dbg!` macro is intended as a debugging tool
+  --> $DIR/dbg_macro.rs:115:22
+   |
+LL |         print!("{}", dbg!(s));
+   |                      ^^^^^^^
+   |
+help: remove the invocation before committing it to a version control system
+   |
+LL |         print!("{}", s);
+   |                      ~
+
+error: aborting due to 21 previous errors