From fed2f28223c43e56da6a31e6eb8c3fd5d84606d2 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sat, 23 Mar 2024 00:24:30 +0100 Subject: [PATCH] Do not rewrite `.as_ref().map(Arc::clone)` and similar --- clippy_lints/src/methods/useless_asref.rs | 4 ++- tests/ui/useless_asref.fixed | 18 ++++++++++++ tests/ui/useless_asref.rs | 18 ++++++++++++ tests/ui/useless_asref.stderr | 36 +++++++++++------------ 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/methods/useless_asref.rs b/clippy_lints/src/methods/useless_asref.rs index b8baad18cc8..474103fccd2 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::walk_ptrs_ty_depth; +use clippy_utils::ty::{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, paths, peel_blocks, strip_pat_refs, }; @@ -93,6 +93,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str, // And that it only has one argument. && let [arg] = args && 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) { 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 c98f2928e03..ddbb9255b46 100644 --- a/tests/ui/useless_asref.fixed +++ b/tests/ui/useless_asref.fixed @@ -8,6 +8,8 @@ )] use std::fmt::Debug; +use std::rc::{Rc, Weak as RcWeak}; +use std::sync::{Arc, Weak as ArcWeak}; struct FakeAsRef; @@ -180,6 +182,22 @@ mod issue12135 { } } +fn issue_12528() { + struct Foo; + + let opt = Some(Arc::new(Foo)); + let _ = opt.as_ref().map(Arc::clone); + + let opt = Some(Rc::new(Foo)); + let _ = opt.as_ref().map(Rc::clone); + + let opt = Some(Arc::downgrade(&Arc::new(Foo))); + let _ = opt.as_ref().map(ArcWeak::clone); + + let opt = Some(Rc::downgrade(&Rc::new(Foo))); + let _ = opt.as_ref().map(RcWeak::clone); +} + fn main() { not_ok(); ok(); diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs index f9d603f116d..b0405e930a2 100644 --- a/tests/ui/useless_asref.rs +++ b/tests/ui/useless_asref.rs @@ -8,6 +8,8 @@ )] use std::fmt::Debug; +use std::rc::{Rc, Weak as RcWeak}; +use std::sync::{Arc, Weak as ArcWeak}; struct FakeAsRef; @@ -180,6 +182,22 @@ pub fn f(x: &Struct) -> Option { } } +fn issue_12528() { + struct Foo; + + let opt = Some(Arc::new(Foo)); + let _ = opt.as_ref().map(Arc::clone); + + let opt = Some(Rc::new(Foo)); + let _ = opt.as_ref().map(Rc::clone); + + let opt = Some(Arc::downgrade(&Arc::new(Foo))); + let _ = opt.as_ref().map(ArcWeak::clone); + + let opt = Some(Rc::downgrade(&Rc::new(Foo))); + let _ = opt.as_ref().map(RcWeak::clone); +} + fn main() { not_ok(); ok(); diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr index c7d622ec2dc..5f495c39670 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:48:18 + --> tests/ui/useless_asref.rs:50: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:50:20 + --> tests/ui/useless_asref.rs:52:20 | LL | foo_rslice(rslice.as_ref()); | ^^^^^^^^^^^^^^^ help: try: `rslice` error: this call to `as_mut` does nothing - --> tests/ui/useless_asref.rs:54:21 + --> tests/ui/useless_asref.rs:56:21 | LL | foo_mrslice(mrslice.as_mut()); | ^^^^^^^^^^^^^^^^ help: try: `mrslice` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:56:20 + --> tests/ui/useless_asref.rs:58:20 | LL | foo_rslice(mrslice.as_ref()); | ^^^^^^^^^^^^^^^^ help: try: `mrslice` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:63:20 + --> tests/ui/useless_asref.rs:65:20 | LL | foo_rslice(rrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^ help: try: `rrrrrslice` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:65:18 + --> tests/ui/useless_asref.rs:67:18 | LL | foo_rstr(rrrrrstr.as_ref()); | ^^^^^^^^^^^^^^^^^ help: try: `rrrrrstr` error: this call to `as_mut` does nothing - --> tests/ui/useless_asref.rs:70:21 + --> tests/ui/useless_asref.rs:72:21 | LL | foo_mrslice(mrrrrrslice.as_mut()); | ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:72:20 + --> tests/ui/useless_asref.rs:74:20 | LL | foo_rslice(mrrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^^ help: try: `mrrrrrslice` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:76:16 + --> tests/ui/useless_asref.rs:78:16 | LL | foo_rrrrmr((&&&&MoreRef).as_ref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `(&&&&MoreRef)` error: this call to `as_mut` does nothing - --> tests/ui/useless_asref.rs:126:13 + --> tests/ui/useless_asref.rs:128:13 | LL | foo_mrt(mrt.as_mut()); | ^^^^^^^^^^^^ help: try: `mrt` error: this call to `as_ref` does nothing - --> tests/ui/useless_asref.rs:128:12 + --> tests/ui/useless_asref.rs:130:12 | LL | foo_rt(mrt.as_ref()); | ^^^^^^^^^^^^ help: try: `mrt` error: this call to `as_ref.map(...)` does nothing - --> tests/ui/useless_asref.rs:139:13 + --> tests/ui/useless_asref.rs:141: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:141:13 + --> tests/ui/useless_asref.rs:143: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:143:13 + --> tests/ui/useless_asref.rs:145: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:167:9 + --> tests/ui/useless_asref.rs:169: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:169:9 + --> tests/ui/useless_asref.rs:171: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:171:9 + --> tests/ui/useless_asref.rs:173: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:176:9 + --> tests/ui/useless_asref.rs:178:9 | LL | Some(1).as_ref().map(|&x| x.clone()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()`