Skip to content

Commit fe4e0ac

Browse files
J-ZhengLiJ-ZhengLi
committed
checks dbg inside other macros as well (but no ext macro);
some refractoring;
1 parent 0535f55 commit fe4e0ac

File tree

4 files changed

+113
-83
lines changed

4 files changed

+113
-83
lines changed

‎clippy_lints/src/dbg_macro.rs‎

Lines changed: 82 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::macros::root_macro_call;
2+
use clippy_utils::macros::{macro_backtrace, MacroCall};
33
use clippy_utils::source::snippet_with_applicability;
44
use clippy_utils::{is_in_cfg_test, is_in_test_function};
55
use rustc_data_structures::fx::FxHashSet;
66
use rustc_errors::Applicability;
7-
use rustc_hir::{Expr, ExprKind, Node};
7+
use rustc_hir::{Expr, ExprKind, HirId, Node};
88
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::lint::in_external_macro;
910
use rustc_session::impl_lint_pass;
10-
use rustc_span::{sym, Span};
11+
use rustc_span::{sym, Span, SyntaxContext};
1112

1213
declare_clippy_lint! {
1314
/// ### What it does
@@ -35,9 +36,10 @@ declare_clippy_lint! {
3536
#[derive(Clone)]
3637
pub struct DbgMacro {
3738
allow_dbg_in_tests: bool,
38-
/// Keep tracks the `dbg!` macro callsites that are already checked,
39-
/// so that we can save some performance by skipping any expressions from the same expansion.
39+
/// Tracks the `dbg!` macro callsites that are already checked.
4040
checked_dbg_call_site: FxHashSet<Span>,
41+
/// Tracks the previous `SyntaxContext`, to avoid walking the same context chain.
42+
prev_ctxt: SyntaxContext,
4143
}
4244

4345
impl_lint_pass!(DbgMacro => [DBG_MACRO]);
@@ -47,80 +49,90 @@ impl DbgMacro {
4749
DbgMacro {
4850
allow_dbg_in_tests,
4951
checked_dbg_call_site: FxHashSet::default(),
52+
prev_ctxt: SyntaxContext::root(),
5053
}
5154
}
5255
}
5356

5457
impl LateLintPass<'_> for DbgMacro {
5558
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
56-
let Some(macro_call) =
57-
root_macro_call(expr.span).filter(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
58-
else {
59-
return;
60-
};
61-
if self.checked_dbg_call_site.contains(&macro_call.span) {
62-
return;
63-
}
64-
self.checked_dbg_call_site.insert(macro_call.span);
59+
let cur_syntax_ctxt = expr.span.ctxt();
6560

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))
61+
if cur_syntax_ctxt != self.prev_ctxt &&
62+
let Some(macro_call) = first_dbg_macro_in_expansion(cx, expr.span) &&
63+
!in_external_macro(cx.sess(), macro_call.span) &&
64+
self.checked_dbg_call_site.insert(macro_call.span) &&
65+
// allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml
66+
!(self.allow_dbg_in_tests && is_in_test(cx, expr.hir_id))
6867
{
69-
return;
70-
}
71-
let mut applicability = Applicability::MachineApplicable;
68+
let mut applicability = Applicability::MachineApplicable;
69+
70+
let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
71+
// dbg!()
72+
ExprKind::Block(..) => {
73+
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
74+
// remove the whole statement.
75+
if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
76+
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
77+
{
78+
(macro_call.span.to(semi_span), String::new())
79+
} else {
80+
(macro_call.span, String::from("()"))
81+
}
82+
},
83+
// dbg!(1)
84+
ExprKind::Match(val, ..) => (
85+
macro_call.span,
86+
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
87+
),
88+
// dbg!(2, 3)
89+
ExprKind::Tup(
90+
[
91+
Expr {
92+
kind: ExprKind::Match(first, ..),
93+
..
94+
},
95+
..,
96+
Expr {
97+
kind: ExprKind::Match(last, ..),
98+
..
99+
},
100+
],
101+
) => {
102+
let snippet = snippet_with_applicability(
103+
cx,
104+
first.span.source_callsite().to(last.span.source_callsite()),
105+
"..",
106+
&mut applicability,
107+
);
108+
(macro_call.span, format!("({snippet})"))
109+
},
110+
_ => return,
111+
};
72112

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-
};
113+
self.prev_ctxt = cur_syntax_ctxt;
114+
115+
span_lint_and_sugg(
116+
cx,
117+
DBG_MACRO,
118+
sugg_span,
119+
"the `dbg!` macro is intended as a debugging tool",
120+
"remove the invocation before committing it to a version control system",
121+
suggestion,
122+
applicability,
123+
);
124+
}
125+
}
115126

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-
);
127+
fn check_crate_post(&mut self, _: &LateContext<'_>) {
128+
self.checked_dbg_call_site = FxHashSet::default();
125129
}
126130
}
131+
132+
fn is_in_test(cx: &LateContext<'_>, hir_id: HirId) -> bool {
133+
is_in_test_function(cx.tcx, hir_id) || is_in_cfg_test(cx.tcx, hir_id)
134+
}
135+
136+
fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option<MacroCall> {
137+
macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
138+
}

‎tests/ui/dbg_macro/dbg_macro.fixed‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ fn issue9914() {
4040
}
4141
macro_rules! expand_to_dbg {
4242
() => {
43-
dbg!();
43+
44+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
4445
};
4546
}
4647

