new lint: double_neg

gnieto · Jun 29, 2016 · a12e839 · a12e839
1 parent d6e3fa8
commit a12e839
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,7 +2,7 @@
 All notable changes to this project will be documented in this file.
 
 ## 0.0.78 - TBA
-* New lints: [`wrong_transmute`]
+* New lints: [`wrong_transmute`, `double_neg`]
 * For compatibility, `cargo clippy` does not defines the `clippy` feature
   introduced in 0.0.76 anymore
 * [`collapsible_if`] now considers `if let`
@@ -153,6 +153,7 @@ All notable changes to this project will be documented in this file.
 [`deprecated_semver`]: https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver
 [`derive_hash_xor_eq`]: https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq
 [`doc_markdown`]: https://github.com/Manishearth/rust-clippy/wiki#doc_markdown
+[`double_neg`]: https://github.com/Manishearth/rust-clippy/wiki#double_neg
 [`drop_ref`]: https://github.com/Manishearth/rust-clippy/wiki#drop_ref
 [`duplicate_underscore_argument`]: https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument
 [`empty_loop`]: https://github.com/Manishearth/rust-clippy/wiki#empty_loop

diff --git a/README.md b/README.md
@@ -17,7 +17,7 @@ Table of contents:
 
 ## Lints
 
-There are 156 lints included in this crate:
+There are 157 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -49,6 +49,7 @@ name
 [deprecated_semver](https://github.com/Manishearth/rust-clippy/wiki#deprecated_semver)                               | warn    | `Warn` on `#[deprecated(since = "x")]` where x is not semver
 [derive_hash_xor_eq](https://github.com/Manishearth/rust-clippy/wiki#derive_hash_xor_eq)                             | warn    | deriving `Hash` but implementing `PartialEq` explicitly
 [doc_markdown](https://github.com/Manishearth/rust-clippy/wiki#doc_markdown)                                         | warn    | checks for the presence of `_`, `::` or camel-case outside ticks in documentation
+[double_neg](https://github.com/Manishearth/rust-clippy/wiki#double_neg)                                             | warn    | --x is a double negation of x and not a pre-decrement as in C or C++
 [drop_ref](https://github.com/Manishearth/rust-clippy/wiki#drop_ref)                                                 | warn    | call to `std::mem::drop` with a reference instead of an owned value, which will not call the `Drop::drop` method on the underlying value
 [duplicate_underscore_argument](https://github.com/Manishearth/rust-clippy/wiki#duplicate_underscore_argument)       | warn    | Function arguments having names which only differ by an underscore
 [empty_loop](https://github.com/Manishearth/rust-clippy/wiki#empty_loop)                                             | warn    | empty `loop {}` detected

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -369,6 +369,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         misc::MODULO_ONE,
         misc::REDUNDANT_PATTERN,
         misc::TOPLEVEL_REF_ARG,
+        misc_early::DOUBLE_NEG,
         misc_early::DUPLICATE_UNDERSCORE_ARGUMENT,
         misc_early::REDUNDANT_CLOSURE_CALL,
         misc_early::UNNEEDED_FIELD_PATTERN,

diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs
@@ -40,12 +40,25 @@ declare_lint! {
     "Closures should not be called in the expression they are defined"
 }
 
+/// **What it does:** This lint detects expressions of the form `--x`
+///
+/// **Why is this bad?** It can mislead C/C++ programmers to think `x` was decremented.
+///
+/// **Known problems:** None.
+///
+/// **Example:** `--x;`
+declare_lint! {
+    pub DOUBLE_NEG, Warn,
+    "--x is a double negation of x and not a pre-decrement as in C or C++"
+}
+
+
 #[derive(Copy, Clone)]
 pub struct MiscEarly;
 
 impl LintPass for MiscEarly {
     fn get_lints(&self) -> LintArray {
-        lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL)
+        lint_array!(UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, REDUNDANT_CLOSURE_CALL, DOUBLE_NEG)
     }
 }
 
@@ -126,21 +139,32 @@ impl EarlyLintPass for MiscEarly {
     }
 
     fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
-        if let ExprKind::Call(ref paren, _) = expr.node {
-            if let ExprKind::Paren(ref closure) = paren.node {
-                if let ExprKind::Closure(_, ref decl, ref block, _) = closure.node {
-                    span_lint_and_then(cx,
-                                       REDUNDANT_CLOSURE_CALL,
-                                       expr.span,
-                                       "Try not to call a closure in the expression where it is declared.",
-                                       |db| {
-                                           if decl.inputs.is_empty() {
-                                               let hint = format!("{}", snippet(cx, block.span, ".."));
-                                               db.span_suggestion(expr.span, "Try doing something like: ", hint);
-                                           }
-                                       });
+        match expr.node {
+            ExprKind::Call(ref paren, _) => {
+                if let ExprKind::Paren(ref closure) = paren.node {
+                    if let ExprKind::Closure(_, ref decl, ref block, _) = closure.node {
+                        span_lint_and_then(cx,
+                                           REDUNDANT_CLOSURE_CALL,
+                                           expr.span,
+                                           "Try not to call a closure in the expression where it is declared.",
+                                           |db| {
+                                               if decl.inputs.is_empty() {
+                                                   let hint = format!("{}", snippet(cx, block.span, ".."));
+                                                   db.span_suggestion(expr.span, "Try doing something like: ", hint);
+                                               }
+                                           });
+                    }
                 }
             }
+            ExprKind::Unary(UnOp::Neg, ref inner) => {
+                if let ExprKind::Unary(UnOp::Neg, _) = inner.node {
+                    span_lint(cx,
+                              DOUBLE_NEG,
+                              expr.span,
+                              "`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op");
+    }
+            }
+            _ => ()
         }
     }
 

diff --git a/tests/compile-fail/double_neg.rs b/tests/compile-fail/double_neg.rs
@@ -0,0 +1,10 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[deny(double_neg)]
+fn main() {
+    let x = 1;
+    -x;
+    -(-x);
+    --x; //~ERROR: `--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op
+}