clippy_lints/src/loops/mod.rs

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