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!(ITERATOR_STEP_BY_ZERO, RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, RANGE_MINUS_ONE)
  86     }
  87 }
  88
  89 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
  90     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
  91         if let ExprMethodCall(ref path, _, ref args) = expr.node {
  92             let name = path.name.as_str();
  93
  94             // Range with step_by(0).
  95             if name == "step_by" && args.len() == 2 && has_step_by(cx, &args[0]) {
  96                 use consts::{constant, Constant};
  97                 use rustc_const_math::ConstInt::Usize;
  98                 if let Some((Constant::Int(Usize(us)), _)) = constant(cx, &args[1]) {
  99                     if us.as_u64() == 0 {
 100                         span_lint(
 101                             cx,
 102                             ITERATOR_STEP_BY_ZERO,
 103                             expr.span,
 104                             "Iterator::step_by(0) will panic at runtime",
 105                         );
 106                     }
 107                 }
 108             } else if name == "zip" && args.len() == 2 {
 109                 let iter = &args[0].node;
 110                 let zip_arg = &args[1];
 111                 if_chain! {
 112                     // .iter() call
 113                     if let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter;
 114                     if iter_path.name == "iter";
 115                     // range expression in .zip() call: 0..x.len()
 116                     if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg);
 117                     if is_integer_literal(start, 0);
 118                     // .len() call
 119                     if let ExprMethodCall(ref len_path, _, ref len_args) = end.node;
 120                     if len_path.name == "len" && len_args.len() == 1;
 121                     // .iter() and .len() called on same Path
 122                     if let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node;
 123                     if let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node;
 124                     if iter_path.segments == len_path.segments;
 125                      then {
 126                          span_lint(cx,
 127                                    RANGE_ZIP_WITH_LEN,
 128                                    expr.span,
 129                                    &format!("It is more idiomatic to use {}.iter().enumerate()",
 130                                             snippet(cx, iter_args[0].span, "_")));
 131                     }
 132                 }
 133             }
 134         }
 135
 136         // exclusive range plus one: x..(y+1)
 137         if_chain! {
 138             if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr);
 139             if let Some(y) = y_plus_one(end);
 140             then {
 141                 span_lint_and_then(
 142                     cx,
 143                     RANGE_PLUS_ONE,
 144                     expr.span,
 145                     "an inclusive range would be more readable",
 146                     |db| {
 147                         let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 148                         let end = Sugg::hir(cx, y, "y");
 149                         db.span_suggestion(expr.span,
 150                                            "use",
 151                                            format!("{}..={}", start, end));
 152                     },
 153                 );
 154             }
 155         }
 156
 157         // inclusive range minus one: x..=(y-1)
 158         if_chain! {
 159             if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr);
 160             if let Some(y) = y_minus_one(end);
 161             then {
 162                 span_lint_and_then(
 163                     cx,
 164                     RANGE_MINUS_ONE,
 165                     expr.span,
 166                     "an exclusive range would be more readable",
 167                     |db| {
 168                         let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 169                         let end = Sugg::hir(cx, y, "y");
 170                         db.span_suggestion(expr.span,
 171                                            "use",
 172                                            format!("{}..{}", start, end));
 173                     },
 174                 );
 175             }
 176         }
 177     }
 178 }
 179
 180 fn has_step_by(cx: &LateContext, expr: &Expr) -> bool {
 181     // No need for walk_ptrs_ty here because step_by moves self, so it
 182     // can't be called on a borrowed range.
 183     let ty = cx.tables.expr_ty_adjusted(expr);
 184
 185     get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
 186 }
 187
 188 fn y_plus_one(expr: &Expr) -> Option<&Expr> {
 189     match expr.node {
 190         ExprBinary(Spanned { node: BiAdd, .. }, ref lhs, ref rhs) => if is_integer_literal(lhs, 1) {
 191             Some(rhs)
 192         } else if is_integer_literal(rhs, 1) {
 193             Some(lhs)
 194         } else {
 195             None
 196         },
 197         _ => None,
 198     }
 199 }
 200
 201 fn y_minus_one(expr: &Expr) -> Option<&Expr> {
 202     match expr.node {
 203         ExprBinary(Spanned { node: BiSub, .. }, ref lhs, ref rhs) if is_integer_literal(rhs, 1) => Some(lhs),
 204         _ => None,
 205     }
 206 }