Skip to content

Commit

Permalink
Auto merge of rust-lang#10143 - EricWu2003:field_reassign_with_defaul…
Browse files Browse the repository at this point in the history
…t-FP, r=Manishearth

don't lint field_reassign when field in closure

fixes rust-lang#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
[rust-lang#10143](rust-lang/rust-clippy#10143)
<!-- changelog_checked -->
  • Loading branch information
bors committed Jan 3, 2023
2 parents 1a46dc0 + cf4e981 commit 4334919
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 8 deletions.
6 changes: 5 additions & 1 deletion clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,11 @@ fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbo
.iter()
.filter(|&&(_, name)| !name.as_str().starts_with('_'))
.any(|&(_, name)| {
let mut walker = ContainsName { name, result: false };
let mut walker = ContainsName {
name,
result: false,
cx,
};

// Scan block
block
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl<'tcx> LateLintPass<'tcx> for Default {
// find out if and which field was set by this `consecutive_statement`
if let Some((field_ident, assign_rhs)) = field_reassigned_by_stmt(consecutive_statement, binding_name) {
// interrupt and cancel lint if assign_rhs references the original binding
if contains_name(binding_name, assign_rhs) {
if contains_name(binding_name, assign_rhs, cx) {
cancel_lint = true;
break;
}
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/loops/needless_range_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub(super) fn check<'tcx>(

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

if is_len_call(end, indexed) || is_end_eq_array_len(cx, end, limits, indexed_ty) {
String::new()
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr) {
} else if visitor.indexed_mut.contains(&indexed) && contains_name(indexed, take_expr, cx) {
return;
} else {
match limits {
Expand Down
21 changes: 17 additions & 4 deletions clippy_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ use crate::consts::{constant, Constant};
use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param};
use crate::visitors::for_each_expr;

use rustc_middle::hir::nested_filter;

#[macro_export]
macro_rules! extract_msrv_attr {
($context:ident) => {
Expand Down Expand Up @@ -1253,22 +1255,33 @@ pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> {
}
}

pub struct ContainsName {
pub struct ContainsName<'a, 'tcx> {
pub cx: &'a LateContext<'tcx>,
pub name: Symbol,
pub result: bool,
}

impl<'tcx> Visitor<'tcx> for ContainsName {
impl<'a, 'tcx> Visitor<'tcx> for ContainsName<'a, 'tcx> {
type NestedFilter = nested_filter::OnlyBodies;

fn visit_name(&mut self, name: Symbol) {
if self.name == name {
self.result = true;
}
}

fn nested_visit_map(&mut self) -> Self::Map {
self.cx.tcx.hir()
}
}

/// Checks if an `Expr` contains a certain name.
pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool {
let mut cn = ContainsName { name, result: false };
pub fn contains_name<'tcx>(name: Symbol, expr: &'tcx Expr<'_>, cx: &LateContext<'tcx>) -> bool {
let mut cn = ContainsName {
name,
result: false,
cx,
};
cn.visit_expr(expr);
cn.result
}
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/field_reassign_with_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,24 @@ mod issue6312 {
}
}
}

struct Collection {
items: Vec<i32>,
len: usize,
}

impl Default for Collection {
fn default() -> Self {
Self {
items: vec![1, 2, 3],
len: 0,
}
}
}

#[allow(clippy::redundant_closure_call)]
fn issue10136() {
let mut c = Collection::default();
// don't lint, since c.items was used to calculate this value
c.len = (|| c.items.len())();
}

0 comments on commit 4334919

Please sign in to comment.