about summary refs log tree commit diff
diff options
context:
space:
mode:
authorTimo <30553356+y21@users.noreply.github.com>2025-02-22 14:04:57 +0000
committerGitHub <noreply@github.com>2025-02-22 14:04:57 +0000
commitd5488b3b631826269e2e6ea5f67a31b7d3fa7d0c (patch)
tree53d0b015f0cd9febd88e1668a9c9fea378d7aa03
parente479a9ff959df44e8b582834d123705e4b3477e1 (diff)
parent91548d0fe33eeab33db48e81d7a07d7edee0965a (diff)
downloadrust-d5488b3b631826269e2e6ea5f67a31b7d3fa7d0c.tar.gz
rust-d5488b3b631826269e2e6ea5f67a31b7d3fa7d0c.zip
`useless_asref`: add a check for `Clone` before suggesting the use of `.clone()` (#14174)
fixes #12357

changelog: [`useless_asref`]: don't suggest to use `.clone()` if the
target type doesn't implement the `Clone` trait
-rw-r--r--clippy_lints/src/methods/useless_asref.rs5
-rw-r--r--tests/ui/useless_asref.fixed30
-rw-r--r--tests/ui/useless_asref.rs30
-rw-r--r--tests/ui/useless_asref.stderr36
4 files changed, 82 insertions, 19 deletions
diff --git a/clippy_lints/src/methods/useless_asref.rs b/clippy_lints/src/methods/useless_asref.rs
index 1d43c585269..0cbf6004be3 100644
--- a/clippy_lints/src/methods/useless_asref.rs
+++ b/clippy_lints/src/methods/useless_asref.rs
@@ -1,6 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::ty::{should_call_clone_as_function, walk_ptrs_ty_depth};
+use clippy_utils::ty::{implements_trait, should_call_clone_as_function, walk_ptrs_ty_depth};
 use clippy_utils::{
     get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, peel_blocks, strip_pat_refs,
 };
@@ -101,6 +101,9 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str,
             && is_calling_clone(cx, arg)
             // And that we are not recommending recv.clone() over Arc::clone() or similar
             && !should_call_clone_as_function(cx, rcv_ty)
+            // https://github.com/rust-lang/rust-clippy/issues/12357
+            && let Some(clone_trait) = cx.tcx.lang_items().clone_trait()
+            && implements_trait(cx, cx.typeck_results().expr_ty(recvr), clone_trait, &[])
         {
             lint_as_ref_clone(cx, expr.span.with_hi(parent.span.hi()), recvr, call_name);
         }
diff --git a/tests/ui/useless_asref.fixed b/tests/ui/useless_asref.fixed
index e399bd76fc1..8c1f948fb58 100644
--- a/tests/ui/useless_asref.fixed
+++ b/tests/ui/useless_asref.fixed
@@ -8,6 +8,7 @@
 )]
 
 use std::fmt::Debug;
+use std::ops::Deref;
 use std::rc::{Rc, Weak as RcWeak};
 use std::sync::{Arc, Weak as ArcWeak};
 
@@ -218,6 +219,35 @@ fn issue_14088() {
     let _: Option<&str> = s.as_ref().map(|x| x.as_ref());
 }
 
+pub struct Wrap<T> {
+    inner: T,
+}
+
+impl<T> Deref for Wrap<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+struct NonCloneableError;
+
+pub struct Issue12357 {
+    current: Option<Wrap<Arc<u32>>>,
+}
+
+impl Issue12357 {
+    fn f(&self) -> Option<Arc<u32>> {
+        self.current.as_ref().map(|p| Arc::clone(p))
+    }
+
+    fn g(&self) {
+        let result: Result<String, NonCloneableError> = Ok("Hello".to_string());
+        let cloned = result.as_ref().map(|s| s.clone());
+    }
+}
+
 fn main() {
     not_ok();
     ok();
diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs
index 4c76a2ecf7b..d9db2d4f559 100644
--- a/tests/ui/useless_asref.rs
+++ b/tests/ui/useless_asref.rs
@@ -8,6 +8,7 @@
 )]
 
 use std::fmt::Debug;
+use std::ops::Deref;
 use std::rc::{Rc, Weak as RcWeak};
 use std::sync::{Arc, Weak as ArcWeak};
 
@@ -218,6 +219,35 @@ fn issue_14088() {
     let _: Option<&str> = s.as_ref().map(|x| x.as_ref());
 }
 
