add neg_multiply lint (#862)

add neg_multiply lint

CHANGELOG.md

@@ -145,6 +145,7 @@ All notable changes to this project will be documented in this file.
 [`needless_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop
 [`needless_return`]: https://github.com/Manishearth/rust-clippy/wiki#needless_return
 [`needless_update`]: https://github.com/Manishearth/rust-clippy/wiki#needless_update
+[`neg_multiply`]: https://github.com/Manishearth/rust-clippy/wiki#neg_multiply
 [`new_ret_no_self`]: https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self
 [`new_without_default`]: https://github.com/Manishearth/rust-clippy/wiki#new_without_default
 [`no_effect`]: https://github.com/Manishearth/rust-clippy/wiki#no_effect

README.md

@@ -14,7 +14,7 @@ Table of contents:
 * [License](#license)
 
 ##Lints
-There are 141 lints included in this crate:
+There are 142 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -95,6 +95,7 @@ name
 [needless_range_loop](https://github.com/Manishearth/rust-clippy/wiki#needless_range_loop)                           | warn    | for-looping over a range of indices where an iterator over items would do
 [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return)                                   | warn    | using a return statement like `return expr;` where an expression would suffice
 [needless_update](https://github.com/Manishearth/rust-clippy/wiki#needless_update)                                   | warn    | using `{ ..base }` when there are no missing fields
+[neg_multiply](https://github.com/Manishearth/rust-clippy/wiki#neg_multiply)                                         | warn    | Warns on multiplying integers with -1
 [new_ret_no_self](https://github.com/Manishearth/rust-clippy/wiki#new_ret_no_self)                                   | warn    | not returning `Self` in a `new` method
 [new_without_default](https://github.com/Manishearth/rust-clippy/wiki#new_without_default)                           | warn    | `fn new() -> Self` method without `Default` implementation
 [no_effect](https://github.com/Manishearth/rust-clippy/wiki#no_effect)                                               | warn    | statements with no effect

src/consts.rs

@@ -177,8 +177,9 @@ impl PartialOrd for Constant {
     }
 }
 
+/// parse a `LitKind` to a `Constant`
 #[allow(cast_possible_wrap)]
-fn lit_to_constant(lit: &LitKind) -> Constant {
+pub fn lit_to_constant(lit: &LitKind) -> Constant {
     match *lit {
         LitKind::Str(ref is, style) => Constant::Str(is.to_string(), style),
         LitKind::Byte(b) => Constant::Int(ConstInt::U8(b)),

src/lib.rs

@@ -88,6 +88,7 @@ pub mod mut_reference;
 pub mod mutex_atomic;
 pub mod needless_bool;
 pub mod needless_update;
+pub mod neg_multiply;
 pub mod new_without_default;
 pub mod no_effect;
 pub mod non_expressive_names;
@@ -232,6 +233,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names));
     reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
     reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents));
+    reg.register_late_lint_pass(box neg_multiply::NegMultiply);
 
     reg.register_lint_group("clippy_pedantic", vec![
         array_indexing::INDEXING_SLICING,
@@ -343,6 +345,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         needless_bool::BOOL_COMPARISON,
         needless_bool::NEEDLESS_BOOL,
         needless_update::NEEDLESS_UPDATE,
+        neg_multiply::NEG_MULTIPLY,
         new_without_default::NEW_WITHOUT_DEFAULT,
         no_effect::NO_EFFECT,
         non_expressive_names::MANY_SINGLE_CHAR_NAMES,

src/neg_multiply.rs

@@ -0,0 +1,57 @@
+use rustc::hir::*;
+use rustc::lint::*;
+use syntax::codemap::{Span, Spanned};
+
+use consts::{self, Constant};
+use utils::span_lint;
+
+/// **What it does:** Checks for multiplication by -1 as a form of negation.
+///
+/// **Why is this bad?** It's more readable to just negate.
+///
+/// **Known problems:** This only catches integers (for now)
+///
+/// **Example:** `x * -1`
+declare_lint! {
+    pub NEG_MULTIPLY,
+    Warn,
+    "Warns on multiplying integers with -1"
+}
+
+#[derive(Copy, Clone)]
+pub struct NegMultiply;
+
+impl LintPass for NegMultiply {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(NEG_MULTIPLY)
+    }
+}
+
+#[allow(match_same_arms)]
+impl LateLintPass for NegMultiply {
+    fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
+        if let ExprBinary(Spanned { node: BiMul, .. }, ref l, ref r) = e.node {
+            match (&l.node, &r.node) {
+                (&ExprUnary(..), &ExprUnary(..)) => (),
+                (&ExprUnary(UnNeg, ref lit), _) => check_mul(cx, e.span, lit, r),
+                (_, &ExprUnary(UnNeg, ref lit)) => check_mul(cx, e.span, lit, l),
+                _ => ()
+            }
+        }
+    }
+}
+
+fn check_mul(cx: &LateContext, span: Span, lit: &Expr, exp: &Expr) {
+    if_let_chain!([
+        let ExprLit(ref l) = lit.node,
+        let Constant::Int(ref ci) = consts::lit_to_constant(&l.node),
+        let Some(val) = ci.to_u64(),
+        val == 1,
+        cx.tcx.expr_ty(exp).is_integral()
+    ], {
+        span_lint(cx, 
+                  NEG_MULTIPLY,
+                  span,
+                  "Negation by multiplying with -1");
+    })
+}

tests/compile-fail/neg_multiply.rs

@@ -0,0 +1,40 @@
+#![feature(plugin)]
+
+#![plugin(clippy)]
+#![deny(neg_multiply)]
+#![allow(no_effect)]
+
+use std::ops::Mul;
+
+struct X;
+
+impl Mul<isize> for X {
+    type Output = X;
+    
+    fn mul(self, _r: isize) -> Self {
+        self
+    }
+}
+
+impl Mul<X> for isize {
+    type Output = X;
+    
+    fn mul(self, _r: X) -> X {
+        X
+    }
+}
+
+fn main() {
+    let x = 0;
+
+    x * -1;
+    //~^ ERROR Negation by multiplying with -1
+
+    -1 * x;
+    //~^ ERROR Negation by multiplying with -1
+
+    -1 * -1; // should be ok
+    
+    X * -1; // should be ok
+    -1 * X; // should also be ok
+}