Merge pull request #2136 from ykrivopalov/identity_op_fixing

Identity/erasing operation lints

Author: oli-obk

clippy_lints/src/erasing_op.rs

@@ -0,0 +1,63 @@
+use consts::{constant_simple, Constant};
+use rustc::hir::*;
+use rustc::lint::*;
+use syntax::codemap::Span;
+use utils::{in_macro, span_lint};
+
+/// **What it does:** Checks for erasing operations, e.g. `x * 0`.
+///
+/// **Why is this bad?** The whole expression can be replaced by zero.
+/// This is most likely not the intended outcome and should probably be
+/// corrected
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// 0 / x; 0 * x; x & 0
+/// ```
+declare_lint! {
+    pub ERASING_OP,
+    Warn,
+    "using erasing operations, e.g. `x * 0` or `y & 0`"
+}
+
+#[derive(Copy, Clone)]
+pub struct ErasingOp;
+
+impl LintPass for ErasingOp {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(ERASING_OP)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ErasingOp {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
+        if in_macro(e.span) {
+            return;
+        }
+        if let ExprBinary(ref cmp, ref left, ref right) = e.node {
+            match cmp.node {
+                BiMul | BiBitAnd => {
+                    check(cx, left, e.span);
+                    check(cx, right, e.span);
+                },
+                BiDiv => check(cx, left, e.span),
+                _ => (),
+            }
+        }
+    }
+}
+
+fn check(cx: &LateContext, e: &Expr, span: Span) {
+    if let Some(Constant::Int(v)) = constant_simple(cx, e) {
+        if v.to_u128_unchecked() == 0 {
+            span_lint(
+                cx,
+                ERASING_OP,
+                span,
+                "this operation will always return zero. This is likely not the intended outcome",
+            );
+        }
+    }
+}

clippy_lints/src/identity_op.rs

@@ -1,9 +1,9 @@
 use consts::{constant_simple, Constant};
-use rustc::lint::*;
 use rustc::hir::*;
+use rustc::lint::*;
+use rustc_const_math::ConstInt;
 use syntax::codemap::Span;
 use utils::{in_macro, snippet, span_lint};
-use syntax::attr::IntType::{SignedInt, UnsignedInt};
 
 /// **What it does:** Checks for identity operations, e.g. `x + 0`.
 ///
@@ -58,15 +58,28 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp {
     }
 }
 
+fn all_ones(v: &ConstInt) -> bool {
+    match *v {
+        ConstInt::I8(i) => i == !0,
+        ConstInt::I16(i) => i == !0,
+        ConstInt::I32(i) => i == !0,
+        ConstInt::I64(i) => i == !0,
+        ConstInt::I128(i) => i == !0,
+        ConstInt::U8(i) => i == !0,
+        ConstInt::U16(i) => i == !0,
+        ConstInt::U32(i) => i == !0,
+        ConstInt::U64(i) => i == !0,
+        ConstInt::U128(i) => i == !0,
+        _ => false
+    }
+}
+
 #[allow(cast_possible_wrap)]
 fn check(cx: &LateContext, e: &Expr, m: i8, span: Span, arg: Span) {
     if let Some(Constant::Int(v)) = constant_simple(cx, e) {
         if match m {
             0 => v.to_u128_unchecked() == 0,
-            -1 => match v.int_type() {
-                SignedInt(_) => (v.to_u128_unchecked() as i128 == -1),
-                UnsignedInt(_) => false,
-            },
+            -1 => all_ones(&v),
             1 => v.to_u128_unchecked() == 1,
             _ => unreachable!(),
         } {

clippy_lints/src/lib.rs

@@ -88,6 +88,7 @@ pub mod entry;
 pub mod enum_clike;
 pub mod enum_glob_use;
 pub mod enum_variants;
+pub mod erasing_op;
 pub mod eq_op;
 pub mod escape;
 pub mod eta_reduction;
@@ -253,6 +254,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_early_lint_pass(box needless_continue::NeedlessContinue);
     reg.register_late_lint_pass(box eta_reduction::EtaPass);
     reg.register_late_lint_pass(box identity_op::IdentityOp);
+    reg.register_late_lint_pass(box erasing_op::ErasingOp);
     reg.register_early_lint_pass(box items_after_statements::ItemsAfterStatements);
     reg.register_late_lint_pass(box mut_mut::MutMut);
     reg.register_late_lint_pass(box mut_reference::UnnecessaryMutPassed);

tests/ui/bit_masks.stderr

@@ -6,6 +6,14 @@ error: &-masking with zero
    |
    = note: `-D bad-bit-mask` implied by `-D warnings`
 
+error: this operation will always return zero. This is likely not the intended outcome
+  --> $DIR/bit_masks.rs:12:5
+   |
+12 |     x & 0 == 0;
+   |     ^^^^^
+   |
+   = note: `-D erasing-op` implied by `-D warnings`
+
 error: incompatible bit mask: `_ & 2` can never be equal to `1`
   --> $DIR/bit_masks.rs:15:5
    |
@@ -48,6 +56,12 @@ error: &-masking with zero
 35 |     0 & x == 0;
    |     ^^^^^^^^^^
 
+error: this operation will always return zero. This is likely not the intended outcome
+  --> $DIR/bit_masks.rs:35:5
+   |
+35 |     0 & x == 0;
+   |     ^^^^^
+
 error: incompatible bit mask: `_ | 2` will always be higher than `1`
   --> $DIR/bit_masks.rs:39:5
    |

tests/ui/erasing_op.rs

@@ -0,0 +1,12 @@
+
+
+
+#[allow(no_effect)]
+#[warn(erasing_op)]
+fn main() {
+    let x: u8 = 0;
+
+    x * 0;
+    0 & x;
+    0 / x;
+}

tests/ui/erasing_op.stderr

@@ -0,0 +1,20 @@
+error: this operation will always return zero. This is likely not the intended outcome
+ --> $DIR/erasing_op.rs:9:5
+  |
+9 |     x * 0;
+  |     ^^^^^
+  |
+  = note: `-D erasing-op` implied by `-D warnings`
+
+error: this operation will always return zero. This is likely not the intended outcome
+  --> $DIR/erasing_op.rs:10:5
+   |
+10 |     0 & x;
+   |     ^^^^^
+
+error: this operation will always return zero. This is likely not the intended outcome
+  --> $DIR/erasing_op.rs:11:5
+   |
+11 |     0 / x;
+   |     ^^^^^
+

tests/ui/identity_op.rs

@@ -27,4 +27,7 @@ fn main() {
 
     x & NEG_ONE;  //no error, as we skip lookups (for now)
     -1 & x;
+
+    let u : u8 = 0;
+    u & 255;
 }

tests/ui/identity_op.stderr

@@ -42,3 +42,9 @@ error: the operation is ineffective. Consider reducing it to `x`
 29 |     -1 & x;
    |     ^^^^^^
 
+error: the operation is ineffective. Consider reducing it to `u`
+  --> $DIR/identity_op.rs:32:5
+   |
+32 |     u & 255;
+   |     ^^^^^^^
+