Skip to content

Commit 4334919

Browse files
committed
Auto merge of #10143 - EricWu2003:field_reassign_with_default-FP, r=Manishearth
don't lint field_reassign when field in closure fixes #10136 This change makes the ContainsName struct visit all interior expressions, which means that ContainsName will return true even if `name` is used in a closure within `expr`. --- changelog: FP: [`field_reassign_with_default`]: No longer lints cases, where values are initializes from closures capturing struct values [#10143](#10143) <!-- changelog_checked -->
2 parents 1a46dc0 + cf4e981 commit 4334919

File tree

5 files changed

+46
-8
lines changed

5 files changed

+46
-8
lines changed

clippy_lints/src/copies.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,11 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
525525
.iter()
526526
.filter(|&&(_, name)| !name.as_str().starts_with('_'))
527527
.any(|&(_, name)| {
528-
let mut walker = ContainsName { name, result: false };
528+
let mut walker = ContainsName {
529+
name,
530+
result: false,
531+
cx,
532+
};
529533

530534
// Scan block
531535
block

clippy_lints/src/default.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
170170
// find out if and which field was set by this `consecutive_statement`
171171
if let Some((field_ident, assign_rhs)) = field_reassigned_by_stmt(consecutive_statement, binding_name) {
172172
// interrupt and cancel lint if assign_rhs references the original binding
173-
if contains_name(binding_name, assign_rhs) {
173+
if contains_name(binding_name, assign_rhs, cx) {
174174
cancel_lint = true;
175175
break;
176176
}

clippy_lints/src/loops/needless_range_loop.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ pub(super) fn check<'tcx>(
8181

8282
let skip = if starts_at_zero {
8383
String::new()
84-
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, start) {
84+
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, start, cx) {
8585
return;
8686
} else {
8787
format!(".skip({})", snippet(cx, start.span, ".."))
@@ -109,7 +109,7 @@ pub(super) fn check<'tcx>(
109109

110110
if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
111111
String::new()
112-
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
112+
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr, cx) {
113113
return;
114114
} else {
115115
match limits {

clippy_utils/src/lib.rs

+17-4
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ use crate::consts::{constant, Constant};
116116
use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param};
117117
use crate::visitors::for_each_expr;
118118

119+
use rustc_middle::hir::nested_filter;
120+
119121
#[macro_export]
120122
macro_rules! extract_msrv_attr {
121123
($context:ident) => {
@@ -1253,22 +1255,33 @@ pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> {
12531255
}
12541256
}
12551257

1256-
pub struct ContainsName {
1258+
pub struct ContainsName<'a, 'tcx> {
1259+
pub cx: &'a LateContext<'tcx>,
12571260
pub name: Symbol,
12581261
pub result: bool,
12591262
}
12601263

1261-
impl<'tcx> Visitor<'tcx> for ContainsName {
1264+
impl<'a, 'tcx> Visitor<'tcx> for ContainsName<'a, 'tcx> {
1265+
type NestedFilter = nested_filter::OnlyBodies;
1266+
12621267
fn visit_name(&mut self, name: Symbol) {
12631268
if self.name == name {
12641269
self.result = true;
12651270
}
12661271
}
1272+
1273+
fn nested_visit_map(&mut self) -> Self::Map {
1274+
self.cx.tcx.hir()
1275+
}
12671276
}
12681277

12691278
/// Checks if an `Expr` contains a certain name.
1270-
pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool {
1271-
let mut cn = ContainsName { name, result: false };
1279+
pub fn contains_name<'tcx>(name: Symbol, expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> bool {
1280+
let mut cn = ContainsName {
1281+
name,
1282+
result: false,
1283+
cx,
1284+
};
12721285
cn.visit_expr(expr);
12731286
cn.result
12741287
}

tests/ui/field_reassign_with_default.rs

+21
Original file line numberDiff line numberDiff line change
@@ -247,3 +247,24 @@ mod issue6312 {
247247
}
248248
}
249249
}
250+
251+
struct Collection {
252+
items: Vec<i32>,
253+
len: usize,
254+
}
255+
256+
impl Default for Collection {
257+
fn default() -> Self {
258+
Self {
259+
items: vec![1, 2, 3],
260+
len: 0,
261+
}
262+
}
263+
}
264+
265+
#[allow(clippy::redundant_closure_call)]
266+
fn issue10136() {
267+
let mut c = Collection::default();
268+
// don't lint, since c.items was used to calculate this value
269+
c.len = (|| c.items.len())();
270+
}

0 commit comments

Comments
 (0)