Merge pull request #56 from Manishearth/identity_op

new lint: identity_op, refactored bit_masks a bit

README.md

@@ -21,6 +21,7 @@ Lints included in this crate:
  - `float_cmp`: Warns on `==` or `!=` comparisons of floaty typed values. As floating-point operations usually involve rounding errors, it is always better to check for approximate equality within some small bounds
  - `precedence`: Warns on expressions where precedence may trip up the unwary reader of the source and suggests adding parenthesis, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
  - `redundant_closure`: Warns on usage of eta-reducible closures like `|a| foo(a)` (which can be written as just `foo`)
+ - `identity_op`: Warns on identity operations like `x + 0` or `y / 1` (which can be reduced to `x` and `y`, respectively)
 
 To use, add the following lines to your Cargo.toml:
 

src/identity_op.rs

@@ -0,0 +1,82 @@
+use rustc::plugin::Registry;
+use rustc::lint::*;
+use rustc::middle::const_eval::lookup_const_by_id;
+use rustc::middle::def::*;
+use syntax::ast::*;
+use syntax::ast_util::{is_comparison_binop, binop_to_string};
+use syntax::ptr::P;
+use syntax::codemap::Span;
+
+declare_lint! { pub IDENTITY_OP, Warn,
+    "Warn on identity operations, e.g. '_ + 0'"}
+    
+#[derive(Copy,Clone)]
+pub struct IdentityOp;
+
+impl LintPass for IdentityOp {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(IDENTITY_OP)
+    }
+    
+    fn check_expr(&mut self, cx: &Context, e: &Expr) {
+		if let ExprBinary(ref cmp, ref left, ref right) = e.node {
+			match cmp.node {
+				BiAdd | BiBitOr | BiBitXor => {
+					check(cx, left, 0, e.span, right.span);
+					check(cx, right, 0, e.span, left.span);
+				},
+				BiShl | BiShr | BiSub => 
+					check(cx, right, 0, e.span, left.span),
+				BiMul => {
+					check(cx, left, 1, e.span, right.span);
+					check(cx, right, 1, e.span, left.span);
+				},
+				BiDiv =>
+					check(cx, right, 1, e.span, left.span),
+				BiBitAnd => {
+					check(cx, left, -1, e.span, right.span);
+					check(cx, right, -1, e.span, left.span);
+				},
+				_ => ()
+			}
+		}
+    }
+}
+
+
+fn check(cx: &Context, e: &Expr, m: i8, span: Span, arg: Span) {
+	if have_lit(cx, e, m) {
+		let map = cx.sess().codemap();
+		cx.span_lint(IDENTITY_OP, span, &format!(
+			"The operation is ineffective. Consider reducing it to '{}'", 
+			&*map.span_to_snippet(arg).unwrap_or("..".to_string())));
+	}
+}
+
+fn have_lit(cx: &Context, e : &Expr, m: i8) -> bool {
+	match &e.node {
+		&ExprUnary(UnNeg, ref litexp) => have_lit(cx, litexp, -m), 
+		&ExprLit(ref lit) => {
+			match (&lit.node, m) {
+				(&LitInt(0, _), 0) => true,
+				(&LitInt(1, SignedIntLit(_, Plus)), 1) => true,
+				(&LitInt(1, UnsuffixedIntLit(Plus)), 1) => true,
+				(&LitInt(1, SignedIntLit(_, Minus)), -1) => true,
+				(&LitInt(1, UnsuffixedIntLit(Minus)), -1) => true,
+				_ => false
+			}
+		},
+		&ExprParen(ref p) => have_lit(cx, p, m),
+		&ExprPath(_, _) => {
+                match cx.tcx.def_map.borrow().get(&e.id) {
+                    Some(&PathResolution { base_def: DefConst(def_id), ..}) => 
+						match lookup_const_by_id(cx.tcx, def_id, Option::None) {
+							Some(l) => have_lit(cx, l, m),
+							None => false
+						},
+                    _ => false
+                }
+            }
+		_ => false
+	}
+}

src/lib.rs

@@ -22,6 +22,7 @@ pub mod ptr_arg;
 pub mod needless_bool;
 pub mod approx_const;
 pub mod eta_reduction;
+pub mod identity_op;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -38,6 +39,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box misc::FloatCmp as LintPassObject);
     reg.register_lint_pass(box misc::Precedence as LintPassObject);
     reg.register_lint_pass(box eta_reduction::EtaPass as LintPassObject);
+    reg.register_lint_pass(box identity_op::IdentityOp as LintPassObject);
     
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
@@ -50,5 +52,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            misc::CMP_NAN, misc::FLOAT_CMP,
                                            misc::PRECEDENCE,
                                            eta_reduction::REDUNDANT_CLOSURE,
+                                           identity_op::IDENTITY_OP,
                                            ]);
 }

tests/compile-fail/bit_masks.rs

@@ -5,7 +5,7 @@ const THREE_BITS : i64 = 7;
 const EVEN_MORE_REDIRECTION : i64 = THREE_BITS;
 
 #[deny(bad_bit_mask)]
-#[allow(ineffective_bit_mask)]
+#[allow(ineffective_bit_mask, identity_op)]
 fn main() {
 	let x = 5;
 	

tests/compile-fail/eq_op.rs

@@ -6,6 +6,7 @@ fn id<X>(x: X) -> X {
 }
 
 #[deny(eq_op)]
+#[allow(identity_op)]
 fn main() {
 	// simple values and comparisons
 	1 == 1; //~ERROR

tests/compile-fail/identity_op.rs

@@ -0,0 +1,24 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+const ONE : i64 = 1;
+const NEG_ONE : i64 = -1;
+const ZERO : i64 = 0;
+
+#[deny(identity_op)]
+fn main() {
+	let x = 0;
+	
+	x + 0; //~ERROR
+	0 + x; //~ERROR
+	x - ZERO; //~ERROR
+	x | (0); //~ERROR
+	((ZERO)) | x; //~ERROR
+		
+	x * 1; //~ERROR
+	1 * x; //~ERROR
+	x / ONE; //~ERROR
+	
+	x & NEG_ONE; //~ERROR
+	-1 & x; //~ERROR
+}