clippy_lints/src/loops/mod.rs

   1 mod empty_loop;
   2 mod explicit_counter_loop;
   3 mod explicit_into_iter_loop;
   4 mod explicit_iter_loop;
   5 mod for_kv_map;
   6 mod for_loops_over_fallibles;
   7 mod iter_next_loop;
   8 mod manual_flatten;
   9 mod manual_memcpy;
  10 mod mut_range_bound;
  11 mod needless_collect;
  12 mod needless_range_loop;
  13 mod never_loop;
  14 mod same_item_push;
  15 mod single_element_loop;
  16 mod utils;
  17 mod while_immutable_condition;
  18 mod while_let_loop;
  19 mod while_let_on_iterator;
  20
  21 use clippy_utils::higher;
  22 use rustc_hir::{Expr, ExprKind, LoopSource, Pat};
  23 use rustc_lint::{LateContext, LateLintPass};
  24 use rustc_session::{declare_lint_pass, declare_tool_lint};
  25 use rustc_span::source_map::Span;
  26 use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor};
  27
  28 declare_clippy_lint! {
  29     /// ### What it does
  30     /// Checks for for-loops that manually copy items between
  31     /// slices that could be optimized by having a memcpy.
  32     ///
  33     /// ### Why is this bad?
  34     /// It is not as fast as a memcpy.
  35     ///
  36     /// ### Example
  37     /// ```rust
  38     /// # let src = vec![1];
  39     /// # let mut dst = vec![0; 65];
  40     /// for i in 0..src.len() {
  41     ///     dst[i + 64] = src[i];
  42     /// }
  43     /// ```
  44     /// Could be written as:
  45     /// ```rust
  46     /// # let src = vec![1];
  47     /// # let mut dst = vec![0; 65];
  48     /// dst[64..(src.len() + 64)].clone_from_slice(&src[..]);
  49     /// ```
  50     #[clippy::version = "pre 1.29.0"]
  51     pub MANUAL_MEMCPY,
  52     perf,
  53     "manually copying items between slices"
  54 }
  55
  56 declare_clippy_lint! {
  57     /// ### What it does
  58     /// Checks for looping over the range of `0..len` of some
  59     /// collection just to get the values by index.
  60     ///
  61     /// ### Why is this bad?
  62     /// Just iterating the collection itself makes the intent
  63     /// more clear and is probably faster.
  64     ///
  65     /// ### Example
  66     /// ```rust
  67     /// let vec = vec!['a', 'b', 'c'];
  68     /// for i in 0..vec.len() {
  69     ///     println!("{}", vec[i]);
  70     /// }
  71     /// ```
  72     /// Could be written as:
  73     /// ```rust
  74     /// let vec = vec!['a', 'b', 'c'];
  75     /// for i in vec {
  76     ///     println!("{}", i);
  77     /// }
  78     /// ```
  79     #[clippy::version = "pre 1.29.0"]
  80     pub NEEDLESS_RANGE_LOOP,
  81     style,
  82     "for-looping over a range of indices where an iterator over items would do"
  83 }
  84
  85 declare_clippy_lint! {
  86     /// ### What it does
  87     /// Checks for loops on `x.iter()` where `&x` will do, and
  88     /// suggests the latter.
  89     ///
  90     /// ### Why is this bad?
  91     /// Readability.
  92     ///
  93     /// ### Known problems
  94     /// False negatives. We currently only warn on some known
  95     /// types.
  96     ///
  97     /// ### Example
  98     /// ```rust
  99     /// // with `y` a `Vec` or slice:
 100     /// # let y = vec![1];
 101     /// for x in y.iter() {
 102     ///     // ..
 103     /// }
 104     /// ```
 105     /// can be rewritten to
 106     /// ```rust
 107     /// # let y = vec![1];
 108     /// for x in &y {
 109     ///     // ..
 110     /// }
 111     /// ```
 112     #[clippy::version = "pre 1.29.0"]
 113     pub EXPLICIT_ITER_LOOP,
 114     pedantic,
 115     "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
 116 }
 117
 118 declare_clippy_lint! {
 119     /// ### What it does
 120     /// Checks for loops on `y.into_iter()` where `y` will do, and
 121     /// suggests the latter.
 122     ///
 123     /// ### Why is this bad?
 124     /// Readability.
 125     ///
 126     /// ### Example
 127     /// ```rust
 128     /// # let y = vec![1];
 129     /// // with `y` a `Vec` or slice:
 130     /// for x in y.into_iter() {
 131     ///     // ..
 132     /// }
 133     /// ```
 134     /// can be rewritten to
 135     /// ```rust
 136     /// # let y = vec![1];
 137     /// for x in y {
 138     ///     // ..
 139     /// }
 140     /// ```
 141     #[clippy::version = "pre 1.29.0"]
 142     pub EXPLICIT_INTO_ITER_LOOP,
 143     pedantic,
 144     "for-looping over `_.into_iter()` when `_` would do"
 145 }
 146
 147 declare_clippy_lint! {
 148     /// ### What it does
 149     /// Checks for loops on `x.next()`.
 150     ///
 151     /// ### Why is this bad?
 152     /// `next()` returns either `Some(value)` if there was a
 153     /// value, or `None` otherwise. The insidious thing is that `Option<_>`
 154     /// implements `IntoIterator`, so that possibly one value will be iterated,
 155     /// leading to some hard to find bugs. No one will want to write such code
 156     /// [except to win an Underhanded Rust
 157     /// Contest](https://www.reddit.com/r/rust/comments/3hb0wm/underhanded_rust_contest/cu5yuhr).
 158     ///
 159     /// ### Example
 160     /// ```ignore
 161     /// for x in y.next() {
 162     ///     ..
 163     /// }
 164     /// ```
 165     #[clippy::version = "pre 1.29.0"]
 166     pub ITER_NEXT_LOOP,
 167     correctness,
 168     "for-looping over `_.next()` which is probably not intended"
 169 }
 170
 171 declare_clippy_lint! {
 172     /// ### What it does
 173     /// Checks for `for` loops over `Option` or `Result` values.
 174     ///
 175     /// ### Why is this bad?
 176     /// Readability. This is more clearly expressed as an `if
 177     /// let`.
 178     ///
 179     /// ### Example
 180     /// ```rust
 181     /// # let opt = Some(1);
 182     ///
 183     /// // Bad
 184     /// for x in opt {
 185     ///     // ..
 186     /// }
 187     ///
 188     /// // Good
 189     /// if let Some(x) = opt {
 190     ///     // ..
 191     /// }
 192     /// ```
 193     ///
 194     /// // or
 195     ///
 196     /// ```rust
 197     /// # let res: Result<i32, std::io::Error> = Ok(1);
 198     ///
 199     /// // Bad
 200     /// for x in &res {
 201     ///     // ..
 202     /// }
 203     ///
 204     /// // Good
 205     /// if let Ok(x) = res {
 206     ///     // ..
 207     /// }
 208     /// ```
 209     #[clippy::version = "1.45.0"]
 210     pub FOR_LOOPS_OVER_FALLIBLES,
 211     suspicious,
 212     "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
 213 }
 214
 215 declare_clippy_lint! {
 216     /// ### What it does
 217     /// Detects `loop + match` combinations that are easier
 218     /// written as a `while let` loop.
 219     ///
 220     /// ### Why is this bad?
 221     /// The `while let` loop is usually shorter and more
 222     /// readable.
 223     ///
 224     /// ### Known problems
 225     /// Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)).
 226     ///
 227     /// ### Example
 228     /// ```rust,no_run
 229     /// # let y = Some(1);
 230     /// loop {
 231     ///     let x = match y {
 232     ///         Some(x) => x,
 233     ///         None => break,
 234     ///     };
 235     ///     // .. do something with x
 236     /// }
 237     /// // is easier written as
 238     /// while let Some(x) = y {
 239     ///     // .. do something with x
 240     /// };
 241     /// ```
 242     #[clippy::version = "pre 1.29.0"]
 243     pub WHILE_LET_LOOP,
 244     complexity,
 245     "`loop { if let { ... } else break }`, which can be written as a `while let` loop"
 246 }
 247
 248 declare_clippy_lint! {
 249     /// ### What it does
 250     /// Checks for functions collecting an iterator when collect
 251     /// is not needed.
 252     ///
 253     /// ### Why is this bad?
 254     /// `collect` causes the allocation of a new data structure,
 255     /// when this allocation may not be needed.
 256     ///
 257     /// ### Example
 258     /// ```rust
 259     /// # let iterator = vec![1].into_iter();
 260     /// let len = iterator.clone().collect::<Vec<_>>().len();
 261     /// // should be
 262     /// let len = iterator.count();
 263     /// ```
 264     #[clippy::version = "1.30.0"]
 265     pub NEEDLESS_COLLECT,
 266     perf,
 267     "collecting an iterator when collect is not needed"
 268 }
 269
 270 declare_clippy_lint! {
 271     /// ### What it does
 272     /// Checks `for` loops over slices with an explicit counter
 273     /// and suggests the use of `.enumerate()`.
 274     ///
 275     /// ### Why is this bad?
 276     /// Using `.enumerate()` makes the intent more clear,
 277     /// declutters the code and may be faster in some instances.
 278     ///
 279     /// ### Example
 280     /// ```rust
 281     /// # let v = vec![1];
 282     /// # fn bar(bar: usize, baz: usize) {}
 283     /// let mut i = 0;
 284     /// for item in &v {
 285     ///     bar(i, *item);
 286     ///     i += 1;
 287     /// }
 288     /// ```
 289     /// Could be written as
 290     /// ```rust
 291     /// # let v = vec![1];
 292     /// # fn bar(bar: usize, baz: usize) {}
 293     /// for (i, item) in v.iter().enumerate() { bar(i, *item); }
 294     /// ```
 295     #[clippy::version = "pre 1.29.0"]
 296     pub EXPLICIT_COUNTER_LOOP,
 297     complexity,
 298     "for-looping with an explicit counter when `_.enumerate()` would do"
 299 }
 300
 301 declare_clippy_lint! {
 302     /// ### What it does
 303     /// Checks for empty `loop` expressions.
 304     ///
 305     /// ### Why is this bad?
 306     /// These busy loops burn CPU cycles without doing
 307     /// anything. It is _almost always_ a better idea to `panic!` than to have
 308     /// a busy loop.
 309     ///
 310     /// If panicking isn't possible, think of the environment and either:
 311     ///   - block on something
 312     ///   - sleep the thread for some microseconds
 313     ///   - yield or pause the thread
 314     ///
 315     /// For `std` targets, this can be done with
 316     /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
 317     /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
 318     ///
 319     /// For `no_std` targets, doing this is more complicated, especially because
 320     /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
 321     /// probably need to invoke some target-specific intrinsic. Examples include:
 322     ///   - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
 323     ///   - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
 324     ///
 325     /// ### Example
 326     /// ```no_run
 327     /// loop {}
 328     /// ```
 329     #[clippy::version = "pre 1.29.0"]
 330     pub EMPTY_LOOP,
 331     suspicious,
 332     "empty `loop {}`, which should block or sleep"
 333 }
 334
 335 declare_clippy_lint! {
 336     /// ### What it does
 337     /// Checks for `while let` expressions on iterators.
 338     ///
 339     /// ### Why is this bad?
 340     /// Readability. A simple `for` loop is shorter and conveys
 341     /// the intent better.
 342     ///
 343     /// ### Example
 344     /// ```ignore
 345     /// while let Some(val) = iter() {
 346     ///     ..
 347     /// }
 348     /// ```
 349     #[clippy::version = "pre 1.29.0"]
 350     pub WHILE_LET_ON_ITERATOR,
 351     style,
 352     "using a `while let` loop instead of a for loop on an iterator"
 353 }
 354
 355 declare_clippy_lint! {
 356     /// ### What it does
 357     /// Checks for iterating a map (`HashMap` or `BTreeMap`) and
 358     /// ignoring either the keys or values.
 359     ///
 360     /// ### Why is this bad?
 361     /// Readability. There are `keys` and `values` methods that
 362     /// can be used to express that don't need the values or keys.
 363     ///
 364     /// ### Example
 365     /// ```ignore
 366     /// for (k, _) in &map {
 367     ///     ..
 368     /// }
 369     /// ```
 370     ///
 371     /// could be replaced by
 372     ///
 373     /// ```ignore
 374     /// for k in map.keys() {
 375     ///     ..
 376     /// }
 377     /// ```
 378     #[clippy::version = "pre 1.29.0"]
 379     pub FOR_KV_MAP,
 380     style,
 381     "looping on a map using `iter` when `keys` or `values` would do"
 382 }
 383
 384 declare_clippy_lint! {
 385     /// ### What it does
 386     /// Checks for loops that will always `break`, `return` or
 387     /// `continue` an outer loop.
 388     ///
 389     /// ### Why is this bad?
 390     /// This loop never loops, all it does is obfuscating the
 391     /// code.
 392     ///
 393     /// ### Example
 394     /// ```rust
 395     /// loop {
 396     ///     ..;
 397     ///     break;
 398     /// }
 399     /// ```
 400     #[clippy::version = "pre 1.29.0"]
 401     pub NEVER_LOOP,
 402     correctness,
 403     "any loop that will always `break` or `return`"
 404 }
 405
 406 declare_clippy_lint! {
 407     /// ### What it does
 408     /// Checks for loops which have a range bound that is a mutable variable
 409     ///
 410     /// ### Why is this bad?
 411     /// One might think that modifying the mutable variable changes the loop bounds
 412     ///
 413     /// ### Known problems
 414     /// False positive when mutation is followed by a `break`, but the `break` is not immediately
 415     /// after the mutation:
 416     ///
 417     /// ```rust
 418     /// let mut x = 5;
 419     /// for _ in 0..x {
 420     ///     x += 1; // x is a range bound that is mutated
 421     ///     ..; // some other expression
 422     ///     break; // leaves the loop, so mutation is not an issue
 423     /// }
 424     /// ```
 425     ///
 426     /// False positive on nested loops ([#6072](https://github.com/rust-lang/rust-clippy/issues/6072))
 427     ///
 428     /// ### Example
 429     /// ```rust
 430     /// let mut foo = 42;
 431     /// for i in 0..foo {
 432     ///     foo -= 1;
 433     ///     println!("{}", i); // prints numbers from 0 to 42, not 0 to 21
 434     /// }
 435     /// ```
 436     #[clippy::version = "pre 1.29.0"]
 437     pub MUT_RANGE_BOUND,
 438     suspicious,
 439     "for loop over a range where one of the bounds is a mutable variable"
 440 }
 441
 442 declare_clippy_lint! {
 443     /// ### What it does
 444     /// Checks whether variables used within while loop condition
 445     /// can be (and are) mutated in the body.
 446     ///
 447     /// ### Why is this bad?
 448     /// If the condition is unchanged, entering the body of the loop
 449     /// will lead to an infinite loop.
 450     ///
 451     /// ### Known problems
 452     /// If the `while`-loop is in a closure, the check for mutation of the
 453     /// condition variables in the body can cause false negatives. For example when only `Upvar` `a` is
 454     /// in the condition and only `Upvar` `b` gets mutated in the body, the lint will not trigger.
 455     ///
 456     /// ### Example
 457     /// ```rust
 458     /// let i = 0;
 459     /// while i > 10 {
 460     ///     println!("let me loop forever!");
 461     /// }
 462     /// ```
 463     #[clippy::version = "pre 1.29.0"]
 464     pub WHILE_IMMUTABLE_CONDITION,
 465     correctness,
 466     "variables used within while expression are not mutated in the body"
 467 }
 468
 469 declare_clippy_lint! {
 470     /// ### What it does
 471     /// Checks whether a for loop is being used to push a constant
 472     /// value into a Vec.
 473     ///
 474     /// ### Why is this bad?
 475     /// This kind of operation can be expressed more succinctly with
 476     /// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
 477     /// have better performance.
 478     ///
 479     /// ### Example
 480     /// ```rust
 481     /// let item1 = 2;
 482     /// let item2 = 3;
 483     /// let mut vec: Vec<u8> = Vec::new();
 484     /// for _ in 0..20 {
 485     ///    vec.push(item1);
 486     /// }
 487     /// for _ in 0..30 {
 488     ///     vec.push(item2);
 489     /// }
 490     /// ```
 491     /// could be written as
 492     /// ```rust
 493     /// let item1 = 2;
 494     /// let item2 = 3;
 495     /// let mut vec: Vec<u8> = vec![item1; 20];
 496     /// vec.resize(20 + 30, item2);
 497     /// ```
 498     #[clippy::version = "1.47.0"]
 499     pub SAME_ITEM_PUSH,
 500     style,
 501     "the same item is pushed inside of a for loop"
 502 }
 503
 504 declare_clippy_lint! {
 505     /// ### What it does
 506     /// Checks whether a for loop has a single element.
 507     ///
 508     /// ### Why is this bad?
 509     /// There is no reason to have a loop of a
 510     /// single element.
 511     ///
 512     /// ### Example
 513     /// ```rust
 514     /// let item1 = 2;
 515     /// for item in &[item1] {
 516     ///     println!("{}", item);
 517     /// }
 518     /// ```
 519     /// could be written as
 520     /// ```rust
 521     /// let item1 = 2;
 522     /// let item = &item1;
 523     /// println!("{}", item);
 524     /// ```
 525     #[clippy::version = "1.49.0"]
 526     pub SINGLE_ELEMENT_LOOP,
 527     complexity,
 528     "there is no reason to have a single element loop"
 529 }
 530
 531 declare_clippy_lint! {
 532     /// ### What it does
 533     /// Check for unnecessary `if let` usage in a for loop
 534     /// where only the `Some` or `Ok` variant of the iterator element is used.
 535     ///
 536     /// ### Why is this bad?
 537     /// It is verbose and can be simplified
 538     /// by first calling the `flatten` method on the `Iterator`.
 539     ///
 540     /// ### Example
 541     ///
 542     /// ```rust
 543     /// let x = vec![Some(1), Some(2), Some(3)];
 544     /// for n in x {
 545     ///     if let Some(n) = n {
 546     ///         println!("{}", n);
 547     ///     }
 548     /// }
 549     /// ```
 550     /// Use instead:
 551     /// ```rust
 552     /// let x = vec![Some(1), Some(2), Some(3)];
 553     /// for n in x.into_iter().flatten() {
 554     ///     println!("{}", n);
 555     /// }
 556     /// ```
 557     #[clippy::version = "1.52.0"]
 558     pub MANUAL_FLATTEN,
 559     complexity,
 560     "for loops over `Option`s or `Result`s with a single expression can be simplified"
 561 }
 562
 563 declare_lint_pass!(Loops => [
 564     MANUAL_MEMCPY,
 565     MANUAL_FLATTEN,
 566     NEEDLESS_RANGE_LOOP,
 567     EXPLICIT_ITER_LOOP,
 568     EXPLICIT_INTO_ITER_LOOP,
 569     ITER_NEXT_LOOP,
 570     FOR_LOOPS_OVER_FALLIBLES,
 571     WHILE_LET_LOOP,
 572     NEEDLESS_COLLECT,
 573     EXPLICIT_COUNTER_LOOP,
 574     EMPTY_LOOP,
 575     WHILE_LET_ON_ITERATOR,
 576     FOR_KV_MAP,
 577     NEVER_LOOP,
 578     MUT_RANGE_BOUND,
 579     WHILE_IMMUTABLE_CONDITION,
 580     SAME_ITEM_PUSH,
 581     SINGLE_ELEMENT_LOOP,
 582 ]);
 583
 584 impl<'tcx> LateLintPass<'tcx> for Loops {
 585     #[allow(clippy::too_many_lines)]
 586     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
 587         let for_loop = higher::ForLoop::hir(expr);
 588         if let Some(higher::ForLoop {
 589             pat,
 590             arg,
 591             body,
 592             loop_id,
 593             span,
 594         }) = for_loop
 595         {
 596             // we don't want to check expanded macros
 597             // this check is not at the top of the function
 598             // since higher::for_loop expressions are marked as expansions
 599             if body.span.from_expansion() {
 600                 return;
 601             }
 602             check_for_loop(cx, pat, arg, body, expr, span);
 603             if let ExprKind::Block(block, _) = body.kind {
 604                 never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
 605             }
 606         }
 607
 608         // we don't want to check expanded macros
 609         if expr.span.from_expansion() {
 610             return;
 611         }
 612
 613         // check for never_loop
 614         if let ExprKind::Loop(block, ..) = expr.kind {
 615             never_loop::check(cx, block, expr.hir_id, expr.span, None);
 616         }
 617
 618         // check for `loop { if let {} else break }` that could be `while let`
 619         // (also matches an explicit "match" instead of "if let")
 620         // (even if the "match" or "if let" is used for declaration)
 621         if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind {
 622             // also check for empty `loop {}` statements, skipping those in #[panic_handler]
 623             empty_loop::check(cx, expr, block);
 624             while_let_loop::check(cx, expr, block);
 625         }
 626
 627         while_let_on_iterator::check(cx, expr);
 628
 629         if let Some(higher::While { condition, body }) = higher::While::hir(expr) {
 630             while_immutable_condition::check(cx, condition, body);
 631         }
 632
 633         needless_collect::check(expr, cx);
 634     }
 635 }
 636
 637 fn check_for_loop<'tcx>(
 638     cx: &LateContext<'tcx>,
 639     pat: &'tcx Pat<'_>,
 640     arg: &'tcx Expr<'_>,
 641     body: &'tcx Expr<'_>,
 642     expr: &'tcx Expr<'_>,
 643     span: Span,
 644 ) {
 645     let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
 646     if !is_manual_memcpy_triggered {
 647         needless_range_loop::check(cx, pat, arg, body, expr);
 648         explicit_counter_loop::check(cx, pat, arg, body, expr);
 649     }
 650     check_for_loop_arg(cx, pat, arg);
 651     for_kv_map::check(cx, pat, arg, body);
 652     mut_range_bound::check(cx, arg, body);
 653     single_element_loop::check(cx, pat, arg, body, expr);
 654     same_item_push::check(cx, pat, arg, body, expr);
 655     manual_flatten::check(cx, pat, arg, body, span);
 656 }
 657
 658 fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
 659     let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
 660
 661     if let ExprKind::MethodCall(method, _, [self_arg], _) = arg.kind {
 662         let method_name = method.ident.as_str();
 663         // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
 664         match method_name {
 665             "iter" | "iter_mut" => explicit_iter_loop::check(cx, self_arg, arg, method_name),
 666             "into_iter" => {
 667                 explicit_iter_loop::check(cx, self_arg, arg, method_name);
 668                 explicit_into_iter_loop::check(cx, self_arg, arg);
 669             },
 670             "next" => {
 671                 next_loop_linted = iter_next_loop::check(cx, arg);
 672             },
 673             _ => {},
 674         }
 675     }
 676
 677     if !next_loop_linted {
 678         for_loops_over_fallibles::check(cx, pat, arg);
 679     }
 680 }