Auto merge of rust-lang#12482 - J-ZhengLi:issue12131, r=Jarcho

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

fixes: rust-lang#12131

It appears that [`root_macro_call_first_node`] only detects `println!` in the following example:
```rust
println!("{:?}", dbg!(s));
```
---

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

(re-opening b'cuz bors doesn't like my previous one)

Author: bors

clippy_lints/src/dbg_macro.rs

@@ -1,12 +1,14 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::macros::root_macro_call_first_node;
+use clippy_utils::macros::{macro_backtrace, MacroCall};
 use clippy_utils::source::snippet_with_applicability;
 use clippy_utils::{is_in_cfg_test, is_in_test_function};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, Node};
+use rustc_hir::{Expr, ExprKind, HirId, Node};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
 use rustc_session::impl_lint_pass;
-use rustc_span::sym;
+use rustc_span::{sym, Span, SyntaxContext};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -31,31 +33,38 @@ declare_clippy_lint! {
     "`dbg!` macro is intended as a debugging tool"
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone)]
 pub struct DbgMacro {
     allow_dbg_in_tests: bool,
+    /// Tracks the `dbg!` macro callsites that are already checked.
+    checked_dbg_call_site: FxHashSet<Span>,
+    /// Tracks the previous `SyntaxContext`, to avoid walking the same context chain.
+    prev_ctxt: SyntaxContext,
 }
 
 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,
+            checked_dbg_call_site: FxHashSet::default(),
+            prev_ctxt: SyntaxContext::root(),
+        }
     }
 }
 
 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 {
-            return;
-        };
-        if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) {
+        let cur_syntax_ctxt = expr.span.ctxt();
+
+        if cur_syntax_ctxt != self.prev_ctxt &&
+            let Some(macro_call) = first_dbg_macro_in_expansion(cx, expr.span) &&
+            !in_external_macro(cx.sess(), macro_call.span) &&
+            self.checked_dbg_call_site.insert(macro_call.span) &&
             // 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;
-            }
+            !(self.allow_dbg_in_tests && is_in_test(cx, expr.hir_id))
+        {
             let mut applicability = Applicability::MachineApplicable;
 
             let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
@@ -101,6 +110,8 @@ impl LateLintPass<'_> for DbgMacro {
                 _ => return,
             };
 
+            self.prev_ctxt = cur_syntax_ctxt;
+
             span_lint_and_sugg(
                 cx,
                 DBG_MACRO,
@@ -112,4 +123,16 @@ impl LateLintPass<'_> for DbgMacro {
             );
         }
     }
+
+    fn check_crate_post(&mut self, _: &LateContext<'_>) {
+        self.checked_dbg_call_site = FxHashSet::default();
+    }
+}
+
+fn is_in_test(cx: &LateContext<'_>, hir_id: HirId) -> bool {
+    is_in_test_function(cx.tcx, hir_id) || is_in_cfg_test(cx.tcx, hir_id)
+}
+
+fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option<MacroCall> {
+    macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
 }

tests/ui-toml/dbg_macro/dbg_macro.fixed

@@ -0,0 +1,38 @@
+//@compile-flags: --test
+#![warn(clippy::dbg_macro)]
+#![allow(clippy::unnecessary_operation, clippy::no_effect)]
+
+fn foo(n: u32) -> u32 {
+    if let Some(n) = n.checked_sub(4) { n } else { n }
+}
+
+fn factorial(n: u32) -> u32 {
+    if n <= 1 {
+        1
+    } else {
+        n * factorial(n - 1)
+    }
+}
+
+fn main() {
+    42;
+    foo(3) + factorial(4);
+    (1, 2, 3, 4, 5);
+}
+
+#[test]
+pub fn issue8481() {
+    dbg!(1);
+}
+
+#[cfg(test)]
+fn foo2() {
+    dbg!(1);
+}
+
+#[cfg(test)]
+mod mod1 {
+    fn func() {
+        dbg!(1);
+    }
+}

tests/ui-toml/dbg_macro/dbg_macro.rs

@@ -1,6 +1,7 @@
 //@compile-flags: --test
 #![warn(clippy::dbg_macro)]
-//@no-rustfix
+#![allow(clippy::unnecessary_operation, clippy::no_effect)]
+
 fn foo(n: u32) -> u32 {
     if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
 }
