fix(clone_on_ref_ptr): only name the generic type if possible

Author: ada4a

clippy_lints/src/methods/clone_on_ref_ptr.rs

@@ -3,7 +3,7 @@ use clippy_utils::source::snippet_with_context;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
-use rustc_middle::ty;
+use rustc_middle::ty::{self, IsSuggestable};
 use rustc_span::symbol::sym;
 
 use super::CLONE_ON_REF_PTR;
@@ -30,12 +30,22 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::
                 // Sometimes unnecessary ::<_> after Rc/Arc/Weak
                 let mut app = Applicability::Unspecified;
                 let snippet = snippet_with_context(cx, receiver.span, expr.span.ctxt(), "..", &mut app).0;
-                diag.span_suggestion(
-                    expr.span,
-                    "try",
-                    format!("{caller_type}::<{}>::clone(&{snippet})", subst.type_at(0)),
-                    app,
-                );
+                let generic = subst.type_at(0);
+                if generic.is_suggestable(cx.tcx, true) {
+                    diag.span_suggestion(
+                        expr.span,
+                        "try",
+                        format!("{caller_type}::<{generic}>::clone(&{snippet})"),
+                        app,
+                    );
+                } else {
+                    diag.span_suggestion(
+                        expr.span,
+                        "try",
+                        format!("{caller_type}::</* generic */>::clone(&{snippet})"),
+                        Applicability::HasPlaceholders,
+                    );
+                }
             },
         );
     }

tests/ui/clone_on_ref_ptr.fixed

@@ -49,3 +49,33 @@ mod issue2076 {
         //~^ clone_on_ref_ptr
     }
 }
+
+#[allow(
+    clippy::needless_borrow,
+    reason = "the suggestion creates `Weak::clone(&rec)`, but `rec` is already a reference"
+)]
+mod issue15009 {
+    use std::rc::{Rc, Weak};
+    use std::sync::atomic::{AtomicU32, Ordering};
+
+    fn main() {
+        let counter = AtomicU32::new(0);
+        let counter_ref = &counter;
+        let factorial = Rc::new_cyclic(move |rec| {
+            let rec = std::rc::Weak::</* generic */>::clone(&rec) as Weak<dyn Fn(u32) -> u32>;
+            //~^ clone_on_ref_ptr
+            move |x| {
+                // can capture env
+                counter_ref.fetch_add(1, Ordering::Relaxed);
+                match x {
+                    0 => 1,
+                    x => x * rec.upgrade().unwrap()(x - 1),
+                }
+            }
+        });
+        println!("{}", factorial(5)); // 120
+        println!("{}", counter.load(Ordering::Relaxed)); // 6
+        println!("{}", factorial(7)); // 5040
+        println!("{}", counter.load(Ordering::Relaxed)); // 14
+    }
+}

tests/ui/clone_on_ref_ptr.rs

@@ -49,3 +49,33 @@ mod issue2076 {
         //~^ clone_on_ref_ptr
     }
 }
+
+#[allow(
+    clippy::needless_borrow,
+    reason = "the suggestion creates `Weak::clone(&rec)`, but `rec` is already a reference"
+)]
+mod issue15009 {
+    use std::rc::{Rc, Weak};
+    use std::sync::atomic::{AtomicU32, Ordering};
+
+    fn main() {
+        let counter = AtomicU32::new(0);
+        let counter_ref = &counter;
+        let factorial = Rc::new_cyclic(move |rec| {
+            let rec = rec.clone() as Weak<dyn Fn(u32) -> u32>;
+            //~^ clone_on_ref_ptr
+            move |x| {
+                // can capture env
+                counter_ref.fetch_add(1, Ordering::Relaxed);
+                match x {
+                    0 => 1,
+                    x => x * rec.upgrade().unwrap()(x - 1),
+                }
+            }
+        });
+        println!("{}", factorial(5)); // 120
+        println!("{}", counter.load(Ordering::Relaxed)); // 6
+        println!("{}", factorial(7)); // 5040
+        println!("{}", counter.load(Ordering::Relaxed)); // 14
+    }
+}

tests/ui/clone_on_ref_ptr.stderr

@@ -37,5 +37,11 @@ error: using `.clone()` on a ref-counted pointer
 LL |         Some(try_opt!(Some(rc)).clone())
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::rc::Rc::<u8>::clone(&try_opt!(Some(rc)))`
 
-error: aborting due to 6 previous errors
+error: using `.clone()` on a ref-counted pointer
+  --> tests/ui/clone_on_ref_ptr.rs:65:23
+   |
+LL |             let rec = rec.clone() as Weak<dyn Fn(u32) -> u32>;
+   |                       ^^^^^^^^^^^ help: try: `std::rc::Weak::</* generic */>::clone(&rec)`
+
+error: aborting due to 7 previous errors