Auto merge of rust-lang#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 rust-lang#12776

changelog: [`assigning_clones`]: use `hir_with_context` to produce correct snippets

Author: bors

clippy_lints/src/assigning_clones.rs

@@ -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})")
                     },
                 }

tests/ui/assigning_clones.fixed

@@ -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`

tests/ui/assigning_clones.rs

@@ -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`

tests/ui/assigning_clones.stderr

@@ -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