+pub struct Wrap<T> {
+    inner: T,
+}
+
+impl<T> Deref for Wrap<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.inner
+    }
+}
+
+struct NonCloneableError;
+
+pub struct Issue12357 {
+    current: Option<Wrap<Arc<u32>>>,
+}
+
+impl Issue12357 {
+    fn f(&self) -> Option<Arc<u32>> {
+        self.current.as_ref().map(|p| Arc::clone(p))
+    }
+
+    fn g(&self) {
+        let result: Result<String, NonCloneableError> = Ok("Hello".to_string());
+        let cloned = result.as_ref().map(|s| s.clone());
+    }
+}
+
 fn main() {
     not_ok();
     ok();
diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr
index d0ca10936fd..8255f5d9d2a 100644
--- a/tests/ui/useless_asref.stderr
+++ b/tests/ui/useless_asref.stderr
@@ -1,5 +1,5 @@
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:50:18
+  --> tests/ui/useless_asref.rs:51:18
    |
 LL |         foo_rstr(rstr.as_ref());
    |                  ^^^^^^^^^^^^^ help: try: `rstr`
@@ -11,103 +11,103 @@ LL | #![deny(clippy::useless_asref)]
    |         ^^^^^^^^^^^^^^^^^^^^^
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:53:20
+  --> tests/ui/useless_asref.rs:54:20
    |
 LL |         foo_rslice(rslice.as_ref());
    |                    ^^^^^^^^^^^^^^^ help: try: `rslice`
 
 error: this call to `as_mut` does nothing
-  --> tests/ui/useless_asref.rs:58:21
+  --> tests/ui/useless_asref.rs:59:21
    |
 LL |         foo_mrslice(mrslice.as_mut());
    |                     ^^^^^^^^^^^^^^^^ help: try: `mrslice`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:61:20
+  --> tests/ui/useless_asref.rs:62:20
    |
 LL |         foo_rslice(mrslice.as_ref());
    |                    ^^^^^^^^^^^^^^^^ help: try: `mrslice`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:69:20
+  --> tests/ui/useless_asref.rs:70:20
    |
 LL |         foo_rslice(rrrrrslice.as_ref());
    |                    ^^^^^^^^^^^^^^^^^^^ help: try: `rrrrrslice`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:72:18
+  --> tests/ui/useless_asref.rs:73:18
    |
 LL |         foo_rstr(rrrrrstr.as_ref());
    |                  ^^^^^^^^^^^^^^^^^ help: try: `rrrrrstr`
 
 error: this call to `as_mut` does nothing
-  --> tests/ui/useless_asref.rs:78:21
+  --> tests/ui/useless_asref.rs:79:21
    |
 LL |         foo_mrslice(mrrrrrslice.as_mut());
    |                     ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:81:20
+  --> tests/ui/useless_asref.rs:82:20
    |
 LL |         foo_rslice(mrrrrrslice.as_ref());
    |                    ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:86:16
+  --> tests/ui/useless_asref.rs:87:16
    |
 LL |     foo_rrrrmr((&&&&MoreRef).as_ref());
    |                ^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&&&&MoreRef)`
 
 error: this call to `as_mut` does nothing
-  --> tests/ui/useless_asref.rs:137:13
+  --> tests/ui/useless_asref.rs:138:13
    |
 LL |     foo_mrt(mrt.as_mut());
    |             ^^^^^^^^^^^^ help: try: `mrt`
 
 error: this call to `as_ref` does nothing
-  --> tests/ui/useless_asref.rs:140:12
+  --> tests/ui/useless_asref.rs:141:12
    |
 LL |     foo_rt(mrt.as_ref());
    |            ^^^^^^^^^^^^ help: try: `mrt`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:152:13
+  --> tests/ui/useless_asref.rs:153:13
    |
 LL |     let z = x.as_ref().map(String::clone);
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:155:13
+  --> tests/ui/useless_asref.rs:156:13
    |
 LL |     let z = x.as_ref().map(|z| z.clone());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:158:13
+  --> tests/ui/useless_asref.rs:159:13
    |
 LL |     let z = x.as_ref().map(|z| String::clone(z));
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:182:9
+  --> tests/ui/useless_asref.rs:183:9
    |
 LL |         x.field.as_ref().map(|v| v.clone());
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:185:9
+  --> tests/ui/useless_asref.rs:186:9
    |
 LL |         x.field.as_ref().map(Clone::clone);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:188:9
+  --> tests/ui/useless_asref.rs:189:9
    |
 LL |         x.field.as_ref().map(|v| Clone::clone(v));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
 
 error: this call to `as_ref.map(...)` does nothing
-  --> tests/ui/useless_asref.rs:193:9
+  --> tests/ui/useless_asref.rs:194:9
    |
 LL |         Some(1).as_ref().map(|&x| x.clone());
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()`