From 17bcf0e86519fa6147dec7271660e278cda98404 Mon Sep 17 00:00:00 2001
From: llogiq <bogusandre@gmail.com>
Date: Wed, 6 May 2015 12:59:08 +0200
Subject: [PATCH] New lint: precedence, see issue #41

---
 README.md                        |  1 +
 src/lib.rs                       |  2 ++
 src/misc.rs                      | 46 +++++++++++++++++++++++++++++++-
 tests/compile-fail/precedence.rs | 15 +++++++++++
 4 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 tests/compile-fail/precedence.rs

diff --git a/README.md b/README.md
index 3014a1a3ecd..7d3e3fd8c65 100644
--- a/README.md
+++ b/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:
 
diff --git a/src/lib.rs b/src/lib.rs
index 5b2f71bb4fb..ffee122f776 100644
--- a/src/lib.rs
+++ b/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,
                                            ]);
 }
diff --git a/src/misc.rs b/src/misc.rs
index cdc39a4b819..eae6d518950 100644
--- a/src/misc.rs
+++ b/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
+	}
+}
diff --git a/tests/compile-fail/precedence.rs b/tests/compile-fail/precedence.rs
new file mode 100644
index 00000000000..7969be0f371
--- /dev/null
+++ b/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
+
+}