From a929316aed324598825e5a74a4082d09a846df32 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Fri, 23 Dec 2022 12:44:47 -0800
Subject: [PATCH] Suggest `.clone()` on method call move errors

---
 Cargo.lock                                    |  1 +
 compiler/rustc_borrowck/Cargo.toml            |  1 +
 .../rustc_borrowck/src/diagnostics/mod.rs     | 43 +++++++++++++++----
 src/test/ui/moves/suggest-clone.fixed         | 11 +++++
 src/test/ui/moves/suggest-clone.rs            | 11 +++++
 src/test/ui/moves/suggest-clone.stderr        | 22 ++++++++++
 6 files changed, 80 insertions(+), 9 deletions(-)
 create mode 100644 src/test/ui/moves/suggest-clone.fixed
 create mode 100644 src/test/ui/moves/suggest-clone.rs
 create mode 100644 src/test/ui/moves/suggest-clone.stderr

diff --git a/Cargo.lock b/Cargo.lock
index 5d05a09f038..6de8ba0f8e6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3447,6 +3447,7 @@ dependencies = [
  "rustc_errors",
  "rustc_graphviz",
  "rustc_hir",
+ "rustc_hir_analysis",
  "rustc_index",
  "rustc_infer",
  "rustc_lexer",
diff --git a/compiler/rustc_borrowck/Cargo.toml b/compiler/rustc_borrowck/Cargo.toml
index 87c113f3e30..f5001172754 100644
--- a/compiler/rustc_borrowck/Cargo.toml
+++ b/compiler/rustc_borrowck/Cargo.toml
@@ -15,6 +15,7 @@ rustc_data_structures = { path = "../rustc_data_structures" }
 rustc_errors = { path = "../rustc_errors" }
 rustc_graphviz = { path = "../rustc_graphviz" }
 rustc_hir = { path = "../rustc_hir" }
+rustc_hir_analysis = { path = "../rustc_hir_analysis" }
 rustc_index = { path = "../rustc_index" }
 rustc_infer = { path = "../rustc_infer" }
 rustc_lexer = { path = "../rustc_lexer" }
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
index 213b8b064f9..5250f39003d 100644
--- a/compiler/rustc_borrowck/src/diagnostics/mod.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -6,6 +6,7 @@ use rustc_errors::{Applicability, Diagnostic};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, Namespace};
 use rustc_hir::GeneratorKind;
