From dfe28debb914dc944bf8de270016071f6742098d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Wed, 13 Mar 2024 22:45:25 +0000
Subject: [PATCH] Account for assign binops in clone suggestions

Explicitly look for `expr += other_expr;` and avoid suggesting
`expr.clone() += other_expr;`, instead suggesting `expr += other_expr.clone();`.
---
 .../src/diagnostics/conflict_errors.rs        | 34 +++++++++++++++++--
 tests/ui/augmented-assignments.rs             |  2 +-
 tests/ui/augmented-assignments.stderr         |  2 +-
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
index 27cb0bbe537..9abc29f52b2 100644
--- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -991,9 +991,39 @@ pub(crate) fn suggest_cloning(
         &self,
         err: &mut Diag<'_>,
         ty: Ty<'tcx>,
-        expr: &hir::Expr<'_>,
-        other_expr: Option<&hir::Expr<'_>>,
+        mut expr: &'cx hir::Expr<'cx>,
+        mut other_expr: Option<&'cx hir::Expr<'cx>>,
     ) {
+        if let Some(some_other_expr) = other_expr
+            && let Some(parent_binop) =
+                self.infcx.tcx.hir().parent_iter(expr.hir_id).find_map(|n| {
+                    if let (hir_id, hir::Node::Expr(e)) = n
+                        && let hir::ExprKind::AssignOp(_binop, target, _arg) = e.kind
+                        && target.hir_id == expr.hir_id
+                    {
+                        Some(hir_id)
+                    } else {
+                        None
+                    }
+                })
+            && let Some(other_parent_binop) =
+                self.infcx.tcx.hir().parent_iter(some_other_expr.hir_id).find_map(|n| {
+                    if let (hir_id, hir::Node::Expr(expr)) = n
+                        && let hir::ExprKind::AssignOp(..) = expr.kind
+                    {
+                        Some(hir_id)
+                    } else {
+                        None
+                    }
+                })
+            && { true }
+            && parent_binop == other_parent_binop
+        {
+            // Explicitly look for `expr += other_expr;` and avoid suggesting
+            // `expr.clone() += other_expr;`, instead suggesting `expr += other_expr.clone();`.
+            other_expr = Some(expr);
+            expr = some_other_expr;
+        }
         'outer: {
             if let ty::Ref(..) = ty.kind() {
                 // We check for either `let binding = foo(expr, other_expr);` or
diff --git a/tests/ui/augmented-assignments.rs b/tests/ui/augmented-assignments.rs
index ca12395e432..8b263e03593 100644
--- a/tests/ui/augmented-assignments.rs
+++ b/tests/ui/augmented-assignments.rs
@@ -13,11 +13,11 @@ fn main() {
     let mut x = Int(1); //~ NOTE binding `x` declared here
     x
     //~^ NOTE borrow of `x` occurs here
-    //~| HELP consider cloning
     +=
     x;
     //~^ ERROR cannot move out of `x` because it is borrowed
     //~| move out of `x` occurs here
+    //~| HELP consider cloning
 
     let y = Int(2);
     //~^ HELP consider changing this to be mutable
diff --git a/tests/ui/augmented-assignments.stderr b/tests/ui/augmented-assignments.stderr
index 465b88fcd5b..6b2900dd5d1 100644
--- a/tests/ui/augmented-assignments.stderr
+++ b/tests/ui/augmented-assignments.stderr
@@ -11,7 +11,7 @@ LL |     x;
    |
 help: consider cloning the value if the performance cost is acceptable
    |
-LL |     x.clone()
+LL |     x.clone();
    |      ++++++++
 
 error[E0596]: cannot borrow `y` as mutable, as it is not declared as mutable