diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index b24e0cdfced..63ec98a5b3a 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/clippy_lints/src/assign_ops.rs @@ -68,52 +68,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps { match &expr.node { hir::ExprKind::AssignOp(op, lhs, rhs) => { if let hir::ExprKind::Binary(binop, l, r) = &rhs.node { - if op.node == binop.node { - let lint = |assignee: &hir::Expr, rhs_other: &hir::Expr| { - 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_other.span)) - { - let a = &sugg::Sugg::hir(cx, assignee, ".."); - let r = &sugg::Sugg::hir(cx, rhs, ".."); - let long = - format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r)); - db.span_suggestion( - expr.span, - &format!( - "Did you mean {} = {} {} {} or {}? Consider replacing it with", - snip_a, - snip_a, - op.node.as_str(), - snip_r, - long - ), - format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), - Applicability::MachineApplicable, - ); - db.span_suggestion( - expr.span, - "or", - long, - Applicability::MachineApplicable, // snippet - ); - } - }, - ); - }; - // 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); - } + if op.node != binop.node { return; } + // lhs op= l op r + if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) { + lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, r); + } + // lhs op= l commutative_op r + if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) { + lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, l); } } }, @@ -234,6 +196,44 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps { } } +fn lint_misrefactored_assign_op(cx: &LateContext<'_, '_>, expr: &hir::Expr, op: hir::BinOp, rhs: &hir::Expr, assignee: &hir::Expr, rhs_other: &hir::Expr) { + 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_other.span)) + { + let a = &sugg::Sugg::hir(cx, assignee, ".."); + let r = &sugg::Sugg::hir(cx, rhs, ".."); + let long = + format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r)); + db.span_suggestion( + expr.span, + &format!( + "Did you mean {} = {} {} {} or {}? Consider replacing it with", + snip_a, + snip_a, + op.node.as_str(), + snip_r, + long + ), + format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), + Applicability::MachineApplicable, + ); + db.span_suggestion( + expr.span, + "or", + long, + Applicability::MachineApplicable, // snippet + ); + } + }, + ); +} + fn is_commutative(op: hir::BinOpKind) -> bool { use rustc::hir::BinOpKind::*; match op {