Skip to content

Fix dbg_macro semi span calculation #11743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 2, 2023
Merged

Fix dbg_macro semi span calculation #11743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 4 additions & 29 deletions clippy_lints/src/dbg_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_errors::Applicability;
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, BytePos, Pos, Span};
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
Expand All @@ -31,31 +31,6 @@ 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 Down Expand Up @@ -88,10 +63,10 @@ impl LateLintPass<'_> for DbgMacro {
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())
if let Some(Node::Stmt(_)) = cx.tcx.hir().find_parent(expr.hir_id)
&& let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
{
(span, String::new())
(macro_call.span.to(semi_span), String::new())
} else {
(macro_call.span, String::from("()"))
}
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/dbg_macro/auxiliary/submodule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn f() {
dbg!();
}
4 changes: 3 additions & 1 deletion tests/ui/dbg_macro.rs → tests/ui/dbg_macro/dbg_macro.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

#![warn(clippy::dbg_macro)]

#[path = "auxiliary/submodule.rs"]
mod submodule;

fn foo(n: u32) -> u32 {
if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
//~^ ERROR: the `dbg!` macro is intended as a debugging tool
//~| NOTE: `-D clippy::dbg-macro` implied by `-D warnings`
}
fn bar(_: ()) {}

Expand Down
54 changes: 33 additions & 21 deletions tests/ui/dbg_macro.stderr → tests/ui/dbg_macro/dbg_macro.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:6:22
--> $DIR/auxiliary/submodule.rs:2:5
|
LL | if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
| ^^^^^^^^^^^^^^^^^^^^^^
LL | dbg!();
| ^^^^^^^
|
= note: `-D clippy::dbg-macro` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::dbg_macro)]`
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:9:22
|
LL | if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n }
| ^^^^^^^^^^^^^^^^^^^^^^
|
help: remove the invocation before committing it to a version control system
|
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:13:8
--> $DIR/dbg_macro.rs:15:8
|
LL | if dbg!(n <= 1) {
| ^^^^^^^^^^^^
Expand All @@ -23,7 +35,7 @@ LL | if n <= 1 {
| ~~~~~~

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

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

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

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

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

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

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

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

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:56:13
--> $DIR/dbg_macro.rs:58:13
|
LL | let _ = dbg!();
| ^^^^^^
Expand All @@ -123,7 +135,7 @@ LL | let _ = ();
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:58:9
--> $DIR/dbg_macro.rs:60:9
|
LL | bar(dbg!());
| ^^^^^^
Expand All @@ -134,7 +146,7 @@ LL | bar(());
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:60:10
--> $DIR/dbg_macro.rs:62:10
|
LL | foo!(dbg!());
| ^^^^^^
Expand All @@ -145,7 +157,7 @@ LL | foo!(());
| ~~

error: the `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:62:16
--> $DIR/dbg_macro.rs:64:16
|
LL | foo2!(foo!(dbg!()));
| ^^^^^^
Expand All @@ -156,7 +168,7 @@ LL | foo2!(foo!(()));
| ~~

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

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

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

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

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