Skip to content
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.

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

Implement dbg_macro rule #3723

Merged
merged 11 commits into from
Feb 3, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -772,6 +772,7 @@ All notable changes to this project will be documented in this file.
[`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
[`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
[`cyclomatic_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cyclomatic_complexity
[`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro
[`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation
[`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const
[`default_trait_access`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_trait_access
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 294 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 295 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
81 changes: 81 additions & 0 deletions clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
use crate::utils::{span_help_and_lint, span_lint_and_sugg, snippet_opt};
use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
use rustc::{declare_tool_lint, lint_array};
use syntax::ast;
use rustc_errors::Applicability;
use syntax::tokenstream::TokenStream;
use syntax::source_map::Span;

/// **What it does:** Checks for usage of dbg!() macro.
///
/// **Why is this bad?** `dbg!` macro is intended as a debugging tool. It
/// should not be in version control.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust,ignore
/// // Bad
/// dbg!(true)
///
/// // Good
/// true
/// ```
declare_clippy_lint! {
pub DBG_MACRO,
restriction,
"`dbg!` macro is intended as a debugging tool"
}

#[derive(Copy, Clone, Debug)]
pub struct Pass;

impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DBG_MACRO)
}

fn name(&self) -> &'static str {
"DbgMacro"
}
}

impl EarlyLintPass for Pass {
fn check_mac(&mut self, cx: &EarlyContext<'_>, mac: &ast::Mac) {
if mac.node.path == "dbg" {
rhysd marked this conversation as resolved.
Show resolved Hide resolved
match tts_span(mac.node.tts.clone()).and_then(|span| snippet_opt(cx, span)) {
Some(sugg) => {
span_lint_and_sugg(
cx,
DBG_MACRO,
mac.span,
"`dbg!` macro is intended as a debugging tool",
"ensure to avoid having uses of it in version control",
sugg,
Applicability::MaybeIncorrect,
);
}
None => {
span_help_and_lint(
cx,
DBG_MACRO,
mac.span,
"`dbg!` macro is intended as a debugging tool",
"ensure to avoid having uses of it in version control",
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still used span_help_and_lint when snippet cannot be made from the token stream.

}
};
}
}
}

// Get span enclosing entire the token stream.
fn tts_span(tts: TokenStream) -> Option<Span> {
let mut cursor = tts.into_trees();
let first = cursor.next()?.span();
let span = match cursor.last() {
Some(tree) => first.to(tree.span()),
None => first,
};
Some(span)
}
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub mod const_static_lifetime;
pub mod copies;
pub mod copy_iterator;
pub mod cyclomatic_complexity;
pub mod dbg_macro;
pub mod default_trait_access;
pub mod derive;
pub mod doc;
Expand Down Expand Up @@ -231,6 +232,7 @@ pub fn register_pre_expansion_lints(
},
);
store.register_pre_expansion_pass(Some(session), true, false, box attrs::CfgAttrPass);
store.register_pre_expansion_pass(Some(session), true, false, box dbg_macro::Pass);
}

pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf {
Expand Down Expand Up @@ -494,6 +496,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
arithmetic::FLOAT_ARITHMETIC,
arithmetic::INTEGER_ARITHMETIC,
dbg_macro::DBG_MACRO,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
implicit_return::IMPLICIT_RETURN,
indexing_slicing::INDEXING_SLICING,
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/dbg_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![warn(clippy::dbg_macro)]

fn foo(n: u32) -> u32 {
if let Some(n) = dbg!(n.checked_sub(4)) {
n
} else {
n
}
}

fn factorial(n: u32) -> u32 {
if dbg!(n <= 1) {
dbg!(1)
} else {
dbg!(n * factorial(n - 1))
}
}

fn main() {
dbg!(42);
rhysd marked this conversation as resolved.
Show resolved Hide resolved
dbg!(dbg!(dbg!(42)));
foo(3) + dbg!(factorial(4));
}
74 changes: 74 additions & 0 deletions tests/ui/dbg_macro.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:4:22
|
LL | if let Some(n) = dbg!(n.checked_sub(4)) {
| ^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::dbg-macro` implied by `-D warnings`
help: ensure to avoid having uses of it in version control
|
LL | if let Some(n) = n.checked_sub(4) {
| ^^^^^^^^^^^^^^^^

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:12:8
|
LL | if dbg!(n <= 1) {
| ^^^^^^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | if n <= 1 {
| ^^^^^^

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:13:9
|
LL | dbg!(1)
| ^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | 1
|

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:15:9
|
LL | dbg!(n * factorial(n - 1))
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | n * factorial(n - 1)
|

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:20:5
|
LL | dbg!(42);
| ^^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | 42;
| ^^

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:21:5
|
LL | dbg!(dbg!(dbg!(42)));
| ^^^^^^^^^^^^^^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | dbg!(dbg!(42));
| ^^^^^^^^^^^^^^

error: `dbg!` macro is intended as a debugging tool
--> $DIR/dbg_macro.rs:22:14
|
LL | foo(3) + dbg!(factorial(4));
| ^^^^^^^^^^^^^^^^^^
help: ensure to avoid having uses of it in version control
|
LL | foo(3) + factorial(4);
| ^^^^^^^^^^^^

error: aborting due to 7 previous errors