src/loops.rs

   1 use rustc::lint::*;
   2 use syntax::ast::*;
   3 use syntax::visit::{Visitor, walk_expr};
   4 use rustc::middle::ty;
   5 use std::collections::HashSet;
   6
   7 use utils::{snippet, span_lint, get_parent_expr, match_def_path};
   8
   9 declare_lint!{ pub NEEDLESS_RANGE_LOOP, Warn,
  10                "for-looping over a range of indices where an iterator over items would do" }
  11
  12 declare_lint!{ pub EXPLICIT_ITER_LOOP, Warn,
  13                "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do" }
  14
  15 declare_lint!{ pub ITER_NEXT_LOOP, Warn,
  16                "for-looping over `_.next()` which is probably not intended" }
  17
  18 #[derive(Copy, Clone)]
  19 pub struct LoopsPass;
  20
  21 impl LintPass for LoopsPass {
  22     fn get_lints(&self) -> LintArray {
  23         lint_array!(NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, ITER_NEXT_LOOP)
  24     }
  25
  26     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
  27         if let Some((pat, arg, body)) = recover_for_loop(expr) {
  28             // check for looping over a range and then indexing a sequence with it
  29             // -> the iteratee must be a range literal
  30             if let ExprRange(_, _) = arg.node {
  31                 // the var must be a single name
  32                 if let PatIdent(_, ref ident, _) = pat.node {
  33                     let mut visitor = VarVisitor { cx: cx, var: ident.node.name,
  34                                                    indexed: HashSet::new(), nonindex: false };
  35                     walk_expr(&mut visitor, body);
  36                     // linting condition: we only indexed one variable
  37                     if visitor.indexed.len() == 1 {
  38                         let indexed = visitor.indexed.into_iter().next().expect("Len was nonzero, but no contents found");
  39                         if visitor.nonindex {
  40                             span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
  41                                 "the loop variable `{}` is used to index `{}`. Consider using \
  42                                  `for ({}, item) in {}.iter().enumerate()` or similar iterators",
  43                                 ident.node.name, indexed, ident.node.name, indexed));
  44                         } else {
  45                             span_lint(cx, NEEDLESS_RANGE_LOOP, expr.span, &format!(
  46                                 "the loop variable `{}` is only used to index `{}`. \
  47                                  Consider using `for item in &{}` or similar iterators",
  48                                 ident.node.name, indexed, indexed));
  49                         }
  50                     }
  51                 }
  52             }
  53
  54             if let ExprMethodCall(ref method, _, ref args) = arg.node {
  55                 // just the receiver, no arguments
  56                 if args.len() == 1 {
  57                     let method_name = method.node.name;
  58                     // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
  59                     if method_name == "iter" {
  60                         let object = snippet(cx, args[0].span, "_");
  61                         span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!(
  62                             "it is more idiomatic to loop over `&{}` instead of `{}.iter()`",
  63                             object, object));
  64                     } else if method_name == "iter_mut" {
  65                         let object = snippet(cx, args[0].span, "_");
  66                         span_lint(cx, EXPLICIT_ITER_LOOP, expr.span, &format!(
  67                             "it is more idiomatic to loop over `&mut {}` instead of `{}.iter_mut()`",
  68                             object, object));
  69                     // check for looping over Iterator::next() which is not what you want
  70                     } else if method_name == "next" {
  71                         let method_call = ty::MethodCall::expr(arg.id);
  72                         let trt_id = cx.tcx.tables
  73                                            .borrow().method_map.get(&method_call)
  74                                            .and_then(|callee| cx.tcx.trait_of_item(callee.def_id));
  75                         if let Some(trt_id) = trt_id {
  76                             if match_def_path(cx, trt_id, &["core", "iter", "Iterator"]) {
  77                                 span_lint(cx, ITER_NEXT_LOOP, expr.span,
  78                                           "you are iterating over `Iterator::next()` which is an Option; \
  79                                            this will compile but is probably not what you want");
  80                             }
  81                         }
  82                     }
  83                 }
  84             }
  85         }
  86     }
  87 }
  88
  89 /// Recover the essential nodes of a desugared for loop:
  90 /// `for pat in arg { body }` becomes `(pat, arg, body)`.
  91 fn recover_for_loop(expr: &Expr) -> Option<(&Pat, &Expr, &Expr)> {
  92     if_let_chain! {
  93         [
  94             let ExprMatch(ref iterexpr, ref arms, _) = expr.node,
  95             let ExprCall(_, ref iterargs) = iterexpr.node,
  96             iterargs.len() == 1 && arms.len() == 1 && arms[0].guard.is_none(),
  97             let ExprLoop(ref block, _) = arms[0].body.node,
  98             block.stmts.is_empty(),
  99             let Some(ref loopexpr) = block.expr,
 100             let ExprMatch(_, ref innerarms, MatchSource::ForLoopDesugar) = loopexpr.node,
 101             innerarms.len() == 2 && innerarms[0].pats.len() == 1,
 102             let PatEnum(_, Some(ref somepats)) = innerarms[0].pats[0].node,
 103             somepats.len() == 1
 104         ], {
 105             return Some((&*somepats[0],
 106                          &*iterargs[0],
 107                          &*innerarms[0].body));
 108         }
 109     }
 110     None
 111 }
 112
 113 struct VarVisitor<'v, 't: 'v> {
 114     cx: &'v Context<'v, 't>, // context reference
 115     var: Name,               // var name to look for as index
 116     indexed: HashSet<Name>,  // indexed variables
 117     nonindex: bool,          // has the var been used otherwise?
 118 }
 119
 120 impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> {
 121     fn visit_expr(&mut self, expr: &'v Expr) {
 122         if let ExprPath(None, ref path) = expr.node {
 123             if path.segments.len() == 1 && path.segments[0].identifier.name == self.var {
 124                 // we are referencing our variable! now check if it's as an index
 125                 if_let_chain! {
 126                     [
 127                         let Some(parexpr) = get_parent_expr(self.cx, expr),
 128                         let ExprIndex(ref seqexpr, _) = parexpr.node,
 129                         let ExprPath(None, ref seqvar) = seqexpr.node,
 130                         seqvar.segments.len() == 1
 131                     ], {
 132                         self.indexed.insert(seqvar.segments[0].identifier.name);
 133                         return;  // no need to walk further
 134                     }
 135                 }
 136                 // we are not indexing anything, record that
 137                 self.nonindex = true;
 138                 return;
 139             }
 140         }
 141         walk_expr(self, expr);
 142     }
 143 }