diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 7522e21d0ef09..3c0ab275a6bf6 100644 --- 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 index ac995b59caff1..194b82253ea5f 100644 --- 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 index fa519281be531..af4f83b7874a5 100644 --- 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 new file mode 100644 index 0000000000000..40d030943bda3 --- /dev/null +++ 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); + +impl Clone for ConditionalClone { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +pub struct DifferentlyConditionalClone(T); + +impl Clone for DifferentlyConditionalClone { + 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 index 8db5c889d1c37..0f2fba70eb28f 100644 --- 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>) { + v.clone(); + //~^ WARN call to `.clone()` on a reference in this situation does nothing +} + +fn with_only_foreign_types_1(v: &ConditionalClone) { + v.clone(); + //~^ WARN call to `.clone()` on a reference in this situation does nothing +} + +fn with_only_foreign_types_2(v: &ConditionalClone) { + v.clone(); +} + +fn different_impl_bound(v: &DifferentlyConditionalClone>) { + 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 index 8823644ac2d5f..f72c6e5612a7d 100644 --- 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,7 +8,7 @@ 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 @@ -16,7 +16,7 @@ LL | let _ = &encoded.clone(); = 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 = non_clone_type_ref.clone(); | ^^^^^^^^ @@ -27,14 +27,14 @@ help: remove this redundant call LL - let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); LL + let non_clone_type_ref_clone: &PlainType = non_clone_type_ref; | -help: if you meant to clone `PlainType`, implement `Clone` for it +help: if you meant to clone `PlainType`, implement `Clone` for `PlainType` | LL + #[derive(Clone)] LL | struct PlainType(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 = non_deref_type.deref(); | ^^^^^^^^ @@ -45,14 +45,14 @@ help: remove this redundant call LL - let non_deref_type_deref: &PlainType = non_deref_type.deref(); LL + let non_deref_type_deref: &PlainType = non_deref_type; | -help: if you meant to clone `PlainType`, implement `Clone` for it +help: if you meant to clone `PlainType`, implement `Clone` for `PlainType` | LL + #[derive(Clone)] LL | struct PlainType(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 = non_borrow_type.borrow(); | ^^^^^^^^^ @@ -63,14 +63,14 @@ help: remove this redundant call LL - let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); LL + let non_borrow_type_borrow: &PlainType = non_borrow_type; | -help: if you meant to clone `PlainType`, implement `Clone` for it +help: if you meant to clone `PlainType`, implement `Clone` for `PlainType` | LL + #[derive(Clone)] LL | struct PlainType(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`, implement `Clone` for it +help: if you meant to clone `PlainType`, implement `Clone` for `PlainType` | LL + #[derive(Clone)] LL | struct PlainType(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`, implement `Clone` for it +help: if you meant to clone `PlainType`, implement `Clone` for `PlainType` | LL + #[derive(Clone)] LL | struct PlainType(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>` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone>` 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>`, implement `Clone` for `PlainType` + | +LL + #[derive(Clone)] +LL | struct PlainType(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` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone` 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>` does not implement `Clone`, so calling `clone` on `&non_clone_types::DifferentlyConditionalClone>` 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>`, implement `Clone` for `PlainType` + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | + +warning: 11 warnings emitted