Rollup merge of #112995 - strottos:ref-clone-suggestions, r=fee1-dead

Check for `<&NotClone as Clone>::clone()` calls and suggest to add Clone trait appropriately

Added recursive checking back up the HIR to see if a `Clone` suggestion would be helpful.

Addresses https://github.com/rust-lang/rust/issues/112857

Largely based on: https://github.com/rust-lang/rust/pull/112977

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -1523,9 +1523,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         found_ty: Ty<'tcx>,
         expr: &hir::Expr<'_>,
     ) {
+        // When `expr` is `x` in something like `let x = foo.clone(); x`, need to recurse up to get
+        // `foo` and `clone`.
+        let expr = self.note_type_is_not_clone_inner_expr(expr);
+
+        // If we've recursed to an `expr` of `foo.clone()`, get `foo` and `clone`.
         let hir::ExprKind::MethodCall(segment, callee_expr, &[], _) = expr.kind else {
             return;
         };
+
         let Some(clone_trait_did) = self.tcx.lang_items().clone_trait() else {
             return;
         };
@@ -1578,6 +1584,84 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         }
     }
 
+    /// Given a type mismatch error caused by `&T` being cloned instead of `T`, and
+    /// the `expr` as the source of this type mismatch, try to find the method call
+    /// as the source of this error and return that instead. Otherwise, return the
+    /// original expression.
+    fn note_type_is_not_clone_inner_expr<'b>(
+        &'b self,
+        expr: &'b hir::Expr<'b>,
+    ) -> &'b hir::Expr<'b> {
+        match expr.peel_blocks().kind {
+            hir::ExprKind::Path(hir::QPath::Resolved(
+                None,
+                hir::Path { segments: [_], res: crate::Res::Local(binding), .. },
+            )) => {
+                let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding)
+                else {
+                    return expr;
+                };
+                let Some(parent) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id)) else {
+                    return expr;
+                };
+
+                match parent {
+                    // foo.clone()
+                    hir::Node::Local(hir::Local { init: Some(init), .. }) => {
+                        self.note_type_is_not_clone_inner_expr(init)
+                    }
+                    // When `expr` is more complex like a tuple
+                    hir::Node::Pat(hir::Pat {
+                        hir_id: pat_hir_id,
+                        kind: hir::PatKind::Tuple(pats, ..),
+                        ..
+                    }) => {
+                        let Some(hir::Node::Local(hir::Local { init: Some(init), .. })) =
+                                self.tcx.hir().find(self.tcx.hir().parent_id(*pat_hir_id)) else {
+                            return expr;
+                        };
+
+                        match init.peel_blocks().kind {
+                            ExprKind::Tup(init_tup) => {
+                                if let Some(init) = pats
+                                    .iter()
+                                    .enumerate()
+                                    .filter(|x| x.1.hir_id == *hir_id)
+                                    .map(|(i, _)| init_tup.get(i).unwrap())
+                                    .next()
+                                {
+                                    self.note_type_is_not_clone_inner_expr(init)
+                                } else {
+                                    expr
+                                }
+                            }
+                            _ => expr,
+                        }
+                    }
+                    _ => expr,
+                }
+            }
+            // If we're calling into a closure that may not be typed recurse into that call. no need
+            // to worry if it's a call to a typed function or closure as this would ne handled
+            // previously.
+            hir::ExprKind::Call(Expr { kind: call_expr_kind, .. }, _) => {
+                if let hir::ExprKind::Path(hir::QPath::Resolved(None, call_expr_path)) = call_expr_kind
+                    && let hir::Path { segments: [_], res: crate::Res::Local(binding), .. } = call_expr_path
+                    && let Some(hir::Node::Pat(hir::Pat { hir_id, .. })) = self.tcx.hir().find(*binding)
+                    && let Some(closure) = self.tcx.hir().find(self.tcx.hir().parent_id(*hir_id))
+                    && let hir::Node::Local(hir::Local { init: Some(init), .. }) = closure
+                    && let Expr { kind: hir::ExprKind::Closure(hir::Closure { body: body_id, .. }), ..} = init
+                {
+                    let hir::Body { value: body_expr, .. } = self.tcx.hir().body(*body_id);
+                    self.note_type_is_not_clone_inner_expr(body_expr)
+                } else {
+                    expr
+                }
+            }
+            _ => expr,
+        }
+    }
+
     /// A common error is to add an extra semicolon:
     ///
     /// ```compile_fail,E0308

tests/ui/typeck/explain_clone_autoref.rs

@@ -11,3 +11,119 @@ fn clone_thing(nc: &NotClone) -> NotClone {
     //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
     //~| NOTE expected `NotClone`, found `&NotClone`
 }
