-use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::source::snippet;
-use clippy_utils::{contains_name, higher, iter_input_pats};
-use rustc_hir::intravisit::FnKind;
-use rustc_hir::{
- Block, Body, Expr, ExprKind, FnDecl, Guard, HirId, Local, MutTy, Pat, PatKind, Path, QPath, StmtKind, Ty, TyKind,
- UnOp,
-};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::lint::in_external_macro;
-use rustc_middle::ty;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::source_map::Span;
-use rustc_span::symbol::Symbol;
+use clippy_utils::visitors::is_local_used;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_hir::def::Res;
+use rustc_hir::def_id::LocalDefId;
+use rustc_hir::hir_id::ItemLocalId;
+use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Node, Pat, PatKind, QPath, UnOp};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::{Span, Symbol};
declare_clippy_lint! {
/// ### What it does
/// code. Still, some may opt to avoid it in their code base, they can set this
/// lint to `Warn`.
///
- /// ### Known problems
- /// This lint, as the other shadowing related lints,
- /// currently only catches very simple patterns.
- ///
/// ### Example
/// ```rust
/// # let x = 1;
/// because a value may be bound to different things depending on position in
/// the code.
///
- /// ### Known problems
- /// This lint, as the other shadowing related lints,
- /// currently only catches very simple patterns.
- ///
/// ### Example
/// ```rust
/// let x = 2;
/// any place in the code. This can be alleviated by either giving more specific
/// names to bindings or introducing more scopes to contain the bindings.
///
- /// ### Known problems
- /// This lint, as the other shadowing related lints,
- /// currently only catches very simple patterns. Note that
- /// `allow`/`warn`/`deny`/`forbid` attributes only work on the function level
- /// for this lint.
- ///
/// ### Example
/// ```rust
/// # let y = 1;
/// let w = z; // use different variable name
/// ```
pub SHADOW_UNRELATED,
- pedantic,
+ restriction,
"rebinding a name without even using the original value"
}
-declare_lint_pass!(Shadow => [SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED]);
+#[derive(Default)]
+pub(crate) struct Shadow {
+ bindings: Vec<FxHashMap<Symbol, Vec<ItemLocalId>>>,
+}
+
+impl_lint_pass!(Shadow => [SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED]);
impl<'tcx> LateLintPass<'tcx> for Shadow {
- fn check_fn(
- &mut self,
- cx: &LateContext<'tcx>,
- _: FnKind<'tcx>,
- decl: &'tcx FnDecl<'_>,
- body: &'tcx Body<'_>,
- _: Span,
- _: HirId,
- ) {
- if in_external_macro(cx.sess(), body.value.span) {
+ fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
+ let (id, ident) = match pat.kind {
+ PatKind::Binding(_, hir_id, ident, _) => (hir_id, ident),
+ _ => return,
+ };
+ if ident.span.from_expansion() || ident.span.is_dummy() {
return;
}
- check_fn(cx, decl, body);
- }
-}
+ let HirId { owner, local_id } = id;
-fn check_fn<'tcx>(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>) {
- let mut bindings = Vec::with_capacity(decl.inputs.len());
- for arg in iter_input_pats(decl, body) {
- if let PatKind::Binding(.., ident, _) = arg.pat.kind {
- bindings.push((ident.name, ident.span));
+ // get (or insert) the list of items for this owner and symbol
+ let data = self.bindings.last_mut().unwrap();
+ let items_with_name = data.entry(ident.name).or_default();
+
+ // check other bindings with the same name, most recently seen first
+ for &prev in items_with_name.iter().rev() {
+ if prev == local_id {
+ // repeated binding in an `Or` pattern
+ return;
+ }
+
+ if is_shadow(cx, owner, prev, local_id) {
+ let prev_hir_id = HirId { owner, local_id: prev };
+ lint_shadow(cx, pat, prev_hir_id, ident.span);
+ // only lint against the "nearest" shadowed binding
+ break;
+ }
}
+ // store the binding
+ items_with_name.push(local_id);
}
- check_expr(cx, &body.value, &mut bindings);
-}
-fn check_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'_>, bindings: &mut Vec<(Symbol, Span)>) {
- let len = bindings.len();
- for stmt in block.stmts {
- match stmt.kind {
- StmtKind::Local(local) => check_local(cx, local, bindings),
- StmtKind::Expr(e) | StmtKind::Semi(e) => check_expr(cx, e, bindings),
- StmtKind::Item(..) => {},
+ fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
+ let hir = cx.tcx.hir();
+ if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
+ self.bindings.push(FxHashMap::default());
}
}
- if let Some(o) = block.expr {
- check_expr(cx, o, bindings);
- }
- bindings.truncate(len);
-}
-fn check_local<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>, bindings: &mut Vec<(Symbol, Span)>) {
- if in_external_macro(cx.sess(), local.span) {
- return;
- }
- if higher::is_from_for_desugar(local) {
- return;
- }
- let Local {
- pat,
- ref ty,
- ref init,
- span,
- ..
- } = *local;
- if let Some(t) = *ty {
- check_ty(cx, t, bindings);
- }
- if let Some(o) = *init {
- check_expr(cx, o, bindings);
- check_pat(cx, pat, Some(o), span, bindings);
- } else {
- check_pat(cx, pat, None, span, bindings);
+ fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
+ let hir = cx.tcx.hir();
+ if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
+ self.bindings.pop();
+ }
}
}
-fn is_binding(cx: &LateContext<'_>, pat_id: HirId) -> bool {
- let var_ty = cx.typeck_results().node_type_opt(pat_id);
- var_ty.map_or(false, |var_ty| !matches!(var_ty.kind(), ty::Adt(..)))
+fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
+ let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
+ let first_scope = scope_tree.var_scope(first);
+ let second_scope = scope_tree.var_scope(second);
+ scope_tree.is_subscope_of(second_scope, first_scope)
}
-fn check_pat<'tcx>(
- cx: &LateContext<'tcx>,
- pat: &'tcx Pat<'_>,
- init: Option<&'tcx Expr<'_>>,
- span: Span,
- bindings: &mut Vec<(Symbol, Span)>,
-) {
- // TODO: match more stuff / destructuring
- match pat.kind {
- PatKind::Binding(.., ident, ref inner) => {
- let name = ident.name;
- if is_binding(cx, pat.hir_id) {
- let mut new_binding = true;
- for tup in bindings.iter_mut() {
- if tup.0 == name {
- lint_shadow(cx, name, span, pat.span, init, tup.1);
- tup.1 = ident.span;
- new_binding = false;
- break;
- }
- }
- if new_binding {
- bindings.push((name, ident.span));
- }
- }
- if let Some(p) = *inner {
- check_pat(cx, p, init, span, bindings);
- }
- },
- PatKind::Struct(_, pfields, _) => {
- if let Some(init_struct) = init {
- if let ExprKind::Struct(_, efields, _) = init_struct.kind {
- for field in pfields {
- let name = field.ident.name;
- let efield = efields
- .iter()
- .find_map(|f| if f.ident.name == name { Some(&*f.expr) } else { None });
- check_pat(cx, field.pat, efield, span, bindings);
- }
- } else {
- for field in pfields {
- check_pat(cx, field.pat, init, span, bindings);
- }
- }
- } else {
- for field in pfields {
- check_pat(cx, field.pat, None, span, bindings);
- }
- }
+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) => {
+ let msg = format!(
+ "`{}` is shadowed by itself in `{}`",
+ snippet(cx, pat.span, "_"),
+ snippet(cx, expr.span, "..")
+ );
+ (SHADOW_SAME, msg)
},
- PatKind::Tuple(inner, _) => {
- if let Some(init_tup) = init {
- if let ExprKind::Tup(tup) = init_tup.kind {
- for (i, p) in inner.iter().enumerate() {
- check_pat(cx, p, Some(&tup[i]), p.span, bindings);
- }
- } else {
- for p in inner {
- check_pat(cx, p, init, span, bindings);
- }
- }
- } else {
- for p in inner {
- check_pat(cx, p, None, span, bindings);
- }
- }
+ Some(expr) if is_local_used(cx, expr, shadowed) => {
+ let msg = format!(
+ "`{}` is shadowed by `{}` which reuses the original value",
+ snippet(cx, pat.span, "_"),
+ snippet(cx, expr.span, "..")
+ );
+ (SHADOW_REUSE, msg)
},
- PatKind::Box(inner) => {
- if let Some(initp) = init {
- if let ExprKind::Box(inner_init) = initp.kind {
- check_pat(cx, inner, Some(inner_init), span, bindings);
- } else {
- check_pat(cx, inner, init, span, bindings);
- }
- } else {
- check_pat(cx, inner, init, span, bindings);
- }
+ _ => {
+ let msg = format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
+ (SHADOW_UNRELATED, msg)
},
- PatKind::Ref(inner, _) => check_pat(cx, inner, init, span, bindings),
- // PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
- _ => (),
- }
+ };
+ span_lint_and_note(
+ cx,
+ lint,
+ span,
+ &msg,
+ Some(cx.tcx.hir().span(shadowed)),
+ "previous binding is here",
+ );
}
-fn lint_shadow<'tcx>(
- cx: &LateContext<'tcx>,
- name: Symbol,
- span: Span,
- pattern_span: Span,
- init: Option<&'tcx Expr<'_>>,
- prev_span: Span,
-) {
- if let Some(expr) = init {
- if is_self_shadow(name, expr) {
- span_lint_and_then(
- cx,
- SHADOW_SAME,
- span,
- &format!(
- "`{}` is shadowed by itself in `{}`",
- snippet(cx, pattern_span, "_"),
- snippet(cx, expr.span, "..")
- ),
- |diag| {
- diag.span_note(prev_span, "previous binding is here");
- },
- );
- } else if contains_name(name, expr) {
- span_lint_and_then(
- cx,
- SHADOW_REUSE,
- pattern_span,
- &format!(
- "`{}` is shadowed by `{}` which reuses the original value",
- snippet(cx, pattern_span, "_"),
- snippet(cx, expr.span, "..")
- ),
- |diag| {
- diag.span_note(expr.span, "initialization happens here");
- diag.span_note(prev_span, "previous binding is here");
- },
- );
- } else {
- span_lint_and_then(
- cx,
- SHADOW_UNRELATED,
- pattern_span,
- &format!("`{}` is being shadowed", snippet(cx, pattern_span, "_")),
- |diag| {
- diag.span_note(expr.span, "initialization happens here");
- diag.span_note(prev_span, "previous binding is here");
+/// Returns true if the expression is a simple transformation of a local binding such as `&x`
+fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_id: HirId) -> bool {
+ let hir = cx.tcx.hir();
+ let is_direct_binding = hir
+ .parent_iter(pat.hir_id)
+ .map_while(|(_id, node)| match node {
+ Node::Pat(pat) => Some(pat),
+ _ => None,
+ })
+ .all(|pat| matches!(pat.kind, PatKind::Ref(..) | PatKind::Or(_)));
+ if !is_direct_binding {
+ return false;
+ }
+ loop {
+ expr = match expr.kind {
+ ExprKind::Box(e)
+ | ExprKind::AddrOf(_, _, e)
+ | ExprKind::Block(
+ &Block {
+ stmts: [],
+ expr: Some(e),
+ ..
},
- );
+ _,
+ )
+ | ExprKind::Unary(UnOp::Deref, e) => e,
+ ExprKind::Path(QPath::Resolved(None, path)) => break path.res == Res::Local(hir_id),
+ _ => break false,
}
- } else {
- span_lint_and_then(
- cx,
- SHADOW_UNRELATED,
- span,
- &format!("`{}` shadows a previous declaration", snippet(cx, pattern_span, "_")),
- |diag| {
- diag.span_note(prev_span, "previous binding is here");
- },
- );
}
}
-fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, bindings: &mut Vec<(Symbol, Span)>) {
- if in_external_macro(cx.sess(), expr.span) {
- return;
- }
- match expr.kind {
- ExprKind::Unary(_, e) | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) | ExprKind::Box(e) => {
- check_expr(cx, e, bindings);
- },
- ExprKind::Block(block, _) | ExprKind::Loop(block, ..) => check_block(cx, block, bindings),
- // ExprKind::Call
- // ExprKind::MethodCall
- ExprKind::Array(v) | ExprKind::Tup(v) => {
- for e in v {
- check_expr(cx, e, bindings);
- }
- },
- ExprKind::If(cond, then, ref otherwise) => {
- check_expr(cx, cond, bindings);
- check_expr(cx, then, bindings);
- if let Some(o) = *otherwise {
- check_expr(cx, o, bindings);
- }
- },
- ExprKind::Match(init, arms, _) => {
- check_expr(cx, init, bindings);
- let len = bindings.len();
- for arm in arms {
- check_pat(cx, arm.pat, Some(init), arm.pat.span, bindings);
- // This is ugly, but needed to get the right type
- if let Some(ref guard) = arm.guard {
- match guard {
- Guard::If(if_expr) => check_expr(cx, if_expr, bindings),
- Guard::IfLet(guard_pat, guard_expr) => {
- check_pat(cx, guard_pat, Some(*guard_expr), guard_pat.span, bindings);
- check_expr(cx, guard_expr, bindings);
- },
- }
- }
- check_expr(cx, arm.body, bindings);
- bindings.truncate(len);
- }
- },
- _ => (),
- }
-}
-
-fn check_ty<'tcx>(cx: &LateContext<'tcx>, ty: &'tcx Ty<'_>, bindings: &mut Vec<(Symbol, Span)>) {
- match ty.kind {
- TyKind::Slice(sty) => check_ty(cx, sty, bindings),
- TyKind::Array(fty, ref anon_const) => {
- check_ty(cx, fty, bindings);
- check_expr(cx, &cx.tcx.hir().body(anon_const.body).value, bindings);
- },
- TyKind::Ptr(MutTy { ty: mty, .. }) | TyKind::Rptr(_, MutTy { ty: mty, .. }) => check_ty(cx, mty, bindings),
- TyKind::Tup(tup) => {
- for t in tup {
- check_ty(cx, t, bindings);
- }
- },
- TyKind::Typeof(ref anon_const) => check_expr(cx, &cx.tcx.hir().body(anon_const.body).value, bindings),
- _ => (),
- }
-}
-
-fn is_self_shadow(name: Symbol, expr: &Expr<'_>) -> bool {
- match expr.kind {
- ExprKind::Box(inner) | ExprKind::AddrOf(_, _, inner) => is_self_shadow(name, inner),
- ExprKind::Block(block, _) => {
- block.stmts.is_empty() && block.expr.as_ref().map_or(false, |e| is_self_shadow(name, e))
- },
- ExprKind::Unary(op, inner) => (UnOp::Deref == op) && is_self_shadow(name, inner),
- ExprKind::Path(QPath::Resolved(_, path)) => path_eq_name(name, path),
- _ => false,
+/// Finds the "init" expression for a pattern: `let <pat> = <init>;` or
+/// `match <init> { .., <pat> => .., .. }`
+fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> {
+ for (_, node) in cx.tcx.hir().parent_iter(hir_id) {
+ let init = match node {
+ Node::Arm(_) | Node::Pat(_) => continue,
+ Node::Expr(expr) => match expr.kind {
+ ExprKind::Match(e, _, _) => Some(e),
+ _ => None,
+ },
+ Node::Local(local) => local.init,
+ _ => None,
+ };
+ return init;
}
-}
-
-fn path_eq_name(name: Symbol, path: &Path<'_>) -> bool {
- !path.is_global() && path.segments.len() == 1 && path.segments[0].ident.name == name
+ None
}
projects / rust.git / blobdiff