Merge pull request #48 from Manishearth/precedence

New lint: precedence, see issue #41

README.md

@@ -18,6 +18,7 @@ Lints included in this crate:
  - `approx_constant`: Warns if the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found and suggests to use the constant
  - `cmp_nan`: Denies comparisons to NAN (which will always return false, which is probably not intended)
  - `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)`
 
 To use, add the following lines to your Cargo.toml:
 

src/lib.rs

@@ -35,6 +35,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_lint_pass(box needless_bool::NeedlessBool as LintPassObject);
     reg.register_lint_pass(box approx_const::ApproxConstant as LintPassObject);
     reg.register_lint_pass(box misc::FloatCmp as LintPassObject);
+    reg.register_lint_pass(box misc::Precedence as LintPassObject);
     
     reg.register_lint_group("clippy", vec![types::BOX_VEC, types::LINKEDLIST,
                                            misc::SINGLE_MATCH, misc::STR_TO_STRING,
@@ -43,5 +44,6 @@ pub fn plugin_registrar(reg: &mut Registry) {
                                            needless_bool::NEEDLESS_BOOL,
                                            approx_const::APPROX_CONSTANT,
                                            misc::CMP_NAN, misc::FLOAT_CMP,
+                                           misc::PRECEDENCE,
                                            ]);
 }

src/misc.rs

@@ -5,7 +5,7 @@ use syntax::ast_util::{is_comparison_binop, binop_to_string};
 use syntax::visit::{FnKind};
 use rustc::lint::{Context, LintPass, LintArray, Lint, Level};
 use rustc::middle::ty::{self, expr_ty, ty_str, ty_ptr, ty_rptr, ty_float};
-use syntax::codemap::Span;
+use syntax::codemap::{Span, Spanned};
 
 
 use types::span_note_and_lint;
@@ -169,3 +169,47 @@ impl LintPass for FloatCmp {
 fn is_float(cx: &Context, expr: &Expr) -> bool {
 	if let ty_float(_) = walk_ty(expr_ty(cx.tcx, expr)).sty { true } else { false }
 }
+
+declare_lint!(pub PRECEDENCE, Warn,
+			  "Warn on mixing bit ops with integer arithmetic without parenthesis");
+			  
+#[derive(Copy,Clone)]
+pub struct Precedence;
+
+impl LintPass for Precedence {
+	fn get_lints(&self) -> LintArray {
+        lint_array!(PRECEDENCE)
+	}
+	
+	fn check_expr(&mut self, cx: &Context, expr: &Expr) {
+		if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node {
+			if is_bit_op(op) {
+				if let ExprBinary(Spanned { node: lop, ..}, _, _) = left.node {
+					if is_arith_op(lop) {
+						cx.span_lint(PRECEDENCE, expr.span, "Operator precedence can trip the unwary. Please consider adding parenthesis to the subexpression to make the meaning more clear.");
+					}
+				} else {
+					if let ExprBinary(Spanned { node: rop, ..}, _, _) = right.node {
+						if is_arith_op(rop) {
+							cx.span_lint(PRECEDENCE, expr.span, "Operator precedence can trip the unwary. Please consider adding parenthesis to the subexpression to make the meaning more clear.");
+						}
+					}
+				}
+			}
+		}
+	}
+}
+
+fn is_bit_op(op : BinOp_) -> bool {
+	match op {
+		BiBitXor | BiBitAnd | BiBitOr | BiShl | BiShr => true,
+		_ => false
+	}
+}
+
+fn is_arith_op(op : BinOp_) -> bool {
+	match op {
+		BiAdd | BiSub | BiMul | BiDiv | BiRem => true,
+		_ => false
+	}
+}

tests/compile-fail/precedence.rs

@@ -0,0 +1,15 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[deny(precedence)]
+#[allow(eq_op)]
+fn main() {
+	format!("{} vs. {}", 1 << 2 + 3, (1 << 2) + 3); //~ERROR
+    format!("{} vs. {}", 1 + 2 << 3, 1 + (2 << 3)); //~ERROR
+    format!("{} vs. {}", 4 >> 1 + 1, (4 >> 1) + 1); //~ERROR
+    format!("{} vs. {}", 1 + 3 >> 2, 1 + (3 >> 2)); //~ERROR
+    format!("{} vs. {}", 1 ^ 1 - 1, (1 ^ 1) - 1);   //~ERROR
+    format!("{} vs. {}", 3 | 2 - 1, (3 | 2) - 1);   //~ERROR
+    format!("{} vs. {}", 3 & 5 - 2, (3 & 5) - 2);   //~ERROR
+
+}