+use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::tcx::PlaceTy;
 use rustc_middle::mir::{
@@ -1066,18 +1067,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 }
                 CallKind::Normal { self_arg, desugaring, method_did } => {
                     let self_arg = self_arg.unwrap();
+                    let tcx = self.infcx.tcx;
                     if let Some((CallDesugaringKind::ForLoopIntoIter, _)) = desugaring {
-                        let ty = moved_place.ty(self.body, self.infcx.tcx).ty;
-                        let suggest = match self.infcx.tcx.get_diagnostic_item(sym::IntoIterator) {
+                        let ty = moved_place.ty(self.body, tcx).ty;
+                        let suggest = match tcx.get_diagnostic_item(sym::IntoIterator) {
                             Some(def_id) => {
                                 let infcx = self.infcx.tcx.infer_ctxt().build();
                                 type_known_to_meet_bound_modulo_regions(
                                     &infcx,
                                     self.param_env,
-                                    infcx.tcx.mk_imm_ref(
-                                        infcx.tcx.lifetimes.re_erased,
-                                        infcx.tcx.erase_regions(ty),
-                                    ),
+                                    tcx.mk_imm_ref(tcx.lifetimes.re_erased, tcx.erase_regions(ty)),
                                     def_id,
                                     DUMMY_SP,
                                 )
@@ -1133,8 +1132,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 place_name, partially_str, loop_message
                             ),
                         );
-                        if let ty::Adt(def, ..)
-                            = moved_place.ty(self.body, self.infcx.tcx).ty.kind()
+                        let ty = moved_place.ty(self.body, self.infcx.tcx).ty;
+                        if let ty::Adt(def, ..) = ty.kind()
                             && Some(def.did()) == self.infcx.tcx.lang_items().pin_type()
                         {
                             err.span_suggestion_verbose(
@@ -1144,8 +1143,34 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                                 Applicability::MaybeIncorrect,
                             );
                         }
+                        if let Some(clone_trait) = tcx.lang_items().clone_trait() {
+                            // We can't use `predicate_may_hold` or `can_eq` without ICEs in
+                            // borrowck because of the inference context, so we do a poor-man's
+                            // version here.
+                            for impl_def_id in tcx.all_impls(clone_trait) {
+                                if let Some(def_id) = impl_def_id.as_local()
+                                    && let hir_id = tcx.hir().local_def_id_to_hir_id(def_id)
+                                    && let hir::Node::Item(hir::Item {
+                                        kind: hir::ItemKind::Impl(hir::Impl {
+                                            self_ty,
+                                            ..
+                                        }),
+                                        ..
+                                    }) = tcx.hir().get(hir_id)
+                                {
+                                    if ty == hir_ty_to_ty(tcx, self_ty) {
+                                        err.span_suggestion_verbose(
+                                            fn_call_span.shrink_to_lo(),
+                                            "you can `clone` the value and consume it, but this \
+                                             might not be your desired behavior",
+                                            "clone().".to_string(),
+                                            Applicability::MaybeIncorrect,
+                                        );
+                                    }
+                                }
+                            }
+                        }
                     }
-                    let tcx = self.infcx.tcx;
                     // Avoid pointing to the same function in multiple different
                     // error messages.
                     if span != DUMMY_SP && self.fn_self_span_reported.insert(self_arg.span) {
diff --git a/src/test/ui/moves/suggest-clone.fixed b/src/test/ui/moves/suggest-clone.fixed
new file mode 100644
index 00000000000..204bfdb10b0
--- /dev/null
+++ b/src/test/ui/moves/suggest-clone.fixed
@@ -0,0 +1,11 @@
+// run-rustfix
+
+#[derive(Clone)]
+struct Foo;
+impl Foo {
+    fn foo(self) {}
+}
+fn main() {
+    let foo = &Foo;
+    foo.clone().foo(); //~ ERROR cannot move out
+}
diff --git a/src/test/ui/moves/suggest-clone.rs b/src/test/ui/moves/suggest-clone.rs
new file mode 100644
index 00000000000..25dd9f006f9
--- /dev/null
+++ b/src/test/ui/moves/suggest-clone.rs
@@ -0,0 +1,11 @@
+// run-rustfix
+
+#[derive(Clone)]
+struct Foo;
+impl Foo {
+    fn foo(self) {}
+}
+fn main() {
+    let foo = &Foo;
+    foo.foo(); //~ ERROR cannot move out
+}
diff --git a/src/test/ui/moves/suggest-clone.stderr b/src/test/ui/moves/suggest-clone.stderr
new file mode 100644
index 00000000000..cbb3dfea3ba
--- /dev/null
+++ b/src/test/ui/moves/suggest-clone.stderr
@@ -0,0 +1,22 @@
+error[E0507]: cannot move out of `*foo` which is behind a shared reference
+  --> $DIR/suggest-clone.rs:10:5
+   |
+LL |     foo.foo();
+   |     ^^^^-----
+   |     |   |
+   |     |   `*foo` moved due to this method call
+   |     move occurs because `*foo` has type `Foo`, which does not implement the `Copy` trait
+   |
+note: `Foo::foo` takes ownership of the receiver `self`, which moves `*foo`
+  --> $DIR/suggest-clone.rs:6:12
+   |
+LL |     fn foo(self) {}
+   |            ^^^^
+help: you can `clone` the value and consume it, but this might not be your desired behavior
+   |
+LL |     foo.clone().foo();
+   |         ++++++++
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0507`.