Skip to content

Commit 99a42ba

Browse files
shanretooshanretoo
committed
fix wrong suggestions of assigning_clones when the assignment is a macro call
1 parent befb659 commit 99a42ba

File tree

4 files changed

+92
-20
lines changed

4 files changed

+92
-20
lines changed

‎clippy_lints/src/assigning_clones.rs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_middle::ty::{self, Instance, Mutability};
1010
use rustc_session::impl_lint_pass;
1111
use rustc_span::def_id::DefId;
1212
use rustc_span::symbol::sym;
13-
use rustc_span::ExpnKind;
13+
use rustc_span::{ExpnKind, SyntaxContext};
1414

1515
declare_clippy_lint! {
1616
/// ### What it does
@@ -68,7 +68,8 @@ impl_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);
6868
impl<'tcx> LateLintPass<'tcx> for AssigningClones {
6969
fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx Expr<'_>) {
7070
// Do not fire the lint in macros
71-
let expn_data = assign_expr.span().ctxt().outer_expn_data();
71+
let ctxt = assign_expr.span().ctxt();
72+
let expn_data = ctxt.outer_expn_data();
7273
match expn_data.kind {
7374
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) | ExpnKind::Macro(..) => return,
7475
ExpnKind::Root => {},
@@ -83,7 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
8384
};
8485

8586
if is_ok_to_suggest(cx, lhs, &call, &self.msrv) {
86-
suggest(cx, assign_expr, lhs, &call);
87+
suggest(cx, ctxt, assign_expr, lhs, &call);
8788
}
8889
}
8990

@@ -221,14 +222,20 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
221222
implemented_fns.contains_key(&provided_fn.def_id)
222223
}
223224