‎tests/ui/dbg_macro/dbg_macro.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ fn issue9914() {
4141
macro_rules! expand_to_dbg {
4242
() => {
4343
dbg!();
44+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
4445
};
4546
}
4647

‎tests/ui/dbg_macro/dbg_macro.stderr‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ LL | (1, 2, 3, 4, 5);
7878
| ~~~~~~~~~~~~~~~
7979

8080
error: the `dbg!` macro is intended as a debugging tool
81-
--> tests/ui/dbg_macro/dbg_macro.rs:47:5
81+
--> tests/ui/dbg_macro/dbg_macro.rs:48:5
8282
|
8383
LL | dbg!();
8484
| ^^^^^^^
@@ -90,7 +90,7 @@ LL +
9090
|
9191

9292
error: the `dbg!` macro is intended as a debugging tool
93-
--> tests/ui/dbg_macro/dbg_macro.rs:50:13
93+
--> tests/ui/dbg_macro/dbg_macro.rs:51:13
9494
|
9595
LL | let _ = dbg!();
9696
| ^^^^^^
@@ -101,7 +101,7 @@ LL | let _ = ();
101101
| ~~
102102

103103
error: the `dbg!` macro is intended as a debugging tool
104-
--> tests/ui/dbg_macro/dbg_macro.rs:52:9
104+
--> tests/ui/dbg_macro/dbg_macro.rs:53:9
105105
|
106106
LL | bar(dbg!());
107107
| ^^^^^^
@@ -112,7 +112,7 @@ LL | bar(());
112112
| ~~
113113

114114
error: the `dbg!` macro is intended as a debugging tool
115-
--> tests/ui/dbg_macro/dbg_macro.rs:54:10
115+
--> tests/ui/dbg_macro/dbg_macro.rs:55:10
116116
|
117117
LL | foo!(dbg!());
118118
| ^^^^^^
@@ -123,7 +123,7 @@ LL | foo!(());
123123
| ~~
124124

125125
error: the `dbg!` macro is intended as a debugging tool
126-
--> tests/ui/dbg_macro/dbg_macro.rs:56:16
126+
--> tests/ui/dbg_macro/dbg_macro.rs:57:16
127127
|
128128
LL | foo2!(foo!(dbg!()));
129129
| ^^^^^^
@@ -134,7 +134,23 @@ LL | foo2!(foo!(()));
134134
| ~~
135135

136136
error: the `dbg!` macro is intended as a debugging tool
137-
--> tests/ui/dbg_macro/dbg_macro.rs:78:9
137+
--> tests/ui/dbg_macro/dbg_macro.rs:43:13
138+
|
139+
LL | dbg!();
140+
| ^^^^^^^
141+
...
142+
LL | expand_to_dbg!();
143+
| ---------------- in this macro invocation
144+
|
145+
= note: this error originates in the macro `expand_to_dbg` (in Nightly builds, run with -Z macro-backtrace for more info)
146+
help: remove the invocation before committing it to a version control system
147+
|
148+
LL - dbg!();
149+
LL +
150+
|
151+
152+
error: the `dbg!` macro is intended as a debugging tool
153+
--> tests/ui/dbg_macro/dbg_macro.rs:79:9
138154
|
139155
LL | dbg!(2);
140156
| ^^^^^^^
@@ -145,7 +161,7 @@ LL | 2;
145161
| ~
146162

147163
error: the `dbg!` macro is intended as a debugging tool
148-
--> tests/ui/dbg_macro/dbg_macro.rs:85:5
164+
--> tests/ui/dbg_macro/dbg_macro.rs:86:5
149165
|
150166
LL | dbg!(1);
151167
| ^^^^^^^
@@ -156,7 +172,7 @@ LL | 1;
156172
| ~
157173

158174
error: the `dbg!` macro is intended as a debugging tool
159-
--> tests/ui/dbg_macro/dbg_macro.rs:91:5
175+
--> tests/ui/dbg_macro/dbg_macro.rs:92:5
160176
|
161177
LL | dbg!(1);
162178
| ^^^^^^^
@@ -167,7 +183,7 @@ LL | 1;
167183
| ~
168184

169185
error: the `dbg!` macro is intended as a debugging tool
170-
--> tests/ui/dbg_macro/dbg_macro.rs:98:9
186+
--> tests/ui/dbg_macro/dbg_macro.rs:99:9
171187
|
172188
LL | dbg!(1);
173189
| ^^^^^^^
@@ -178,7 +194,7 @@ LL | 1;
178194
| ~
179195

180196
error: the `dbg!` macro is intended as a debugging tool
181-
--> tests/ui/dbg_macro/dbg_macro.rs:105:31
197+
--> tests/ui/dbg_macro/dbg_macro.rs:106:31
182198
|
183199
LL | println!("dbg: {:?}", dbg!(s));
184200
| ^^^^^^^
@@ -189,7 +205,7 @@ LL | println!("dbg: {:?}", s);
189205
| ~
190206

191207
error: the `dbg!` macro is intended as a debugging tool
192-
--> tests/ui/dbg_macro/dbg_macro.rs:107:22
208+
--> tests/ui/dbg_macro/dbg_macro.rs:108:22
193209
|
194210
LL | print!("{}", dbg!(s));
195211
| ^^^^^^^
@@ -199,5 +215,5 @@ help: remove the invocation before committing it to a version control system
199215
LL | print!("{}", s);
200216
| ~
201217

202-
error: aborting due to 18 previous errors
218+
error: aborting due to 19 previous errors
203219

0 commit comments

Comments
 (0)