about summary refs log tree commit diff
diff options
context:
space:
mode:
authorAlex Macleod <alex@macleod.io>2024-05-01 12:04:11 +0000
committerAlex Macleod <alex@macleod.io>2024-05-01 12:22:50 +0000
commitc313ef51df3b395f2819d972223809427cd5072a (patch)
tree560131ef3d253cbd797beb4732858ffd6e9bf106
parentc6bf9548d53233f8a08928955a996f4bf30ea1bd (diff)
downloadrust-c313ef51df3b395f2819d972223809427cd5072a.tar.gz
rust-c313ef51df3b395f2819d972223809427cd5072a.zip
Don't lint assigning_clones on nested late init locals
-rw-r--r--clippy_lints/src/assigning_clones.rs8
-rw-r--r--clippy_utils/src/lib.rs15
-rw-r--r--tests/ui/assigning_clones.fixed6
-rw-r--r--tests/ui/assigning_clones.rs6
-rw-r--r--tests/ui/assigning_clones.stderr18
5 files changed, 39 insertions, 14 deletions
diff --git a/clippy_lints/src/assigning_clones.rs b/clippy_lints/src/assigning_clones.rs
index d88ca84fb97..1beeb4f7139 100644
--- a/clippy_lints/src/assigning_clones.rs
+++ b/clippy_lints/src/assigning_clones.rs
@@ -2,9 +2,9 @@ use clippy_config::msrvs::{self, Msrv};
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::macros::HirNode;
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{is_trait_method, path_to_local};
+use clippy_utils::{is_trait_method, local_is_initialized, path_to_local};
 use rustc_errors::Applicability;
-use rustc_hir::{self as hir, Expr, ExprKind, Node};
+use rustc_hir::{self as hir, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::{self, Instance, Mutability};
 use rustc_session::impl_lint_pass;
@@ -163,9 +163,7 @@ fn is_ok_to_suggest<'tcx>(cx: &LateContext<'tcx>, lhs: &Expr<'tcx>, call: &CallC
         // TODO: This check currently bails if the local variable has no initializer.
         // That is overly conservative - the lint should fire even if there was no initializer,
         // but the variable has been initialized before `lhs` was evaluated.
-        if let Some(Node::LetStmt(local)) = cx.tcx.hir().parent_id_iter(local).next().map(|p| cx.tcx.hir_node(p))
-            && local.init.is_none()
-        {
+        if !local_is_initialized(cx, local) {
             return false;
         }
     }
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index fccd75d8153..649515a29b8 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -192,6 +192,21 @@ pub fn find_binding_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<
     None
 }
 
+/// Checks if the given local has an initializer or is from something other than a `let` statement
+///
+/// e.g. returns true for `x` in `fn f(x: usize) { .. }` and `let x = 1;` but false for `let x;`
+pub fn local_is_initialized(cx: &LateContext<'_>, local: HirId) -> bool {
+    for (_, node) in cx.tcx.hir().parent_iter(local) {
+        match node {
+            Node::Pat(..) | Node::PatField(..) => {},
+            Node::LetStmt(let_stmt) => return let_stmt.init.is_some(),
+            _ => return true,
+        }
+    }
+
+    false
+}
+
 /// Returns `true` if the given `NodeId` is inside a constant context
 ///
 /// # Example
diff --git a/tests/ui/assigning_clones.fixed b/tests/ui/assigning_clones.fixed
index 8387c7d6156..394d2a67ca3 100644
--- a/tests/ui/assigning_clones.fixed
+++ b/tests/ui/assigning_clones.fixed
@@ -86,6 +86,12 @@ fn assign_to_uninit_mut_var(b: HasCloneFrom) {
     a = b.clone();
 }
 
+fn late_init_let_tuple() {
+    let (p, q): (String, String);
+    p = "ghi".to_string();
+    q = p.clone();
+}
+
 #[derive(Clone)]
 pub struct HasDeriveClone;
 
diff --git a/tests/ui/assigning_clones.rs b/tests/ui/assigning_clones.rs
index 6f4da9f652c..df6b760d5fd 100644
--- a/tests/ui/assigning_clones.rs
+++ b/tests/ui/assigning_clones.rs
@@ -86,6 +86,12 @@ fn assign_to_uninit_mut_var(b: HasCloneFrom) {
     a = b.clone();
 }
 
+fn late_init_let_tuple() {
+    let (p, q): (String, String);
+    p = "ghi".to_string();
+    q = p.clone();
+}
+
 #[derive(Clone)]
 pub struct HasDeriveClone;
 
diff --git a/tests/ui/assigning_clones.stderr b/tests/ui/assigning_clones.stderr
index 793927bd1cb..87f63ca604f 100644
--- a/tests/ui/assigning_clones.stderr
+++ b/tests/ui/assigning_clones.stderr
@@ -68,55 +68,55 @@ 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
+  --> tests/ui/assigning_clones.rs:139: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
+  --> tests/ui/assigning_clones.rs:146: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:141:5
+  --> tests/ui/assigning_clones.rs:147: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:171:5
+  --> tests/ui/assigning_clones.rs:177: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:175:5
+  --> tests/ui/assigning_clones.rs:181: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:196:5
+  --> tests/ui/assigning_clones.rs:202: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:200:5
+  --> tests/ui/assigning_clones.rs:206: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:204:5
+  --> tests/ui/assigning_clones.rs:210: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:208:5
+  --> tests/ui/assigning_clones.rs:214:5
    |
 LL |     mut_thing = ToOwned::to_owned(ref_str);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `clone_into()`: `ToOwned::clone_into(ref_str, &mut mut_thing)`