clippy_lints/src/ranges.rs

   1 use rustc::lint::*;
   2 use rustc::hir::*;
   3 use syntax::ast::RangeLimits;
   4 use syntax::codemap::Spanned;
   5 use utils::{is_integer_literal, paths, snippet, span_lint, span_lint_and_then};
   6 use utils::{get_trait_def_id, higher, implements_trait};
   7 use utils::sugg::Sugg;
   8
   9 /// **What it does:** Checks for calling `.step_by(0)` on iterators,
  10 /// which never terminates.
  11 ///
  12 /// **Why is this bad?** This very much looks like an oversight, since with
  13 /// `loop { .. }` there is an obvious better way to endlessly loop.
  14 ///
  15 /// **Known problems:** None.
  16 ///
  17 /// **Example:**
  18 /// ```rust
  19 /// for x in (5..5).step_by(0) { .. }
  20 /// ```
  21 declare_lint! {
  22     pub ITERATOR_STEP_BY_ZERO,
  23     Warn,
  24     "using `Iterator::step_by(0)`, which produces an infinite iterator"
  25 }
  26
  27 /// **What it does:** Checks for zipping a collection with the range of
  28 /// `0.._.len()`.
  29 ///
  30 /// **Why is this bad?** The code is better expressed with `.enumerate()`.
  31 ///
  32 /// **Known problems:** None.
  33 ///
  34 /// **Example:**
  35 /// ```rust
  36 /// x.iter().zip(0..x.len())
  37 /// ```
  38 declare_lint! {
  39     pub RANGE_ZIP_WITH_LEN,
  40     Warn,
  41     "zipping iterator with a range when `enumerate()` would do"
  42 }
  43
  44 /// **What it does:** Checks for exclusive ranges where 1 is added to the
  45 /// upper bound, e.g. `x..(y+1)`.
  46 ///
  47 /// **Why is this bad?** The code is more readable with an inclusive range
  48 /// like `x..=y`.
  49 ///
  50 /// **Known problems:** None.
  51 ///
  52 /// **Example:**
  53 /// ```rust
  54 /// for x..(y+1) { .. }
  55 /// ```
  56 declare_lint! {
  57     pub RANGE_PLUS_ONE,
  58     Allow,
  59     "`x..(y+1)` reads better as `x..=y`"
  60 }
  61
  62 /// **What it does:** Checks for inclusive ranges where 1 is subtracted from
  63 /// the upper bound, e.g. `x..=(y-1)`.
  64 ///
  65 /// **Why is this bad?** The code is more readable with an exclusive range
  66 /// like `x..y`.
  67 ///
  68 /// **Known problems:** None.
  69 ///
  70 /// **Example:**
  71 /// ```rust
  72 /// for x..=(y-1) { .. }
  73 /// ```
  74 declare_lint! {
  75     pub RANGE_MINUS_ONE,
  76     Warn,
  77     "`x..=(y-1)` reads better as `x..y`"
  78 }
  79
  80 #[derive(Copy, Clone)]
  81 pub struct Pass;
  82
  83 impl LintPass for Pass {
  84     fn get_lints(&self) -> LintArray {
  85         lint_array!(
  86             ITERATOR_STEP_BY_ZERO,
  87             RANGE_ZIP_WITH_LEN,
  88             RANGE_PLUS_ONE,
  89             RANGE_MINUS_ONE
  90         )
  91     }
  92 }
  93
  94 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
  95     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
  96         if let ExprMethodCall(ref path, _, ref args) = expr.node {
  97             let name = path.name.as_str();
  98
  99             // Range with step_by(0).
 100             if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
 101                 use consts::{constant, Constant};
 102                 use rustc_const_math::ConstInt::Usize;
 103                 if let Some((Constant::Int(Usize(us)), _)) = constant(cx, &args[1]) {
 104                     if us.as_u64() == 0 {
 105                         span_lint(
 106                             cx,
 107                             ITERATOR_STEP_BY_ZERO,
 108                             expr.span,
 109                             "Iterator::step_by(0) will panic at runtime",
 110                         );
 111                     }
 112                 }
 113             } else if name == "zip" && args.len() == 2 {
 114                 let iter = &args[0].node;
 115                 let zip_arg = &args[1];
 116                 if_let_chain! {[
 117                     // .iter() call
 118                     let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter,
 119                     iter_path.name == "iter",
 120                     // range expression in .zip() call: 0..x.len()
 121                     let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg),
 122                     is_integer_literal(start, 0),
 123                     // .len() call
 124                     let ExprMethodCall(ref len_path, _, ref len_args) = end.node,
 125                     len_path.name == "len" && len_args.len() == 1,
 126                     // .iter() and .len() called on same Path
 127                     let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node,
 128                     let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node,
 129                     iter_path.segments == len_path.segments
 130                  ], {
 131                      span_lint(cx,
 132                                RANGE_ZIP_WITH_LEN,
 133                                expr.span,
 134                                &format!("It is more idiomatic to use {}.iter().enumerate()",
 135                                         snippet(cx, iter_args[0].span, "_")));
 136                 }}
 137             }
 138         }
 139
 140         // exclusive range plus one: x..(y+1)
 141         if_let_chain! {[
 142             let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr),
 143             let Some(y) = y_plus_one(end),
 144         ], {
 145             span_lint_and_then(
 146                 cx,
 147                 RANGE_PLUS_ONE,
 148                 expr.span,
 149                 "an inclusive range would be more readable",
 150                 |db| {
 151                     let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 152                     let end = Sugg::hir(cx, y, "y");
 153                     db.span_suggestion(expr.span,
 154                                        "use",
 155                                        format!("{}..={}", start, end));
 156                 },
 157             );
 158         }}
 159
 160         // inclusive range minus one: x..=(y-1)
 161         if_let_chain! {[
 162             let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr),
 163             let Some(y) = y_minus_one(end),
 164         ], {
 165             span_lint_and_then(
 166                 cx,
 167                 RANGE_MINUS_ONE,
 168                 expr.span,
 169                 "an exclusive range would be more readable",
 170                 |db| {
 171                     let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 172                     let end = Sugg::hir(cx, y, "y");
 173                     db.span_suggestion(expr.span,
 174                                        "use",
 175                                        format!("{}..{}", start, end));
 176                 },
 177             );
 178         }}
 179     }
 180 }
 181
 182 fn has_step_by(cx: &LateContext, expr: &Expr) -> bool {
 183     // No need for walk_ptrs_ty here because step_by moves self, so it
 184     // can't be called on a borrowed range.
 185     let ty = cx.tables.expr_ty_adjusted(expr);
 186
 187     get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
 188 }
 189
 190 fn y_plus_one(expr: &Expr) -> Option<&Expr> {
 191     match expr.node {
 192         ExprBinary(Spanned { node: BiAdd, .. }, ref lhs, ref rhs) => {
 193             if is_integer_literal(lhs, 1) {
 194                 Some(rhs)
 195             } else if is_integer_literal(rhs, 1) {
 196                 Some(lhs)
 197             } else {
 198                 None
 199             }
 200         },
 201         _ => None,
 202     }
 203 }
 204
 205 fn y_minus_one(expr: &Expr) -> Option<&Expr> {
 206     match expr.node {
 207         ExprBinary(Spanned { node: BiSub, .. }, ref lhs, ref rhs) if is_integer_literal(rhs, 1) => Some(lhs),
 208         _ => None,
 209     }
 210 }