Skip to content

Commit 860859c

Browse files
J-ZhengLiJ-ZhengLi
committed
fix [dbg_macro] FN when dbg is inside some complex macros
1 parent 88b5d51 commit 860859c

File tree

3 files changed

+107
-65
lines changed

3 files changed

+107
-65
lines changed

‎clippy_lints/src/dbg_macro.rs

+75-64
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::macros::root_macro_call_first_node;
2+
use clippy_utils::macros::root_macro_call;
33
use clippy_utils::source::snippet_with_applicability;
44
use clippy_utils::{is_in_cfg_test, is_in_test_function};
55
use rustc_errors::Applicability;
66
use rustc_hir::{Expr, ExprKind, Node};
77
use rustc_lint::{LateContext, LateLintPass, LintContext};
88
use rustc_session::impl_lint_pass;
9-
use rustc_span::sym;
9+
use rustc_span::{sym, Span};
1010

1111
declare_clippy_lint! {
1212
/// ### What it does
@@ -34,82 +34,93 @@ declare_clippy_lint! {
3434
#[derive(Copy, Clone)]
3535
pub struct DbgMacro {
3636
allow_dbg_in_tests: bool,
37+
/// Keep tracking of the previous `dbg!` macro call site in order to
38+
/// skip any other expressions from the same expansion, including nested macro calls.
39+
prev_dbg_call_site: Span,
3740
}
3841

3942
impl_lint_pass!(DbgMacro => [DBG_MACRO]);
4043

4144
impl DbgMacro {
4245
pub fn new(allow_dbg_in_tests: bool) -> Self {
43-
DbgMacro { allow_dbg_in_tests }
46+
DbgMacro {
47+
allow_dbg_in_tests,
48+
prev_dbg_call_site: Span::default(),
49+
}
4450
}
4551
}
4652

4753
impl LateLintPass<'_> for DbgMacro {
4854
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
49-
let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
55+
let Some(macro_call) =
56+
root_macro_call(expr.span).filter(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
57+
else {
5058
return;
5159
};
52-
if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) {
53-
// allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
54-
if self.allow_dbg_in_tests
55-
&& (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id))
56-
{
57-
return;
58-
}
59-
let mut applicability = Applicability::MachineApplicable;
60-
61-
let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
62-
// dbg!()
63-
ExprKind::Block(..) => {
64-
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
65-
// remove the whole statement.
66-
if let Some(Node::Stmt(_)) = cx.tcx.hir().find_parent(expr.hir_id)
67-
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
68-
{
69-
(macro_call.span.to(semi_span), String::new())
70-
} else {
71-
(macro_call.span, String::from("()"))
72-
}
73-
},
74-
// dbg!(1)
75-
ExprKind::Match(val, ..) => (
76-
macro_call.span,
77-
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
78-
),
79-
// dbg!(2, 3)
80-
ExprKind::Tup(
81-
[
82-
Expr {
83-
kind: ExprKind::Match(first, ..),
84-
..
85-
},
86-
..,
87-
Expr {
88-
kind: ExprKind::Match(last, ..),
89-
..
90-
},
91-
],
92-
) => {
93-
let snippet = snippet_with_applicability(
94-
cx,
95-
first.span.source_callsite().to(last.span.source_callsite()),
96-
"..",
97-
&mut applicability,
98-
);
99-
(macro_call.span, format!("({snippet})"))
100-
},
101-
_ => return,
102-
};
60+
// skip previous checked exprs
61+
if self.prev_dbg_call_site.contains(macro_call.span) {
62+
return;
63+
}
64+
self.prev_dbg_call_site = macro_call.span;
10365

104-
span_lint_and_sugg(
105-
cx,
106-
DBG_MACRO,
107-
sugg_span,
108-
"the `dbg!` macro is intended as a debugging tool",
109-
"remove the invocation before committing it to a version control system",
110-
suggestion,
111-
applicability,
112-
);
66+
// allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
67+
if self.allow_dbg_in_tests && (is_in_test_function(cx.tcx, expr.hir_id) || is_in_cfg_test(cx.tcx, expr.hir_id))
68+
{
69+
return;
11370
}
71+
let mut applicability = Applicability::MachineApplicable;
72+
73+
let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
74+
// dbg!()
75+
ExprKind::Block(..) => {
76+
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
77+
// remove the whole statement.
78+
if let Some(Node::Stmt(_)) = cx.tcx.hir().find_parent(expr.hir_id)
79+
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
80+
{
81+
(macro_call.span.to(semi_span), String::new())
82+
} else {
83+
(macro_call.span, String::from("()"))
84+
}
85+
},
86+
// dbg!(1)
87+
ExprKind::Match(val, ..) => (
88+
macro_call.span,
89+
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
90+
),
91+
// dbg!(2, 3)
92+
ExprKind::Tup(
93+
[
94+
Expr {
95+
kind: ExprKind::Match(first, ..),
96+
..
97+
},
98+
..,
99+
Expr {
100+
kind: ExprKind::Match(last, ..),
101+
..
102+
},
103+
],
104+
) => {
105+
let snippet = snippet_with_applicability(
106+
cx,
107+
first.span.source_callsite().to(last.span.source_callsite()),
108+
"..",
109+
&mut applicability,
110+
);
111+
(macro_call.span, format!("({snippet})"))
112+
},
113+
_ => return,
114+
};
115+
116+
span_lint_and_sugg(
117+
cx,
118+
DBG_MACRO,
119+
sugg_span,
120+
"the `dbg!` macro is intended as a debugging tool",
121+
"remove the invocation before committing it to a version control system",
122+
suggestion,
123+
applicability,
124+
);
114125
}
115126
}

‎tests/ui/dbg_macro/dbg_macro.rs

+9
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,12 @@ mod mod1 {
107107
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
108108
}
109109
}
110+
111+
mod issue12131 {
112+
fn dbg_in_print(s: &str) {
113+
println!("dbg: {:?}", dbg!(s));
114+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
115+
print!("{}", dbg!(s));
116+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
117+
}
118+
}

‎tests/ui/dbg_macro/dbg_macro.stderr

+23-1
Original file line numberDiff line numberDiff line change
@@ -211,5 +211,27 @@ help: remove the invocation before committing it to a version control system
211211
LL | 1;
212212
| ~
213213

214-
error: aborting due to 19 previous errors
214+
error: the `dbg!` macro is intended as a debugging tool
215+
--> $DIR/dbg_macro.rs:113:31
216+
|
217+
LL | println!("dbg: {:?}", dbg!(s));
218+
| ^^^^^^^
219+
|
220+
help: remove the invocation before committing it to a version control system
221+
|
222+
LL | println!("dbg: {:?}", s);
223+
| ~
224+
225+
error: the `dbg!` macro is intended as a debugging tool
226+
--> $DIR/dbg_macro.rs:115:22
227+
|
228+
LL | print!("{}", dbg!(s));
229+
| ^^^^^^^
230+
|
231+
help: remove the invocation before committing it to a version control system
232+
|
233+
LL | print!("{}", s);
234+
| ~
235+
236+
error: aborting due to 21 previous errors
215237

0 commit comments

Comments
 (0)