+
+fn clone_thing2(nc: &NotClone) -> NotClone {
+    let nc: NotClone = nc.clone();
+    //~^ ERROR mismatched type
+    //~| NOTE expected due to this
+    //~| NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    //~| NOTE expected `NotClone`, found `&NotClone`
+    nc
+}
+
+fn clone_thing3(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let nc = nc.clone();
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    nc
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing4(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let nc = nc.clone();
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    let nc2 = nc;
+    nc2
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+impl NotClone {
+    fn other_fn(&self) {}
+    fn get_ref_notclone(&self) -> &Self {
+        self
+    }
+}
+
+fn clone_thing5(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let nc = nc.clone();
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    let nc2 = nc;
+    nc2.other_fn();
+    let nc3 = nc2;
+    nc3
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing6(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let (ret, _) = (nc.clone(), 1);
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    let _ = nc.clone();
+    ret
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing7(nc: Vec<&NotClone>) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let ret = nc[0].clone();
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    ret
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing8(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let ret = {
+        let a = nc.clone();
+        //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+        a
+    };
+    ret
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing9(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let cl = || nc.clone();
+    //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    let ret = cl();
+    ret
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing10(nc: &NotClone) -> (NotClone, NotClone) {
+    let (a, b) = {
+        let a = nc.clone();
+        //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+        (a, nc.clone())
+        //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+    };
+    (a, b)
+    //~^ ERROR mismatched type
+    //~| ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}
+
+fn clone_thing11(nc: &NotClone) -> NotClone {
+    //~^ NOTE expected `NotClone` because of return type
+    let a = {
+        let nothing = nc.clone();
+        let a = nc.clone();
+        //~^ NOTE `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+        let nothing = nc.clone();
+        a
+    };
+    a
+    //~^ ERROR mismatched type
+    //~| NOTE expected `NotClone`, found `&NotClone`
+}

tests/ui/typeck/explain_clone_autoref.stderr

@@ -18,6 +18,219 @@ LL + #[derive(Clone)]
 LL | struct NotClone;
    |
 
-error: aborting due to previous error
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:16:24
+   |
+LL |     let nc: NotClone = nc.clone();
+   |             --------   ^^^^^^^^^^ expected `NotClone`, found `&NotClone`
+   |             |
+   |             expected due to this
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:16:24
+   |
+LL |     let nc: NotClone = nc.clone();
+   |                        ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:28:5
+   |
+LL | fn clone_thing3(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     nc
+   |     ^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:26:14
+   |
+LL |     let nc = nc.clone();
+   |              ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:38:5
+   |
+LL | fn clone_thing4(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     nc2
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:35:14
+   |
+LL |     let nc = nc.clone();
+   |              ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:57:5
+   |
+LL | fn clone_thing5(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     nc3
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:52:14
+   |
+LL |     let nc = nc.clone();
+   |              ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:67:5
+   |
+LL | fn clone_thing6(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     ret
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:64:21
+   |
+LL |     let (ret, _) = (nc.clone(), 1);
+   |                     ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:76:5
+   |
+LL | fn clone_thing7(nc: Vec<&NotClone>) -> NotClone {
+   |                                        -------- expected `NotClone` because of return type
+...
+LL |     ret
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:74:15
+   |
+LL |     let ret = nc[0].clone();
+   |               ^^^^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:88:5
+   |
+LL | fn clone_thing8(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     ret
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:84:17
+   |
+LL |         let a = nc.clone();
+   |                 ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:98:5
+   |
+LL | fn clone_thing9(nc: &NotClone) -> NotClone {
+   |                                   -------- expected `NotClone` because of return type
+...
+LL |     ret
+   |     ^^^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:95:17
+   |
+LL |     let cl = || nc.clone();
+   |                 ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:110:6
+   |
+LL |     (a, b)
+   |      ^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:105:17
+   |
+LL |         let a = nc.clone();
+   |                 ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:110:9
+   |
+LL |     (a, b)
+   |         ^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:107:13
+   |
+LL |         (a, nc.clone())
+   |             ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error[E0308]: mismatched types
+  --> $DIR/explain_clone_autoref.rs:126:5
+   |
+LL | fn clone_thing11(nc: &NotClone) -> NotClone {
+   |                                    -------- expected `NotClone` because of return type
+...
+LL |     a
+   |     ^ expected `NotClone`, found `&NotClone`
+   |
+note: `NotClone` does not implement `Clone`, so `&NotClone` was cloned instead
+  --> $DIR/explain_clone_autoref.rs:121:17
+   |
+LL |         let a = nc.clone();
+   |                 ^^
+help: consider annotating `NotClone` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClone;
+   |
+
+error: aborting due to 12 previous errors
 
 For more information about this error, try `rustc --explain E0308`.