diff --git a/README.md b/README.md index 3ea3659dd..47747d86e 100644 --- a/README.md +++ b/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: diff --git a/src/identity_op.rs b/src/identity_op.rs new file mode 100644 index 000000000..ec4495539 --- /dev/null +++ b/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 + } +} diff --git a/src/lib.rs b/src/lib.rs index 1655faaf8..c9585d2eb 100644 --- a/src/lib.rs +++ b/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, ]); } diff --git a/tests/compile-fail/bit_masks.rs b/tests/compile-fail/bit_masks.rs index d2646d258..e45b78980 100644 --- a/tests/compile-fail/bit_masks.rs +++ b/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; diff --git a/tests/compile-fail/eq_op.rs b/tests/compile-fail/eq_op.rs index 910b5e848..07b15625b 100644 --- a/tests/compile-fail/eq_op.rs +++ b/tests/compile-fail/eq_op.rs @@ -6,6 +6,7 @@ fn id(x: X) -> X { } #[deny(eq_op)] +#[allow(identity_op)] fn main() { // simple values and comparisons 1 == 1; //~ERROR diff --git a/tests/compile-fail/identity_op.rs b/tests/compile-fail/identity_op.rs new file mode 100644 index 000000000..6c17d30fa --- /dev/null +++ b/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 +}