224-
fn suggest<'tcx>(cx: &LateContext<'tcx>, assign_expr: &Expr<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) {
225+
fn suggest<'tcx>(
226+
cx: &LateContext<'tcx>,
227+
ctxt: SyntaxContext,
228+
assign_expr: &Expr<'tcx>,
229+
lhs: &Expr<'tcx>,
230+
call: &CallCandidate<'tcx>,
231+
) {
225232
span_lint_and_then(cx, ASSIGNING_CLONES, assign_expr.span, call.message(), |diag| {
226233
let mut applicability = Applicability::Unspecified;
227234

228235
diag.span_suggestion(
229236
assign_expr.span,
230237
call.suggestion_msg(),
231-
call.suggested_replacement(cx, lhs, &mut applicability),
238+
call.suggested_replacement(cx, ctxt, lhs, &mut applicability),
232239
applicability,
233240
);
234241
});
@@ -274,6 +281,7 @@ impl<'tcx> CallCandidate<'tcx> {
274281
fn suggested_replacement(
275282
&self,
276283
cx: &LateContext<'tcx>,
284+
ctxt: SyntaxContext,
277285
lhs: &Expr<'tcx>,
278286
applicability: &mut Applicability,
279287
) -> String {
@@ -293,7 +301,7 @@ impl<'tcx> CallCandidate<'tcx> {
293301
// Determine whether we need to reference the argument to clone_from().
294302
let clone_receiver_type = cx.typeck_results().expr_ty(receiver);
295303
let clone_receiver_adj_type = cx.typeck_results().expr_ty_adjusted(receiver);
296-
let mut arg_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
304+
let mut arg_sugg = Sugg::hir_with_context(cx, receiver, ctxt, "_", applicability);
297305
if clone_receiver_type != clone_receiver_adj_type {
298306
// The receiver may have been a value type, so we need to add an `&` to
299307
// be sure the argument to clone_from will be a reference.
@@ -311,7 +319,7 @@ impl<'tcx> CallCandidate<'tcx> {
311319
Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr()
312320
};
313321
// The RHS had to be exactly correct before the call, there is no auto-deref for function calls.
314-
let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
322+
let rhs_sugg = Sugg::hir_with_context(cx, self_arg, ctxt, "_", applicability);
315323

316324
format!("Clone::clone_from({self_sugg}, {rhs_sugg})")
317325
},
@@ -340,11 +348,11 @@ impl<'tcx> CallCandidate<'tcx> {
340348

341349
match self.kind {
342350
CallKind::MethodCall { receiver } => {
343-
let receiver_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
351+
let receiver_sugg = Sugg::hir_with_context(cx, receiver, ctxt, "_", applicability);
344352
format!("{receiver_sugg}.clone_into({rhs_sugg})")
345353
},
346354
CallKind::FunctionCall { self_arg, .. } => {
347-
let self_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
355+
let self_sugg = Sugg::hir_with_context(cx, self_arg, ctxt, "_", applicability);
348356
format!("ToOwned::clone_into({self_sugg}, {rhs_sugg})")
349357
},
350358
}

‎tests/ui/assigning_clones.fixed‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,16 @@ fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFr
6262
mut_thing.clone_from(ref_thing + ref_thing);
6363
}
6464

65+
fn clone_method_macro() {
66+
let mut s = String::from("");
67+
s.clone_from(&format!("{} {}", "hello", "world"));
68+
}
69+
70+
fn clone_function_macro() {
71+
let mut s = String::from("");
72+
Clone::clone_from(&mut s, &format!("{} {}", "hello", "world"));
73+
}
74+
6575
fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
6676
let mut a = HasCloneFrom;
6777
for _ in 1..10 {
@@ -214,6 +224,16 @@ fn owned_function_val(mut mut_thing: String, ref_str: &str) {
214224
ToOwned::clone_into(ref_str, &mut mut_thing);
215225
}
216226

227+
fn owned_method_macro() {
228+
let mut s = String::from("");
229+
format!("{} {}", "hello", "world").clone_into(&mut s);
230+
}
231+
232+
fn owned_function_macro() {
233+
let mut s = String::from("");
234+
ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s);
235+
}
236+
217237
struct FakeToOwned;
218238
impl FakeToOwned {
219239
/// This looks just like `ToOwned::to_owned`

‎tests/ui/assigning_clones.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,16 @@ fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFr
6262
*mut_thing = (ref_thing + ref_thing).clone();
6363
}
6464

65+
fn clone_method_macro() {
66+
let mut s = String::from("");
67+
s = format!("{} {}", "hello", "world").clone();
68+
}
69+
70+
fn clone_function_macro() {
71+
let mut s = String::from("");
72+
s = Clone::clone(&format!("{} {}", "hello", "world"));
73+
}
74+
6575
fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
6676
let mut a = HasCloneFrom;
6777
for _ in 1..10 {
@@ -214,6 +224,16 @@ fn owned_function_val(mut mut_thing: String, ref_str: &str) {
214224
mut_thing = ToOwned::to_owned(ref_str);
215225
}
216226

227+
fn owned_method_macro() {
228+
let mut s = String::from("");
229+
s = format!("{} {}", "hello", "world").to_owned();
230+
}
231+
232+
fn owned_function_macro() {
233+
let mut s = String::from("");
234+
s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
235+
}
236+
217237
struct FakeToOwned;
218238
impl FakeToOwned {
219239
/// This looks just like `ToOwned::to_owned`

‎tests/ui/assigning_clones.stderr‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -62,64 +62,88 @@ LL | *mut_thing = (ref_thing + ref_thing).clone();
6262
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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:67:5
66+
|
67+
LL | s = format!("{} {}", "hello", "world").clone();
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `s.clone_from(&format!("{} {}", "hello", "world"))`
69+
70+
error: assigning the result of `Clone::clone()` may be inefficient
71+
--> tests/ui/assigning_clones.rs:72:5
72+
|
73+
LL | s = Clone::clone(&format!("{} {}", "hello", "world"));
74+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut s, &format!("{} {}", "hello", "world"))`
75+
76+
error: assigning the result of `Clone::clone()` may be inefficient
77+
--> tests/ui/assigning_clones.rs:78:9
6678
|
6779
LL | a = b.clone();
6880
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
6981

7082
error: assigning the result of `Clone::clone()` may be inefficient
71-
--> tests/ui/assigning_clones.rs:139:5
83+
--> tests/ui/assigning_clones.rs:149:5
7284
|
7385
LL | a = b.clone();
7486
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
7587

7688
error: assigning the result of `Clone::clone()` may be inefficient
77-
--> tests/ui/assigning_clones.rs:146:5
89+
--> tests/ui/assigning_clones.rs:156:5
7890
|
7991
LL | a = b.clone();
8092
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
8193

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

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

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

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

106118
error: assigning the result of `ToOwned::to_owned()` may be inefficient
107-
--> tests/ui/assigning_clones.rs:206:5
119+
--> tests/ui/assigning_clones.rs:216:5
108120
|
109121
LL | **mut_box_string = ref_str.to_owned();
110122
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
111123

112124
error: assigning the result of `ToOwned::to_owned()` may be inefficient
113-
--> tests/ui/assigning_clones.rs:210:5
125+
--> tests/ui/assigning_clones.rs:220:5
114126
|
115127
LL | *mut_thing = ToOwned::to_owned(ref_str);
116128
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`
117129

118130
error: assigning the result of `ToOwned::to_owned()` may be inefficient
119-
--> tests/ui/assigning_clones.rs:214:5
131+
--> tests/ui/assigning_clones.rs:224:5
120132
|
121133
LL | mut_thing = ToOwned::to_owned(ref_str);
122134
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`
123135

124-
error: aborting due to 20 previous errors
136+
error: assigning the result of `ToOwned::to_owned()` may be inefficient
137+
--> tests/ui/assigning_clones.rs:229:5
138+
|
139+
LL | s = format!("{} {}", "hello", "world").to_owned();
140+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `format!("{} {}", "hello", "world").clone_into(&mut s)`
141+
142+
error: assigning the result of `ToOwned::to_owned()` may be inefficient
143+
--> tests/ui/assigning_clones.rs:234:5
144+
|
145+
LL | s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
146+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s)`
147+
148+
error: aborting due to 24 previous errors
125149

0 commit comments

Comments
 (0)