Auto merge of #3723 - rhysd:issue3721, r=oli-obk

Implement dbg_macro rule

Fixes  #3721

This patch adds new `dbg_macro` rule to check `dbg!` macro use.

Since this is my first patch to clippy, I'm not confident about following points:

- ~~Currently only checks `dbg!` span. Is it possible to check if the `dbg!` macro is provided by standard library or user-defined? If it's possible, I can make the check more strict.~~ Resolved as #3723 (comment)
- ~~Is category `style` correct for this rule?~~'restriction' is used instead
- ~~Should I use `span_lint_and_sugg` instead of `span_lint`? Currently entire message is put as `msg`.  But later part `ensure to avoid having uses of it in version control` may be put as suggestion.~~ Done
- I'm not native English speaker. The message and doc may not be natural as English.

Author: bors

CHANGELOG.md

@@ -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

README.md

@@ -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 295 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 296 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:
 

clippy_lints/src/dbg_macro.rs

@@ -0,0 +1,78 @@
+use crate::utils::{snippet_opt, span_help_and_lint, span_lint_and_sugg};
+use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_errors::Applicability;
+use syntax::ast;
+use syntax::source_map::Span;
+use syntax::tokenstream::TokenStream;
+
+/// **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" {
+            if let Some(sugg) = tts_span(mac.node.tts.clone()).and_then(|span| snippet_opt(cx, span)) {
+                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,
+                );
+            } else {
+                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",
+                );
+            }
+        }
+    }
+}
+
+// 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)
+}

clippy_lints/src/lib.rs

@@ -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;
@@ -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 {
@@ -495,6 +497,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,

tests/ui/dbg_macro.rs

@@ -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);
+    dbg!(dbg!(dbg!(42)));
+    foo(3) + dbg!(factorial(4));
+}

tests/ui/dbg_macro.stderr

@@ -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
+