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     /// **Example:**
  49     /// ```rust,ignore
  50     /// for x..(y+1) { .. }
  51     /// ```
  52     /// Could be written as
  53     /// ```rust,ignore
  54     /// for x..=y { .. }
  55     /// ```
  56     pub RANGE_PLUS_ONE,
  57     complexity,
  58     "`x..(y+1)` reads better as `x..=y`"
  59 }
  60
  61 declare_clippy_lint! {
  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,ignore
  72     /// for x..=(y-1) { .. }
  73     /// ```
  74     /// Could be written as
  75     /// ```rust,ignore
  76     /// for x..y { .. }
  77     /// ```
  78     pub RANGE_MINUS_ONE,
  79     complexity,
  80     "`x..=(y-1)` reads better as `x..y`"
  81 }
  82
  83 declare_lint_pass!(Ranges => [
  84     RANGE_ZIP_WITH_LEN,
  85     RANGE_PLUS_ONE,
  86     RANGE_MINUS_ONE
  87 ]);
  88
  89 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges {
  90     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
  91         if let ExprKind::MethodCall(ref path, _, ref args) = expr.kind {
  92             let name = path.ident.as_str();
  93             if name == "zip" && args.len() == 2 {
  94                 let iter = &args[0].kind;
  95                 let zip_arg = &args[1];
  96                 if_chain! {
  97                     // `.iter()` call
  98                     if let ExprKind::MethodCall(ref iter_path, _, ref iter_args ) = *iter;
  99                     if iter_path.ident.name == sym!(iter);
 100                     // range expression in `.zip()` call: `0..x.len()`
 101                     if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(cx, zip_arg);
 102                     if is_integer_const(cx, start, 0);
 103                     // `.len()` call
 104                     if let ExprKind::MethodCall(ref len_path, _, ref len_args) = end.kind;
 105                     if len_path.ident.name == sym!(len) && len_args.len() == 1;
 106                     // `.iter()` and `.len()` called on same `Path`
 107                     if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind;
 108                     if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind;
 109                     if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments);
 110                      then {
 111                          span_lint(cx,
 112                                    RANGE_ZIP_WITH_LEN,
 113                                    expr.span,
 114                                    &format!("It is more idiomatic to use `{}.iter().enumerate()`",
 115                                             snippet(cx, iter_args[0].span, "_")));
 116                     }
 117                 }
 118             }
 119         }
 120
 121         check_exclusive_range_plus_one(cx, expr);
 122         check_inclusive_range_minus_one(cx, expr);
 123     }
 124 }
 125
 126 // exclusive range plus one: `x..(y+1)`
 127 fn check_exclusive_range_plus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
 128     if_chain! {
 129         if let Some(higher::Range {
 130             start,
 131             end: Some(end),
 132             limits: RangeLimits::HalfOpen
 133         }) = higher::range(cx, expr);
 134         if let Some(y) = y_plus_one(cx, end);
 135         then {
 136             let span = if expr.span.from_expansion() {
 137                 expr.span
 138                     .ctxt()
 139                     .outer_expn_data()
 140                     .call_site
 141             } else {
 142                 expr.span
 143             };
 144             span_lint_and_then(
 145                 cx,
 146                 RANGE_PLUS_ONE,
 147                 span,
 148                 "an inclusive range would be more readable",
 149                 |db| {
 150                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 151                     let end = Sugg::hir(cx, y, "y");
 152                     if let Some(is_wrapped) = &snippet_opt(cx, span) {
 153                         if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
 154                             db.span_suggestion(
 155                                 span,
 156                                 "use",
 157                                 format!("({}..={})", start, end),
 158                                 Applicability::MaybeIncorrect,
 159                             );
 160                         } else {
 161                             db.span_suggestion(
 162                                 span,
 163                                 "use",
 164                                 format!("{}..={}", start, end),
 165                                 Applicability::MachineApplicable, // snippet
 166                             );
 167                         }
 168                     }
 169                 },
 170             );
 171         }
 172     }
 173 }
 174
 175 // inclusive range minus one: `x..=(y-1)`
 176 fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
 177     if_chain! {
 178         if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(cx, expr);
 179         if let Some(y) = y_minus_one(cx, end);
 180         then {
 181             span_lint_and_then(
 182                 cx,
 183                 RANGE_MINUS_ONE,
 184                 expr.span,
 185                 "an exclusive range would be more readable",
 186                 |db| {
 187                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 188                     let end = Sugg::hir(cx, y, "y");
 189                     db.span_suggestion(
 190                         expr.span,
 191                         "use",
 192                         format!("{}..{}", start, end),
 193                         Applicability::MachineApplicable, // snippet
 194                     );
 195                 },
 196             );
 197         }
 198     }
 199 }
 200
 201 fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
 202     match expr.kind {
 203         ExprKind::Binary(
 204             Spanned {
 205                 node: BinOpKind::Add, ..
 206             },
 207             ref lhs,
 208             ref rhs,
 209         ) => {
 210             if is_integer_const(cx, lhs, 1) {
 211                 Some(rhs)
 212             } else if is_integer_const(cx, rhs, 1) {
 213                 Some(lhs)
 214             } else {
 215                 None
 216             }
 217         },
 218         _ => None,
 219     }
 220 }
 221
 222 fn y_minus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
 223     match expr.kind {
 224         ExprKind::Binary(
 225             Spanned {
 226                 node: BinOpKind::Sub, ..
 227             },
 228             ref lhs,
 229             ref rhs,
 230         ) if is_integer_const(cx, rhs, 1) => Some(lhs),
 231         _ => None,
 232     }
 233 }