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