From c313ef51df3b395f2819d972223809427cd5072a Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Wed, 1 May 2024 12:04:11 +0000 Subject: [PATCH] Don't lint assigning_clones on nested late init locals --- clippy_lints/src/assigning_clones.rs | 8 +++----- clippy_utils/src/lib.rs | 15 +++++++++++++++ tests/ui/assigning_clones.fixed | 6 ++++++ tests/ui/assigning_clones.rs | 6 ++++++ tests/ui/assigning_clones.stderr | 18 +++++++++--------- 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_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)`