Skip to content

Commit 975a775

Browse files
ada4aada4a
committed
feat(clone_on_ref_ptr): don't add a & to the receiver if it's a reference
1 parent 6b697db commit 975a775

File tree

4 files changed

+97
-18
lines changed

4 files changed

+97
-18
lines changed

‎clippy_lints/src/methods/clone_on_ref_ptr.rs‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::source::snippet_with_context;
2+
use clippy_utils::sugg::Sugg;
3+
use clippy_utils::ty::peel_and_count_ty_refs;
34
use rustc_errors::Applicability;
45
use rustc_hir as hir;
56
use rustc_lint::LateContext;
@@ -9,9 +10,10 @@ use rustc_span::symbol::sym;
910
use super::CLONE_ON_REF_PTR;
1011

1112
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::Expr<'_>) {
12-
let obj_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
13+
let receiver_ty = cx.typeck_results().expr_ty(receiver);
14+
let (receiver_ty_peeled, n_refs, _) = peel_and_count_ty_refs(receiver_ty);
1315

14-
if let ty::Adt(adt, subst) = obj_ty.kind()
16+
if let ty::Adt(adt, subst) = receiver_ty_peeled.kind()
1517
&& let Some(name) = cx.tcx.get_diagnostic_name(adt.did())
1618
{
1719
let caller_type = match name {
@@ -29,20 +31,23 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, receiver: &hir::
2931
|diag| {
3032
// Sometimes unnecessary ::<_> after Rc/Arc/Weak
3133
let mut app = Applicability::Unspecified;
32-
let snippet = snippet_with_context(cx, receiver.span, expr.span.ctxt(), "..", &mut app).0;
34+
let mut sugg = Sugg::hir_with_context(cx, receiver, expr.span.ctxt(), "..", &mut app);
35+
if n_refs == 0 {
36+
sugg = sugg.addr();
37+
}
3338
let generic = subst.type_at(0);
3439
if generic.is_suggestable(cx.tcx, true) {
3540
diag.span_suggestion(
3641
expr.span,
3742
"try",
38-
format!("{caller_type}::<{generic}>::clone(&{snippet})"),
43+
format!("{caller_type}::<{generic}>::clone({sugg})"),
3944
app,
4045
);
4146
} else {
4247
diag.span_suggestion(
4348
expr.span,
4449
"try",
45-
format!("{caller_type}::</* generic */>::clone(&{snippet})"),
50+
format!("{caller_type}::</* generic */>::clone({sugg})"),
4651
Applicability::HasPlaceholders,
4752
);
4853
}

‎tests/ui/clone_on_ref_ptr.fixed‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ mod issue2076 {
5050
}
5151
}
5252

53-
#[allow(
54-
clippy::needless_borrow,
55-
reason = "the suggestion creates `Weak::clone(&rec)`, but `rec` is already a reference"
56-
)]
5753
mod issue15009 {
5854
use std::rc::{Rc, Weak};
5955
use std::sync::atomic::{AtomicU32, Ordering};
@@ -62,7 +58,7 @@ mod issue15009 {
6258
let counter = AtomicU32::new(0);
6359
let counter_ref = &counter;
6460
let factorial = Rc::new_cyclic(move |rec| {
65-
let rec = std::rc::Weak::</* generic */>::clone(&rec) as Weak<dyn Fn(u32) -> u32>;
61+
let rec = std::rc::Weak::</* generic */>::clone(rec) as Weak<dyn Fn(u32) -> u32>;
6662
//~^ clone_on_ref_ptr
6763
move |x| {
6864
// can capture env
@@ -79,3 +75,26 @@ mod issue15009 {
7975
println!("{}", counter.load(Ordering::Relaxed)); // 14
8076
}
8177
}
78+
79+
fn issue15741(mut rc: Rc<str>, ref_rc: &Rc<str>, refmut_rc: &mut Rc<str>) {
80+
std::rc::Rc::<str>::clone(&rc);
81+
//~^ clone_on_ref_ptr
82+
std::rc::Rc::<str>::clone(ref_rc);
83+
//~^ clone_on_ref_ptr
84+
std::rc::Rc::<str>::clone(refmut_rc);
85+
//~^ clone_on_ref_ptr
86+
87+
// The following cases already cause warn-by-default lints to fire, and the suggestion just makes
88+
// another set of warn-by-default lints to fire, so this is probably fine
89+
90+
#[allow(clippy::needless_borrow, clippy::unnecessary_mut_passed)] // before the suggestion
91+
#[allow(clippy::double_parens)] // after the suggestion
92+
{
93+
std::rc::Rc::<str>::clone(&(rc));
94+
//~^ clone_on_ref_ptr
95+
std::rc::Rc::<str>::clone((&rc));
96+
//~^ clone_on_ref_ptr
97+
std::rc::Rc::<str>::clone((&mut rc));
98+
//~^ clone_on_ref_ptr
99+
};
100+
}

‎tests/ui/clone_on_ref_ptr.rs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ mod issue2076 {
5050
}
5151
}
5252

53-
#[allow(
54-
clippy::needless_borrow,
55-
reason = "the suggestion creates `Weak::clone(&rec)`, but `rec` is already a reference"
56-
)]
5753
mod issue15009 {
5854
use std::rc::{Rc, Weak};
5955
use std::sync::atomic::{AtomicU32, Ordering};
@@ -79,3 +75,26 @@ mod issue15009 {
7975
println!("{}", counter.load(Ordering::Relaxed)); // 14
8076
}
8177
}
78+
79+
fn issue15741(mut rc: Rc<str>, ref_rc: &Rc<str>, refmut_rc: &mut Rc<str>) {
80+
rc.clone();
81+
//~^ clone_on_ref_ptr
82+
ref_rc.clone();
83+
//~^ clone_on_ref_ptr
84+
refmut_rc.clone();
85+
//~^ clone_on_ref_ptr
86+
87+
// The following cases already cause warn-by-default lints to fire, and the suggestion just makes
88+
// another set of warn-by-default lints to fire, so this is probably fine
89+
90+
#[allow(clippy::needless_borrow, clippy::unnecessary_mut_passed)] // before the suggestion
91+
#[allow(clippy::double_parens)] // after the suggestion
92+
{
93+
(rc).clone();
94+
//~^ clone_on_ref_ptr
95+
(&rc).clone();
96+
//~^ clone_on_ref_ptr
97+
(&mut rc).clone();
98+
//~^ clone_on_ref_ptr
99+
};
100+
}

‎tests/ui/clone_on_ref_ptr.stderr‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,46 @@ LL | Some(try_opt!(Some(rc)).clone())
3838
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::rc::Rc::<u8>::clone(&try_opt!(Some(rc)))`
3939

4040
error: using `.clone()` on a ref-counted pointer
41-
--> tests/ui/clone_on_ref_ptr.rs:65:23
41+
--> tests/ui/clone_on_ref_ptr.rs:61:23
4242
|
4343
LL | let rec = rec.clone() as Weak<dyn Fn(u32) -> u32>;
44-
| ^^^^^^^^^^^ help: try: `std::rc::Weak::</* generic */>::clone(&rec)`
44+
| ^^^^^^^^^^^ help: try: `std::rc::Weak::</* generic */>::clone(rec)`
4545

46-
error: aborting due to 7 previous errors
46+
error: using `.clone()` on a ref-counted pointer
47+
--> tests/ui/clone_on_ref_ptr.rs:80:5
48+
|
49+
LL | rc.clone();
50+
| ^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone(&rc)`
51+
52+
error: using `.clone()` on a ref-counted pointer
53+
--> tests/ui/clone_on_ref_ptr.rs:82:5
54+
|
55+
LL | ref_rc.clone();
56+
| ^^^^^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone(ref_rc)`
57+
58+
error: using `.clone()` on a ref-counted pointer
59+
--> tests/ui/clone_on_ref_ptr.rs:84:5
60+
|
61+
LL | refmut_rc.clone();
62+
| ^^^^^^^^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone(refmut_rc)`
63+
64+
error: using `.clone()` on a ref-counted pointer
65+
--> tests/ui/clone_on_ref_ptr.rs:93:9
66+
|
67+
LL | (rc).clone();
68+
| ^^^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone(&(rc))`
69+
70+
error: using `.clone()` on a ref-counted pointer
71+
--> tests/ui/clone_on_ref_ptr.rs:95:9
72+
|
73+
LL | (&rc).clone();
74+
| ^^^^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone((&rc))`
75+
76+
error: using `.clone()` on a ref-counted pointer
77+
--> tests/ui/clone_on_ref_ptr.rs:97:9
78+
|
79+
LL | (&mut rc).clone();
80+
| ^^^^^^^^^^^^^^^^^ help: try: `std::rc::Rc::<str>::clone((&mut rc))`
81+
82+
error: aborting due to 13 previous errors
4783

0 commit comments

Comments
 (0)