about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJ-ZhengLi <lizheng135@huawei.com>2024-03-06 23:08:59 +0800
committerJ-ZhengLi <lizheng135@huawei.com>2024-03-14 09:32:05 +0800
commitfe4e0aca73666b1be9a7f285fb9ae6e03d58a54c (patch)
tree3cac46dbcb44115e6777ece8e2d869c805eab6ff
parent0535f558317ca934bccbf5cda3e0f9779dbfd972 (diff)
downloadrust-fe4e0aca73666b1be9a7f285fb9ae6e03d58a54c.tar.gz
rust-fe4e0aca73666b1be9a7f285fb9ae6e03d58a54c.zip
checks `dbg` inside other macros as well (but no ext macro);
some refractoring;
-rw-r--r--clippy_lints/src/dbg_macro.rs152
-rw-r--r--tests/ui/dbg_macro/dbg_macro.fixed3
-rw-r--r--tests/ui/dbg_macro/dbg_macro.rs1
-rw-r--r--tests/ui/dbg_macro/dbg_macro.stderr40
4 files changed, 113 insertions, 83 deletions
diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs
index bd61260d94e..e2296767431 100644
--- a/clippy_lints/src/dbg_macro.rs
+++ b/clippy_lints/src/dbg_macro.rs
@@ -1,13 +1,14 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::macros::root_macro_call;
+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, Span};
+use rustc_span::{sym, Span, SyntaxContext};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -35,9 +36,10 @@ declare_clippy_lint! {
 #[derive(Clone)]
 pub struct DbgMacro {
     allow_dbg_in_tests: bool,
-    /// Keep tracks the `dbg!` macro callsites that are already checked,
-    /// so that we can save some performance by skipping any expressions from the same expansion.
+    /// 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]);
@@ -47,80 +49,90 @@ impl DbgMacro {
         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(expr.span).filter(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
-        else {
-            return;
-        };
-        if self.checked_dbg_call_site.contains(&macro_call.span) {
-            return;
-        }
-        self.checked_dbg_call_site.insert(macro_call.span);
+        let cur_syntax_ctxt = expr.span.ctxt();
 
-        // 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))
+        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
+            !(self.allow_dbg_in_tests && is_in_test(cx, expr.hir_id))
         {
-            return;
-        }
-        let mut applicability = Applicability::MachineApplicable;
+            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,
+            };
 
-        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,
-        };
+            self.prev_ctxt = cur_syntax_ctxt;
+
+            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,
+            );
+        }
+    }
 
-        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,
-        );
+    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))
+}
diff --git a/tests/ui/dbg_macro/dbg_macro.fixed b/tests/ui/dbg_macro/dbg_macro.fixed
index 7197c3c8f47..e3525191423 100644
--- a/tests/ui/dbg_macro/dbg_macro.fixed
+++ b/tests/ui/dbg_macro/dbg_macro.fixed
@@ -40,7 +40,8 @@ fn issue9914() {
     }
     macro_rules! expand_to_dbg {
         () => {
-            dbg!();
+            
+            //~^ ERROR: the `dbg!` macro is intended as a debugging tool
         };
     }
 
diff --git a/tests/ui/dbg_macro/dbg_macro.rs b/tests/ui/dbg_macro/dbg_macro.rs
index 0815d9f38dc..80606c2db05 100644
--- a/tests/ui/dbg_macro/dbg_macro.rs
+++ b/tests/ui/dbg_macro/dbg_macro.rs
@@ -41,6 +41,7 @@ fn issue9914() {
     macro_rules! expand_to_dbg {
         () => {
             dbg!();
+            //~^ ERROR: the `dbg!` macro is intended as a debugging tool
         };
     }
 
diff --git a/tests/ui/dbg_macro/dbg_macro.stderr b/tests/ui/dbg_macro/dbg_macro.stderr
index 5ff49816d00..86667701da0 100644
--- a/tests/ui/dbg_macro/dbg_macro.stderr
+++ b/tests/ui/dbg_macro/dbg_macro.stderr
@@ -78,7 +78,7 @@ LL |     (1, 2, 3, 4, 5);
    |     ~~~~~~~~~~~~~~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:47:5
+  --> tests/ui/dbg_macro/dbg_macro.rs:48:5
    |
 LL |     dbg!();
    |     ^^^^^^^
@@ -90,7 +90,7 @@ LL +
    |
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:50:13
+  --> tests/ui/dbg_macro/dbg_macro.rs:51:13
    |
 LL |     let _ = dbg!();
    |             ^^^^^^
@@ -101,7 +101,7 @@ LL |     let _ = ();
    |             ~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:52:9
+  --> tests/ui/dbg_macro/dbg_macro.rs:53:9
    |
 LL |     bar(dbg!());
    |         ^^^^^^
@@ -112,7 +112,7 @@ LL |     bar(());
    |         ~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:54:10
+  --> tests/ui/dbg_macro/dbg_macro.rs:55:10
    |
 LL |     foo!(dbg!());
    |          ^^^^^^
@@ -123,7 +123,7 @@ LL |     foo!(());
    |          ~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:56:16
+  --> tests/ui/dbg_macro/dbg_macro.rs:57:16
    |
 LL |     foo2!(foo!(dbg!()));
    |                ^^^^^^
@@ -134,7 +134,23 @@ LL |     foo2!(foo!(()));
    |                ~~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:78:9
+  --> tests/ui/dbg_macro/dbg_macro.rs:43:13
+   |
+LL |             dbg!();
+   |             ^^^^^^^
+...
+LL |     expand_to_dbg!();
+   |     ---------------- in this macro invocation
+   |
+   = note: this error originates in the macro `expand_to_dbg` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: remove the invocation before committing it to a version control system
+   |
+LL -             dbg!();
+LL +             
+   |
+
+error: the `dbg!` macro is intended as a debugging tool
+  --> tests/ui/dbg_macro/dbg_macro.rs:79:9
    |
 LL |         dbg!(2);
    |         ^^^^^^^
@@ -145,7 +161,7 @@ LL |         2;
    |         ~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:85:5
+  --> tests/ui/dbg_macro/dbg_macro.rs:86:5
    |
 LL |     dbg!(1);
    |     ^^^^^^^
@@ -156,7 +172,7 @@ LL |     1;
    |     ~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:91:5
+  --> tests/ui/dbg_macro/dbg_macro.rs:92:5
    |
 LL |     dbg!(1);
    |     ^^^^^^^
@@ -167,7 +183,7 @@ LL |     1;
    |     ~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:98:9
+  --> tests/ui/dbg_macro/dbg_macro.rs:99:9
    |
 LL |         dbg!(1);
    |         ^^^^^^^
@@ -178,7 +194,7 @@ LL |         1;
    |         ~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:105:31
+  --> tests/ui/dbg_macro/dbg_macro.rs:106:31
    |
 LL |         println!("dbg: {:?}", dbg!(s));
    |                               ^^^^^^^
@@ -189,7 +205,7 @@ LL |         println!("dbg: {:?}", s);
    |                               ~
 
 error: the `dbg!` macro is intended as a debugging tool
-  --> tests/ui/dbg_macro/dbg_macro.rs:107:22
+  --> tests/ui/dbg_macro/dbg_macro.rs:108:22
    |
 LL |         print!("{}", dbg!(s));
    |                      ^^^^^^^
@@ -199,5 +215,5 @@ help: remove the invocation before committing it to a version control system
 LL |         print!("{}", s);
    |                      ~
 
-error: aborting due to 18 previous errors
+error: aborting due to 19 previous errors