Skip to content

Commit 815a07e

Browse files
borsbors
committed
Auto merge of #10725 - y21:issue9914, r=Jarcho
don't remove `dbg!` in arbitrary expressions Fixes #9914 The `dbg_macro` lint replaces empty `dbg!` invocations with the empty string in its suggestion, which is not always valid code in certain contexts (e.g. `let _ = dbg!();` becomes `let _ = ;`). This PR changes it to `()`, which should always be valid where `dbg!()` is valid (`dbg!()` with no arguments evaluates to `()`). It also special-cases "standalone" `dbg!();` expression statements, where it will suggest removing the whole statement entirely like it did before. changelog: [`dbg_macro`]: don't remove `dbg!()` in arbitrary expressions as it sometimes results in syntax errors
2 parents cf182b9 + f0be0ee commit 815a07e

File tree

3 files changed

+143
-23
lines changed

3 files changed

+143
-23
lines changed

‎clippy_lints/src/dbg_macro.rs‎

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ use clippy_utils::macros::root_macro_call_first_node;
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;
6-
use rustc_hir::{Expr, ExprKind};
7-
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_hir::{Expr, ExprKind, Node};
7+
use rustc_lint::{LateContext, LateLintPass, LintContext};
88
use rustc_session::{declare_tool_lint, impl_lint_pass};
9-
use rustc_span::sym;
9+
use rustc_span::{sym, BytePos, Pos, Span};
1010

