Skip to content

Commit 12ecaa8

Browse files
borsbors
committed
Auto merge of #12482 - J-ZhengLi:issue12131, r=Jarcho
fix [`dbg_macro`] FN when dbg is inside some complex macros fixes: #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)
2 parents 59a5ad4 + fe4e0ac commit 12ecaa8

File tree

9 files changed

+344
-106
lines changed

9 files changed

+344
-106
lines changed

‎clippy_lints/src/dbg_macro.rs‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::macros::root_macro_call_first_node;
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};
5+
use rustc_data_structures::fx::FxHashSet;
56
use rustc_errors::Applicability;
6-
use rustc_hir::{Expr, ExprKind, Node};
7+
use rustc_hir::{Expr, ExprKind, HirId, Node};
78
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::lint::in_external_macro;
810
use rustc_session::impl_lint_pass;
9-
use rustc_span::sym;
11+
use rustc_span::{sym, Span, SyntaxContext};
1012

1113
declare_clippy_lint! {
1214
/// ### What it does
@@ -31,31 +33,38 @@ declare_clippy_lint! {
3133
"`dbg!` macro is intended as a debugging tool"
3234
}
3335

34-
#[derive(Copy, Clone)]
36+
#[derive(Clone)]
3537
pub struct DbgMacro {
3638
allow_dbg_in_tests: bool,
39+
/// Tracks the `dbg!` macro callsites that are already checked.
40+
checked_dbg_call_site: FxHashSet<Span>,
41+
/// Tracks the previous `SyntaxContext`, to avoid walking the same context chain.
42+
prev_ctxt: SyntaxContext,
3743
}
3844

3945
impl_lint_pass!(DbgMacro => [DBG_MACRO]);
4046

4147
impl DbgMacro {
4248
pub fn new(allow_dbg_in_tests: bool) -> Self {
43-
DbgMacro { allow_dbg_in_tests }
49+
DbgMacro {
50+
allow_dbg_in_tests,
51+
checked_dbg_call_site: FxHashSet::default(),
52+
prev_ctxt: SyntaxContext::root(),
53+
}
4454
}
4555
}
4656

4757
impl LateLintPass<'_> for DbgMacro {
4858
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
49-
let Some(macro_call) = root_macro_call_first_node(cx, expr) else {
50-
return;
51-
};
52-
if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) {
59+
let cur_syntax_ctxt = expr.span.ctxt();
60+
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) &&
5365
// 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-
}
66+
!(self.allow_dbg_in_tests && is_in_test(cx, expr.hir_id))
67+
{
5968
let mut applicability = Applicability::MachineApplicable;
6069

6170
let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
@@ -101,6 +110,8 @@ impl LateLintPass<'_> for DbgMacro {
101110
_ => return,
102111
};
103112

113+
self.prev_ctxt = cur_syntax_ctxt;
114+
104115
span_lint_and_sugg(
105116
cx,
106117
DBG_MACRO,
@@ -112,4 +123,16 @@ impl LateLintPass<'_> for DbgMacro {
112123
);
113124
}
114125
}
126+
127+
fn check_crate_post(&mut self, _: &LateContext<'_>) {
128+
self.checked_dbg_call_site = FxHashSet::default();
129+
}
130+
}
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))
115138
}

‎tests/ui-toml/dbg_macro/dbg_macro.fixed‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//@compile-flags: --test
2+
#![warn(clippy::dbg_macro)]
3+
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
4+
5+
fn foo(n: u32) -> u32 {
6+
if let Some(n) = n.checked_sub(4) { n } else { n }
7+
}
8+
9+
fn factorial(n: u32) -> u32 {
10+
if n <= 1 {
11+
1
12+
} else {
13+
n * factorial(n - 1)
14+
}
15+
}
16+
17+
fn main() {
18+
42;
19+
foo(3) + factorial(4);
20+
(1, 2, 3, 4, 5);
21+
}
22+
23+
#[test]
24+
pub fn issue8481() {
25+
dbg!(1);
26+
}
27+
28+
#[cfg(test)]
29+
fn foo2() {
30+
dbg!(1);
31+
}
32+
33+
#[cfg(test)]
34+
mod mod1 {
35+
fn func() {
36+
dbg!(1);
37+
}
38+
}

