diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index cea712f2fee..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, Node, Stmt, StmtKind}; +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, Span}; +use rustc_span::{sym, BytePos, Pos, Span}; declare_clippy_lint! { /// ### What it does @@ -31,9 +31,29 @@ declare_clippy_lint! { "`dbg!` macro is intended as a debugging tool" } -fn span_including_semi(cx: &LateContext<'_>, span: Span) -> Span { - let span = cx.sess().source_map().span_extend_to_next_char(span, ';', true); - span.with_hi(span.hi() + rustc_span::BytePos(1)) +/// 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)] @@ -62,16 +82,17 @@ impl LateLintPass<'_> for DbgMacro { let mut applicability = Applicability::MachineApplicable; let (sugg_span, suggestion) = match expr.peel_drop_temps().kind { - ExprKind::Block(..) => match cx.tcx.hir().find_parent(expr.hir_id) { - // dbg!() as a standalone statement, suggest removing the whole statement entirely - Some(Node::Stmt( - stmt @ Stmt { - kind: StmtKind::Semi(_), - .. - }, - )) => (span_including_semi(cx, stmt.span.source_callsite()), String::new()), - // empty dbg!() in arbitrary position (e.g. `foo(dbg!())`), suggest replacing with `foo(())` - _ => (macro_call.span, String::from("()")), + // dbg!() + 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, ..) => ( diff --git a/tests/ui/dbg_macro.rs b/tests/ui/dbg_macro.rs index ae2075e7dad..10788d40481 100644 --- a/tests/ui/dbg_macro.rs +++ b/tests/ui/dbg_macro.rs @@ -23,10 +23,29 @@ fn main() { } 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 { diff --git a/tests/ui/dbg_macro.stderr b/tests/ui/dbg_macro.stderr index 34329a06b97..530e7663317 100644 --- a/tests/ui/dbg_macro.stderr +++ b/tests/ui/dbg_macro.stderr @@ -99,7 +99,7 @@ LL | (1, 2, 3, 4, 5); | ~~~~~~~~~~~~~~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:26:5 + --> $DIR/dbg_macro.rs:42:5 | LL | dbg!(); | ^^^^^^^ @@ -111,7 +111,7 @@ LL + | error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:28:13 + --> $DIR/dbg_macro.rs:44:13 | LL | let _ = dbg!(); | ^^^^^^ @@ -122,7 +122,7 @@ LL | let _ = (); | ~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:29:9 + --> $DIR/dbg_macro.rs:45:9 | LL | bar(dbg!()); | ^^^^^^ @@ -133,7 +133,29 @@ LL | bar(()); | ~~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:49:9 + --> $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); | ^^^^^^^ @@ -144,7 +166,7 @@ LL | 2; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:55:5 + --> $DIR/dbg_macro.rs:74:5 | LL | dbg!(1); | ^^^^^^^ @@ -155,7 +177,7 @@ LL | 1; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:60:5 + --> $DIR/dbg_macro.rs:79:5 | LL | dbg!(1); | ^^^^^^^ @@ -166,7 +188,7 @@ LL | 1; | ~ error: the `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:66:9 + --> $DIR/dbg_macro.rs:85:9 | LL | dbg!(1); | ^^^^^^^ @@ -176,5 +198,5 @@ help: remove the invocation before committing it to a version control system LL | 1; | ~ -error: aborting due to 16 previous errors +error: aborting due to 18 previous errors