Skip to content

Commit

Permalink
Auto merge of #10725 - y21:issue9914, r=Jarcho
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@bors
bors committed May 18, 2023
2 parents cf182b9 + f0be0ee commit 815a07e
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 23 deletions.
57 changes: 47 additions & 10 deletions clippy_lints/src/dbg_macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use clippy_utils::macros::root_macro_call_first_node;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{is_in_cfg_test, is_in_test_function};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_hir::{Expr, ExprKind, Node};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;
use rustc_span::{sym, BytePos, Pos, Span};

declare_clippy_lint! {
/// ### What it does
Expand All @@ -31,6 +31,31 @@ declare_clippy_lint! {
"`dbg!` macro is intended as a debugging tool"
}

/// Gets the span of the statement up to the next semicolon, if and only if the next
/// non-whitespace character actually is a semicolon.
/// E.g.
/// ```rust,ignore
///
/// dbg!();
/// ^^^^^^^ this span is returned
///
/// foo!(dbg!());
/// no span is returned
/// ```
fn span_including_semi(cx: &LateContext<'_>, span: Span) -> Option<Span> {
let sm = cx.sess().source_map();
let sf = sm.lookup_source_file(span.hi());
let src = sf.src.as_ref()?.get(span.hi().to_usize()..)?;
let first_non_whitespace = src.find(|c: char| !c.is_whitespace())?;

if src.as_bytes()[first_non_whitespace] == b';' {
let hi = span.hi() + BytePos::from_usize(first_non_whitespace + 1);
Some(span.with_hi(hi))
} else {
None
}
}

#[derive(Copy, Clone)]
pub struct DbgMacro {
allow_dbg_in_tests: bool,
Expand All @@ -55,13 +80,25 @@ impl LateLintPass<'_> for DbgMacro {
return;
}
let mut applicability = Applicability::MachineApplicable;
let suggestion = match expr.peel_drop_temps().kind {

let (sugg_span, suggestion) = match expr.peel_drop_temps().kind {
// dbg!()
ExprKind::Block(_, _) => String::new(),
// dbg!(1)
ExprKind::Match(val, ..) => {
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability).to_string()
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(stmt)) = cx.tcx.hir().find_parent(expr.hir_id)
&& let Some(span) = span_including_semi(cx, stmt.span.source_callsite())
{
(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(
[
Expand All @@ -82,15 +119,15 @@ impl LateLintPass<'_> for DbgMacro {
"..",
&mut applicability,
);
format!("({snippet})")
(macro_call.span, format!("({snippet})"))
},
_ => return,
};

span_lint_and_sugg(
cx,
DBG_MACRO,
macro_call.span,
sugg_span,
"the `dbg!` macro is intended as a debugging tool",
"remove the invocation before committing it to a version control system",
suggestion,
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/dbg_macro.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
fn foo(n: u32) -> u32 {
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
}
fn bar(_: ()) {}

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

fn issue9914() {
macro_rules! foo {
($x:expr) => {
$x;
};
}
macro_rules! foo2 {
($x:expr) => {
$x;
};
}
macro_rules! expand_to_dbg {
() => {
dbg!();
};
}

dbg!();
#[allow(clippy::let_unit_value)]
let _ = dbg!();
bar(dbg!());
foo!(dbg!());
foo2!(foo!(dbg!()));
expand_to_dbg!();
}

mod issue7274 {
trait Thing<'b> {
fn foo(&self);
Expand Down
82 changes: 69 additions & 13 deletions tests/ui/dbg_macro.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ LL | if let Some(n) = n.checked_sub(4) { n } else { n }
| ~~~~~~~~~~~~~~~~

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

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:10:9
--> $DIR/dbg_macro.rs:11:9
|
LL | dbg!(1)
| ^^^^^^^
Expand All @@ -33,7 +33,7 @@ LL | 1
|

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

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

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

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

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

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

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:41:9
--> $DIR/dbg_macro.rs:42:5
|
LL | dbg!();
| ^^^^^^^
|
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
--> $DIR/dbg_macro.rs:44:13
|
LL | let _ = dbg!();
| ^^^^^^
|
help: remove the invocation before committing it to a version control system
|
LL | let _ = ();
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:45:9
|
LL | bar(dbg!());
| ^^^^^^
|
help: remove the invocation before committing it to a version control system
|
LL | bar(());
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:46:10
|
LL | foo!(dbg!());
| ^^^^^^
|
help: remove the invocation before committing it to a version control system
|
LL | foo!(());
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:47:16
|
LL | foo2!(foo!(dbg!()));
| ^^^^^^
|
help: remove the invocation before committing it to a version control system
|
LL | foo2!(foo!(()));
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:68:9
|
LL | dbg!(2);
| ^^^^^^^
Expand All @@ -110,7 +166,7 @@ LL | 2;
| ~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:47:5
--> $DIR/dbg_macro.rs:74:5
|
LL | dbg!(1);
| ^^^^^^^
Expand All @@ -121,7 +177,7 @@ LL | 1;
| ~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:52:5
--> $DIR/dbg_macro.rs:79:5
|
LL | dbg!(1);
| ^^^^^^^
Expand All @@ -132,7 +188,7 @@ LL | 1;
| ~

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

error: aborting due to 13 previous errors
error: aborting due to 18 previous errors

0 comments on commit 815a07e

Please sign in to comment.