Skip to content

Commit

Permalink
Auto merge of #12742 - Alexendoo:assigning-clones-nested-late-init, r…
Browse files Browse the repository at this point in the history
…=dswij

Don't lint assigning_clones on nested late init locals

Fixes #12741

changelog: none
  • Loading branch information
bors committed May 4, 2024
2 parents 993d8ae + c313ef5 commit 2800251
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 14 deletions.
8 changes: 3 additions & 5 deletions clippy_lints/src/assigning_clones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -164,9 +164,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;
}
}
Expand Down
15 changes: 15 additions & 0 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/assigning_clones.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/assigning_clones.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
18 changes: 9 additions & 9 deletions tests/ui/assigning_clones.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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)`
Expand Down

0 comments on commit 2800251

Please sign in to comment.