src/tools/clippy/clippy_lints/src/ranges.rs

   1 use crate::consts::{constant, Constant};
   2 use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
   3 use clippy_utils::source::{snippet, snippet_opt, snippet_with_applicability};
   4 use clippy_utils::sugg::Sugg;
   5 use clippy_utils::{get_parent_expr, in_constant, is_integer_const, meets_msrv, single_segment_path};
   6 use clippy_utils::{higher, SpanlessEq};
   7 use if_chain::if_chain;
   8 use rustc_ast::ast::RangeLimits;
   9 use rustc_errors::Applicability;
  10 use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, QPath};
  11 use rustc_lint::{LateContext, LateLintPass, LintContext};
  12 use rustc_middle::ty;
  13 use rustc_semver::RustcVersion;
  14 use rustc_session::{declare_tool_lint, impl_lint_pass};
  15 use rustc_span::source_map::{Span, Spanned};
  16 use rustc_span::sym;
  17 use rustc_span::symbol::Ident;
  18 use std::cmp::Ordering;
  19
  20 declare_clippy_lint! {
  21     /// **What it does:** Checks for zipping a collection with the range of
  22     /// `0.._.len()`.
  23     ///
  24     /// **Why is this bad?** The code is better expressed with `.enumerate()`.
  25     ///
  26     /// **Known problems:** None.
  27     ///
  28     /// **Example:**
  29     /// ```rust
  30     /// # let x = vec![1];
  31     /// x.iter().zip(0..x.len());
  32     /// ```
  33     /// Could be written as
  34     /// ```rust
  35     /// # let x = vec![1];
  36     /// x.iter().enumerate();
  37     /// ```
  38     pub RANGE_ZIP_WITH_LEN,
  39     complexity,
  40     "zipping iterator with a range when `enumerate()` would do"
  41 }
  42
  43 declare_clippy_lint! {
  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:** Will add unnecessary pair of parentheses when the
  51     /// expression is not wrapped in a pair but starts with a opening parenthesis
  52     /// and ends with a closing one.
  53     /// I.e., `let _ = (f()+1)..(f()+1)` results in `let _ = ((f()+1)..=f())`.
  54     ///
  55     /// Also in many cases, inclusive ranges are still slower to run than
  56     /// exclusive ranges, because they essentially add an extra branch that
  57     /// LLVM may fail to hoist out of the loop.
  58     ///
  59     /// This will cause a warning that cannot be fixed if the consumer of the
  60     /// range only accepts a specific range type, instead of the generic
  61     /// `RangeBounds` trait
  62     /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
  63     ///
  64     /// **Example:**
  65     /// ```rust,ignore
  66     /// for x..(y+1) { .. }
  67     /// ```
  68     /// Could be written as
  69     /// ```rust,ignore
  70     /// for x..=y { .. }
  71     /// ```
  72     pub RANGE_PLUS_ONE,
  73     pedantic,
  74     "`x..(y+1)` reads better as `x..=y`"
  75 }
  76
  77 declare_clippy_lint! {
  78     /// **What it does:** Checks for inclusive ranges where 1 is subtracted from
  79     /// the upper bound, e.g., `x..=(y-1)`.
  80     ///
  81     /// **Why is this bad?** The code is more readable with an exclusive range
  82     /// like `x..y`.
  83     ///
  84     /// **Known problems:** This will cause a warning that cannot be fixed if
  85     /// the consumer of the range only accepts a specific range type, instead of
  86     /// the generic `RangeBounds` trait
  87     /// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
  88     ///
  89     /// **Example:**
  90     /// ```rust,ignore
  91     /// for x..=(y-1) { .. }
  92     /// ```
  93     /// Could be written as
  94     /// ```rust,ignore
  95     /// for x..y { .. }
  96     /// ```
  97     pub RANGE_MINUS_ONE,
  98     pedantic,
  99     "`x..=(y-1)` reads better as `x..y`"
 100 }
 101
 102 declare_clippy_lint! {
 103     /// **What it does:** Checks for range expressions `x..y` where both `x` and `y`
 104     /// are constant and `x` is greater or equal to `y`.
 105     ///
 106     /// **Why is this bad?** Empty ranges yield no values so iterating them is a no-op.
 107     /// Moreover, trying to use a reversed range to index a slice will panic at run-time.
 108     ///
 109     /// **Known problems:** None.
 110     ///
 111     /// **Example:**
 112     ///
 113     /// ```rust,no_run
 114     /// fn main() {
 115     ///     (10..=0).for_each(|x| println!("{}", x));
 116     ///
 117     ///     let arr = [1, 2, 3, 4, 5];
 118     ///     let sub = &arr[3..1];
 119     /// }
 120     /// ```
 121     /// Use instead:
 122     /// ```rust
 123     /// fn main() {
 124     ///     (0..=10).rev().for_each(|x| println!("{}", x));
 125     ///
 126     ///     let arr = [1, 2, 3, 4, 5];
 127     ///     let sub = &arr[1..3];
 128     /// }
 129     /// ```
 130     pub REVERSED_EMPTY_RANGES,
 131     correctness,
 132     "reversing the limits of range expressions, resulting in empty ranges"
 133 }
 134
 135 declare_clippy_lint! {
 136     /// **What it does:** Checks for expressions like `x >= 3 && x < 8` that could
 137     /// be more readably expressed as `(3..8).contains(x)`.
 138     ///
 139     /// **Why is this bad?** `contains` expresses the intent better and has less
 140     /// failure modes (such as fencepost errors or using `||` instead of `&&`).
 141     ///
 142     /// **Known problems:** None.
 143     ///
 144     /// **Example:**
 145     ///
 146     /// ```rust
 147     /// // given
 148     /// let x = 6;
 149     ///
 150     /// assert!(x >= 3 && x < 8);
 151     /// ```
 152     /// Use instead:
 153     /// ```rust
 154     ///# let x = 6;
 155     /// assert!((3..8).contains(&x));
 156     /// ```
 157     pub MANUAL_RANGE_CONTAINS,
 158     style,
 159     "manually reimplementing {`Range`, `RangeInclusive`}`::contains`"
 160 }
 161
 162 const MANUAL_RANGE_CONTAINS_MSRV: RustcVersion = RustcVersion::new(1, 35, 0);
 163
 164 pub struct Ranges {
 165     msrv: Option<RustcVersion>,
 166 }
 167
 168 impl Ranges {
 169     #[must_use]
 170     pub fn new(msrv: Option<RustcVersion>) -> Self {
 171         Self { msrv }
 172     }
 173 }
 174
 175 impl_lint_pass!(Ranges => [
 176     RANGE_ZIP_WITH_LEN,
 177     RANGE_PLUS_ONE,
 178     RANGE_MINUS_ONE,
 179     REVERSED_EMPTY_RANGES,
 180     MANUAL_RANGE_CONTAINS,
 181 ]);
 182
 183 impl<'tcx> LateLintPass<'tcx> for Ranges {
 184     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 185         match expr.kind {
 186             ExprKind::MethodCall(ref path, _, ref args, _) => {
 187                 check_range_zip_with_len(cx, path, args, expr.span);
 188             },
 189             ExprKind::Binary(ref op, ref l, ref r) => {
 190                 if meets_msrv(self.msrv.as_ref(), &MANUAL_RANGE_CONTAINS_MSRV) {
 191                     check_possible_range_contains(cx, op.node, l, r, expr);
 192                 }
 193             },
 194             _ => {},
 195         }
 196
 197         check_exclusive_range_plus_one(cx, expr);
 198         check_inclusive_range_minus_one(cx, expr);
 199         check_reversed_empty_range(cx, expr);
 200     }
 201     extract_msrv_attr!(LateContext);
 202 }
 203
 204 fn check_possible_range_contains(cx: &LateContext<'_>, op: BinOpKind, l: &Expr<'_>, r: &Expr<'_>, expr: &Expr<'_>) {
 205     if in_constant(cx, expr.hir_id) {
 206         return;
 207     }
 208
 209     let span = expr.span;
 210     let combine_and = match op {
 211         BinOpKind::And | BinOpKind::BitAnd => true,
 212         BinOpKind::Or | BinOpKind::BitOr => false,
 213         _ => return,
 214     };
 215     // value, name, order (higher/lower), inclusiveness
 216     if let (Some((lval, lname, name_span, lval_span, lord, linc)), Some((rval, rname, _, rval_span, rord, rinc))) =
 217         (check_range_bounds(cx, l), check_range_bounds(cx, r))
 218     {
 219         // we only lint comparisons on the same name and with different
 220         // direction
 221         if lname != rname || lord == rord {
 222             return;
 223         }
 224         let ord = Constant::partial_cmp(cx.tcx, cx.typeck_results().expr_ty(l), &lval, &rval);
 225         if combine_and && ord == Some(rord) {
 226             // order lower bound and upper bound
 227             let (l_span, u_span, l_inc, u_inc) = if rord == Ordering::Less {
 228                 (lval_span, rval_span, linc, rinc)
 229             } else {
 230                 (rval_span, lval_span, rinc, linc)
 231             };
 232             // we only lint inclusive lower bounds
 233             if !l_inc {
 234                 return;
 235             }
 236             let (range_type, range_op) = if u_inc {
 237                 ("RangeInclusive", "..=")
 238             } else {
 239                 ("Range", "..")
 240             };
 241             let mut applicability = Applicability::MachineApplicable;
 242             let name = snippet_with_applicability(cx, name_span, "_", &mut applicability);
 243             let lo = snippet_with_applicability(cx, l_span, "_", &mut applicability);
 244             let hi = snippet_with_applicability(cx, u_span, "_", &mut applicability);
 245             let space = if lo.ends_with('.') { " " } else { "" };
 246             span_lint_and_sugg(
 247                 cx,
 248                 MANUAL_RANGE_CONTAINS,
 249                 span,
 250                 &format!("manual `{}::contains` implementation", range_type),
 251                 "use",
 252                 format!("({}{}{}{}).contains(&{})", lo, space, range_op, hi, name),
 253                 applicability,
 254             );
 255         } else if !combine_and && ord == Some(lord) {
 256             // `!_.contains(_)`
 257             // order lower bound and upper bound
 258             let (l_span, u_span, l_inc, u_inc) = if lord == Ordering::Less {
 259                 (lval_span, rval_span, linc, rinc)
 260             } else {
 261                 (rval_span, lval_span, rinc, linc)
 262             };
 263             if l_inc {
 264                 return;
 265             }
 266             let (range_type, range_op) = if u_inc {
 267                 ("Range", "..")
 268             } else {
 269                 ("RangeInclusive", "..=")
 270             };
 271             let mut applicability = Applicability::MachineApplicable;
 272             let name = snippet_with_applicability(cx, name_span, "_", &mut applicability);
 273             let lo = snippet_with_applicability(cx, l_span, "_", &mut applicability);
 274             let hi = snippet_with_applicability(cx, u_span, "_", &mut applicability);
 275             let space = if lo.ends_with('.') { " " } else { "" };
 276             span_lint_and_sugg(
 277                 cx,
 278                 MANUAL_RANGE_CONTAINS,
 279                 span,
 280                 &format!("manual `!{}::contains` implementation", range_type),
 281                 "use",
 282                 format!("!({}{}{}{}).contains(&{})", lo, space, range_op, hi, name),
 283                 applicability,
 284             );
 285         }
 286     }
 287 }
 288
 289 fn check_range_bounds(cx: &LateContext<'_>, ex: &Expr<'_>) -> Option<(Constant, Ident, Span, Span, Ordering, bool)> {
 290     if let ExprKind::Binary(ref op, ref l, ref r) = ex.kind {
 291         let (inclusive, ordering) = match op.node {
 292             BinOpKind::Gt => (false, Ordering::Greater),
 293             BinOpKind::Ge => (true, Ordering::Greater),
 294             BinOpKind::Lt => (false, Ordering::Less),
 295             BinOpKind::Le => (true, Ordering::Less),
 296             _ => return None,
 297         };
 298         if let Some(id) = match_ident(l) {
 299             if let Some((c, _)) = constant(cx, cx.typeck_results(), r) {
 300                 return Some((c, id, l.span, r.span, ordering, inclusive));
 301             }
 302         } else if let Some(id) = match_ident(r) {
 303             if let Some((c, _)) = constant(cx, cx.typeck_results(), l) {
 304                 return Some((c, id, r.span, l.span, ordering.reverse(), inclusive));
 305             }
 306         }
 307     }
 308     None
 309 }
 310
 311 fn match_ident(e: &Expr<'_>) -> Option<Ident> {
 312     if let ExprKind::Path(ref qpath) = e.kind {
 313         if let Some(seg) = single_segment_path(qpath) {
 314             if seg.args.is_none() {
 315                 return Some(seg.ident);
 316             }
 317         }
 318     }
 319     None
 320 }
 321
 322 fn check_range_zip_with_len(cx: &LateContext<'_>, path: &PathSegment<'_>, args: &[Expr<'_>], span: Span) {
 323     let name = path.ident.as_str();
 324     if name == "zip" && args.len() == 2 {
 325         let iter = &args[0].kind;
 326         let zip_arg = &args[1];
 327         if_chain! {
 328             // `.iter()` call
 329             if let ExprKind::MethodCall(ref iter_path, _, ref iter_args, _) = *iter;
 330             if iter_path.ident.name == sym::iter;
 331             // range expression in `.zip()` call: `0..x.len()`
 332             if let Some(higher::Range { start: Some(start), end: Some(end), .. }) = higher::range(zip_arg);
 333             if is_integer_const(cx, start, 0);
 334             // `.len()` call
 335             if let ExprKind::MethodCall(ref len_path, _, ref len_args, _) = end.kind;
 336             if len_path.ident.name == sym!(len) && len_args.len() == 1;
 337             // `.iter()` and `.len()` called on same `Path`
 338             if let ExprKind::Path(QPath::Resolved(_, ref iter_path)) = iter_args[0].kind;
 339             if let ExprKind::Path(QPath::Resolved(_, ref len_path)) = len_args[0].kind;
 340             if SpanlessEq::new(cx).eq_path_segments(&iter_path.segments, &len_path.segments);
 341             then {
 342                 span_lint(cx,
 343                     RANGE_ZIP_WITH_LEN,
 344                     span,
 345                     &format!("it is more idiomatic to use `{}.iter().enumerate()`",
 346                         snippet(cx, iter_args[0].span, "_"))
 347                 );
 348             }
 349         }
 350     }
 351 }
 352
 353 // exclusive range plus one: `x..(y+1)`
 354 fn check_exclusive_range_plus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
 355     if_chain! {
 356         if let Some(higher::Range {
 357             start,
 358             end: Some(end),
 359             limits: RangeLimits::HalfOpen
 360         }) = higher::range(expr);
 361         if let Some(y) = y_plus_one(cx, end);
 362         then {
 363             let span = if expr.span.from_expansion() {
 364                 expr.span
 365                     .ctxt()
 366                     .outer_expn_data()
 367                     .call_site
 368             } else {
 369                 expr.span
 370             };
 371             span_lint_and_then(
 372                 cx,
 373                 RANGE_PLUS_ONE,
 374                 span,
 375                 "an inclusive range would be more readable",
 376                 |diag| {
 377                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 378                     let end = Sugg::hir(cx, y, "y");
 379                     if let Some(is_wrapped) = &snippet_opt(cx, span) {
 380                         if is_wrapped.starts_with('(') && is_wrapped.ends_with(')') {
 381                             diag.span_suggestion(
 382                                 span,
 383                                 "use",
 384                                 format!("({}..={})", start, end),
 385                                 Applicability::MaybeIncorrect,
 386                             );
 387                         } else {
 388                             diag.span_suggestion(
 389                                 span,
 390                                 "use",
 391                                 format!("{}..={}", start, end),
 392                                 Applicability::MachineApplicable, // snippet
 393                             );
 394                         }
 395                     }
 396                 },
 397             );
 398         }
 399     }
 400 }
 401
 402 // inclusive range minus one: `x..=(y-1)`
 403 fn check_inclusive_range_minus_one(cx: &LateContext<'_>, expr: &Expr<'_>) {
 404     if_chain! {
 405         if let Some(higher::Range { start, end: Some(end), limits: RangeLimits::Closed }) = higher::range(expr);
 406         if let Some(y) = y_minus_one(cx, end);
 407         then {
 408             span_lint_and_then(
 409                 cx,
 410                 RANGE_MINUS_ONE,
 411                 expr.span,
 412                 "an exclusive range would be more readable",
 413                 |diag| {
 414                     let start = start.map_or(String::new(), |x| Sugg::hir(cx, x, "x").to_string());
 415                     let end = Sugg::hir(cx, y, "y");
 416                     diag.span_suggestion(
 417                         expr.span,
 418                         "use",
 419                         format!("{}..{}", start, end),
 420                         Applicability::MachineApplicable, // snippet
 421                     );
 422                 },
 423             );
 424         }
 425     }
 426 }
 427
 428 fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
 429     fn inside_indexing_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 430         matches!(
 431             get_parent_expr(cx, expr),
 432             Some(Expr {
 433                 kind: ExprKind::Index(..),
 434                 ..
 435             })
 436         )
 437     }
 438
 439     fn is_for_loop_arg(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 440         let mut cur_expr = expr;
 441         while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
 442             match higher::for_loop(parent_expr) {
 443                 Some((_, args, _, _)) if args.hir_id == expr.hir_id => return true,
 444                 _ => cur_expr = parent_expr,
 445             }
 446         }
 447
 448         false
 449     }
 450
 451     fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
 452         match limits {
 453             RangeLimits::HalfOpen => ordering != Ordering::Less,
 454             RangeLimits::Closed => ordering == Ordering::Greater,
 455         }
 456     }
 457
 458     if_chain! {
 459         if let Some(higher::Range { start: Some(start), end: Some(end), limits }) = higher::range(expr);
 460         let ty = cx.typeck_results().expr_ty(start);
 461         if let ty::Int(_) | ty::Uint(_) = ty.kind();
 462         if let Some((start_idx, _)) = constant(cx, cx.typeck_results(), start);
 463         if let Some((end_idx, _)) = constant(cx, cx.typeck_results(), end);
 464         if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
 465         if is_empty_range(limits, ordering);
 466         then {
 467             if inside_indexing_expr(cx, expr) {
 468                 // Avoid linting `N..N` as it has proven to be useful, see #5689 and #5628 ...
 469                 if ordering != Ordering::Equal {
 470                     span_lint(
 471                         cx,
 472                         REVERSED_EMPTY_RANGES,
 473                         expr.span,
 474                         "this range is reversed and using it to index a slice will panic at run-time",
 475                     );
 476                 }
 477             // ... except in for loop arguments for backwards compatibility with `reverse_range_loop`
 478             } else if ordering != Ordering::Equal || is_for_loop_arg(cx, expr) {
 479                 span_lint_and_then(
 480                     cx,
 481                     REVERSED_EMPTY_RANGES,
 482                     expr.span,
 483                     "this range is empty so it will yield no values",
 484                     |diag| {
 485                         if ordering != Ordering::Equal {
 486                             let start_snippet = snippet(cx, start.span, "_");
 487                             let end_snippet = snippet(cx, end.span, "_");
 488                             let dots = match limits {
 489                                 RangeLimits::HalfOpen => "..",
 490                                 RangeLimits::Closed => "..="
 491                             };
 492
 493                             diag.span_suggestion(
 494                                 expr.span,
 495                                 "consider using the following if you are attempting to iterate over this \
 496                                  range in reverse",
 497                                 format!("({}{}{}).rev()", end_snippet, dots, start_snippet),
 498                                 Applicability::MaybeIncorrect,
 499                             );
 500                         }
 501                     },
 502                 );
 503             }
 504         }
 505     }
 506 }
 507
 508 fn y_plus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
 509     match expr.kind {
 510         ExprKind::Binary(
 511             Spanned {
 512                 node: BinOpKind::Add, ..
 513             },
 514             ref lhs,
 515             ref rhs,
 516         ) => {
 517             if is_integer_const(cx, lhs, 1) {
 518                 Some(rhs)
 519             } else if is_integer_const(cx, rhs, 1) {
 520                 Some(lhs)
 521             } else {
 522                 None
 523             }
 524         },
 525         _ => None,
 526     }
 527 }
 528
 529 fn y_minus_one<'t>(cx: &LateContext<'_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> {
 530     match expr.kind {
 531         ExprKind::Binary(
 532             Spanned {
 533                 node: BinOpKind::Sub, ..
 534             },
 535             ref lhs,
 536             ref rhs,
 537         ) if is_integer_const(cx, rhs, 1) => Some(lhs),
 538         _ => None,
 539     }
 540 }