‎tests/ui-toml/dbg_macro/dbg_macro.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//@compile-flags: --test
22
#![warn(clippy::dbg_macro)]
3-
//@no-rustfix
3+
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
4+
45
fn foo(n: u32) -> u32 {
56
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
67
}
@@ -15,9 +16,7 @@ fn factorial(n: u32) -> u32 {
1516

1617
fn main() {
1718
dbg!(42);
18-
dbg!(dbg!(dbg!(42)));
1919
foo(3) + dbg!(factorial(4));
20-
dbg!(1, 2, dbg!(3, 4));
2120
dbg!(1, 2, 3, 4, 5);
2221
}
2322

‎tests/ui-toml/dbg_macro/dbg_macro.stderr‎

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: the `dbg!` macro is intended as a debugging tool
2-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:5:22
2+
--> tests/ui-toml/dbg_macro/dbg_macro.rs:6:22
33
|
44
LL | if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
55
| ^^^^^^^^^^^^^^^^^^^^^^
@@ -12,7 +12,7 @@ LL | if let Some(n) = n.checked_sub(4) { n } else { n }
1212
| ~~~~~~~~~~~~~~~~
1313

1414
error: the `dbg!` macro is intended as a debugging tool
15-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:9:8
15+
--> tests/ui-toml/dbg_macro/dbg_macro.rs:10:8
1616
|
1717
LL | if dbg!(n <= 1) {
1818
| ^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL | if n <= 1 {
2323
| ~~~~~~
2424

2525
error: the `dbg!` macro is intended as a debugging tool
26-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:10:9
26+
--> tests/ui-toml/dbg_macro/dbg_macro.rs:11:9
2727
|
2828
LL | dbg!(1)
2929
| ^^^^^^^
@@ -34,7 +34,7 @@ LL | 1
3434
|
3535

3636
error: the `dbg!` macro is intended as a debugging tool
37-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:12:9
37+
--> tests/ui-toml/dbg_macro/dbg_macro.rs:13:9
3838
|
3939
LL | dbg!(n * factorial(n - 1))
4040
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -45,7 +45,7 @@ LL | n * factorial(n - 1)
4545
|
4646

4747
error: the `dbg!` macro is intended as a debugging tool
48-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:17:5
48+
--> tests/ui-toml/dbg_macro/dbg_macro.rs:18:5
4949
|
5050
LL | dbg!(42);
5151
| ^^^^^^^^
@@ -55,17 +55,6 @@ help: remove the invocation before committing it to a version control system
5555
LL | 42;
5656
| ~~
5757

58-
error: the `dbg!` macro is intended as a debugging tool
59-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:18:5
60-
|
61-
LL | dbg!(dbg!(dbg!(42)));
62-
| ^^^^^^^^^^^^^^^^^^^^
63-
|
64-
help: remove the invocation before committing it to a version control system
65-
|
66-
LL | dbg!(dbg!(42));
67-
| ~~~~~~~~~~~~~~
68-
6958
error: the `dbg!` macro is intended as a debugging tool
7059
--> tests/ui-toml/dbg_macro/dbg_macro.rs:19:14
7160
|
@@ -80,17 +69,6 @@ LL | foo(3) + factorial(4);
8069
error: the `dbg!` macro is intended as a debugging tool
8170
--> tests/ui-toml/dbg_macro/dbg_macro.rs:20:5
8271
|
83-
LL | dbg!(1, 2, dbg!(3, 4));
84-
| ^^^^^^^^^^^^^^^^^^^^^^
85-
|
86-
help: remove the invocation before committing it to a version control system
87-
|
88-
LL | (1, 2, dbg!(3, 4));
89-
| ~~~~~~~~~~~~~~~~~~
90-
91-
error: the `dbg!` macro is intended as a debugging tool
92-
--> tests/ui-toml/dbg_macro/dbg_macro.rs:21:5
93-
|
9472
LL | dbg!(1, 2, 3, 4, 5);
9573
| ^^^^^^^^^^^^^^^^^^^
9674
|
@@ -99,5 +77,5 @@ help: remove the invocation before committing it to a version control system
9977
LL | (1, 2, 3, 4, 5);
10078
| ~~~~~~~~~~~~~~~
10179

102-
error: aborting due to 9 previous errors
80+
error: aborting due to 7 previous errors
10381

‎tests/ui/dbg_macro/dbg_macro.fixed‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
#![warn(clippy::dbg_macro)]
2+
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
3+
4+
fn foo(n: u32) -> u32 {
5+
if let Some(n) = n.checked_sub(4) { n } else { n }
6+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
7+
}
8+
fn bar(_: ()) {}
9+
10+
fn factorial(n: u32) -> u32 {
11+
if n <= 1 {
12+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
13+
1
14+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
15+
} else {
16+
n * factorial(n - 1)
17+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
18+
}
19+
}
20+
21+
fn main() {
22+
42;
23+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
24+
foo(3) + factorial(4);
25+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
26+
(1, 2, 3, 4, 5);
27+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
28+
}
29+
30+
fn issue9914() {
31+
macro_rules! foo {
32+
($x:expr) => {
33+
$x;
34+
};
35+
}
36+
macro_rules! foo2 {
37+
($x:expr) => {
38+
$x;
39+
};
40+
}
41+
macro_rules! expand_to_dbg {
42+
() => {
43+
44+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
45+
};
46+
}
47+
48+
49+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
50+
#[allow(clippy::let_unit_value)]
51+
let _ = ();
52+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
53+
bar(());
54+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
55+
foo!(());
56+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
57+
foo2!(foo!(()));
58+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
59+
expand_to_dbg!();
60+
}
61+
62+
mod issue7274 {
63+
trait Thing<'b> {
64+
fn foo(&self);
65+
}
66+
67+
macro_rules! define_thing {
68+
($thing:ident, $body:expr) => {
69+
impl<'a> Thing<'a> for $thing {
70+
fn foo<'b>(&self) {
71+
$body
72+
}
73+
}
74+
};
75+
}
76+
77+
struct MyThing;
78+
define_thing!(MyThing, {
79+
2;
80+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
81+
});
82+
}
83+
84+
#[test]
85+
pub fn issue8481() {
86+
1;
87+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
88+
}
89+
90+
#[cfg(test)]
91+
fn foo2() {
92+
1;
93+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
94+
}
95+
96+
#[cfg(test)]
97+
mod mod1 {
98+
fn func() {
99+
1;
100+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
101+
}
102+
}
103+
104+
mod issue12131 {
105+
fn dbg_in_print(s: &str) {
106+
println!("dbg: {:?}", s);
107+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
108+
print!("{}", s);
109+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
110+
}
111+
}

‎tests/ui/dbg_macro/dbg_macro.rs‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
1-
//@no-rustfix
2-
31
#![warn(clippy::dbg_macro)]
4-
5-
#[path = "auxiliary/submodule.rs"]
6-
mod submodule;
2+
#![allow(clippy::unnecessary_operation, clippy::no_effect)]
73

84
fn foo(n: u32) -> u32 {
95
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
@@ -25,12 +21,8 @@ fn factorial(n: u32) -> u32 {
2521
fn main() {
2622
dbg!(42);
2723
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
28-
dbg!(dbg!(dbg!(42)));
29-
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
3024
foo(3) + dbg!(factorial(4));
3125
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
32-
dbg!(1, 2, dbg!(3, 4));
33-
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
3426
dbg!(1, 2, 3, 4, 5);
3527
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
3628
}
@@ -49,6 +41,7 @@ fn issue9914() {
4941
macro_rules! expand_to_dbg {
5042
() => {
5143
dbg!();
44+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
5245
};
5346
}
5447

@@ -107,3 +100,12 @@ mod mod1 {
107100
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
108101
}
109102
}
103+
104+
mod issue12131 {
105+
fn dbg_in_print(s: &str) {
106+
println!("dbg: {:?}", dbg!(s));
107+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
108+
print!("{}", dbg!(s));
109+
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
110+
}
111+
}

0 commit comments

Comments
 (0)