rust-lang · cyrgani · Dec 29, 2024 · cyrgani · Jan 3, 2025 · estebank
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -615,7 +615,7 @@ lint_non_upper_case_global = {$sort} `{$name}` should have an upper case name
 lint_noop_method_call = call to `.{$method}()` on a reference in this situation does nothing
     .suggestion = remove this redundant call
     .note = the type `{$orig_ty}` does not implement `{$trait_}`, so calling `{$method}` on `&{$orig_ty}` copies the reference, which does not do anything and can be removed
-    .derive_suggestion = if you meant to clone `{$orig_ty}`, implement `Clone` for it
+    .derive_suggestion = if you meant to clone `{$orig_ty}`, implement `Clone` for `{$non_clone_ty}`
 
 lint_only_cast_u8_to_char = only `u8` can be cast into `char`
     .suggestion = use a `char` literal instead

diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -1346,6 +1346,7 @@ pub(crate) struct NoopMethodCallDiag<'a> {
     pub method: Symbol,
     pub orig_ty: Ty<'a>,
     pub trait_: Symbol,
+    pub non_clone_ty: Ty<'a>,
     #[suggestion(code = "", applicability = "machine-applicable")]
     pub label: Span,
     #[suggestion(

diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs
@@ -1,6 +1,7 @@
 use rustc_hir::def::DefKind;
 use rustc_hir::{Expr, ExprKind};
 use rustc_middle::ty;
+use rustc_middle::ty::ClauseKind;
 use rustc_middle::ty::adjustment::Adjust;
 use rustc_session::{declare_lint, declare_lint_pass};
 use rustc_span::sym;
