diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 89b57ed1a8de..f1e223d9a48c 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -54,7 +54,7 @@ declare_lint_pass!(SuspiciousImpl => [SUSPICIOUS_ARITHMETIC_IMPL, SUSPICIOUS_OP_ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { - if let hir::ExprKind::Binary(binop, _, _) = expr.kind { + if let hir::ExprKind::Binary(binop, _, _) | hir::ExprKind::AssignOp(binop, ..) = expr.kind { match binop.node { hir::BinOpKind::Eq | hir::BinOpKind::Lt @@ -65,14 +65,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl { _ => {}, } // Check if the binary expression is part of another bi/unary expression - // as a child node + // or operator assignment as a child node let mut parent_expr = cx.tcx.hir().get_parent_node(expr.hir_id); while parent_expr != hir::CRATE_HIR_ID { if let hir::Node::Expr(e) = cx.tcx.hir().get(parent_expr) { match e.kind { hir::ExprKind::Binary(..) | hir::ExprKind::Unary(hir::UnOp::UnNot, _) - | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => return, + | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) + | hir::ExprKind::AssignOp(..) => return, _ => {}, } } @@ -191,7 +192,8 @@ impl<'a, 'tcx> Visitor<'tcx> for BinaryExprVisitor { match expr.kind { hir::ExprKind::Binary(..) | hir::ExprKind::Unary(hir::UnOp::UnNot, _) - | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => self.in_binary_expr = true, + | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) + | hir::ExprKind::AssignOp(..) => self.in_binary_expr = true, _ => {}, } diff --git a/tests/ui/suspicious_arithmetic_impl.rs b/tests/ui/suspicious_arithmetic_impl.rs index 6ee924d3b2ed..1f5b98118870 100644 --- a/tests/ui/suspicious_arithmetic_impl.rs +++ b/tests/ui/suspicious_arithmetic_impl.rs @@ -1,5 +1,5 @@ #![warn(clippy::suspicious_arithmetic_impl)] -use std::ops::{Add, AddAssign, Div, Mul, Sub}; +use std::ops::{Add, AddAssign, BitOrAssign, Div, DivAssign, Mul, MulAssign, Sub}; #[derive(Copy, Clone)] struct Foo(u32); @@ -18,6 +18,25 @@ impl AddAssign for Foo { } } +impl BitOrAssign for Foo { + fn bitor_assign(&mut self, other: Foo) { + let idx = other.0; + self.0 |= 1 << idx; // OK: BinOpKind::Shl part of AssignOp as child node + } +} + +impl MulAssign for Foo { + fn mul_assign(&mut self, other: Foo) { + self.0 /= other.0; + } +} + +impl DivAssign for Foo { + fn div_assign(&mut self, other: Foo) { + self.0 /= other.0; // OK: BinOpKind::Div == DivAssign + } +} + impl Mul for Foo { type Output = Foo; diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index e8a6efc4c4d2..7e42d72c30b2 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -14,5 +14,11 @@ LL | *self = *self - other; | = note: `#[deny(clippy::suspicious_op_assign_impl)]` on by default -error: aborting due to 2 previous errors +error: Suspicious use of binary operator in `MulAssign` impl + --> $DIR/suspicious_arithmetic_impl.rs:30:16 + | +LL | self.0 /= other.0; + | ^^ + +error: aborting due to 3 previous errors