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