diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index bd61260d94e9..c3383068b88f 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, + /// Tracks the previous `SyntaxContext`, to avoid walking the same context chain. + prev_ctxt: SyntaxContext, } impl_lint_pass!(DbgMacro => [DBG_MACRO]); @@ -47,80 +49,91 @@ 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(¯o_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.contains(¯o_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 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, + }; - 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; + self.checked_dbg_call_site.insert(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, + ); + } + } - 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 { + 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 7197c3c8f477..d1fb8bf34ee7 100644 --- a/tests/ui/dbg_macro/dbg_macro.fixed +++ b/tests/ui/dbg_macro/dbg_macro.fixed @@ -40,7 +40,7 @@ fn issue9914() { } macro_rules! expand_to_dbg { () => { - dbg!(); + }; } @@ -56,6 +56,7 @@ fn issue9914() { foo2!(foo!(())); //~^ ERROR: the `dbg!` macro is intended as a debugging tool expand_to_dbg!(); + //~^ ERROR: the `dbg!` macro is intended as a debugging tool } mod issue7274 { diff --git a/tests/ui/dbg_macro/dbg_macro.rs b/tests/ui/dbg_macro/dbg_macro.rs index 0815d9f38dc8..99cfd14c76a7 100644 --- a/tests/ui/dbg_macro/dbg_macro.rs +++ b/tests/ui/dbg_macro/dbg_macro.rs @@ -56,6 +56,7 @@ fn issue9914() { foo2!(foo!(dbg!())); //~^ ERROR: the `dbg!` macro is intended as a debugging tool expand_to_dbg!(); + //~^ ERROR: the `dbg!` macro is intended as a debugging tool } mod issue7274 { diff --git a/tests/ui/dbg_macro/dbg_macro.stderr b/tests/ui/dbg_macro/dbg_macro.stderr index 5ff49816d00c..2fa9efcceeb5 100644 --- a/tests/ui/dbg_macro/dbg_macro.stderr +++ b/tests/ui/dbg_macro/dbg_macro.stderr @@ -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