From 9c9af1d88525de5649375e08abf16437426349b8 Mon Sep 17 00:00:00 2001
From: Jacek Pospychala <jacek.pospychala@gmail.com>
Date: Sun, 5 Apr 2020 20:40:51 +0200
Subject: [PATCH] Include OpAssign in suspicious_op_assign_impl

---
 clippy_lints/src/suspicious_trait_impl.rs  | 10 ++++++----
 tests/ui/suspicious_arithmetic_impl.rs     | 21 ++++++++++++++++++++-
 tests/ui/suspicious_arithmetic_impl.stderr |  8 +++++++-
 3 files changed, 33 insertions(+), 6 deletions(-)

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