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