Skip to content

Commit cdd073f

Browse files
committed
avoid suggest let mut if possible
1 parent 2cd8619 commit cdd073f

File tree

3 files changed

+68
-12
lines changed

3 files changed

+68
-12
lines changed

clippy_lints/src/explicit_reinitialization.rs

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_opt;
3-
use clippy_utils::{fn_has_unsatisfiable_preds, is_from_proc_macro};
3+
use clippy_utils::visitors::for_each_local_use_after_expr;
4+
use clippy_utils::{fn_has_unsatisfiable_preds, get_parent_expr, is_from_proc_macro};
45
use rustc_data_structures::fx::FxHashSet;
56
use rustc_data_structures::graph::dominators::Dominators;
67
use rustc_data_structures::graph::iterate::DepthFirstSearch;
78
use rustc_data_structures::graph::WithSuccessors;
89
use rustc_errors::Applicability;
10+
use rustc_hir::def::Res;
911
use rustc_hir::def_id::LocalDefId;
1012
use rustc_hir::{
11-
Closure, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Node, Path, PathSegment, QPath, StmtKind,
12-
TraitFn, TraitItem, TraitItemKind,
13+
Closure, Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind, Mutability, Node, Path, PathSegment, QPath,
14+
StmtKind, TraitFn, TraitItem, TraitItemKind, UnOp,
1315
};
1416
use rustc_lint::{LateContext, LateLintPass};
1517
use rustc_middle::lint::in_external_macro;
@@ -18,6 +20,7 @@ use rustc_middle::mir::{self, BasicBlock, Body, Local, Location, Place, Statemen
1820
use rustc_session::{declare_lint_pass, declare_tool_lint, Session};
1921
use rustc_span::Span;
2022
use std::collections::BTreeSet;
23+
use std::ops::ControlFlow;
2124

2225
declare_clippy_lint! {
2326
/// ### What it does
@@ -81,6 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
8184
None,
8285
Path {
8386
segments: [PathSegment { args: None, .. }],
87+
res: Res::Local(local_id),
8488
..
8589
},
8690
)),
@@ -90,6 +94,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
9094
right,
9195
_,
9296
),
97+
hir_id: expr_id,
9398
..
9499
},
95100
) = stmt.kind
@@ -123,13 +128,64 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitReinitialization {
123128
assert!(start_location.dominates(location, dominators));
124129

125130
if dominate_all_usage(mir, dominators, local, start_location) {
131+
// copy from `vec_init_then_push.rs`
132+
let mut needs_mut = false;
133+
let _ = for_each_local_use_after_expr(cx, *local_id, *expr_id, |e| {
134+
let Some(parent) = get_parent_expr(cx, e) else {
135+
return ControlFlow::Continue(());
136+
};
137+
let adjusted_ty = cx.typeck_results().expr_ty_adjusted(e);
138+
let adjusted_mut = adjusted_ty.ref_mutability().unwrap_or(Mutability::Not);
139+
needs_mut |= adjusted_mut == Mutability::Mut;
140+
match parent.kind {
141+
ExprKind::AddrOf(_, Mutability::Mut, _) => {
142+
needs_mut = true;
143+
return ControlFlow::Break(true);
144+
},
145+
ExprKind::Unary(UnOp::Deref, _) | ExprKind::Index(..) if !needs_mut => {
146+
let mut last_place = parent;
147+
while let Some(parent) = get_parent_expr(cx, last_place) {
148+
if matches!(parent.kind, ExprKind::Unary(UnOp::Deref, _) | ExprKind::Field(..))
149+
|| matches!(parent.kind, ExprKind::Index(e, _, _) if e.hir_id == last_place.hir_id)
150+
{
151+
last_place = parent;
152+
} else {
153+
break;
154+
}
155+
}
156+
needs_mut |= cx.typeck_results().expr_ty_adjusted(last_place).ref_mutability()
157+
== Some(Mutability::Mut)
158+
|| get_parent_expr(cx, last_place)
159+
.map_or(false, |e| matches!(e.kind, ExprKind::AddrOf(_, Mutability::Mut, _)));
160+
},
161+
ExprKind::MethodCall(_, recv, ..)
162+
if recv.hir_id == e.hir_id
163+
&& adjusted_mut == Mutability::Mut
164+
&& !adjusted_ty.peel_refs().is_slice() =>
165+
{
166+
// No need to set `needs_mut` to true. The receiver will be either explicitly borrowed, or it
167+
// will be implicitly borrowed via an adjustment. Both of these cases
168+
// are already handled by this point.
169+
return ControlFlow::Break(true);
170+
},
171+
ExprKind::Assign(lhs, ..) if e.hir_id == lhs.hir_id => {
172+
needs_mut = true;
173+
return ControlFlow::Break(false);
174+
},
175+
_ => (),
176+
}
177+
ControlFlow::Continue(())
178+
});
179+
180+
let mut_str = if needs_mut { "mut " } else { "" };
181+
126182
span_lint_and_sugg(
127183
cx,
128184
EXPLICIT_REINITIALIZATION,
129185
stmt.span,
130186
"create a fresh variable is more explicit",
131187
"create a fresh variable instead of reinitialization",
132-
format!("let mut {snip}"),
188+
format!("let {mut_str}{snip}"),
133189
Applicability::MachineApplicable,
134190
);
135191
}

tests/ui/explicit_reinitialization.fixed

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,21 @@
44
fn test_copy() {
55
let mut x = 1;
66
println!("{x}");
7-
let mut x = 2;
7+
let x = 2;
88
println!("{x}");
99
}
1010

1111
fn test_move() {
1212
let mut x = String::new();
1313
println!("{x}");
14-
let mut x = String::new();
14+
let x = String::new();
1515
println!("{x}");
1616
}
1717

1818
#[allow(unused_assignments, clippy::misrefactored_assign_op)]
1919
fn should_lint() {
2020
let mut a = 1;
21-
let mut a = a * a * a;
21+
let a = a * a * a;
2222
}
2323

2424
#[allow(clippy::ptr_arg)]
@@ -91,7 +91,7 @@ fn known_false_negative() {
9191
let mut x = 1;
9292
println!("{x}");
9393
{
94-
let mut x = 2;
94+
let x = 2;
9595
}
9696
println!("{x}");
9797
}

tests/ui/explicit_reinitialization.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: create a fresh variable is more explicit
22
--> $DIR/explicit_reinitialization.rs:7:5
33
|
44
LL | x = 2;
5-
| ^^^^^^ help: create a fresh variable instead of reinitialization: `let mut x = 2;`
5+
| ^^^^^^ help: create a fresh variable instead of reinitialization: `let x = 2;`
66
|
77
= note: `-D clippy::explicit-reinitialization` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::explicit_reinitialization)]`
@@ -11,13 +11,13 @@ error: create a fresh variable is more explicit
1111
--> $DIR/explicit_reinitialization.rs:14:5
1212
|
1313
LL | x = String::new();
14-
| ^^^^^^^^^^^^^^^^^^ help: create a fresh variable instead of reinitialization: `let mut x = String::new();`
14+
| ^^^^^^^^^^^^^^^^^^ help: create a fresh variable instead of reinitialization: `let x = String::new();`
1515

1616
error: create a fresh variable is more explicit
1717
--> $DIR/explicit_reinitialization.rs:21:5
1818
|
1919
LL | a = a * a * a;
20-
| ^^^^^^^^^^^^^^ help: create a fresh variable instead of reinitialization: `let mut a = a * a * a;`
20+
| ^^^^^^^^^^^^^^ help: create a fresh variable instead of reinitialization: `let a = a * a * a;`
2121

2222
error: create a fresh variable is more explicit
2323
--> $DIR/explicit_reinitialization.rs:28:5
@@ -35,7 +35,7 @@ error: create a fresh variable is more explicit
3535
--> $DIR/explicit_reinitialization.rs:94:9
3636
|
3737
LL | x = 2;
38-
| ^^^^^^ help: create a fresh variable instead of reinitialization: `let mut x = 2;`
38+
| ^^^^^^ help: create a fresh variable instead of reinitialization: `let x = 2;`
3939

4040
error: aborting due to 6 previous errors
4141

0 commit comments

Comments
 (0)