@@ -15,9 +16,7 @@ fn factorial(n: u32) -> u32 {
 
 fn main() {
     dbg!(42);
-    dbg!(dbg!(dbg!(42)));
     foo(3) + dbg!(factorial(4));
-    dbg!(1, 2, dbg!(3, 4));
     dbg!(1, 2, 3, 4, 5);
 }
 

tests/ui-toml/dbg_macro/dbg_macro.stderr

@@ -1,5 +1,5 @@
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:5:22
+  --> tests/ui-toml/dbg_macro/dbg_macro.rs:6:22
    |
 LL |     if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
    |                      ^^^^^^^^^^^^^^^^^^^^^^
@@ -12,7 +12,7 @@ LL |     if let Some(n) = n.checked_sub(4) { n } else { n }
    |                      ~~~~~~~~~~~~~~~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:9:8
+  --> tests/ui-toml/dbg_macro/dbg_macro.rs:10:8
    |
 LL |     if dbg!(n <= 1) {
    |        ^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL |     if n <= 1 {
    |        ~~~~~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:10:9
+  --> tests/ui-toml/dbg_macro/dbg_macro.rs:11:9
    |
 LL |         dbg!(1)
    |         ^^^^^^^
@@ -34,7 +34,7 @@ LL |         1
    |
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:12:9
+  --> tests/ui-toml/dbg_macro/dbg_macro.rs:13:9
    |
 LL |         dbg!(n * factorial(n - 1))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -45,7 +45,7 @@ LL |         n * factorial(n - 1)
    |
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:17:5
+  --> tests/ui-toml/dbg_macro/dbg_macro.rs:18:5
    |
 LL |     dbg!(42);
    |     ^^^^^^^^
@@ -55,17 +55,6 @@ help: remove the invocation before committing it to a version control system
 LL |     42;
    |     ~~
 
-error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:18:5
-   |
-LL |     dbg!(dbg!(dbg!(42)));
-   |     ^^^^^^^^^^^^^^^^^^^^
-   |
-help: remove the invocation before committing it to a version control system
-   |
-LL |     dbg!(dbg!(42));
-   |     ~~~~~~~~~~~~~~
-
 error: the `dbg!` macro is intended as a debugging tool
   --> tests/ui-toml/dbg_macro/dbg_macro.rs:19:14
    |
@@ -80,17 +69,6 @@ LL |     foo(3) + factorial(4);
 error: the `dbg!` macro is intended as a debugging tool
   --> tests/ui-toml/dbg_macro/dbg_macro.rs:20:5
    |
-LL |     dbg!(1, 2, dbg!(3, 4));
-   |     ^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: remove the invocation before committing it to a version control system
-   |
-LL |     (1, 2, dbg!(3, 4));
-   |     ~~~~~~~~~~~~~~~~~~
-
-error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui-toml/dbg_macro/dbg_macro.rs:21:5
-   |
 LL |     dbg!(1, 2, 3, 4, 5);
    |     ^^^^^^^^^^^^^^^^^^^
    |
@@ -99,5 +77,5 @@ help: remove the invocation before committing it to a version control system
 LL |     (1, 2, 3, 4, 5);
    |     ~~~~~~~~~~~~~~~
 
-error: aborting due to 9 previous errors
+error: aborting due to 7 previous errors
 

tests/ui/dbg_macro/dbg_macro.fixed

@@ -0,0 +1,111 @@
+#![warn(clippy::dbg_macro)]
+#![allow(clippy::unnecessary_operation, clippy::no_effect)]
+
+fn foo(n: u32) -> u32 {
+    if let Some(n) = n.checked_sub(4) { n } else { n }
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+}
+fn bar(_: ()) {}
+
+fn factorial(n: u32) -> u32 {
+    if n <= 1 {
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+        1
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    } else {
+        n * factorial(n - 1)
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    }
+}
+
+fn main() {
+    42;
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    foo(3) + factorial(4);
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    (1, 2, 3, 4, 5);
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+}
+
+fn issue9914() {
+    macro_rules! foo {
+        ($x:expr) => {
+            $x;
+        };
+    }
+    macro_rules! foo2 {
+        ($x:expr) => {
+            $x;
+        };
+    }
+    macro_rules! expand_to_dbg {
+        () => {
+            
+            //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+        };
+    }
+
+    
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    #[allow(clippy::let_unit_value)]
+    let _ = ();
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    bar(());
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    foo!(());
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    foo2!(foo!(()));
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    expand_to_dbg!();
+}
+
+mod issue7274 {
+    trait Thing<'b> {
+        fn foo(&self);
+    }
+
+    macro_rules! define_thing {
+        ($thing:ident, $body:expr) => {
+            impl<'a> Thing<'a> for $thing {
+                fn foo<'b>(&self) {
+                    $body
+                }
+            }
+        };
+    }
+
+    struct MyThing;
+    define_thing!(MyThing, {
+        2;
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    });
+}
+
+#[test]
+pub fn issue8481() {
+    1;
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+}
+
+#[cfg(test)]
+fn foo2() {
+    1;
+    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+}
+
+#[cfg(test)]
+mod mod1 {
+    fn func() {
+        1;
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    }
+}
+
+mod issue12131 {
+    fn dbg_in_print(s: &str) {
+        println!("dbg: {:?}", s);
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+        print!("{}", s);
+        //~^ ERROR: the `dbg!` macro is intended as a debugging tool
+    }
+}

tests/ui/dbg_macro/dbg_macro.rs

@@ -1,9 +1,5 @@
-//@no-rustfix
-
 #![warn(clippy::dbg_macro)]
-
-#[path = "auxiliary/submodule.rs"]
-mod submodule;
+#![allow(clippy::unnecessary_operation, clippy::no_effect)]
 
 fn foo(n: u32) -> u32 {
     if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
@@ -25,12 +21,8 @@ fn factorial(n: u32) -> u32 {
 fn main() {
     dbg!(42);
     //~^ ERROR: the `dbg!` macro is intended as a debugging tool
-    dbg!(dbg!(dbg!(42)));
-    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
     foo(3) + dbg!(factorial(4));
     //~^ ERROR: the `dbg!` macro is intended as a debugging tool
-    dbg!(1, 2, dbg!(3, 4));
-    //~^ ERROR: the `dbg!` macro is intended as a debugging tool
     dbg!(1, 2, 3, 4, 5);
     //~^ ERROR: the `dbg!` macro is intended as a debugging tool
 }
@@ -49,6 +41,7 @@ fn issue9914() {
     macro_rules! expand_to_dbg {
         () => {
             dbg!();
+            //~^ ERROR: the `dbg!` macro is intended as a debugging tool
         };
     }
 
@@ -107,3 +100,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
+    }
+}