clippy_lints/src/shadow.rs

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