From ef0c933550937ed0db47fb9da9f0aa32e75c865b Mon Sep 17 00:00:00 2001 From: Tim Neumann Date: Sun, 30 Aug 2015 17:32:35 +0200 Subject: [PATCH] add precedence_negative_literal lint --- README.md | 2 +- src/lib.rs | 5 ++- src/misc.rs | 44 --------------------- src/precedence.rs | 65 ++++++++++++++++++++++++++++++++ tests/compile-fail/precedence.rs | 10 +++++ 5 files changed, 79 insertions(+), 47 deletions(-) create mode 100644 src/precedence.rs diff --git a/README.md b/README.md index 4d5c59f2aef..1a1b6fe5956 100644 --- a/README.md +++ b/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`) diff --git a/src/lib.rs b/src/lib.rs index 5e9205e32f9..c72c5b1d7a7 100755 --- a/src/lib.rs +++ b/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, diff --git a/src/misc.rs b/src/misc.rs index 6e438407216..ef9f4248c0c 100644 --- a/src/misc.rs +++ b/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()`"); diff --git a/src/precedence.rs b/src/precedence.rs new file mode 100644 index 00000000000..1d89adf9df8 --- /dev/null +++ b/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 + } +} diff --git a/tests/compile-fail/precedence.rs b/tests/compile-fail/precedence.rs index ebb25f61b75..71dcd493008 100755 --- a/tests/compile-fail/precedence.rs +++ b/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()); }