clippy_lints/src/shadow.rs

   1 use clippy_utils::diagnostics::span_lint_and_then;
   2 use clippy_utils::source::snippet;
   3 use clippy_utils::{contains_name, higher, iter_input_pats};
   4 use rustc_hir::intravisit::FnKind;
   5 use rustc_hir::{
   6     Block, Body, Expr, ExprKind, FnDecl, Guard, HirId, Local, MutTy, Pat, PatKind, Path, QPath, StmtKind, Ty, TyKind,
   7     UnOp,
   8 };
   9 use rustc_lint::{LateContext, LateLintPass, LintContext};
  10 use rustc_middle::lint::in_external_macro;
  11 use rustc_middle::ty;
  12 use rustc_session::{declare_lint_pass, declare_tool_lint};
  13 use rustc_span::source_map::Span;
  14 use rustc_span::symbol::Symbol;
  15
  16 declare_clippy_lint! {
  17     /// ### What it does
  18     /// Checks for bindings that shadow other bindings already in
  19     /// scope, while just changing reference level or mutability.
  20     ///
  21     /// ### Why is this bad?
  22     /// Not much, in fact it's a very common pattern in Rust
  23     /// code. Still, some may opt to avoid it in their code base, they can set this
  24     /// lint to `Warn`.
  25     ///
  26     /// ### Known problems
  27     /// This lint, as the other shadowing related lints,
  28     /// currently only catches very simple patterns.
  29     ///
  30     /// ### Example
  31     /// ```rust
  32     /// # let x = 1;
  33     /// // Bad
  34     /// let x = &x;
  35     ///
  36     /// // Good
  37     /// let y = &x; // use different variable name
  38     /// ```
  39     pub SHADOW_SAME,
  40     restriction,
  41     "rebinding a name to itself, e.g., `let mut x = &mut x`"
  42 }
  43
  44 declare_clippy_lint! {
  45     /// ### What it does
  46     /// Checks for bindings that shadow other bindings already in
  47     /// scope, while reusing the original value.
  48     ///
  49     /// ### Why is this bad?
  50     /// Not too much, in fact it's a common pattern in Rust
  51     /// code. Still, some argue that name shadowing like this hurts readability,
  52     /// because a value may be bound to different things depending on position in
  53     /// the code.
  54     ///
  55     /// ### Known problems
  56     /// This lint, as the other shadowing related lints,
  57     /// currently only catches very simple patterns.
  58     ///
  59     /// ### Example
  60     /// ```rust
  61     /// let x = 2;
  62     /// let x = x + 1;
  63     /// ```
  64     /// use different variable name:
  65     /// ```rust
  66     /// let x = 2;
  67     /// let y = x + 1;
  68     /// ```
  69     pub SHADOW_REUSE,
  70     restriction,
  71     "rebinding a name to an expression that re-uses the original value, e.g., `let x = x + 1`"
  72 }
  73
  74 declare_clippy_lint! {
  75     /// ### What it does
  76     /// Checks for bindings that shadow other bindings already in
  77     /// scope, either without a initialization or with one that does not even use
  78     /// the original value.
  79     ///
  80     /// ### Why is this bad?
  81     /// Name shadowing can hurt readability, especially in
  82     /// large code bases, because it is easy to lose track of the active binding at
  83     /// any place in the code. This can be alleviated by either giving more specific
  84     /// names to bindings or introducing more scopes to contain the bindings.
  85     ///
  86     /// ### Known problems
  87     /// This lint, as the other shadowing related lints,
  88     /// currently only catches very simple patterns. Note that
  89     /// `allow`/`warn`/`deny`/`forbid` attributes only work on the function level
  90     /// for this lint.
  91     ///
  92     /// ### Example
  93     /// ```rust
  94     /// # let y = 1;
  95     /// # let z = 2;
  96     /// let x = y;
  97     ///
  98     /// // Bad
  99     /// let x = z; // shadows the earlier binding
 100     ///
 101     /// // Good
 102     /// let w = z; // use different variable name
 103     /// ```
 104     pub SHADOW_UNRELATED,
 105     pedantic,
 106     "rebinding a name without even using the original value"
 107 }
 108
 109 declare_lint_pass!(Shadow => [SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED]);
 110
 111 impl<'tcx> LateLintPass<'tcx> for Shadow {
 112     fn check_fn(
 113         &mut self,
 114         cx: &LateContext<'tcx>,
 115         _: FnKind<'tcx>,
 116         decl: &'tcx FnDecl<'_>,
 117         body: &'tcx Body<'_>,
 118         _: Span,
 119         _: HirId,
 120     ) {
 121         if in_external_macro(cx.sess(), body.value.span) {
 122             return;
 123         }
 124         check_fn(cx, decl, body);
 125     }
 126 }
 127
 128 fn check_fn<'tcx>(cx: &LateContext<'tcx>, decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>) {
 129     let mut bindings = Vec::with_capacity(decl.inputs.len());
 130     for arg in iter_input_pats(decl, body) {
 131         if let PatKind::Binding(.., ident, _) = arg.pat.kind {
 132             bindings.push((ident.name, ident.span));
 133         }
 134     }
 135     check_expr(cx, &body.value, &mut bindings);
 136 }
 137
 138 fn check_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'_>, bindings: &mut Vec<(Symbol, Span)>) {
 139     let len = bindings.len();
 140     for stmt in block.stmts {
 141         match stmt.kind {
 142             StmtKind::Local(local) => check_local(cx, local, bindings),
 143             StmtKind::Expr(e) | StmtKind::Semi(e) => check_expr(cx, e, bindings),
 144             StmtKind::Item(..) => {},
 145         }
 146     }
 147     if let Some(o) = block.expr {
 148         check_expr(cx, o, bindings);
 149     }
 150     bindings.truncate(len);
 151 }
 152
 153 fn check_local<'tcx>(cx: &LateContext<'tcx>, local: &'tcx Local<'_>, bindings: &mut Vec<(Symbol, Span)>) {
 154     if in_external_macro(cx.sess(), local.span) {
 155         return;
 156     }
 157     if higher::is_from_for_desugar(local) {
 158         return;
 159     }
 160     let Local {
 161         pat,
 162         ref ty,
 163         ref init,
 164         span,
 165         ..
 166     } = *local;
 167     if let Some(t) = *ty {
 168         check_ty(cx, t, bindings);
 169     }
 170     if let Some(o) = *init {
 171         check_expr(cx, o, bindings);
 172         check_pat(cx, pat, Some(o), span, bindings);
 173     } else {
 174         check_pat(cx, pat, None, span, bindings);
 175     }
 176 }
 177
 178 fn is_binding(cx: &LateContext<'_>, pat_id: HirId) -> bool {
 179     let var_ty = cx.typeck_results().node_type_opt(pat_id);
 180     var_ty.map_or(false, |var_ty| !matches!(var_ty.kind(), ty::Adt(..)))
 181 }
 182
 183 fn check_pat<'tcx>(
 184     cx: &LateContext<'tcx>,
 185     pat: &'tcx Pat<'_>,
 186     init: Option<&'tcx Expr<'_>>,
 187     span: Span,
 188     bindings: &mut Vec<(Symbol, Span)>,
 189 ) {
 190     // TODO: match more stuff / destructuring
 191     match pat.kind {
 192         PatKind::Binding(.., ident, ref inner) => {
 193             let name = ident.name;
 194             if is_binding(cx, pat.hir_id) {
 195                 let mut new_binding = true;
 196                 for tup in bindings.iter_mut() {
 197                     if tup.0 == name {
 198                         lint_shadow(cx, name, span, pat.span, init, tup.1);
 199                         tup.1 = ident.span;
 200                         new_binding = false;
 201                         break;
 202                     }
 203                 }
 204                 if new_binding {
 205                     bindings.push((name, ident.span));
 206                 }
 207             }
 208             if let Some(p) = *inner {
 209                 check_pat(cx, p, init, span, bindings);
 210             }
 211         },
 212         PatKind::Struct(_, pfields, _) => {
 213             if let Some(init_struct) = init {
 214                 if let ExprKind::Struct(_, efields, _) = init_struct.kind {
 215                     for field in pfields {
 216                         let name = field.ident.name;
 217                         let efield = efields
 218                             .iter()
 219                             .find_map(|f| if f.ident.name == name { Some(&*f.expr) } else { None });
 220                         check_pat(cx, field.pat, efield, span, bindings);
 221                     }
 222                 } else {
 223                     for field in pfields {
 224                         check_pat(cx, field.pat, init, span, bindings);
 225                     }
 226                 }
 227             } else {
 228                 for field in pfields {
 229                     check_pat(cx, field.pat, None, span, bindings);
 230                 }
 231             }
 232         },
 233         PatKind::Tuple(inner, _) => {
 234             if let Some(init_tup) = init {
 235                 if let ExprKind::Tup(tup) = init_tup.kind {
 236                     for (i, p) in inner.iter().enumerate() {
 237                         check_pat(cx, p, Some(&tup[i]), p.span, bindings);
 238                     }
 239                 } else {
 240                     for p in inner {
 241                         check_pat(cx, p, init, span, bindings);
 242                     }
 243                 }
 244             } else {
 245                 for p in inner {
 246                     check_pat(cx, p, None, span, bindings);
 247                 }
 248             }
 249         },
 250         PatKind::Box(inner) => {
 251             if let Some(initp) = init {
 252                 if let ExprKind::Box(inner_init) = initp.kind {
 253                     check_pat(cx, inner, Some(inner_init), span, bindings);
 254                 } else {
 255                     check_pat(cx, inner, init, span, bindings);
 256                 }
 257             } else {
 258                 check_pat(cx, inner, init, span, bindings);
 259             }
 260         },
 261         PatKind::Ref(inner, _) => check_pat(cx, inner, init, span, bindings),
 262         // PatVec(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),
 263         _ => (),
 264     }
 265 }
 266
 267 fn lint_shadow<'tcx>(
 268     cx: &LateContext<'tcx>,
 269     name: Symbol,
 270     span: Span,
 271     pattern_span: Span,
 272     init: Option<&'tcx Expr<'_>>,
 273     prev_span: Span,
 274 ) {
 275     if let Some(expr) = init {
 276         if is_self_shadow(name, expr) {
 277             span_lint_and_then(
 278                 cx,
 279                 SHADOW_SAME,
 280                 span,
 281                 &format!(
 282                     "`{}` is shadowed by itself in `{}`",
 283                     snippet(cx, pattern_span, "_"),
 284                     snippet(cx, expr.span, "..")
 285                 ),
 286                 |diag| {
 287                     diag.span_note(prev_span, "previous binding is here");
 288                 },
 289             );
 290         } else if contains_name(name, expr) {
 291             span_lint_and_then(
 292                 cx,
 293                 SHADOW_REUSE,
 294                 pattern_span,
 295                 &format!(
 296                     "`{}` is shadowed by `{}` which reuses the original value",
 297                     snippet(cx, pattern_span, "_"),
 298                     snippet(cx, expr.span, "..")
 299                 ),
 300                 |diag| {
 301                     diag.span_note(expr.span, "initialization happens here");
 302                     diag.span_note(prev_span, "previous binding is here");
 303                 },
 304             );
 305         } else {
 306             span_lint_and_then(
 307                 cx,
 308                 SHADOW_UNRELATED,
 309                 pattern_span,
 310                 &format!("`{}` is being shadowed", snippet(cx, pattern_span, "_")),
 311                 |diag| {
 312                     diag.span_note(expr.span, "initialization happens here");
 313                     diag.span_note(prev_span, "previous binding is here");
 314                 },
 315             );
 316         }
 317     } else {
 318         span_lint_and_then(
 319             cx,
 320             SHADOW_UNRELATED,
 321             span,
 322             &format!("`{}` shadows a previous declaration", snippet(cx, pattern_span, "_")),
 323             |diag| {
 324                 diag.span_note(prev_span, "previous binding is here");
 325             },
 326         );
 327     }
 328 }
 329
 330 fn check_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, bindings: &mut Vec<(Symbol, Span)>) {
 331     if in_external_macro(cx.sess(), expr.span) {
 332         return;
 333     }
 334     match expr.kind {
 335         ExprKind::Unary(_, e) | ExprKind::Field(e, _) | ExprKind::AddrOf(_, _, e) | ExprKind::Box(e) => {
 336             check_expr(cx, e, bindings);
 337         },
 338         ExprKind::Block(block, _) | ExprKind::Loop(block, ..) => check_block(cx, block, bindings),
 339         // ExprKind::Call
 340         // ExprKind::MethodCall
 341         ExprKind::Array(v) | ExprKind::Tup(v) => {
 342             for e in v {
 343                 check_expr(cx, e, bindings);
 344             }
 345         },
 346         ExprKind::If(cond, then, ref otherwise) => {
 347             check_expr(cx, cond, bindings);
 348             check_expr(cx, then, bindings);
 349             if let Some(o) = *otherwise {
 350                 check_expr(cx, o, bindings);
 351             }
 352         },
 353         ExprKind::Match(init, arms, _) => {
 354             check_expr(cx, init, bindings);
 355             let len = bindings.len();
 356             for arm in arms {
 357                 check_pat(cx, arm.pat, Some(init), arm.pat.span, bindings);
 358                 // This is ugly, but needed to get the right type
 359                 if let Some(ref guard) = arm.guard {
 360                     match guard {
 361                         Guard::If(if_expr) => check_expr(cx, if_expr, bindings),
 362                         Guard::IfLet(guard_pat, guard_expr) => {
 363                             check_pat(cx, guard_pat, Some(*guard_expr), guard_pat.span, bindings);
 364                             check_expr(cx, guard_expr, bindings);
 365                         },
 366                     }
 367                 }
 368                 check_expr(cx, arm.body, bindings);
 369                 bindings.truncate(len);
 370             }
 371         },
 372         _ => (),
 373     }
 374 }
 375
 376 fn check_ty<'tcx>(cx: &LateContext<'tcx>, ty: &'tcx Ty<'_>, bindings: &mut Vec<(Symbol, Span)>) {
 377     match ty.kind {
 378         TyKind::Slice(sty) => check_ty(cx, sty, bindings),
 379         TyKind::Array(fty, ref anon_const) => {
 380             check_ty(cx, fty, bindings);
 381             check_expr(cx, &cx.tcx.hir().body(anon_const.body).value, bindings);
 382         },
 383         TyKind::Ptr(MutTy { ty: mty, .. }) | TyKind::Rptr(_, MutTy { ty: mty, .. }) => check_ty(cx, mty, bindings),
 384         TyKind::Tup(tup) => {
 385             for t in tup {
 386                 check_ty(cx, t, bindings);
 387             }
 388         },
 389         TyKind::Typeof(ref anon_const) => check_expr(cx, &cx.tcx.hir().body(anon_const.body).value, bindings),
 390         _ => (),
 391     }
 392 }
 393
 394 fn is_self_shadow(name: Symbol, expr: &Expr<'_>) -> bool {
 395     match expr.kind {
 396         ExprKind::Box(inner) | ExprKind::AddrOf(_, _, inner) => is_self_shadow(name, inner),
 397         ExprKind::Block(block, _) => {
 398             block.stmts.is_empty() && block.expr.as_ref().map_or(false, |e| is_self_shadow(name, e))
 399         },
 400         ExprKind::Unary(op, inner) => (UnOp::Deref == op) && is_self_shadow(name, inner),
 401         ExprKind::Path(QPath::Resolved(_, path)) => path_eq_name(name, path),
 402         _ => false,
 403     }
 404 }
 405
 406 fn path_eq_name(name: Symbol, path: &Path<'_>) -> bool {
 407     !path.is_global() && path.segments.len() == 1 && path.segments[0].ident.name == name
 408 }