merge clone_double_ref with noop_method_call

Author: fee1-dead

compiler/rustc_lint/messages.ftl

@@ -6,11 +6,7 @@ lint_array_into_iter =
         or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value
 
 lint_clone_double_ref =
-    using `clone` on a double-reference, which copies the reference of type `{$ty}` instead of cloning the inner type
-
-lint_clone_double_ref_try_deref = try dereferencing it
-
-lint_clone_double_ref_sugg_explicit = or try being explicit if you are sure, that you want to clone a reference
+    using `.{$call}()` on a double reference, which copies `{$ty}` instead of {$op} the inner type
 
 lint_enum_intrinsics_mem_discriminant =
     the return value of `mem::discriminant` is unspecified when called with a non-enum type

compiler/rustc_lint/src/clone_double_ref.rs

@@ -50,7 +50,6 @@ extern crate tracing;
 
 mod array_into_iter;
 pub mod builtin;
-mod clone_double_ref;
 mod context;
 mod deref_into_dyn_supertrait;
 mod early;
@@ -96,7 +95,6 @@ use rustc_span::Span;
 
 use array_into_iter::ArrayIntoIter;
 use builtin::*;
-use clone_double_ref::CloneDoubleRef;
 use deref_into_dyn_supertrait::*;
 use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
 use for_loops_over_fallibles::*;
@@ -201,7 +199,6 @@ late_lint_methods!(
     [
         BuiltinCombinedModuleLateLintPass,
         [
-            CloneDoubleRef: CloneDoubleRef,
             ForLoopsOverFallibles: ForLoopsOverFallibles,
             DerefIntoDynSupertrait: DerefIntoDynSupertrait,
             HardwiredLints: HardwiredLints,

compiler/rustc_lint/src/lib.rs

@@ -567,33 +567,9 @@ pub struct BuiltinUnexpectedCliConfigValue {
 #[derive(LintDiagnostic)]
 #[diag(lint_clone_double_ref)]
 pub struct CloneDoubleRef<'a> {
+    pub call: Symbol,
     pub ty: Ty<'a>,
-    #[subdiagnostic]
-    pub try_deref: CloneDoubleRefTryDeref,
-    #[subdiagnostic]
-    pub explicit: CloneDoubleRefExplicit<'a>,
-}
-
-#[derive(Subdiagnostic)]
-#[multipart_suggestion(lint_clone_double_ref_sugg_explicit, applicability = "maybe-incorrect")]
-pub struct CloneDoubleRefExplicit<'a> {
-    #[suggestion_part(code = "<{refs}{ty}>::clone(")]
-    pub start: Span,
-    #[suggestion_part(code = ")")]
-    pub end: Span,
-    pub refs: String,
-    pub ty: Ty<'a>,
-}
-
-#[derive(Subdiagnostic)]
-#[multipart_suggestion(lint_clone_double_ref_try_deref, applicability = "maybe-incorrect")]
-pub struct CloneDoubleRefTryDeref {
-    #[suggestion_part(code = "{refs}({derefs}")]
-    pub start: Span,
-    #[suggestion_part(code = ").clone()")]
-    pub end: Span,
-    pub refs: String,
-    pub derefs: String,
+    pub op: &'static str,
 }
 
 // deref_into_dyn_supertrait.rs

compiler/rustc_lint/src/lints.rs

@@ -1,10 +1,11 @@
 use crate::context::LintContext;
-use crate::lints::NoopMethodCallDiag;
+use crate::lints::{CloneDoubleRef, NoopMethodCallDiag};
 use crate::LateContext;
 use crate::LateLintPass;
 use rustc_hir::def::DefKind;
 use rustc_hir::{Expr, ExprKind};
 use rustc_middle::ty;
+use rustc_middle::ty::adjustment::Adjust;
 use rustc_span::symbol::sym;
 
 declare_lint! {
@@ -31,18 +32,32 @@ declare_lint! {
     /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything
     /// as references are copy. This lint detects these calls and warns the user about them.
     pub NOOP_METHOD_CALL,
-    Allow,
+    Warn,
     "detects the use of well-known noop methods"
 }
 
-declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]);
+declare_lint! {
+    /// The `clone_double_ref` lint checks for usage of `.clone()` on an `&&T`,
+    /// which copies the inner `&T`, instead of cloning the underlying `T` and
+    /// can be confusing.
+    pub CLONE_DOUBLE_REF,
+    Warn,
+    "using `clone` on `&&T`"
+}
+
+declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL, CLONE_DOUBLE_REF]);
 
 impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         // We only care about method calls.
