diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index 3c4e2cef0448..3327482f09d5 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -38,12 +38,31 @@ declare_lint! { "assigning the result of an operation on a variable to that same variable" } +/// **What it does:** Check for `a op= a op b` or `a op= b op a` patterns. +/// +/// **Why is this bad?** Most likely these are bugs where one meant to write `a op= b` +/// +/// **Known problems:** Someone might actually mean `a op= a op b`, but that should rather be written as `a = (2 * a) op b` where applicable. +/// +/// **Example:** +/// +/// ```rust +/// let mut a = 5; +/// ... +/// a += a + b; +/// ``` +declare_lint! { + pub MISREFACTORED_ASSIGN_OP, + Warn, + "having a variable on both sides of an assign op" +} + #[derive(Copy, Clone, Default)] pub struct AssignOps; impl LintPass for AssignOps { fn get_lints(&self) -> LintArray { - lint_array!(ASSIGN_OPS, ASSIGN_OP_PATTERN) + lint_array!(ASSIGN_OPS, ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP) } } @@ -59,6 +78,40 @@ impl LateLintPass for AssignOps { "replace it with", format!("{} = {}", lhs, sugg::make_binop(higher::binop(op.node), lhs, rhs))); }); + if let hir::ExprBinary(binop, ref l, ref r) = rhs.node { + if op.node == binop.node { + let lint = |assignee: &hir::Expr, rhs: &hir::Expr| { + let ty = cx.tcx.expr_ty(assignee); + if ty.walk_shallow().next().is_some() { + return; // implements_trait does not work with generics + } + let rty = cx.tcx.expr_ty(rhs); + if rty.walk_shallow().next().is_some() { + return; // implements_trait does not work with generics + } + span_lint_and_then(cx, + MISREFACTORED_ASSIGN_OP, + expr.span, + "variable appears on both sides of an assignment operation", + |db| { + if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span), + snippet_opt(cx, rhs.span)) { + db.span_suggestion(expr.span, + "replace it with", + format!("{} {}= {}", snip_a, op.node.as_str(), snip_r)); + } + }); + }; + // lhs op= l op r + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) { + lint(lhs, r); + } + // lhs op= l commutative_op r + if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) { + lint(lhs, l); + } + } + } } hir::ExprAssign(ref assignee, ref e) => { if let hir::ExprBinary(op, ref l, ref r) = e.node { @@ -138,3 +191,27 @@ impl LateLintPass for AssignOps { } } } + +fn is_commutative(op: hir::BinOp_) -> bool { + use rustc::hir::BinOp_::*; + match op { + BiAdd | + BiMul | + BiAnd | + BiOr | + BiBitXor | + BiBitAnd | + BiBitOr | + BiEq | + BiNe => true, + BiSub | + BiDiv | + BiRem | + BiShl | + BiShr | + BiLt | + BiLe | + BiGe | + BiGt => false, + } +} diff --git a/tests/compile-fail/assign_ops2.rs b/tests/compile-fail/assign_ops2.rs new file mode 100644 index 000000000000..1537232bf182 --- /dev/null +++ b/tests/compile-fail/assign_ops2.rs @@ -0,0 +1,36 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#[allow(unused_assignments)] +#[deny(misrefactored_assign_op)] +fn main() { + let mut a = 5; + a += a + 1; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a += 1 + a += 1 + a; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a += 1 + a -= a - 1; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a -= 1 + a *= a * 99; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a *= 99 + a *= 42 * a; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a *= 42 + a /= a / 2; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a /= 2 + a %= a % 5; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a %= 5 + a &= a & 1; //~ ERROR variable appears on both sides of an assignment operation + //~^ HELP replace it with + //~| SUGGESTION a &= 1 + a -= 1 - a; + a /= 5 / a; + a %= 42 % a; + a <<= 6 << a; +}