diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index 7ae0310b6d99..49d22377119f 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -1,6 +1,9 @@ +use std::ops::ControlFlow; + use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::path_to_local_id; use clippy_utils::source::snippet; -use clippy_utils::visitors::is_local_used; +use clippy_utils::visitors::{Descend, Visitable, for_each_expr}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; @@ -175,9 +178,31 @@ fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second false } +/// Checks if the given local is used, except for in child expression of `except`. +/// +/// This is a version of [`is_local_used`](clippy_utils::visitors::is_local_used), used to +/// implement the fix for . +pub fn is_local_used_except<'tcx>( + cx: &LateContext<'tcx>, + visitable: impl Visitable<'tcx>, + id: HirId, + except: Option, +) -> bool { + for_each_expr(cx, visitable, |e| { + if except.is_some_and(|it| it == e.hir_id) { + ControlFlow::Continue(Descend::No) + } else if path_to_local_id(e, id) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(Descend::Yes) + } + }) + .is_some() +} + fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) { let (lint, msg) = match find_init(cx, pat.hir_id) { - Some(expr) if is_self_shadow(cx, pat, expr, shadowed) => { + Some((expr, _)) if is_self_shadow(cx, pat, expr, shadowed) => { let msg = format!( "`{}` is shadowed by itself in `{}`", snippet(cx, pat.span, "_"), @@ -185,7 +210,7 @@ fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) ); (SHADOW_SAME, msg) }, - Some(expr) if is_local_used(cx, expr, shadowed) => { + Some((expr, except)) if is_local_used_except(cx, expr, shadowed, except) => { let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_")); (SHADOW_REUSE, msg) }, @@ -232,15 +257,32 @@ fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_ /// Finds the "init" expression for a pattern: `let = ;` (or `if let`) or /// `match { .., => .., .. }` -fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> { - for (_, node) in cx.tcx.hir().parent_iter(hir_id) { +/// +/// For closure arguments passed to a method call, returns the method call, and the `HirId` of the +/// closure (which will later be skipped). This is for +fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<(&'tcx Expr<'tcx>, Option)> { + for (hir_id, node) in cx.tcx.hir().parent_iter(hir_id) { let init = match node { - Node::Arm(_) | Node::Pat(_) => continue, + Node::Arm(_) | Node::Pat(_) | Node::Param(_) => continue, Node::Expr(expr) => match expr.kind { - ExprKind::Match(e, _, _) | ExprKind::Let(&LetExpr { init: e, .. }) => Some(e), + ExprKind::Match(e, _, _) | ExprKind::Let(&LetExpr { init: e, .. }) => Some((e, None)), + // If we're a closure argument, then a parent call is also an associated item. + ExprKind::Closure(_) => { + if let Some((_, node)) = cx.tcx.hir().parent_iter(hir_id).next() { + match node { + Node::Expr(expr) => match expr.kind { + ExprKind::MethodCall(_, _, _, _) | ExprKind::Call(_, _) => Some((expr, Some(hir_id))), + _ => None, + }, + _ => None, + } + } else { + None + } + }, _ => None, }, - Node::LetStmt(local) => local.init, + Node::LetStmt(local) => local.init.map(|init| (init, None)), _ => None, }; return init; diff --git a/tests/ui/shadow.rs b/tests/ui/shadow.rs index 258dba9dd831..30b96277f99f 100644 --- a/tests/ui/shadow.rs +++ b/tests/ui/shadow.rs @@ -119,4 +119,18 @@ fn ice_8748() { }]; } +// https://github.com/rust-lang/rust-clippy/issues/10780 +fn shadow_closure() { + // These are not shadow_unrelated; but they are correctly shadow_reuse + let x = Some(1); + #[allow(clippy::shadow_reuse)] + let y = x.map(|x| x + 1); + let z = x.map(|x| x + 1); + let a: Vec> = [100u8, 120, 140] + .iter() + .map(|i| i.checked_mul(2)) + .map(|i| i.map(|i| i - 10)) + .collect(); +} + fn main() {} diff --git a/tests/ui/shadow.stderr b/tests/ui/shadow.stderr index fdd149a2216f..5d2d70604506 100644 --- a/tests/ui/shadow.stderr +++ b/tests/ui/shadow.stderr @@ -280,5 +280,29 @@ note: previous binding is here LL | let x = 1; | ^ -error: aborting due to 23 previous errors +error: `x` is shadowed + --> tests/ui/shadow.rs:128:20 + | +LL | let z = x.map(|x| x + 1); + | ^ + | +note: previous binding is here + --> tests/ui/shadow.rs:125:9 + | +LL | let x = Some(1); + | ^ + +error: `i` is shadowed + --> tests/ui/shadow.rs:132:25 + | +LL | .map(|i| i.map(|i| i - 10)) + | ^ + | +note: previous binding is here + --> tests/ui/shadow.rs:132:15 + | +LL | .map(|i| i.map(|i| i - 10)) + | ^ + +error: aborting due to 25 previous errors