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_chain! {
 117                     // .iter() call
 118                     if let ExprMethodCall(ref iter_path, _, ref iter_args ) = *iter;
 119                     if iter_path.name == "iter";
 120                     // range expression in .zip() call: 0..x.len()
 121                     if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg);
 122                     if is_integer_literal(start, 0);
 123                     // .len() call
 124                     if let ExprMethodCall(ref len_path, _, ref len_args) = end.node;
 125                     if len_path.name == "len" && len_args.len() == 1;
 126                     // .iter() and .len() called on same Path
 127                     if let ExprPath(QPath::Resolved(_, ref iter_path)) = iter_args[0].node;
 128                     if let ExprPath(QPath::Resolved(_, ref len_path)) = len_args[0].node;
 129                     if iter_path.segments == len_path.segments;
 130                      then {
 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
 141         // exclusive range plus one: x..(y+1)
 142         if_chain! {
 143             if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::HalfOpen }) = higher::range(expr);
 144             if let Some(y) = y_plus_one(end);
 145             then {
 146                 span_lint_and_then(
 147                     cx,
 148                     RANGE_PLUS_ONE,
 149                     expr.span,
 150                     "an inclusive range would be more readable",
 151                     |db| {
 152                         let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 153                         let end = Sugg::hir(cx, y, "y");
 154                         db.span_suggestion(expr.span,
 155                                            "use",
 156                                            format!("{}..={}", start, end));
 157                     },
 158                 );
 159             }
 160         }
 161
 162         // inclusive range minus one: x..=(y-1)
 163         if_chain! {
 164             if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr);
 165             if let Some(y) = y_minus_one(end);
 166             then {
 167                 span_lint_and_then(
 168                     cx,
 169                     RANGE_MINUS_ONE,
 170                     expr.span,
 171                     "an exclusive range would be more readable",
 172                     |db| {
 173                         let start = start.map_or("".to_owned(), |x| Sugg::hir(cx, x, "x").to_string());
 174                         let end = Sugg::hir(cx, y, "y");
 175                         db.span_suggestion(expr.span,
 176                                            "use",
 177                                            format!("{}..{}", start, end));
 178                     },
 179                 );
 180             }
 181         }
 182     }
 183 }
 184
 185 fn has_step_by(cx: &LateContext, expr: &Expr) -> bool {
 186     // No need for walk_ptrs_ty here because step_by moves self, so it
 187     // can't be called on a borrowed range.
 188     let ty = cx.tables.expr_ty_adjusted(expr);
 189
 190     get_trait_def_id(cx, &paths::ITERATOR).map_or(false, |iterator_trait| implements_trait(cx, ty, iterator_trait, &[]))
 191 }
 192
 193 fn y_plus_one(expr: &Expr) -> Option<&Expr> {
 194     match expr.node {
 195         ExprBinary(Spanned { node: BiAdd, .. }, ref lhs, ref rhs) => {
 196             if is_integer_literal(lhs, 1) {
 197                 Some(rhs)
 198             } else if is_integer_literal(rhs, 1) {
 199                 Some(lhs)
 200             } else {
 201                 None
 202             }
 203         },
 204         _ => None,
 205     }
 206 }
 207
 208 fn y_minus_one(expr: &Expr) -> Option<&Expr> {
 209     match expr.node {
 210         ExprBinary(Spanned { node: BiSub, .. }, ref lhs, ref rhs) if is_integer_literal(rhs, 1) => Some(lhs),
 211         _ => None,
 212     }
 213 }