mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 23:24:24 +00:00
Auto merge of #12783 - shanretoo:fix-assigning-clones, r=blyxyas
fix: use hir_with_context to produce correct snippets for assigning_clones The `assigning_clones` lint is producing wrong output when the assignment is a macro call. Since Applicability level `Unspecified` will never be changed inside `hir_with_applicability`, so it is safe here to replace `hir_with_applicability` with `hir_with_context` to generate snippets of the macro call instead of the expansion. fixes #12776 changelog: [`assigning_clones`]: use `hir_with_context` to produce correct snippets
This commit is contained in:
commit
baf2a23840
4 changed files with 92 additions and 20 deletions
|
@ -10,7 +10,7 @@ use rustc_middle::ty::{self, Instance, Mutability};
|
|||
use rustc_session::impl_lint_pass;
|
||||
use rustc_span::def_id::DefId;
|
||||
use rustc_span::symbol::sym;
|
||||
use rustc_span::ExpnKind;
|
||||
use rustc_span::{ExpnKind, SyntaxContext};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
|
@ -68,7 +68,8 @@ impl_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);
|
|||
impl<'tcx> LateLintPass<'tcx> for AssigningClones {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx Expr<'_>) {
|
||||
// Do not fire the lint in macros
|
||||
let expn_data = assign_expr.span().ctxt().outer_expn_data();
|
||||
let ctxt = assign_expr.span().ctxt();
|
||||
let expn_data = ctxt.outer_expn_data();
|
||||
match expn_data.kind {
|
||||
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) | ExpnKind::Macro(..) => return,
|
||||
ExpnKind::Root => {},
|
||||
|
@ -83,7 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
|
|||
};
|
||||
|
||||
if is_ok_to_suggest(cx, lhs, &call, &self.msrv) {
|
||||
suggest(cx, assign_expr, lhs, &call);
|
||||
suggest(cx, ctxt, assign_expr, lhs, &call);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -221,14 +222,20 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
|
|||
implemented_fns.contains_key(&provided_fn.def_id)
|
||||
}
|
||||
|
||||
fn suggest<'tcx>(cx: &LateContext<'tcx>, assign_expr: &Expr<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) {
|
||||
fn suggest<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
ctxt: SyntaxContext,
|
||||
assign_expr: &Expr<'tcx>,
|
||||
lhs: &Expr<'tcx>,
|
||||
call: &CallCandidate<'tcx>,
|
||||
) {
|
||||
span_lint_and_then(cx, ASSIGNING_CLONES, assign_expr.span, call.message(), |diag| {
|
||||
let mut applicability = Applicability::Unspecified;
|
||||
|
||||
diag.span_suggestion(
|
||||
assign_expr.span,
|
||||
call.suggestion_msg(),
|
||||
call.suggested_replacement(cx, lhs, &mut applicability),
|
||||
call.suggested_replacement(cx, ctxt, lhs, &mut applicability),
|
||||
applicability,
|
||||
);
|
||||
});
|
||||
|
@ -274,6 +281,7 @@ impl<'tcx> CallCandidate<'tcx> {
|
|||
fn suggested_replacement(
|
||||
&self,
|
||||
cx: &LateContext<'tcx>,
|
||||
ctxt: SyntaxContext,
|
||||
lhs: &Expr<'tcx>,
|
||||
applicability: &mut Applicability,
|
||||
) -> String {
|
||||
|
@ -293,7 +301,7 @@ impl<'tcx> CallCandidate<'tcx> {
|
|||
// Determine whether we need to reference the argument to clone_from().
|
||||
let clone_receiver_type = cx.typeck_results().expr_ty(receiver);
|
||||
let clone_receiver_adj_type = cx.typeck_results().expr_ty_adjusted(receiver);
|
||||
let mut arg_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
|
||||
let mut arg_sugg = Sugg::hir_with_context(cx, receiver, ctxt, "_", applicability);
|
||||
if clone_receiver_type != clone_receiver_adj_type {
|
||||
// The receiver may have been a value type, so we need to add an `&` to
|
||||
// be sure the argument to clone_from will be a reference.
|
||||
|
@ -311,7 +319,7 @@ impl<'tcx> CallCandidate<'tcx> {
|
|||
Sugg::hir_with_applicability(cx, lhs, "_", applicability).mut_addr()
|
||||
};
|
||||
// The RHS had to be exactly correct before the call, there is no auto-deref for function calls.
|
||||
let rhs_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
|
||||
let rhs_sugg = Sugg::hir_with_context(cx, self_arg, ctxt, "_", applicability);
|
||||
|
||||
format!("Clone::clone_from({self_sugg}, {rhs_sugg})")
|
||||
},
|
||||
|
@ -340,11 +348,11 @@ impl<'tcx> CallCandidate<'tcx> {
|
|||
|
||||
match self.kind {
|
||||
CallKind::MethodCall { receiver } => {
|
||||
let receiver_sugg = Sugg::hir_with_applicability(cx, receiver, "_", applicability);
|
||||
let receiver_sugg = Sugg::hir_with_context(cx, receiver, ctxt, "_", applicability);
|
||||
format!("{receiver_sugg}.clone_into({rhs_sugg})")
|
||||
},
|
||||
CallKind::FunctionCall { self_arg, .. } => {
|
||||
let self_sugg = Sugg::hir_with_applicability(cx, self_arg, "_", applicability);
|
||||
let self_sugg = Sugg::hir_with_context(cx, self_arg, ctxt, "_", applicability);
|
||||
format!("ToOwned::clone_into({self_sugg}, {rhs_sugg})")
|
||||
},
|
||||
}
|
||||
|
|
|
@ -62,6 +62,16 @@ fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFr
|
|||
mut_thing.clone_from(ref_thing + ref_thing);
|
||||
}
|
||||
|
||||
fn clone_method_macro() {
|
||||
let mut s = String::from("");
|
||||
s.clone_from(&format!("{} {}", "hello", "world"));
|
||||
}
|
||||
|
||||
fn clone_function_macro() {
|
||||
let mut s = String::from("");
|
||||
Clone::clone_from(&mut s, &format!("{} {}", "hello", "world"));
|
||||
}
|
||||
|
||||
fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
|
||||
let mut a = HasCloneFrom;
|
||||
for _ in 1..10 {
|
||||
|
@ -214,6 +224,16 @@ fn owned_function_val(mut mut_thing: String, ref_str: &str) {
|
|||
ToOwned::clone_into(ref_str, &mut mut_thing);
|
||||
}
|
||||
|
||||
fn owned_method_macro() {
|
||||
let mut s = String::from("");
|
||||
format!("{} {}", "hello", "world").clone_into(&mut s);
|
||||
}
|
||||
|
||||
fn owned_function_macro() {
|
||||
let mut s = String::from("");
|
||||
ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s);
|
||||
}
|
||||
|
||||
struct FakeToOwned;
|
||||
impl FakeToOwned {
|
||||
/// This looks just like `ToOwned::to_owned`
|
||||
|
|
|
@ -62,6 +62,16 @@ fn clone_method_rhs_complex(mut_thing: &mut HasCloneFrom, ref_thing: &HasCloneFr
|
|||
*mut_thing = (ref_thing + ref_thing).clone();
|
||||
}
|
||||
|
||||
fn clone_method_macro() {
|
||||
let mut s = String::from("");
|
||||
s = format!("{} {}", "hello", "world").clone();
|
||||
}
|
||||
|
||||
fn clone_function_macro() {
|
||||
let mut s = String::from("");
|
||||
s = Clone::clone(&format!("{} {}", "hello", "world"));
|
||||
}
|
||||
|
||||
fn assign_to_init_mut_var(b: HasCloneFrom) -> HasCloneFrom {
|
||||
let mut a = HasCloneFrom;
|
||||
for _ in 1..10 {
|
||||
|
@ -214,6 +224,16 @@ fn owned_function_val(mut mut_thing: String, ref_str: &str) {
|
|||
mut_thing = ToOwned::to_owned(ref_str);
|
||||
}
|
||||
|
||||
fn owned_method_macro() {
|
||||
let mut s = String::from("");
|
||||
s = format!("{} {}", "hello", "world").to_owned();
|
||||
}
|
||||
|
||||
fn owned_function_macro() {
|
||||
let mut s = String::from("");
|
||||
s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
|
||||
}
|
||||
|
||||
struct FakeToOwned;
|
||||
impl FakeToOwned {
|
||||
/// This looks just like `ToOwned::to_owned`
|
||||
|
|
|
@ -62,64 +62,88 @@ LL | *mut_thing = (ref_thing + ref_thing).clone();
|
|||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `mut_thing.clone_from(ref_thing + ref_thing)`
|
||||
|
||||
error: assigning the result of `Clone::clone()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:68:9
|
||||
--> tests/ui/assigning_clones.rs:67:5
|
||||
|
|
||||
LL | s = format!("{} {}", "hello", "world").clone();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `s.clone_from(&format!("{} {}", "hello", "world"))`
|
||||
|
||||
error: assigning the result of `Clone::clone()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:72:5
|
||||
|
|
||||
LL | s = Clone::clone(&format!("{} {}", "hello", "world"));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_from()`: `Clone::clone_from(&mut s, &format!("{} {}", "hello", "world"))`
|
||||
|
||||
error: assigning the result of `Clone::clone()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:78:9
|
||||
|
|
||||
LL | a = b.clone();
|
||||
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
|
||||
|
||||
error: assigning the result of `Clone::clone()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:139:5
|
||||
--> tests/ui/assigning_clones.rs:149:5
|
||||
|
|
||||
LL | a = b.clone();
|
||||
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
|
||||
|
||||
error: assigning the result of `Clone::clone()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:146:5
|
||||
--> tests/ui/assigning_clones.rs:156:5
|
||||
|
|
||||
LL | a = b.clone();
|
||||
| ^^^^^^^^^^^^^ help: use `clone_from()`: `a.clone_from(&b)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:147:5
|
||||
--> tests/ui/assigning_clones.rs:157:5
|
||||
|
|
||||
LL | a = c.to_owned();
|
||||
| ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `c.clone_into(&mut a)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:177:5
|
||||
--> tests/ui/assigning_clones.rs:187:5
|
||||
|
|
||||
LL | *mut_string = ref_str.to_owned();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(mut_string)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:181:5
|
||||
--> tests/ui/assigning_clones.rs:191:5
|
||||
|
|
||||
LL | mut_string = ref_str.to_owned();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut mut_string)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:202:5
|
||||
--> tests/ui/assigning_clones.rs:212:5
|
||||
|
|
||||
LL | **mut_box_string = ref_str.to_owned();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:206:5
|
||||
--> tests/ui/assigning_clones.rs:216:5
|
||||
|
|
||||
LL | **mut_box_string = ref_str.to_owned();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ref_str.clone_into(&mut (*mut_box_string))`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:210:5
|
||||
--> tests/ui/assigning_clones.rs:220:5
|
||||
|
|
||||
LL | *mut_thing = ToOwned::to_owned(ref_str);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, mut_thing)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:214:5
|
||||
--> tests/ui/assigning_clones.rs:224:5
|
||||
|
|
||||
LL | mut_thing = ToOwned::to_owned(ref_str);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`
|
||||
|
||||
error: aborting due to 20 previous errors
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:229:5
|
||||
|
|
||||
LL | s = format!("{} {}", "hello", "world").to_owned();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `format!("{} {}", "hello", "world").clone_into(&mut s)`
|
||||
|
||||
error: assigning the result of `ToOwned::to_owned()` may be inefficient
|
||||
--> tests/ui/assigning_clones.rs:234:5
|
||||
|
|
||||
LL | s = ToOwned::to_owned(&format!("{} {}", "hello", "world"));
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(&format!("{} {}", "hello", "world"), &mut s)`
|
||||
|
||||
error: aborting due to 24 previous errors
|
||||
|
||||
|
|
Loading…
Reference in a new issue