-        let ExprKind::MethodCall(call, receiver, ..) = &expr.kind else {
-            return
+        let ExprKind::MethodCall(call, receiver, _, call_span) = &expr.kind else {
+            return;
         };
+
+        if call_span.from_expansion() {
+            return;
+        }
+
         // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow`
         // traits and ignore any other method call.
         let did = match cx.typeck_results().type_dependent_def(expr.hir_id) {
@@ -70,25 +85,40 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
         };
         // (Re)check that it implements the noop diagnostic.
         let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return };
-        if !matches!(
-            name,
-            sym::noop_method_borrow | sym::noop_method_clone | sym::noop_method_deref
-        ) {
-            return;
-        }
+
+        let op = match name {
+            sym::noop_method_borrow => "borrowing",
+            sym::noop_method_clone => "cloning",
+            sym::noop_method_deref => "dereferencing",
+            _ => return,
+        };
+
         let receiver_ty = cx.typeck_results().expr_ty(receiver);
         let expr_ty = cx.typeck_results().expr_ty_adjusted(expr);
-        if receiver_ty != expr_ty {
-            // This lint will only trigger if the receiver type and resulting expression \
-            // type are the same, implying that the method call is unnecessary.
+
+        let arg_adjustments = cx.typeck_results().expr_adjustments(receiver);
+
+        // If there is any user defined auto-deref step, then we don't want to warn.
+        // https://github.com/rust-lang/rust-clippy/issues/9272
+        if arg_adjustments.iter().any(|adj| matches!(adj.kind, Adjust::Deref(Some(_)))) {
             return;
         }
+
         let expr_span = expr.span;
         let span = expr_span.with_lo(receiver.span.hi());
-        cx.emit_spanned_lint(
-            NOOP_METHOD_CALL,
-            span,
-            NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span },
-        );
+
+        if receiver_ty == expr_ty {
+            cx.emit_spanned_lint(
+                NOOP_METHOD_CALL,
+                span,
+                NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span },
+            );
+        } else {
+            cx.emit_spanned_lint(
+                CLONE_DOUBLE_REF,
+                span,
+                CloneDoubleRef { call: call.ident.name, ty: expr_ty, op },
+            )
+        }
     }
 }

compiler/rustc_lint/src/noop_method_call.rs