1111
declare_clippy_lint! {
1212
/// ### What it does
@@ -31,6 +31,31 @@ declare_clippy_lint! {
3131
"`dbg!` macro is intended as a debugging tool"
3232
}
3333

34+
/// Gets the span of the statement up to the next semicolon, if and only if the next
35+
/// non-whitespace character actually is a semicolon.
36+
/// E.g.
37+
/// ```rust,ignore
38+
///
39+
/// dbg!();
40+
/// ^^^^^^^ this span is returned
41+
///
42+
/// foo!(dbg!());
43+
/// no span is returned
44+
/// ```
45+
fn span_including_semi(cx: &LateContext<'_>, span: Span) -> Option<Span> {
46+
let sm = cx.sess().source_map();
47+
let sf = sm.lookup_source_file(span.hi());
48+
let src = sf.src.as_ref()?.get(span.hi().to_usize()..)?;
49+
let first_non_whitespace = src.find(|c: char| !c.is_whitespace())?;
50+
51+
if src.as_bytes()[first_non_whitespace] == b';' {
52+
let hi = span.hi() + BytePos::from_usize(first_non_whitespace + 1);
53+
Some(span.with_hi(hi))
54+
} else {
55+
None
56+
}
57+
}
58+
3459
#[derive(Copy, Clone)]
3560
pub struct DbgMacro {
3661
allow_dbg_in_tests: bool,
@@ -55,13 +80,25 @@ impl LateLintPass<'_> for DbgMacro {
5580
return;
5681
}
5782
let mut applicability = Applicability::MachineApplicable;
58-
let suggestion = match expr.peel_drop_temps().kind {
83+
84+
let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
5985
// dbg!()
60-
ExprKind::Block(_, _) => String::new(),
61-
// dbg!(1)
62-
ExprKind::Match(val, ..) => {
63-
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string()
86+
ExprKind::Block(..) => {
87+
// If the `dbg!` macro is a "free" statement and not contained within other expressions,
88+
// remove the whole statement.
89+
if let Some(Node::Stmt(stmt)) = cx.tcx.hir().find_parent(expr.hir_id)
90+
&& let Some(span) = span_including_semi(cx, stmt.span.source_callsite())
91+
{
92+
(span, String::new())
93+
} else {
94+
(macro_call.span, String::from("()"))
95+
}
6496
},
97+
// dbg!(1)
98+
ExprKind::Match(val, ..) => (
99+
macro_call.span,
100+
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string(),
101+
),
65102
// dbg!(2, 3)
66103
ExprKind::Tup(
67104
[
@@ -82,15 +119,15 @@ impl LateLintPass<'_> for DbgMacro {
82119
"..",
83120
&mut applicability,
84121
);
85-
format!("({snippet})")
122+
(macro_call.span, format!("({snippet})"))
86123
},
87124
_ => return,
88125
};
89126

90127
span_lint_and_sugg(
91128
cx,
92129
DBG_MACRO,
93-
macro_call.span,
130+
sugg_span,
94131
"the `dbg!` macro is intended as a debugging tool",
95132
"remove the invocation before committing it to a version control system",
96133
suggestion,

‎tests/ui/dbg_macro.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
fn foo(n: u32) -> u32 {
55
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
66
}
7+
fn bar(_: ()) {}
78

89
fn factorial(n: u32) -> u32 {
910
if dbg!(n <= 1) {
@@ -21,6 +22,32 @@ fn main() {
2122
dbg!(1, 2, 3, 4, 5);
2223
}
2324

25+
fn issue9914() {
26+
macro_rules! foo {
27+
($x:expr) => {
28+
$x;
29+
};
30+
}
31+
macro_rules! foo2 {
32+
($x:expr) => {
33+
$x;
34+
};
35+
}
36+
macro_rules! expand_to_dbg {
37+
() => {
38+
dbg!();
39+
};
40+
}
41+
42+
dbg!();
43+
#[allow(clippy::let_unit_value)]
44+
let _ = dbg!();
45+
bar(dbg!());
46+
foo!(dbg!());
47+
foo2!(foo!(dbg!()));
48+
expand_to_dbg!();
49+
}
50+
2451
mod issue7274 {
2552
trait Thing<'b> {
2653
fn foo(&self);

‎tests/ui/dbg_macro.stderr‎

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ LL | if let Some(n) = n.checked_sub(4) { n } else { n }
1111
| ~~~~~~~~~~~~~~~~
1212

1313
error: the `dbg!` macro is intended as a debugging tool
14-
--> $DIR/dbg_macro.rs:9:8
14+
--> $DIR/dbg_macro.rs:10:8
1515
|
1616
LL | if dbg!(n <= 1) {
1717
| ^^^^^^^^^^^^
@@ -22,7 +22,7 @@ LL | if n <= 1 {
2222
| ~~~~~~
2323

2424
error: the `dbg!` macro is intended as a debugging tool
25-
--> $DIR/dbg_macro.rs:10:9
25+
--> $DIR/dbg_macro.rs:11:9
2626
|
2727
LL | dbg!(1)
2828
| ^^^^^^^
@@ -33,7 +33,7 @@ LL | 1
3333
|
3434

3535
error: the `dbg!` macro is intended as a debugging tool
36-
--> $DIR/dbg_macro.rs:12:9
36+
--> $DIR/dbg_macro.rs:13:9
3737
|
3838
LL | dbg!(n * factorial(n - 1))
3939
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -44,7 +44,7 @@ LL | n * factorial(n - 1)
4444
|
4545

4646
error: the `dbg!` macro is intended as a debugging tool
47-
--> $DIR/dbg_macro.rs:17:5
47+
--> $DIR/dbg_macro.rs:18:5
4848
|
4949
LL | dbg!(42);
5050
| ^^^^^^^^
@@ -55,7 +55,7 @@ LL | 42;
5555
| ~~
5656

5757
error: the `dbg!` macro is intended as a debugging tool
58-
--> $DIR/dbg_macro.rs:18:5
58+
--> $DIR/dbg_macro.rs:19:5
5959
|
6060
LL | dbg!(dbg!(dbg!(42)));
6161
| ^^^^^^^^^^^^^^^^^^^^
@@ -66,7 +66,7 @@ LL | dbg!(dbg!(42));
6666
| ~~~~~~~~~~~~~~
6767

6868
error: the `dbg!` macro is intended as a debugging tool
69-
--> $DIR/dbg_macro.rs:19:14
69+
--> $DIR/dbg_macro.rs:20:14
7070
|
7171
LL | foo(3) + dbg!(factorial(4));
7272
| ^^^^^^^^^^^^^^^^^^
@@ -77,7 +77,7 @@ LL | foo(3) + factorial(4);
7777
| ~~~~~~~~~~~~
7878

7979
error: the `dbg!` macro is intended as a debugging tool
80-
--> $DIR/dbg_macro.rs:20:5
80+
--> $DIR/dbg_macro.rs:21:5
8181
|
8282
LL | dbg!(1, 2, dbg!(3, 4));
8383
| ^^^^^^^^^^^^^^^^^^^^^^
@@ -88,7 +88,7 @@ LL | (1, 2, dbg!(3, 4));
8888
| ~~~~~~~~~~~~~~~~~~
8989

9090
error: the `dbg!` macro is intended as a debugging tool
91-
--> $DIR/dbg_macro.rs:21:5
91+
--> $DIR/dbg_macro.rs:22:5
9292
|
9393
LL | dbg!(1, 2, 3, 4, 5);
9494
| ^^^^^^^^^^^^^^^^^^^
@@ -99,7 +99,63 @@ LL | (1, 2, 3, 4, 5);
9999
| ~~~~~~~~~~~~~~~
100100

101101
error: the `dbg!` macro is intended as a debugging tool
102-
--> $DIR/dbg_macro.rs:41:9
102+
--> $DIR/dbg_macro.rs:42:5
103+
|
104+
LL | dbg!();
105+
| ^^^^^^^
106+
|
107+
help: remove the invocation before committing it to a version control system
108+
|
109+
LL - dbg!();
110+
LL +
111+
|
112+
113+
error: the `dbg!` macro is intended as a debugging tool
114+
--> $DIR/dbg_macro.rs:44:13
115+
|
116+
LL | let _ = dbg!();
117+
| ^^^^^^
118+
|
119+
help: remove the invocation before committing it to a version control system
120+
|
121+
LL | let _ = ();
122+
| ~~
123+
124+
error: the `dbg!` macro is intended as a debugging tool
125+
--> $DIR/dbg_macro.rs:45:9
126+
|
127+
LL | bar(dbg!());
128+
| ^^^^^^
129+
|
130+
help: remove the invocation before committing it to a version control system
131+
|
132+
LL | bar(());
133+
| ~~
134+
135+
error: the `dbg!` macro is intended as a debugging tool
136+
--> $DIR/dbg_macro.rs:46:10
137+
|
138+
LL | foo!(dbg!());
139+
| ^^^^^^
140+
|
141+
help: remove the invocation before committing it to a version control system
142+
|
143+
LL | foo!(());
144+
| ~~
145+
146+
error: the `dbg!` macro is intended as a debugging tool
147+
--> $DIR/dbg_macro.rs:47:16
148+
|
149+
LL | foo2!(foo!(dbg!()));
150+
| ^^^^^^
151+
|
152+
help: remove the invocation before committing it to a version control system
153+
|
154+
LL | foo2!(foo!(()));
155+
| ~~
156+
157+
error: the `dbg!` macro is intended as a debugging tool
158+
--> $DIR/dbg_macro.rs:68:9
103159
|
104160
LL | dbg!(2);
105161
| ^^^^^^^
@@ -110,7 +166,7 @@ LL | 2;
110166
| ~
111167

112168
error: the `dbg!` macro is intended as a debugging tool
113-
--> $DIR/dbg_macro.rs:47:5
169+
--> $DIR/dbg_macro.rs:74:5
114170
|
115171
LL | dbg!(1);
116172
| ^^^^^^^
@@ -121,7 +177,7 @@ LL | 1;
121177
| ~
122178

123179
error: the `dbg!` macro is intended as a debugging tool
124-
--> $DIR/dbg_macro.rs:52:5
180+
--> $DIR/dbg_macro.rs:79:5
125181
|
126182
LL | dbg!(1);
127183
| ^^^^^^^
@@ -132,7 +188,7 @@ LL | 1;
132188
| ~
133189

134190
error: the `dbg!` macro is intended as a debugging tool
135-
--> $DIR/dbg_macro.rs:58:9
191+
--> $DIR/dbg_macro.rs:85:9
136192
|
137193
LL | dbg!(1);
138194
| ^^^^^^^
@@ -142,5 +198,5 @@ help: remove the invocation before committing it to a version control system
142198
LL | 1;
143199
| ~
144200

145-
error: aborting due to 13 previous errors
201+
error: aborting due to 18 previous errors
146202

0 commit comments

Comments
 (0)