Skip to content

Commit 231455b

Browse files
KobzolKobzol
committed
Remove ref special case from assigning_clones 
1 parent 86717f2 commit 231455b

File tree

4 files changed

+72
-49
lines changed

4 files changed

+72
-49
lines changed

‎clippy_lints/src/assigning_clones.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,7 @@ impl<'tcx> CallCandidate<'tcx> {
249249
TargetTrait::Clone => {
250250
match self.kind {
251251
CallKind::MethodCall { receiver } => {
252-
let receiver_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
253-
// `*lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
254-
Sugg::hir_with_applicability(cx, ref_expr, "_", applicability)
255-
} else {
256-
// `lhs = self_expr.clone();` -> `lhs.clone_from(self_expr)`
257-
Sugg::hir_with_applicability(cx, lhs, "_", applicability)
258-
}
259-
.maybe_par();
252+
let receiver_sugg = Sugg::hir_with_applicability(cx, lhs, "_", applicability).maybe_par();
260253

261254
// Determine whether we need to reference the argument to clone_from().
262255
let clone_receiver_type = cx.typeck_results().expr_ty(receiver);
@@ -271,13 +264,7 @@ impl<'tcx> CallCandidate<'tcx> {
271264
format!("{receiver_sugg}.clone_from({arg_sugg})")
272265
},
273266
CallKind::FunctionCall { self_arg, .. } => {
274-
let self_sugg = if let ExprKind::Unary(hir::UnOp::Deref, ref_expr) = lhs.kind {
275-
// `*lhs = Clone::clone(self_expr);` -> `Clone::clone_from(lhs, self_expr)`
276-
Sugg::hir_with_applicability(cx, ref_expr, "_", applicability)
277-
} else {
278-
// `lhs = Clone::clone(self_expr);` -> `Clone::clone_from(&mut lhs, self_expr)`
279-
Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr()
280-
};
267+
let self_sugg = Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr();
281268
// The RHS had to be exactly correct before the call, there is no auto-deref for function calls.
282269
let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
283270

‎tests/ui/assigning_clones.fixed

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use std::borrow::ToOwned;
99
use std::ops::{Add, Deref, DerefMut};
10+
use std::task::{Context, Waker};
1011

1112
// Clone
1213
pub struct HasCloneFrom;
@@ -21,45 +22,45 @@ impl Clone for HasCloneFrom {
2122
}
2223

2324
fn clone_method_rhs_val(mut_thing: &mut HasCloneFrom, value_thing: HasCloneFrom) {
24-
mut_thing.clone_from(&value_thing);
25+
(*mut_thing).clone_from(&value_thing);
2526
}
2627

2728
fn clone_method_rhs_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
28-
mut_thing.clone_from(ref_thing);
29+
(*mut_thing).clone_from(ref_thing);
2930
}
3031

3132
fn clone_method_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
3233
mut_thing.clone_from(ref_thing);
3334
}
3435

3536
fn clone_function_lhs_mut_ref(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
36-
Clone::clone_from(mut_thing, ref_thing);
37+
Clone::clone_from(&mut (*mut_thing), ref_thing);
3738
}
3839

3940
fn clone_function_lhs_val(mut mut_thing: HasCloneFrom, ref_thing: &HasCloneFrom) {
4041
Clone::clone_from(&mut mut_thing, ref_thing);
4142
}
4243

4344
fn clone_function_through_trait(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
44-
Clone::clone_from(mut_thing, ref_thing);
45+
Clone::clone_from(&mut (*mut_thing), ref_thing);
4546
}
4647

4748
fn clone_function_through_type(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
48-
Clone::clone_from(mut_thing, ref_thing);
49+
Clone::clone_from(&mut (*mut_thing), ref_thing);
4950
}
5051

5152
fn clone_function_fully_qualified(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
52-
Clone::clone_from(mut_thing, ref_thing);
53+
Clone::clone_from(&mut (*mut_thing), ref_thing);
5354
}
5455

5556
fn clone_method_lhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
5657
// These parens should be kept as necessary for a receiver
57-
(mut_thing + &mut HasCloneFrom).clone_from(ref_thing);
58+
(*(mut_thing + &mut HasCloneFrom)).clone_from(ref_thing);
5859
}
5960

6061
fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFrom) {
6162
// These parens should be removed since they are not needed in a function argument
62-
mut_thing.clone_from(ref_thing + ref_thing);
63+
(*mut_thing).clone_from(ref_thing + ref_thing);
6364
}
6465

