Merge pull request #190 from Manishearth/fix-189

Fix 189

src/consts.rs

@@ -53,7 +53,7 @@ impl Constant {
 }
 
 /// a Lit_-like enum to fold constant `Expr`s into
-#[derive(PartialEq, Eq, Debug, Clone)]
+#[derive(PartialEq, Eq, Debug, Clone)] //TODO: A better PartialEq, remove Eq
 pub enum ConstantVariant {
     /// a String "abc"
     ConstantStr(String, StrStyle),
@@ -332,10 +332,12 @@ fn constant_binop(cx: &Context, op: BinOp, left: &Expr, right: &Expr)
         //BiBitOr,
         //BiShl,
         //BiShr,
-        //BiEq,
+        BiEq => constant_binop_apply(cx, left, right,
+            |l, r| Some(ConstantBool(l == r))),
         //BiLt,
         //BiLe,
-        //BiNe,
+        BiNe => constant_binop_apply(cx, left, right,
+            |l, r| Some(ConstantBool(l != r))),
         //BiGe,
         //BiGt,
         _ => None,

src/identity_op.rs

@@ -1,10 +1,8 @@
 use rustc::lint::*;
-use rustc::middle::const_eval::lookup_const_by_id;
-use rustc::middle::def::*;
 use syntax::ast::*;
 use syntax::codemap::Span;
 
-use consts::{constant, Constant, is_negative};
+use consts::{constant, is_negative};
 use consts::ConstantVariant::ConstantInt;
 use utils::{span_lint, snippet};
 
@@ -51,8 +49,8 @@ fn check(cx: &Context, e: &Expr, m: i8, span: Span, arg: Span) {
         if let ConstantInt(v, ty) = c.constant {
             if match m {
                 0 => v == 0,
-                -1 => is_negative(ty),
-                1 => !is_negative(ty),
+                -1 => is_negative(ty) && v == 1,
+                1 => !is_negative(ty) && v == 1,
                 _ => unreachable!(),
             } {
                 span_lint(cx, IDENTITY_OP, span, &format!(

src/ranges.rs

@@ -2,7 +2,7 @@ use rustc::lint::{Context, LintArray, LintPass};
 use rustc::middle::ty::TypeVariants::TyStruct;
 use syntax::ast::*;
 use syntax::codemap::Spanned;
-use utils::{match_def_path, walk_ptrs_ty};
+use utils::{match_def_path};
 
 declare_lint! {
     pub RANGE_STEP_BY_ZERO, Warn,

tests/compile-fail/identity_op.rs

@@ -10,7 +10,10 @@ fn main() {
     let x = 0;
 
     x + 0;        //~ERROR the operation is ineffective
+    x + (1 - 1);  //~ERROR the operation is ineffective
+    x + 1;
     0 + x;        //~ERROR the operation is ineffective
+    1 + x;
     x - ZERO;     //no error, as we skip lookups (for now)
     x | (0);      //~ERROR the operation is ineffective
     ((ZERO)) | x; //no error, as we skip lookups (for now)
@@ -19,6 +22,8 @@ fn main() {
     1 * x;        //~ERROR the operation is ineffective
     x / ONE;      //no error, as we skip lookups (for now)
 
+    x / 2;        //no false positive
+
     x & NEG_ONE;  //no error, as we skip lookups (for now)
     -1 & x;       //~ERROR the operation is ineffective
 }

tests/consts.rs

@@ -5,7 +5,7 @@ extern crate clippy;
 extern crate syntax;
 extern crate rustc;
 
-use clippy::consts::constant;
+use clippy::consts::{constant, ConstantVariant};
 use clippy::consts::ConstantVariant::*;
 use syntax::ast::*;
 use syntax::ptr::P;
@@ -14,21 +14,30 @@ use std::mem;
 use rustc::lint::Context;
 
 fn ctx() -> &'static Context<'static, 'static> {
-	unsafe { 
-		let x : *const Context<'static, 'static> = std::ptr::null();
-		mem::transmute(x)
-	}
+    unsafe {
+        let x : *const Context<'static, 'static> = std::ptr::null();
+        mem::transmute(x)
+    }
+}
+
+fn lit(l: Lit_) -> Expr {
+    Expr{
+        id: 1,
+        node: ExprLit(P(Spanned{
+            node: l,
+            span: COMMAND_LINE_SP,
+        })),
+        span: COMMAND_LINE_SP,
+    }
+}
+
+fn check(expect: ConstantVariant, expr: &Expr) {
+    assert_eq!(Some(expect), constant(ctx(), expr).map(|x| x.constant))
 }
 
 #[test]
 fn test_lit() {
-    assert_eq!(Some(ConstantBool(true)), constant(ctx(),
-        &Expr{ 
-			id: 1, 
-			node: ExprLit(P(Spanned{ 
-				node: LitBool(true), 
-				span: COMMAND_LINE_SP,
-			})), 
-			span: COMMAND_LINE_SP,
-		}).map(|x| x.constant));
+    check(ConstantBool(true), &lit(LitBool(true)));
+    check(ConstantBool(false), &lit(LitBool(false)));
+
 }