@@ -89,7 +89,7 @@ macro_rules! specialize_for_lengths {
                 $num => {
                     for s in iter {
                         copy_slice_and_advance!(target, sep_bytes);
-                        let content_bytes = s.borrow().as_ref();
+                        let content_bytes = s.as_ref();
                         copy_slice_and_advance!(target, content_bytes);
                     }
                 },
@@ -98,7 +98,7 @@ macro_rules! specialize_for_lengths {
                 // arbitrary non-zero size fallback
                 for s in iter {
                     copy_slice_and_advance!(target, sep_bytes);
-                    let content_bytes = s.borrow().as_ref();
+                    let content_bytes = s.as_ref();
                     copy_slice_and_advance!(target, content_bytes);
                 }
             }

library/alloc/src/str.rs

@@ -1,6 +1,8 @@
 // run-pass
 // pretty-expanded FIXME #23616
 
+#![allow(noop_method_call)]
+
 struct NoClone;
 
 fn main() {

tests/ui/issues/issue-11820.rs

@@ -1,11 +1,11 @@
-#![feature(once_cell)]
-#![deny(clone_double_ref)]
+#![feature(lazy_cell)]
+#![deny(clone_double_ref, noop_method_call)]
 
 pub fn clone_on_double_ref() {
     let x = vec![1];
     let y = &&x;
     let z: &Vec<_> = y.clone();
-    //~^ ERROR using `clone` on a double-reference, which copies the reference of type `Vec<i32>`
+    //~^ ERROR using `.clone()` on a double reference, which copies `&Vec<i32>`
 
     println!("{:p} {:p}", *y, z);
 }
@@ -22,7 +22,9 @@ fn rust_clippy_issue_9272() {
 
 fn check(mut encoded: &[u8]) {
     let _ = &mut encoded.clone();
+    //~^ ERROR call to `.clone()` on a reference in this situation does nothing
     let _ = &encoded.clone();
+    //~^ ERROR call to `.clone()` on a reference in this situation does nothing
 }
 
 fn main() {}

tests/ui/lint/clone-double-ref.rs

@@ -1,22 +1,35 @@
-error: using `clone` on a double-reference, which copies the reference of type `Vec<i32>` instead of cloning the inner type
-  --> $DIR/clone-double-ref.rs:7:22
+error: using `.clone()` on a double reference, which copies `&Vec<i32>` instead of cloning the inner type
+  --> $DIR/clone-double-ref.rs:7:23
    |
 LL |     let z: &Vec<_> = y.clone();
-   |                      ^^^^^^^^^
+   |                       ^^^^^^^^
    |
 note: the lint level is defined here
   --> $DIR/clone-double-ref.rs:2:9
    |
-LL | #![deny(clone_double_ref)]
+LL | #![deny(clone_double_ref, noop_method_call)]
    |         ^^^^^^^^^^^^^^^^
-help: try dereferencing it
+
+error: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/clone-double-ref.rs:24:25
+   |
+LL |     let _ = &mut encoded.clone();
+   |                         ^^^^^^^^ unnecessary method call
+   |
+   = note: the type `&[u8]` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
+note: the lint level is defined here
+  --> $DIR/clone-double-ref.rs:2:27
+   |
+LL | #![deny(clone_double_ref, noop_method_call)]
+   |                           ^^^^^^^^^^^^^^^^
+
+error: call to `.clone()` on a reference in this situation does nothing
+  --> $DIR/clone-double-ref.rs:26:21
    |
-LL |     let z: &Vec<_> = &(*y).clone();
-   |                      +++ ~~~~~~~~~
-help: or try being explicit if you are sure, that you want to clone a reference
+LL |     let _ = &encoded.clone();
+   |                     ^^^^^^^^ unnecessary method call
    |
-LL |     let z: &Vec<_> = <&Vec<i32>>::clone(y);
-   |                      +++++++++++++++++++ ~
+   = note: the type `&[u8]` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed
 
-error: aborting due to previous error
+error: aborting due to 3 previous errors
 

tests/ui/lint/clone-double-ref.stderr

@@ -15,24 +15,21 @@ fn main() {
     let non_clone_type_ref = &PlainType(1u32);
     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
     //~^ WARNING call to `.clone()` on a reference in this situation does nothing
-    //~| WARNING using `clone` on a double-reference, which copies the reference of type `PlainType<u32>`
 
     let clone_type_ref = &CloneType(1u32);
     let clone_type_ref_clone: CloneType<u32> = clone_type_ref.clone();
 
-    // Calling clone on a double reference doesn't warn since the method call itself
-    // peels the outer reference off
     let clone_type_ref = &&CloneType(1u32);
     let clone_type_ref_clone: &CloneType<u32> = clone_type_ref.clone();
-    //~^ WARNING using `clone` on a double-reference, which copies the reference of type `CloneType<u32>`
+    //~^ WARNING using `.clone()` on a double reference, which copies `&CloneType<u32>`
 
     let non_deref_type = &PlainType(1u32);
     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
     //~^ WARNING call to `.deref()` on a reference in this situation does nothing
 
-    // Dereferencing a &&T does not warn since it has collapsed the double reference
     let non_deref_type = &&PlainType(1u32);
     let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
+    //~^ WARNING using `.deref()` on a double reference, which copies `&PlainType<u32>`
 
     let non_borrow_type = &PlainType(1u32);
     let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
@@ -44,7 +41,7 @@ fn main() {
 
     let xs = ["a", "b", "c"];
     let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead
-    //~^ WARNING using `clone` on a double-reference, which copies the reference of type `str`
+    //~^ WARNING using `.clone()` on a double reference, which copies `&str`
 }
 
 fn generic<T>(non_clone_type: &PlainType<T>) {