6566
fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
@@ -140,6 +141,17 @@ fn clone_inside_macro() {
140141
clone_inside!(a, b);
141142
}
142143

144+
// https://github.com/rust-lang/rust-clippy/issues/12437
145+
fn clone_into_deref_method(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
146+
(*prev).clone_from(cx.waker());
147+
prev
148+
}
149+
150+
fn clone_into_deref_fn(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
151+
Clone::clone_from(&mut (*prev), cx.waker());
152+
prev
153+
}
154+
143155
// ToOwned
144156
fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
145157
ref_str.clone_into(mut_string);

‎tests/ui/assigning_clones.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use std::borrow::ToOwned;
99
use std::ops::{Add, Deref, DerefMut};
10+
use std::task::{Context, Waker};
1011

1112
// Clone
1213
pub struct HasCloneFrom;
@@ -140,6 +141,17 @@ fn clone_inside_macro() {
140141
clone_inside!(a, b);
141142
}
142143

144+
// https://github.com/rust-lang/rust-clippy/issues/12437
145+
fn clone_into_deref_method(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
146+
*prev = cx.waker().clone();
147+
prev
148+
}
149+
150+
fn clone_into_deref_fn(mut prev: Box<Waker>, cx: &mut Context<'_>) -> Box<Waker> {
151+
*prev = Clone::clone(cx.waker());
152+
prev
153+
}
154+
143155
// ToOwned
144156
fn owned_method_mut_ref(mut_string: &mut String, ref_str: &str) {
145157
*mut_string = ref_str.to_owned();

‎tests/ui/assigning_clones.stderr

Lines changed: 38 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,107 +1,119 @@
11
error: assigning the result of `Clone::clone()` may be inefficient
2-
--> tests/ui/assigning_clones.rs:24:5
2+
--> tests/ui/assigning_clones.rs:25:5
33
|
44
LL | *mut_thing = value_thing.clone();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(&value_thing)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(*mut_thing).clone_from(&value_thing)`
66
|
77
= note: `-D clippy::assigning-clones` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::assigning_clones)]`
99

1010
error: assigning the result of `Clone::clone()` may be inefficient
11-
--> tests/ui/assigning_clones.rs:28:5
11+
--> tests/ui/assigning_clones.rs:29:5
1212
|
1313
LL | *mut_thing = ref_thing.clone();
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(*mut_thing).clone_from(ref_thing)`
1515

1616
error: assigning the result of `Clone::clone()` may be inefficient
17-
--> tests/ui/assigning_clones.rs:32:5
17+
--> tests/ui/assigning_clones.rs:33:5
1818
|
1919
LL | mut_thing = ref_thing.clone();
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing)`
2121

2222
error: assigning the result of `Clone::clone()` may be inefficient
23-
--> tests/ui/assigning_clones.rs:36:5
23+
--> tests/ui/assigning_clones.rs:37:5
2424
|
2525
LL | *mut_thing = Clone::clone(ref_thing);
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut (*mut_thing), ref_thing)`
2727

2828
error: assigning the result of `Clone::clone()` may be inefficient
29-
--> tests/ui/assigning_clones.rs:40:5
29+
--> tests/ui/assigning_clones.rs:41:5
3030
|
3131
LL | mut_thing = Clone::clone(ref_thing);
3232
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut mut_thing, ref_thing)`
3333

3434
error: assigning the result of `Clone::clone()` may be inefficient
35-
--> tests/ui/assigning_clones.rs:44:5
35+
--> tests/ui/assigning_clones.rs:45:5
3636
|
3737
LL | *mut_thing = Clone::clone(ref_thing);
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut (*mut_thing), ref_thing)`
3939

4040
error: assigning the result of `Clone::clone()` may be inefficient
41-
--> tests/ui/assigning_clones.rs:48:5
41+
--> tests/ui/assigning_clones.rs:49:5
4242
|
4343
LL | *mut_thing = HasCloneFrom::clone(ref_thing);
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
44+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut (*mut_thing), ref_thing)`
4545