@@ -124,14 +125,48 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
         let orig_ty = expr_ty.peel_refs();
 
         if receiver_ty == expr_ty {
+            let mut non_clone_ty = orig_ty;
+
             let suggest_derive = match orig_ty.kind() {
-                ty::Adt(def, _) => Some(cx.tcx.def_span(def.did()).shrink_to_lo()),
+                ty::Adt(def, args) => {
+                    if def.did().is_local() {
+                        Some(cx.tcx.def_span(def.did()).shrink_to_lo())
+                    } else if let Some(trait_impl_id) = cx.tcx.impl_of_method(i.def_id()) {
+                        // If the type is generic over `T`, implements `Clone` if `T` does
+                        // and the concrete `T` is local, suggest deriving `Clone` for `T` rather than the type.
+                        let predicates = cx.tcx.predicates_of(trait_impl_id);
+
+                        if predicates.predicates.into_iter().all(|&(predicate, _)| {
+                            let ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
+                            else {
+                                return true;
+                            };
+                            cx.tcx.is_diagnostic_item(sym::Clone, trait_predicate.trait_ref.def_id)
+                        }) {
+                            args.iter().find_map(|arg| {
+                                let ty = arg.as_type()?;
+                                let did = ty.ty_adt_def()?.did();
+                                if did.is_local() {
+                                    non_clone_ty = ty;
+                                    Some(cx.tcx.def_span(did))
+                                } else {
+                                    None
+                                }
+                            })
+                        } else {
+                            None
+                        }
+                    } else {
+                        None
+                    }
+                }
                 _ => None,
             };
             cx.emit_span_lint(NOOP_METHOD_CALL, span, NoopMethodCallDiag {
                 method: call.ident.name,
                 orig_ty,
                 trait_,
+                non_clone_ty,
                 label: span,
                 suggest_derive,
             });

diff --git a/tests/ui/lint/auxiliary/non_clone_types.rs b/tests/ui/lint/auxiliary/non_clone_types.rs
@@ -0,0 +1,25 @@
+pub struct NotClone;
+
+pub struct IsClone;
+
+impl Clone for IsClone {
+    fn clone(&self) -> Self {
+        Self
+    }
+}
+
+pub struct ConditionalClone<T>(T);
+
+impl<T: Clone> Clone for ConditionalClone<T> {
+    fn clone(&self) -> Self {
+        Self(self.0.clone())
+    }
+}
+
+pub struct DifferentlyConditionalClone<T>(T);
+
+impl<T: Default> Clone for DifferentlyConditionalClone<T> {
+    fn clone(&self) -> Self {
+        Self(T::default())
+    }
+}
diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs
@@ -1,8 +1,12 @@
 //@ check-pass
+//@ aux-build:non_clone_types.rs
 
 #![feature(rustc_attrs)]
 #![allow(unused)]
 
+extern crate non_clone_types;
+use non_clone_types::*;
+
 use std::borrow::Borrow;
 use std::ops::Deref;
 
@@ -61,3 +65,27 @@ impl Clone for DiagnosticClone {
 fn with_other_diagnostic_item(x: DiagnosticClone) {
     x.clone();
 }
+
+fn with_foreign_type(v: &NotClone) {
+    v.clone();
+    //~^ WARN call to `.clone()` on a reference in this situation does nothing
+}
+
+fn with_foreign_generic_type(v: &ConditionalClone<PlainType<u32>>) {
+    v.clone();
+    //~^ WARN call to `.clone()` on a reference in this situation does nothing
+}
+
+fn with_only_foreign_types_1(v: &ConditionalClone<NotClone>) {
+    v.clone();
+    //~^ WARN call to `.clone()` on a reference in this situation does nothing
+}
+
+fn with_only_foreign_types_2(v: &ConditionalClone<IsClone>) {
+    v.clone();
+}
+
+fn different_impl_bound(v: &DifferentlyConditionalClone<PlainType<u8>>) {
+    v.clone();
+    //~^ WARN call to `.clone()` on a reference in this situation does nothing
+}
diff --git a/tests/ui/lint/noop-method-call.stderr b/tests/ui/lint/noop-method-call.stderr
@@ -1,5 +1,5 @@
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:15:25
+  --> $DIR/noop-method-call.rs:19:25
    |
 LL |     let _ = &mut encoded.clone();
    |                         ^^^^^^^^ help: remove this redundant call
@@ -8,15 +8,15 @@ LL |     let _ = &mut encoded.clone();
    = note: `#[warn(noop_method_call)]` on by default
 
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:17:21
+  --> $DIR/noop-method-call.rs:21:21
    |
 LL |     let _ = &encoded.clone();
    |                     ^^^^^^^^ help: remove this redundant call
    |
    = note: the type `[u8]` does not implement `Clone`, so calling `clone` on `&[u8]` copies the reference, which does not do anything and can be removed
 
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:23:71
+  --> $DIR/noop-method-call.rs:27:71
    |
 LL |     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
    |                                                                       ^^^^^^^^
@@ -27,14 +27,14 @@ help: remove this redundant call
 LL -     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
 LL +     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref;
    |
-help: if you meant to clone `PlainType<u32>`, implement `Clone` for it
+help: if you meant to clone `PlainType<u32>`, implement `Clone` for `PlainType<u32>`
    |
 LL + #[derive(Clone)]
 LL | struct PlainType<T>(T);
    |
 
 warning: call to `.deref()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:31:63
+  --> $DIR/noop-method-call.rs:35:63
    |
 LL |     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
    |                                                               ^^^^^^^^
@@ -45,14 +45,14 @@ help: remove this redundant call
 LL -     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
 LL +     let non_deref_type_deref: &PlainType<u32> = non_deref_type;
    |
-help: if you meant to clone `PlainType<u32>`, implement `Clone` for it
+help: if you meant to clone `PlainType<u32>`, implement `Clone` for `PlainType<u32>`
    |
 LL + #[derive(Clone)]
 LL | struct PlainType<T>(T);
    |
 
 warning: call to `.borrow()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:35:66
+  --> $DIR/noop-method-call.rs:39:66
    |
 LL |     let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
    |                                                                  ^^^^^^^^^
@@ -63,14 +63,14 @@ help: remove this redundant call
 LL -     let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
 LL +     let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type;
    |
-help: if you meant to clone `PlainType<u32>`, implement `Clone` for it
+help: if you meant to clone `PlainType<u32>`, implement `Clone` for `PlainType<u32>`
    |
 LL + #[derive(Clone)]
 LL | struct PlainType<T>(T);
    |
 
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:44:19
+  --> $DIR/noop-method-call.rs:48:19
    |
 LL |     non_clone_type.clone();
    |                   ^^^^^^^^
@@ -81,14 +81,14 @@ help: remove this redundant call
 LL -     non_clone_type.clone();
 LL +     non_clone_type;
    |
-help: if you meant to clone `PlainType<T>`, implement `Clone` for it
+help: if you meant to clone `PlainType<T>`, implement `Clone` for `PlainType<T>`
    |
 LL + #[derive(Clone)]
 LL | struct PlainType<T>(T);
    |
 
 warning: call to `.clone()` on a reference in this situation does nothing
-  --> $DIR/noop-method-call.rs:49:19
+  --> $DIR/noop-method-call.rs:53:19
    |
 LL |     non_clone_type.clone();
    |                   ^^^^^^^^
@@ -99,11 +99,63 @@ help: remove this redundant call
 LL -     non_clone_type.clone();
 LL +     non_clone_type;
    |
-help: if you meant to clone `PlainType<u32>`, implement `Clone` for it
+help: if you meant to clone `PlainType<u32>`, implement `Clone` for `PlainType<u32>`
    |
 LL + #[derive(Clone)]
 LL | struct PlainType<T>(T);
    |
 
-warning: 7 warnings emitted
+warning: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:70:6
+   |
+LL |     v.clone();
+   |      ^^^^^^^^ help: remove this redundant call
+   |
+   = note: the type `non_clone_types::NotClone` does not implement `Clone`, so calling `clone` on `&non_clone_types::NotClone` copies the reference, which does not do anything and can be removed
+
+warning: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:75:6
+   |
+LL |     v.clone();
+   |      ^^^^^^^^
+   |
+   = note: the type `non_clone_types::ConditionalClone<PlainType<u32>>` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone<PlainType<u32>>` copies the reference, which does not do anything and can be removed
+help: remove this redundant call
+   |
+LL -     v.clone();
+LL +     v;
+   |
+help: if you meant to clone `non_clone_types::ConditionalClone<PlainType<u32>>`, implement `Clone` for `PlainType<u32>`
+   |
+LL + #[derive(Clone)]
+LL | struct PlainType<T>(T);
+   |
+
+warning: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:80:6
+   |
+LL |     v.clone();
+   |      ^^^^^^^^ help: remove this redundant call
+   |
+   = note: the type `non_clone_types::ConditionalClone<non_clone_types::NotClone>` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone<non_clone_types::NotClone>` copies the reference, which does not do anything and can be removed
+
+warning: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/noop-method-call.rs:89:6
+   |
+LL |     v.clone();
+   |      ^^^^^^^^
+   |
+   = note: the type `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>` does not implement `Clone`, so calling `clone` on `&non_clone_types::DifferentlyConditionalClone<PlainType<u8>>` copies the reference, which does not do anything and can be removed
+help: remove this redundant call
+   |
+LL -     v.clone();
+LL +     v;
+   |
+help: if you meant to clone `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>`, implement `Clone` for `PlainType<u8>`
+   |
+LL + #[derive(Clone)]
+LL | struct PlainType<T>(T);
+   |
+
+warning: 11 warnings emitted