clippy_lints/src/ranges.rs

   1 use if_chain::if_chain;
   2 use rustc::declare_lint_pass;
   3 use rustc::hir::*;
   4 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
   5 use rustc_errors::Applicability;
   6 use rustc_session::declare_tool_lint;
   7 use syntax::ast::RangeLimits;
   8 use syntax::source_map::Spanned;
   9
  10 use crate::utils::sugg::Sugg;
  11 use crate::utils::{higher, SpanlessEq};
  12 use crate::utils::{is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then};
  13
  14 declare_clippy_lint! {
  15     /// **What it does:** Checks for zipping a collection with the range of
  16     /// `0.._.len()`.
  17     ///
  18     /// **Why is this bad?** The code is better expressed with `.enumerate()`.
  19     ///
  20     /// **Known problems:** None.
  21     ///
  22     /// **Example:**
  23     /// ```rust
  24     /// # let x = vec![1];
  25     /// x.iter().zip(0..x.len());
  26     /// ```
  27     /// Could be written as
  28     /// ```rust
  29     /// # let x = vec![1];
  30     /// x.iter().enumerate();
  31     /// ```
  32     pub RANGE_ZIP_WITH_LEN,
  33     complexity,
  34     "zipping iterator with a range when `enumerate()` would do"
  35 }
  36
  37 declare_clippy_lint! {
  38     /// **What it does:** Checks for exclusive ranges where 1 is added to the
  39     /// upper bound, e.g., `x..(y+1)`.
  40     ///
  41     /// **Why is this bad?** The code is more readable with an inclusive range
  42     /// like `x..=y`.
  43     ///
  44     /// **Known problems:** Will add unnecessary pair of parentheses when the
  45     /// expression is not wrapped in a pair but starts with a opening parenthesis
  46     /// and ends with a closing one.
  47     /// I.e., `let _ = (f()+1)..(f()+1)` results in `let _ = ((f()+1)..=f())`.
  48     ///
  49     /// **Example:**
  50     /// ```rust,ignore
  51     /// for x..(y+1) { .. }
  52     /// ```
  53     /// Could be written as
  54     /// ```rust,ignore
  55     /// for x..=y { .. }
  56     /// ```
  57     pub RANGE_PLUS_ONE,
  58     complexity,
  59     "`x..(y+1)` reads better as `x..=y`"
  60 }
  61
  62 declare_clippy_lint! {
  63     /// **What it does:** Checks for inclusive ranges where 1 is subtracted from
  64     /// the upper bound, e.g., `x..=(y-1)`.
  65     ///
  66     /// **Why is this bad?** The code is more readable with an exclusive range
  67     /// like `x..y`.
  68     ///
  69     /// **Known problems:** None.
  70     ///
  71     /// **Example:**
  72     /// ```rust,ignore
  73     /// for x..=(y-1) { .. }
  74     /// ```
  75     /// Could be written as
  76     /// ```rust,ignore
  77     /// for x..y { .. }
  78     /// ```
  79     pub RANGE_MINUS_ONE,
  80     complexity,
  81     "`x..=(y-1)` reads better as `x..y`"
  82 }
  83
  84 declare_lint_pass!(Ranges => [
  85     RANGE_ZIP_WITH_LEN,
  86     RANGE_PLUS_ONE,
  87     RANGE_MINUS_ONE
  88 ]);
  89
  90 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges {
  91     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
  92         if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind {
  93             let name = path.ident.as_str();
  94             if name == "zip" && args.len() == 2 {
  95                 let iter = &args[0].kind;
  96                 let zip_arg = &args[1];
  97                 if_chain! {
  98                     // `.iter()` call
  99                     if let ExprKind::MethodCall(ref iter_path, _, ref iter_args ) = *iter;
 100                     if iter_path.ident.name == sym!(iter);
 101                     // range expression in `.zip()` call: `0..x.len()`
 102                     if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(cx, zip_arg);
 103                     if is_integer_const(cx, start, 0);
 104                     // `.len()` call
 105                     if let ExprKind::MethodCall(ref len_path, _, ref len_args) = end.kind;
 106                     if len_path.ident.name == sym!(len) && len_args.len() == 1;
 107                     // `.iter()` and `.len()` called on same `Path`
 108                     if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind;
 109                     if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind;
 110                     if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments);
 111                      then {
 112                          span_lint(cx,
 113                                    RANGE_ZIP_WITH_LEN,
 114                                    expr.span,
 115                                    &format!("It is more idiomatic to use {}.iter().enumerate()",
 116                                             snippet(cx, iter_args[0].span, "_")));
 117                     }
 118                 }
 119             }
 120         }
 121
 122         check_exclusive_range_plus_one(cx, expr);
 123         check_inclusive_range_minus_one(cx, expr);
 124     }
 125 }
 126
 127 // exclusive range plus one: `x..(y+1)`
 128 fn check_exclusive_range_plus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
 129     if_chain! {
 130         if let Some(higher::Range {
 131             start,
 132             end: Some(end),
 133             limits: RangeLimits::HalfOpen
 134         }) = higher::range(cx, expr);
 135         if let Some(y) = y_plus_one(cx, end);
 136         then {
 137             let span = if expr.span.from_expansion() {
 138                 expr.span
 139                     .ctxt()
 140                     .outer_expn_data()
 141                     .call_site
 142             } else {
 143                 expr.span
 144             };
 145             span_lint_and_then(
 146                 cx,
 147                 RANGE_PLUS_ONE,
 148                 span,
 149                 "an inclusive range would be more readable",
 150                 |db| {
 151                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 152                     let end = Sugg::hir(cx, y, "y");
 153                     if let Some(is_wrapped) = &snippet_opt(cx, span) {
 154                         if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
 155                             db.span_suggestion(
 156                                 span,
 157                                 "use",
 158                                 format!("({}..={})", start, end),
 159                                 Applicability::MaybeIncorrect,
 160                             );
 161                         } else {
 162                             db.span_suggestion(
 163                                 span,
 164                                 "use",
 165                                 format!("{}..={}", start, end),
 166                                 Applicability::MachineApplicable, // snippet
 167                             );
 168                         }
 169                     }
 170                 },
 171             );
 172         }
 173     }
 174 }
 175
 176 // inclusive range minus one: `x..=(y-1)`
 177 fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr) {
 178     if_chain! {
 179         if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(cx, expr);
 180         if let Some(y) = y_minus_one(cx, end);
 181         then {
 182             span_lint_and_then(
 183                 cx,
 184                 RANGE_MINUS_ONE,
 185                 expr.span,
 186                 "an exclusive range would be more readable",
 187                 |db| {
 188                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 189                     let end = Sugg::hir(cx, y, "y");
 190                     db.span_suggestion(
 191                         expr.span,
 192                         "use",
 193                         format!("{}..{}", start, end),
 194                         Applicability::MachineApplicable, // snippet
 195                     );
 196                 },
 197             );
 198         }
 199     }
 200 }
 201
 202 fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
 203     match expr.kind {
 204         ExprKind::Binary(
 205             Spanned {
 206                 node: BinOpKind::Add, ..
 207             },
 208             ref lhs,
 209             ref rhs,
 210         ) => {
 211             if is_integer_const(cx, lhs, 1) {
 212                 Some(rhs)
 213             } else if is_integer_const(cx, rhs, 1) {
 214                 Some(lhs)
 215             } else {
 216                 None
 217             }
 218         },
 219         _ => None,
 220     }
 221 }
 222
 223 fn y_minus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr) -> Option<&'t Expr> {
 224     match expr.kind {
 225         ExprKind::Binary(
 226             Spanned {
 227                 node: BinOpKind::Sub, ..
 228             },
 229             ref lhs,
 230             ref rhs,
 231         ) if is_integer_const(cx, rhs, 1) => Some(lhs),
 232         _ => None,
 233     }
 234 }