about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-03-20 20:41:41 +0000
committerbors <bors@rust-lang.org>2024-03-20 20:41:41 +0000
commit5b7efe827f255190aaf889387d03d72596775f1f (patch)
tree2d409bef7e92b29fec72798c9b87be515ed67550
parent89aba8d45d116844b7d606b844a9388a4e099081 (diff)
parentdb7c9feaa00dcc5adcbf6d345ddcfa54bcf3debf (diff)
downloadrust-5b7efe827f255190aaf889387d03d72596775f1f.tar.gz
rust-5b7efe827f255190aaf889387d03d72596775f1f.zip
Auto merge of #12516 - humannum14916:assigning_clones_msrv, r=Alexendoo
Make `assigning_clones` MSRV check more precise

Continuation of #12511

`clone_into` is the only suggestion subject to the 1.63 MSRV requirement, and the lint should still emit other suggestions regardless of the MSRV.

changelog: [assigning_clones]: only apply MSRV check to `clone_into` suggestions.
-rw-r--r--clippy_lints/src/assigning_clones.rs14
-rw-r--r--tests/ui/assigning_clones.fixed10
-rw-r--r--tests/ui/assigning_clones.rs10
-rw-r--r--tests/ui/assigning_clones.stderr32
4 files changed, 42 insertions, 24 deletions
diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
index ea5578572ea..88d9f762a87 100644
--- a/clippy_lints/src/assigning_clones.rs
+++ b/clippy_lints/src/assigning_clones.rs
@@ -66,10 +66,6 @@ impl_lint_pass!(AssigningClones => [ASSIGNING_CLONES]);
 
 impl<'tcx> LateLintPass<'tcx> for AssigningClones {
     fn check_expr(&mut self, cx: &LateContext<'tcx>, assign_expr: &'tcx hir::Expr<'_>) {
-        if !self.msrv.meets(msrvs::ASSIGNING_CLONES) {
-            return;
-        }
-
         // Do not fire the lint in macros
         let expn_data = assign_expr.span().ctxt().outer_expn_data();
         match expn_data.kind {
@@ -85,7 +81,7 @@ impl<'tcx> LateLintPass<'tcx> for AssigningClones {
             return;
         };
 
-        if is_ok_to_suggest(cx, lhs, &call) {
+        if is_ok_to_suggest(cx, lhs, &call, &self.msrv) {
             suggest(cx, assign_expr, lhs, &call);
         }
     }
@@ -154,7 +150,13 @@ fn extract_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<
 
 // Return true if we find that the called method has a custom implementation and isn't derived or
 // provided by default by the corresponding trait.
-fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>) -> bool {
+fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallCandidate<'tcx>, msrv: &Msrv) -> bool {
+    // For calls to .to_owned we suggest using .clone_into(), which was only stablilized in 1.63.
+    // If the current MSRV is below that, don't suggest the lint.
+    if !msrv.meets(msrvs::ASSIGNING_CLONES) && matches!(call.target, TargetTrait::ToOwned) {
+        return false;
+    }
+
     // If the left-hand side is a local variable, it might be uninitialized at this point.
     // In that case we do not want to suggest the lint.
     if let Some(local) = path_to_local(lhs) {
diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed
index 771c7f6f56a..160f3b94663 100644
--- a/tests/ui/assigning_clones.fixed
+++ b/tests/ui/assigning_clones.fixed
@@ -129,14 +129,16 @@ fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
 }
 
 #[clippy::msrv = "1.62"]
-fn msrv_1_62(mut a: String, b: &str) {
+fn msrv_1_62(mut a: String, b: String, c: &str) {
+    a.clone_from(&b);
     // Should not be linted, as clone_into wasn't stabilized until 1.63
-    a = b.to_owned();
+    a = c.to_owned();
 }
 
 #[clippy::msrv = "1.63"]
-fn msrv_1_63(mut a: String, b: &str) {
-    b.clone_into(&mut a);
+fn msrv_1_63(mut a: String, b: String, c: &str) {
+    a.clone_from(&b);
+    c.clone_into(&mut a);
 }
 
 macro_rules! clone_inside {
diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs
index 1320481322f..14ba1d4db9a 100644
--- a/tests/ui/assigning_clones.rs
+++ b/tests/ui/assigning_clones.rs
@@ -129,14 +129,16 @@ fn ignore_generic_clone<T: Clone>(a: &mut T, b: &T) {
 }
 
 #[clippy::msrv = "1.62"]
-fn msrv_1_62(mut a: String, b: &str) {
+fn msrv_1_62(mut a: String, b: String, c: &str) {
+    a = b.clone();
     // Should not be linted, as clone_into wasn't stabilized until 1.63
-    a = b.to_owned();
+    a = c.to_owned();
 }
 
 #[clippy::msrv = "1.63"]
-fn msrv_1_63(mut a: String, b: &str) {
-    a = b.to_owned();
+fn msrv_1_63(mut a: String, b: String, c: &str) {
+    a = b.clone();
+    a = c.to_owned();
 }
 
 macro_rules! clone_inside {
diff --git a/tests/ui/assigning_clones.stderr b/tests/ui/assigning_clones.stderr
index b8e20cee160..ba59f067431 100644
--- a/tests/ui/assigning_clones.stderr
+++ b/tests/ui/assigning_clones.stderr
@@ -67,47 +67,59 @@ error: assigning the result of `Clone::clone()` may be inefficient
 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:133: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:140: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:139:5
+  --> tests/ui/assigning_clones.rs:141:5
    |
-LL |     a = b.to_owned();
-   |     ^^^^^^^^^^^^^^^^ help: use `clone_into()`: `b.clone_into(&mut a)`
+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:156:5
+  --> tests/ui/assigning_clones.rs:158: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:160:5
+  --> tests/ui/assigning_clones.rs:162: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:181:5
+  --> tests/ui/assigning_clones.rs:183: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:185:5
+  --> tests/ui/assigning_clones.rs:187: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:189:5
+  --> tests/ui/assigning_clones.rs:191: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:193:5
+  --> tests/ui/assigning_clones.rs:195: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 18 previous errors
+error: aborting due to 20 previous errors