diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index 799e71e847a..ea17e7a6071 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -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 @@ -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 { + 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, @@ -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( [ @@ -82,7 +119,7 @@ impl LateLintPass<'_> for DbgMacro { "..", &mut applicability, ); - format!("({snippet})") + (macro_call.span, format!("({snippet})")) }, _ => return, }; @@ -90,7 +127,7 @@ impl LateLintPass<'_> for DbgMacro { 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, diff --git a/tests/ui/dbg_macro.rs b/tests/ui/dbg_macro.rs index 8701e3cd29f..10788d40481 100644 --- a/tests/ui/dbg_macro.rs +++ b/tests/ui/dbg_macro.rs @@ -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) { @@ -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); diff --git a/tests/ui/dbg_macro.stderr b/tests/ui/dbg_macro.stderr index ddb5f1342e9..530e7663317 100644 --- a/tests/ui/dbg_macro.stderr +++ b/tests/ui/dbg_macro.stderr @@ -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) { | ^^^^^^^^^^^^ @@ -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) | ^^^^^^^ @@ -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)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -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); | ^^^^^^^^ @@ -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))); | ^^^^^^^^^^^^^^^^^^^^ @@ -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)); | ^^^^^^^^^^^^^^^^^^ @@ -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)); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -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); | ^^^^^^^^^^^^^^^^^^^ @@ -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); | ^^^^^^^ @@ -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); | ^^^^^^^ @@ -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); | ^^^^^^^ @@ -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); | ^^^^^^^ @@ -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