clippy_lints/src/ranges.rs

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