4646
error: assigning the result of `Clone::clone()` may be inefficient
47-
--> tests/ui/assigning_clones.rs:52:5
47+
--> tests/ui/assigning_clones.rs:53:5
4848
|
4949
LL | *mut_thing = <HasCloneFrom as Clone>::clone(ref_thing);
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(mut_thing, ref_thing)`
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut (*mut_thing), ref_thing)`
5151

5252
error: assigning the result of `Clone::clone()` may be inefficient
53-
--> tests/ui/assigning_clones.rs:57:5
53+
--> tests/ui/assigning_clones.rs:58:5
5454
|
5555
LL | *(mut_thing + &mut HasCloneFrom) = ref_thing.clone();
56-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(mut_thing + &mut HasCloneFrom).clone_from(ref_thing)`
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(*(mut_thing + &mut HasCloneFrom)).clone_from(ref_thing)`
5757

5858
error: assigning the result of `Clone::clone()` may be inefficient
59-
--> tests/ui/assigning_clones.rs:62:5
59+
--> tests/ui/assigning_clones.rs:63:5
6060
|
6161
LL | *mut_thing = (ref_thing + ref_thing).clone();
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing + ref_thing)`
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(*mut_thing).clone_from(ref_thing + ref_thing)`
6363

6464
error: assigning the result of `Clone::clone()` may be inefficient
65-
--> tests/ui/assigning_clones.rs:68:9
65+
--> tests/ui/assigning_clones.rs:69:9
6666
|
6767
LL | a = b.clone();
6868
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
6969

70+
error: assigning the result of `Clone::clone()` may be inefficient
71+
--> tests/ui/assigning_clones.rs:146:5
72+
|
73+
LL | *prev = cx.waker().clone();
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `(*prev).clone_from(cx.waker())`
75+
76+
error: assigning the result of `Clone::clone()` may be inefficient
77+
--> tests/ui/assigning_clones.rs:151:5
78+
|
79+
LL | *prev = Clone::clone(cx.waker());
80+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut (*prev), cx.waker())`
81+
7082
error: assigning the result of `ToOwned::to_owned()` may be inefficient
71-
--> tests/ui/assigning_clones.rs:145:5
83+
--> tests/ui/assigning_clones.rs:157:5
7284
|
7385
LL | *mut_string = ref_str.to_owned();
7486
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`
7587

7688
error: assigning the result of `ToOwned::to_owned()` may be inefficient
77-
--> tests/ui/assigning_clones.rs:149:5
89+
--> tests/ui/assigning_clones.rs:161:5
7890
|
7991
LL | mut_string = ref_str.to_owned();
8092
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`
8193

8294
error: assigning the result of `ToOwned::to_owned()` may be inefficient
83-
--> tests/ui/assigning_clones.rs:170:5
95+
--> tests/ui/assigning_clones.rs:182:5
8496
|
8597
LL | **mut_box_string = ref_str.to_owned();
8698
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
8799

88100
error: assigning the result of `ToOwned::to_owned()` may be inefficient
89-
--> tests/ui/assigning_clones.rs:174:5
101+
--> tests/ui/assigning_clones.rs:186:5
90102
|
91103
LL | **mut_box_string = ref_str.to_owned();
92104
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
93105

94106
error: assigning the result of `ToOwned::to_owned()` may be inefficient
95-
--> tests/ui/assigning_clones.rs:178:5
107+
--> tests/ui/assigning_clones.rs:190:5
96108
|
97109
LL | *mut_thing = ToOwned::to_owned(ref_str);
98110
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`
99111

100112
error: assigning the result of `ToOwned::to_owned()` may be inefficient
101-
--> tests/ui/assigning_clones.rs:182:5
113+
--> tests/ui/assigning_clones.rs:194:5
102114
|
103115
LL | mut_thing = ToOwned::to_owned(ref_str);
104116
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`
105117

106-
error: aborting due to 17 previous errors
118+
error: aborting due to 19 previous errors
107119

0 commit comments

Comments
 (0)