From 751131b4acce37d0d4d62edd1eb831143c8fa46f Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 31 May 2022 21:57:32 -0400 Subject: [PATCH] Move `AssignOps` into `Operators` lint pass --- clippy_lints/src/assign_ops.rs | 235 ------------------ clippy_lints/src/lib.register_all.rs | 4 +- clippy_lints/src/lib.register_lints.rs | 4 +- clippy_lints/src/lib.register_style.rs | 2 +- clippy_lints/src/lib.register_suspicious.rs | 2 +- clippy_lints/src/lib.rs | 2 - .../src/operators/assign_op_pattern.rs | 101 ++++++++ .../src/operators/misrefactored_assign_op.rs | 84 +++++++ clippy_lints/src/operators/mod.rs | 70 ++++++ 9 files changed, 261 insertions(+), 243 deletions(-) delete mode 100644 clippy_lints/src/assign_ops.rs create mode 100644 clippy_lints/src/operators/assign_op_pattern.rs create mode 100644 clippy_lints/src/operators/misrefactored_assign_op.rs diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs deleted file mode 100644 index f81da2d42233..000000000000 --- a/clippy_lints/src/assign_ops.rs +++ /dev/null @@ -1,235 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_opt; -use clippy_utils::ty::implements_trait; -use clippy_utils::{binop_traits, sugg}; -use clippy_utils::{eq_expr_value, trait_ref_of_method}; -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `a = a op b` or `a = b commutative_op a` - /// patterns. - /// - /// ### Why is this bad? - /// These can be written as the shorter `a op= b`. - /// - /// ### Known problems - /// While forbidden by the spec, `OpAssign` traits may have - /// implementations that differ from the regular `Op` impl. - /// - /// ### Example - /// ```rust - /// let mut a = 5; - /// let b = 0; - /// // ... - /// - /// a = a + b; - /// ``` - /// - /// Use instead: - /// ```rust - /// let mut a = 5; - /// let b = 0; - /// // ... - /// - /// a += b; - /// ``` - #[clippy::version = "pre 1.29.0"] - pub ASSIGN_OP_PATTERN, - style, - "assigning the result of an operation on a variable to that same variable" -} - -declare_clippy_lint! { - /// ### What it does - /// Checks 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 - /// Clippy cannot know for sure if `a op= a op b` should have - /// been `a = a op a op b` or `a = a op b`/`a op= b`. Therefore, it suggests both. - /// If `a op= a op b` is really the correct behavior it should be - /// written as `a = a op a op b` as it's less confusing. - /// - /// ### Example - /// ```rust - /// let mut a = 5; - /// let b = 2; - /// // ... - /// a += a + b; - /// ``` - #[clippy::version = "pre 1.29.0"] - pub MISREFACTORED_ASSIGN_OP, - suspicious, - "having a variable on both sides of an assign op" -} - -declare_lint_pass!(AssignOps => [ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP]); - -impl<'tcx> LateLintPass<'tcx> for AssignOps { - #[allow(clippy::too_many_lines)] - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - match &expr.kind { - hir::ExprKind::AssignOp(op, lhs, rhs) => { - if let hir::ExprKind::Binary(binop, l, r) = &rhs.kind { - if op.node != binop.node { - return; - } - // lhs op= l op r - if eq_expr_value(cx, lhs, l) { - lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, r); - } - // lhs op= l commutative_op r - if is_commutative(op.node) && eq_expr_value(cx, lhs, r) { - lint_misrefactored_assign_op(cx, expr, *op, rhs, lhs, l); - } - } - }, - hir::ExprKind::Assign(assignee, e, _) => { - if let hir::ExprKind::Binary(op, l, r) = &e.kind { - let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| { - let ty = cx.typeck_results().expr_ty(assignee); - let rty = cx.typeck_results().expr_ty(rhs); - if_chain! { - if let Some((_, lang_item)) = binop_traits(op.node); - if let Ok(trait_id) = cx.tcx.lang_items().require(lang_item); - let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id); - if trait_ref_of_method(cx, parent_fn) - .map_or(true, |t| t.path.res.def_id() != trait_id); - if implements_trait(cx, ty, trait_id, &[rty.into()]); - then { - span_lint_and_then( - cx, - ASSIGN_OP_PATTERN, - expr.span, - "manual implementation of an assign operation", - |diag| { - if let (Some(snip_a), Some(snip_r)) = - (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span)) - { - diag.span_suggestion( - expr.span, - "replace it with", - format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), - Applicability::MachineApplicable, - ); - } - }, - ); - } - } - }; - - let mut visitor = ExprVisitor { - assignee, - counter: 0, - cx, - }; - - walk_expr(&mut visitor, e); - - if visitor.counter == 1 { - // a = a op b - if eq_expr_value(cx, assignee, l) { - lint(assignee, r); - } - // a = b commutative_op a - // Limited to primitive type as these ops are know to be commutative - if eq_expr_value(cx, assignee, r) && cx.typeck_results().expr_ty(assignee).is_primitive_ty() { - match op.node { - hir::BinOpKind::Add - | hir::BinOpKind::Mul - | hir::BinOpKind::And - | hir::BinOpKind::Or - | hir::BinOpKind::BitXor - | hir::BinOpKind::BitAnd - | hir::BinOpKind::BitOr => { - lint(assignee, l); - }, - _ => {}, - } - } - } - } - }, - _ => {}, - } - } -} - -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", - |diag| { - 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(op.node.into(), a, r)); - diag.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::MaybeIncorrect, - ); - diag.span_suggestion( - expr.span, - "or", - long, - Applicability::MaybeIncorrect, // snippet - ); - } - }, - ); -} - -#[must_use] -fn is_commutative(op: hir::BinOpKind) -> bool { - use rustc_hir::BinOpKind::{ - Add, And, BitAnd, BitOr, BitXor, Div, Eq, Ge, Gt, Le, Lt, Mul, Ne, Or, Rem, Shl, Shr, Sub, - }; - match op { - Add | Mul | And | Or | BitXor | BitAnd | BitOr | Eq | Ne => true, - Sub | Div | Rem | Shl | Shr | Lt | Le | Ge | Gt => false, - } -} - -struct ExprVisitor<'a, 'tcx> { - assignee: &'a hir::Expr<'a>, - counter: u8, - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - if eq_expr_value(self.cx, self.assignee, expr) { - self.counter += 1; - } - - walk_expr(self, expr); - } -} diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 7728157d2282..b43c11330f0d 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -6,8 +6,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(almost_complete_letter_range::ALMOST_COMPLETE_LETTER_RANGE), LintId::of(approx_const::APPROX_CONSTANT), LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS), - LintId::of(assign_ops::ASSIGN_OP_PATTERN), - LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(async_yields_async::ASYNC_YIELDS_ASYNC), LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(attrs::DEPRECATED_CFG_ATTR), @@ -260,6 +258,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(octal_escapes::OCTAL_ESCAPES), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(operators::ABSURD_EXTREME_COMPARISONS), + LintId::of(operators::ASSIGN_OP_PATTERN), + LintId::of(operators::MISREFACTORED_ASSIGN_OP), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 787ae1e79ec0..231511d46b72 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -42,8 +42,6 @@ store.register_lints(&[ asm_syntax::INLINE_ASM_X86_ATT_SYNTAX, asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX, assertions_on_constants::ASSERTIONS_ON_CONSTANTS, - assign_ops::ASSIGN_OP_PATTERN, - assign_ops::MISREFACTORED_ASSIGN_OP, async_yields_async::ASYNC_YIELDS_ASYNC, attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON, attrs::BLANKET_CLIPPY_RESTRICTION_LINTS, @@ -435,8 +433,10 @@ store.register_lints(&[ only_used_in_recursion::ONLY_USED_IN_RECURSION, open_options::NONSENSICAL_OPEN_OPTIONS, operators::ABSURD_EXTREME_COMPARISONS, + operators::ASSIGN_OP_PATTERN, operators::FLOAT_ARITHMETIC, operators::INTEGER_ARITHMETIC, + operators::MISREFACTORED_ASSIGN_OP, option_env_unwrap::OPTION_ENV_UNWRAP, option_if_let_else::OPTION_IF_LET_ELSE, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs index d52ec50e5422..78df54a4fff6 100644 --- a/clippy_lints/src/lib.register_style.rs +++ b/clippy_lints/src/lib.register_style.rs @@ -4,7 +4,6 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS), - LintId::of(assign_ops::ASSIGN_OP_PATTERN), LintId::of(blacklisted_name::BLACKLISTED_NAME), LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), @@ -98,6 +97,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![ LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), + LintId::of(operators::ASSIGN_OP_PATTERN), LintId::of(ptr::CMP_NULL), LintId::of(ptr::PTR_ARG), LintId::of(ptr_eq::PTR_EQ), diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 7b13713c36e5..4ee89b7bec5d 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -4,7 +4,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![ LintId::of(almost_complete_letter_range::ALMOST_COMPLETE_LETTER_RANGE), - LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), LintId::of(await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE), LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK), @@ -29,6 +28,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(methods::SUSPICIOUS_MAP), LintId::of(mut_key::MUTABLE_KEY_TYPE), LintId::of(octal_escapes::OCTAL_ESCAPES), + LintId::of(operators::MISREFACTORED_ASSIGN_OP), LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f1d282013091..0f9e3b062331 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -174,7 +174,6 @@ mod as_conversions; mod as_underscore; mod asm_syntax; mod assertions_on_constants; -mod assign_ops; mod async_yields_async; mod attrs; mod await_holding_invalid; @@ -704,7 +703,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(doc::DocMarkdown::new(doc_valid_idents.clone()))); store.register_late_pass(|| Box::new(neg_multiply::NegMultiply)); store.register_late_pass(|| Box::new(mem_forget::MemForget)); - store.register_late_pass(|| Box::new(assign_ops::AssignOps)); store.register_late_pass(|| Box::new(let_if_seq::LetIfSeq)); store.register_late_pass(|| Box::new(mixed_read_write_in_expression::EvalOrderDependence)); store.register_late_pass(|| Box::new(missing_doc::MissingDoc::new())); diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs new file mode 100644 index 000000000000..979e0a66707d --- /dev/null +++ b/clippy_lints/src/operators/assign_op_pattern.rs @@ -0,0 +1,101 @@ +use clippy_utils::binop_traits; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::implements_trait; +use clippy_utils::{eq_expr_value, trait_ref_of_method}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_lint::LateContext; + +use super::ASSIGN_OP_PATTERN; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + assignee: &'tcx hir::Expr<'_>, + e: &'tcx hir::Expr<'_>, +) { + if let hir::ExprKind::Binary(op, l, r) = &e.kind { + let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| { + let ty = cx.typeck_results().expr_ty(assignee); + let rty = cx.typeck_results().expr_ty(rhs); + if_chain! { + if let Some((_, lang_item)) = binop_traits(op.node); + if let Ok(trait_id) = cx.tcx.lang_items().require(lang_item); + let parent_fn = cx.tcx.hir().get_parent_item(e.hir_id); + if trait_ref_of_method(cx, parent_fn) + .map_or(true, |t| t.path.res.def_id() != trait_id); + if implements_trait(cx, ty, trait_id, &[rty.into()]); + then { + span_lint_and_then( + cx, + ASSIGN_OP_PATTERN, + expr.span, + "manual implementation of an assign operation", + |diag| { + if let (Some(snip_a), Some(snip_r)) = + (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span)) + { + diag.span_suggestion( + expr.span, + "replace it with", + format!("{} {}= {}", snip_a, op.node.as_str(), snip_r), + Applicability::MachineApplicable, + ); + } + }, + ); + } + } + }; + + let mut visitor = ExprVisitor { + assignee, + counter: 0, + cx, + }; + + walk_expr(&mut visitor, e); + + if visitor.counter == 1 { + // a = a op b + if eq_expr_value(cx, assignee, l) { + lint(assignee, r); + } + // a = b commutative_op a + // Limited to primitive type as these ops are know to be commutative + if eq_expr_value(cx, assignee, r) && cx.typeck_results().expr_ty(assignee).is_primitive_ty() { + match op.node { + hir::BinOpKind::Add + | hir::BinOpKind::Mul + | hir::BinOpKind::And + | hir::BinOpKind::Or + | hir::BinOpKind::BitXor + | hir::BinOpKind::BitAnd + | hir::BinOpKind::BitOr => { + lint(assignee, l); + }, + _ => {}, + } + } + } + } +} + +struct ExprVisitor<'a, 'tcx> { + assignee: &'a hir::Expr<'a>, + counter: u8, + cx: &'a LateContext<'tcx>, +} + +impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> { + fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { + if eq_expr_value(self.cx, self.assignee, expr) { + self.counter += 1; + } + + walk_expr(self, expr); + } +} diff --git a/clippy_lints/src/operators/misrefactored_assign_op.rs b/clippy_lints/src/operators/misrefactored_assign_op.rs new file mode 100644 index 000000000000..0024384d9278 --- /dev/null +++ b/clippy_lints/src/operators/misrefactored_assign_op.rs @@ -0,0 +1,84 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::eq_expr_value; +use clippy_utils::source::snippet_opt; +use clippy_utils::sugg; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; + +use super::MISREFACTORED_ASSIGN_OP; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + op: hir::BinOpKind, + lhs: &'tcx hir::Expr<'_>, + rhs: &'tcx hir::Expr<'_>, +) { + if let hir::ExprKind::Binary(binop, l, r) = &rhs.kind { + if op != binop.node { + return; + } + // lhs op= l op r + if eq_expr_value(cx, lhs, l) { + lint_misrefactored_assign_op(cx, expr, op, rhs, lhs, r); + } + // lhs op= l commutative_op r + if is_commutative(op) && eq_expr_value(cx, lhs, r) { + lint_misrefactored_assign_op(cx, expr, op, rhs, lhs, l); + } + } +} + +fn lint_misrefactored_assign_op( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + op: hir::BinOpKind, + 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", + |diag| { + 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(op.into(), a, r)); + diag.span_suggestion( + expr.span, + &format!( + "did you mean `{} = {} {} {}` or `{}`? Consider replacing it with", + snip_a, + snip_a, + op.as_str(), + snip_r, + long + ), + format!("{} {}= {}", snip_a, op.as_str(), snip_r), + Applicability::MaybeIncorrect, + ); + diag.span_suggestion( + expr.span, + "or", + long, + Applicability::MaybeIncorrect, // snippet + ); + } + }, + ); +} + +#[must_use] +fn is_commutative(op: hir::BinOpKind) -> bool { + use rustc_hir::BinOpKind::{ + Add, And, BitAnd, BitOr, BitXor, Div, Eq, Ge, Gt, Le, Lt, Mul, Ne, Or, Rem, Shl, Shr, Sub, + }; + match op { + Add | Mul | And | Or | BitXor | BitAnd | BitOr | Eq | Ne => true, + Sub | Div | Rem | Shl | Shr | Lt | Le | Ge | Gt => false, + } +} diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index a7495f042da4..c1d038aa2b1a 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -3,6 +3,8 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; mod absurd_extreme_comparisons; +mod assign_op_pattern; +mod misrefactored_assign_op; mod numeric_arithmetic; declare_clippy_lint! { @@ -82,6 +84,68 @@ declare_clippy_lint! { "any floating-point arithmetic statement" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `a = a op b` or `a = b commutative_op a` + /// patterns. + /// + /// ### Why is this bad? + /// These can be written as the shorter `a op= b`. + /// + /// ### Known problems + /// While forbidden by the spec, `OpAssign` traits may have + /// implementations that differ from the regular `Op` impl. + /// + /// ### Example + /// ```rust + /// let mut a = 5; + /// let b = 0; + /// // ... + /// + /// a = a + b; + /// ``` + /// + /// Use instead: + /// ```rust + /// let mut a = 5; + /// let b = 0; + /// // ... + /// + /// a += b; + /// ``` + #[clippy::version = "pre 1.29.0"] + pub ASSIGN_OP_PATTERN, + style, + "assigning the result of an operation on a variable to that same variable" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks 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 + /// Clippy cannot know for sure if `a op= a op b` should have + /// been `a = a op a op b` or `a = a op b`/`a op= b`. Therefore, it suggests both. + /// If `a op= a op b` is really the correct behavior it should be + /// written as `a = a op a op b` as it's less confusing. + /// + /// ### Example + /// ```rust + /// let mut a = 5; + /// let b = 2; + /// // ... + /// a += a + b; + /// ``` + #[clippy::version = "pre 1.29.0"] + pub MISREFACTORED_ASSIGN_OP, + suspicious, + "having a variable on both sides of an assign op" +} + #[derive(Default)] pub struct Operators { arithmetic_context: numeric_arithmetic::Context, @@ -90,6 +154,8 @@ impl_lint_pass!(Operators => [ ABSURD_EXTREME_COMPARISONS, INTEGER_ARITHMETIC, FLOAT_ARITHMETIC, + ASSIGN_OP_PATTERN, + MISREFACTORED_ASSIGN_OP, ]); impl<'tcx> LateLintPass<'tcx> for Operators { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { @@ -102,6 +168,10 @@ impl<'tcx> LateLintPass<'tcx> for Operators { }, ExprKind::AssignOp(op, lhs, rhs) => { self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs); + misrefactored_assign_op::check(cx, e, op.node, lhs, rhs); + }, + ExprKind::Assign(lhs, rhs, _) => { + assign_op_pattern::check(cx, e, lhs, rhs); }, ExprKind::Unary(op, arg) => { if op == UnOp::Neg {