From 6017de46f7864cabcf4770f11c93f319314a250f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Feb 2024 18:01:12 +0000 Subject: [PATCH] When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]` CC #40699. --- compiler/rustc_lint/messages.ftl | 1 + compiler/rustc_lint/src/lints.rs | 6 ++ compiler/rustc_lint/src/noop_method_call.rs | 12 +++- tests/ui/lint/noop-method-call.fixed | 64 ------------------ tests/ui/lint/noop-method-call.rs | 1 - tests/ui/lint/noop-method-call.stderr | 74 +++++++++++++++++---- 6 files changed, 80 insertions(+), 78 deletions(-) delete mode 100644 tests/ui/lint/noop-method-call.fixed diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 1548646c04a28..496ffeddbffbc 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -429,6 +429,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 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 5fb2ddf91990c..a25ddbce8626a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1314,6 +1314,12 @@ pub struct NoopMethodCallDiag<'a> { pub trait_: Symbol, #[suggestion(code = "", applicability = "machine-applicable")] pub label: Span, + #[suggestion( + lint_derive_suggestion, + code = "#[derive(Clone)]\n", + applicability = "maybe-incorrect" + )] + pub suggest_derive: Option, } #[derive(LintDiagnostic)] diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 26c5e4fb4837d..970d411fb0657 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -121,10 +121,20 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { let orig_ty = expr_ty.peel_refs(); if receiver_ty == expr_ty { + let suggest_derive = match orig_ty.kind() { + ty::Adt(def, _) => Some(cx.tcx.def_span(def.did()).shrink_to_lo()), + _ => None, + }; cx.emit_span_lint( NOOP_METHOD_CALL, span, - NoopMethodCallDiag { method: call.ident.name, orig_ty, trait_, label: span }, + NoopMethodCallDiag { + method: call.ident.name, + orig_ty, + trait_, + label: span, + suggest_derive, + }, ); } else { match name { diff --git a/tests/ui/lint/noop-method-call.fixed b/tests/ui/lint/noop-method-call.fixed deleted file mode 100644 index 279dc8a3cd0cc..0000000000000 --- a/tests/ui/lint/noop-method-call.fixed +++ /dev/null @@ -1,64 +0,0 @@ -//@ check-pass -//@ run-rustfix - -#![feature(rustc_attrs)] -#![allow(unused)] - -use std::borrow::Borrow; -use std::ops::Deref; - -struct PlainType(T); - -#[derive(Clone)] -struct CloneType(T); - -fn check(mut encoded: &[u8]) { - let _ = &mut encoded; - //~^ WARN call to `.clone()` on a reference in this situation does nothing - let _ = &encoded; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -fn main() { - let non_clone_type_ref = &PlainType(1u32); - let non_clone_type_ref_clone: &PlainType = non_clone_type_ref; - //~^ WARN call to `.clone()` on a reference in this situation does nothing - - let clone_type_ref = &CloneType(1u32); - let clone_type_ref_clone: CloneType = clone_type_ref.clone(); - - - let non_deref_type = &PlainType(1u32); - let non_deref_type_deref: &PlainType = non_deref_type; - //~^ WARN call to `.deref()` on a reference in this situation does nothing - - let non_borrow_type = &PlainType(1u32); - let non_borrow_type_borrow: &PlainType = non_borrow_type; - //~^ WARN call to `.borrow()` on a reference in this situation does nothing - - // Borrowing a &&T does not warn since it has collapsed the double reference - let non_borrow_type = &&PlainType(1u32); - let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); -} - -fn generic(non_clone_type: &PlainType) { - non_clone_type; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -fn non_generic(non_clone_type: &PlainType) { - non_clone_type; - //~^ WARN call to `.clone()` on a reference in this situation does nothing -} - -struct DiagnosticClone; -impl Clone for DiagnosticClone { - #[rustc_diagnostic_item = "other_clone"] - fn clone(&self) -> Self { - DiagnosticClone - } -} - -fn with_other_diagnostic_item(x: DiagnosticClone) { - x.clone(); -} diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs index 447a4c62410b0..8db5c889d1c37 100644 --- a/tests/ui/lint/noop-method-call.rs +++ b/tests/ui/lint/noop-method-call.rs @@ -1,5 +1,4 @@ //@ check-pass -//@ run-rustfix #![feature(rustc_attrs)] #![allow(unused)] diff --git a/tests/ui/lint/noop-method-call.stderr b/tests/ui/lint/noop-method-call.stderr index d04f44022eefd..8823644ac2d5f 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:16:25 + --> $DIR/noop-method-call.rs:15: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:18:21 + --> $DIR/noop-method-call.rs:17:21 | LL | let _ = &encoded.clone(); | ^^^^^^^^ help: remove this redundant call @@ -16,44 +16,94 @@ 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:24:71 + --> $DIR/noop-method-call.rs:23:71 | LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +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 + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.deref()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:32:63 + --> $DIR/noop-method-call.rs:31:63 | LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Deref`, so calling `deref` on `&PlainType` copies the reference, which does not do anything and can be removed +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 + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.borrow()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:36:66 + --> $DIR/noop-method-call.rs:35:66 | LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); - | ^^^^^^^^^ help: remove this redundant call + | ^^^^^^^^^ | = note: the type `PlainType` does not implement `Borrow`, so calling `borrow` on `&PlainType` copies the reference, which does not do anything and can be removed +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 + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:45:19 + --> $DIR/noop-method-call.rs:44:19 | LL | non_clone_type.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +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 + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: call to `.clone()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:50:19 + --> $DIR/noop-method-call.rs:49:19 | LL | non_clone_type.clone(); - | ^^^^^^^^ help: remove this redundant call + | ^^^^^^^^ | = note: the type `PlainType` does not implement `Clone`, so calling `clone` on `&PlainType` copies the reference, which does not do anything and can be removed +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 + | +LL + #[derive(Clone)] +LL | struct PlainType(T); + | warning: 7 warnings emitted