add precedence_negative_literal lint

README.md

@@ -39,7 +39,7 @@ name
 [needless_return](https://github.com/Manishearth/rust-clippy/wiki#needless_return)                   | warn    | using a return statement like `return expr;` where an expression would suffice
 [non_ascii_literal](https://github.com/Manishearth/rust-clippy/wiki#non_ascii_literal)               | allow   | using any literal non-ASCII chars in a string literal; suggests using the \\u escape instead
 [option_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#option_unwrap_used)             | allow   | using `Option.unwrap()`, which should at least get a better message using `expect()`
-[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence)                             | warn    | expressions where precedence may trip up the unwary reader of the source; suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`
+[precedence](https://github.com/Manishearth/rust-clippy/wiki#precedence)                             | warn    | catches operations where precedence may be unclear. See the wiki for a list of cases caught
 [ptr_arg](https://github.com/Manishearth/rust-clippy/wiki#ptr_arg)                                   | allow   | fn arguments of the type `&Vec<...>` or `&String`, suggesting to use `&[...]` or `&str` instead, respectively
 [range_step_by_zero](https://github.com/Manishearth/rust-clippy/wiki#range_step_by_zero)             | warn    | using Range::step_by(0), which produces an infinite iterator
 [redundant_closure](https://github.com/Manishearth/rust-clippy/wiki#redundant_closure)               | warn    | using redundant closures, i.e. `|a| foo(a)` (which can be written as just `foo`)

src/lib.rs

@@ -40,6 +40,7 @@ pub mod lifetimes;
 pub mod loops;
 pub mod ranges;
 pub mod matches;
+pub mod precedence;
 
 #[plugin_registrar]
 pub fn plugin_registrar(reg: &mut Registry) {
@@ -52,7 +53,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_pass(box precedence::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_pass(box mut_mut::MutMut as LintPassObject);
@@ -109,10 +110,10 @@ pub fn plugin_registrar(reg: &mut Registry) {
         misc::CMP_OWNED,
         misc::FLOAT_CMP,
         misc::MODULO_ONE,
-        misc::PRECEDENCE,
         misc::TOPLEVEL_REF_ARG,
         mut_mut::MUT_MUT,
         needless_bool::NEEDLESS_BOOL,
+        precedence::PRECEDENCE,
         ptr_arg::PTR_ARG,
         ranges::RANGE_STEP_BY_ZERO,
         returns::LET_AND_RETURN,

src/misc.rs

@@ -109,50 +109,6 @@ fn is_float(cx: &Context, expr: &Expr) -> bool {
     }
 }
 
-declare_lint!(pub PRECEDENCE, Warn,
-              "expressions where precedence may trip up the unwary reader of the source; \
-               suggests adding parentheses, e.g. `x << 2 + y` will be parsed as `x << (2 + y)`");
-
-#[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) && (is_arith_expr(left) || is_arith_expr(right)) {
-                span_lint(cx, PRECEDENCE, expr.span,
-                    "operator precedence can trip the unwary. Consider adding parentheses \
-                     to the subexpression");
-            }
-        }
-    }
-}
-
-fn is_arith_expr(expr : &Expr) -> bool {
-    match expr.node {
-        ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op),
-        _ => false
-    }
-}
-
-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
-    }
-}
-
 declare_lint!(pub CMP_OWNED, Warn,
               "creating owned instances for comparing with others, e.g. `x == \"foo\".to_string()`");
 

src/precedence.rs

@@ -0,0 +1,65 @@
+use rustc::lint::*;
+use syntax::ast::*;
+use syntax::codemap::Spanned;
+
+use utils::span_lint;
+
+declare_lint!(pub PRECEDENCE, Warn,
+              "catches operations where precedence may be unclear. See the wiki for a \
+               list of cases caught");
+
+#[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) && (is_arith_expr(left) || is_arith_expr(right)) {
+                span_lint(cx, PRECEDENCE, expr.span,
+                    "operator precedence can trip the unwary. Consider adding parentheses \
+                     to the subexpression");
+            }
+        }
+
+        if let ExprUnary(UnNeg, ref rhs) = expr.node {
+            if let ExprMethodCall(_, _, ref args) = rhs.node {
+                if let Some(slf) = args.first() {
+                    if let ExprLit(ref lit) = slf.node {
+                        match lit.node {
+                            LitInt(..) | LitFloat(..) | LitFloatUnsuffixed(..) =>
+                                span_lint(cx, PRECEDENCE, expr.span,
+                                    "unary minus has lower precedence than method call. Consider \
+                                     adding parentheses to clarify your intent"),
+                                _ => ()
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+fn is_arith_expr(expr : &Expr) -> bool {
+    match expr.node {
+        ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op),
+        _ => false
+    }
+}
+
+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

@@ -13,4 +13,14 @@ fn main() {
     format!("{} vs. {}", 3 | 2 - 1, (3 | 2) - 1);   //~ERROR operator precedence can trip
     format!("{} vs. {}", 3 & 5 - 2, (3 & 5) - 2);   //~ERROR operator precedence can trip
 
+    format!("{} vs. {}", -1i32.abs(), (-1i32).abs()); //~ERROR unary minus has lower precedence
+    format!("{} vs. {}", -1f32.abs(), (-1f32).abs()); //~ERROR unary minus has lower precedence
+
+    // These should not trigger an error
+    let _ = (-1i32).abs();
+    let _ = (-1f32).abs();
+    let _ = -(1i32).abs();
+    let _ = -(1f32).abs();
+    let _ = -(1i32.abs());
+    let _ = -(1f32.abs());
 }