mod empty_loop; mod explicit_counter_loop; mod explicit_into_iter_loop; mod explicit_iter_loop; mod for_kv_map; mod iter_next_loop; mod manual_find; mod manual_flatten; mod manual_memcpy; mod missing_spin_loop; mod mut_range_bound; mod needless_range_loop; mod never_loop; mod same_item_push; mod single_element_loop; mod utils; mod while_immutable_condition; mod while_let_loop; mod while_let_on_iterator; use clippy_utils::higher; use rustc_hir::{Expr, ExprKind, LoopSource, Pat}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; use utils::{make_iterator_snippet, IncrementVisitor, InitializeVisitor}; declare_clippy_lint! { /// ### What it does /// Checks for for-loops that manually copy items between /// slices that could be optimized by having a memcpy. /// /// ### Why is this bad? /// It is not as fast as a memcpy. /// /// ### Example /// ```rust /// # let src = vec![1]; /// # let mut dst = vec![0; 65]; /// for i in 0..src.len() { /// dst[i + 64] = src[i]; /// } /// ``` /// /// Use instead: /// ```rust /// # let src = vec![1]; /// # let mut dst = vec![0; 65]; /// dst[64..(src.len() + 64)].clone_from_slice(&src[..]); /// ``` #[clippy::version = "pre 1.29.0"] pub MANUAL_MEMCPY, perf, "manually copying items between slices" } declare_clippy_lint! { /// ### What it does /// Checks for looping over the range of `0..len` of some /// collection just to get the values by index. /// /// ### Why is this bad? /// Just iterating the collection itself makes the intent /// more clear and is probably faster. /// /// ### Example /// ```rust /// let vec = vec!['a', 'b', 'c']; /// for i in 0..vec.len() { /// println!("{}", vec[i]); /// } /// ``` /// /// Use instead: /// ```rust /// let vec = vec!['a', 'b', 'c']; /// for i in vec { /// println!("{}", i); /// } /// ``` #[clippy::version = "pre 1.29.0"] pub NEEDLESS_RANGE_LOOP, style, "for-looping over a range of indices where an iterator over items would do" } declare_clippy_lint! { /// ### What it does /// Checks for loops on `x.iter()` where `&x` will do, and /// suggests the latter. /// /// ### Why is this bad? /// Readability. /// /// ### Known problems /// False negatives. We currently only warn on some known /// types. /// /// ### Example /// ```rust /// // with `y` a `Vec` or slice: /// # let y = vec![1]; /// for x in y.iter() { /// // .. /// } /// ``` /// /// Use instead: /// ```rust /// # let y = vec![1]; /// for x in &y { /// // .. /// } /// ``` #[clippy::version = "pre 1.29.0"] pub EXPLICIT_ITER_LOOP, pedantic, "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do" } declare_clippy_lint! { /// ### What it does /// Checks for loops on `y.into_iter()` where `y` will do, and /// suggests the latter. /// /// ### Why is this bad? /// Readability. /// /// ### Example /// ```rust /// # let y = vec![1]; /// // with `y` a `Vec` or slice: /// for x in y.into_iter() { /// // .. /// } /// ``` /// can be rewritten to /// ```rust /// # let y = vec![1]; /// for x in y { /// // .. /// } /// ``` #[clippy::version = "pre 1.29.0"] pub EXPLICIT_INTO_ITER_LOOP, pedantic, "for-looping over `_.into_iter()` when `_` would do" } declare_clippy_lint! { /// ### What it does /// Checks for loops on `x.next()`. /// /// ### Why is this bad? /// `next()` returns either `Some(value)` if there was a /// value, or `None` otherwise. The insidious thing is that `Option<_>` /// implements `IntoIterator`, so that possibly one value will be iterated, /// leading to some hard to find bugs. No one will want to write such code /// [except to win an Underhanded Rust /// Contest](https://www.reddit.com/r/rust/comments/3hb0wm/underhanded_rust_contest/cu5yuhr). /// /// ### Example /// ```ignore /// for x in y.next() { /// .. /// } /// ``` #[clippy::version = "pre 1.29.0"] pub ITER_NEXT_LOOP, correctness, "for-looping over `_.next()` which is probably not intended" } declare_clippy_lint! { /// ### What it does /// Detects `loop + match` combinations that are easier /// written as a `while let` loop. /// /// ### Why is this bad? /// The `while let` loop is usually shorter and more /// readable. /// /// ### Known problems /// Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)). /// /// ### Example /// ```rust,no_run /// # let y = Some(1); /// loop { /// let x = match y { /// Some(x) => x, /// None => break, /// }; /// // .. do something with x /// } /// // is easier written as /// while let Some(x) = y { /// // .. do something with x /// }; /// ``` #[clippy::version = "pre 1.29.0"] pub WHILE_LET_LOOP, complexity, "`loop { if let { ... } else break }`, which can be written as a `while let` loop" } declare_clippy_lint! { /// ### What it does /// Checks `for` loops over slices with an explicit counter /// and suggests the use of `.enumerate()`. /// /// ### Why is this bad? /// Using `.enumerate()` makes the intent more clear, /// declutters the code and may be faster in some instances. /// /// ### Example /// ```rust /// # let v = vec![1]; /// # fn bar(bar: usize, baz: usize) {} /// let mut i = 0; /// for item in &v { /// bar(i, *item); /// i += 1; /// } /// ``` /// /// Use instead: /// ```rust /// # let v = vec![1]; /// # fn bar(bar: usize, baz: usize) {} /// for (i, item) in v.iter().enumerate() { bar(i, *item); } /// ``` #[clippy::version = "pre 1.29.0"] pub EXPLICIT_COUNTER_LOOP, complexity, "for-looping with an explicit counter when `_.enumerate()` would do" } declare_clippy_lint! { /// ### What it does /// Checks for empty `loop` expressions. /// /// ### Why is this bad? /// These busy loops burn CPU cycles without doing /// anything. It is _almost always_ a better idea to `panic!` than to have /// a busy loop. /// /// If panicking isn't possible, think of the environment and either: /// - block on something /// - sleep the thread for some microseconds /// - yield or pause the thread /// /// For `std` targets, this can be done with /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html) /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html). /// /// For `no_std` targets, doing this is more complicated, especially because /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will /// probably need to invoke some target-specific intrinsic. Examples include: /// - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html) /// - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html) /// /// ### Example /// ```no_run /// loop {} /// ``` #[clippy::version = "pre 1.29.0"] pub EMPTY_LOOP, suspicious, "empty `loop {}`, which should block or sleep" } declare_clippy_lint! { /// ### What it does /// Checks for `while let` expressions on iterators. /// /// ### Why is this bad? /// Readability. A simple `for` loop is shorter and conveys /// the intent better. /// /// ### Example /// ```ignore /// while let Some(val) = iter.next() { /// .. /// } /// ``` /// /// Use instead: /// ```ignore /// for val in &mut iter { /// .. /// } /// ``` #[clippy::version = "pre 1.29.0"] pub WHILE_LET_ON_ITERATOR, style, "using a `while let` loop instead of a for loop on an iterator" } declare_clippy_lint! { /// ### What it does /// Checks for iterating a map (`HashMap` or `BTreeMap`) and /// ignoring either the keys or values. /// /// ### Why is this bad? /// Readability. There are `keys` and `values` methods that /// can be used to express that don't need the values or keys. /// /// ### Example /// ```ignore /// for (k, _) in &map { /// .. /// } /// ``` /// /// could be replaced by /// /// ```ignore /// for k in map.keys() { /// .. /// } /// ``` #[clippy::version = "pre 1.29.0"] pub FOR_KV_MAP, style, "looping on a map using `iter` when `keys` or `values` would do" } declare_clippy_lint! { /// ### What it does /// Checks for loops that will always `break`, `return` or /// `continue` an outer loop. /// /// ### Why is this bad? /// This loop never loops, all it does is obfuscating the /// code. /// /// ### Example /// ```rust /// loop { /// ..; /// break; /// } /// ``` #[clippy::version = "pre 1.29.0"] pub NEVER_LOOP, correctness, "any loop that will always `break` or `return`" } declare_clippy_lint! { /// ### What it does /// Checks for loops which have a range bound that is a mutable variable /// /// ### Why is this bad? /// One might think that modifying the mutable variable changes the loop bounds /// /// ### Known problems /// False positive when mutation is followed by a `break`, but the `break` is not immediately /// after the mutation: /// /// ```rust /// let mut x = 5; /// for _ in 0..x { /// x += 1; // x is a range bound that is mutated /// ..; // some other expression /// break; // leaves the loop, so mutation is not an issue /// } /// ``` /// /// False positive on nested loops ([#6072](https://github.com/rust-lang/rust-clippy/issues/6072)) /// /// ### Example /// ```rust /// let mut foo = 42; /// for i in 0..foo { /// foo -= 1; /// println!("{}", i); // prints numbers from 0 to 42, not 0 to 21 /// } /// ``` #[clippy::version = "pre 1.29.0"] pub MUT_RANGE_BOUND, suspicious, "for loop over a range where one of the bounds is a mutable variable" } declare_clippy_lint! { /// ### What it does /// Checks whether variables used within while loop condition /// can be (and are) mutated in the body. /// /// ### Why is this bad? /// If the condition is unchanged, entering the body of the loop /// will lead to an infinite loop. /// /// ### Known problems /// If the `while`-loop is in a closure, the check for mutation of the /// condition variables in the body can cause false negatives. For example when only `Upvar` `a` is /// in the condition and only `Upvar` `b` gets mutated in the body, the lint will not trigger. /// /// ### Example /// ```rust /// let i = 0; /// while i > 10 { /// println!("let me loop forever!"); /// } /// ``` #[clippy::version = "pre 1.29.0"] pub WHILE_IMMUTABLE_CONDITION, correctness, "variables used within while expression are not mutated in the body" } declare_clippy_lint! { /// ### What it does /// Checks whether a for loop is being used to push a constant /// value into a Vec. /// /// ### Why is this bad? /// This kind of operation can be expressed more succinctly with /// `vec![item; SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also /// have better performance. /// /// ### Example /// ```rust /// let item1 = 2; /// let item2 = 3; /// let mut vec: Vec = Vec::new(); /// for _ in 0..20 { /// vec.push(item1); /// } /// for _ in 0..30 { /// vec.push(item2); /// } /// ``` /// /// Use instead: /// ```rust /// let item1 = 2; /// let item2 = 3; /// let mut vec: Vec = vec![item1; 20]; /// vec.resize(20 + 30, item2); /// ``` #[clippy::version = "1.47.0"] pub SAME_ITEM_PUSH, style, "the same item is pushed inside of a for loop" } declare_clippy_lint! { /// ### What it does /// Checks whether a for loop has a single element. /// /// ### Why is this bad? /// There is no reason to have a loop of a /// single element. /// /// ### Example /// ```rust /// let item1 = 2; /// for item in &[item1] { /// println!("{}", item); /// } /// ``` /// /// Use instead: /// ```rust /// let item1 = 2; /// let item = &item1; /// println!("{}", item); /// ``` #[clippy::version = "1.49.0"] pub SINGLE_ELEMENT_LOOP, complexity, "there is no reason to have a single element loop" } declare_clippy_lint! { /// ### What it does /// Check for unnecessary `if let` usage in a for loop /// where only the `Some` or `Ok` variant of the iterator element is used. /// /// ### Why is this bad? /// It is verbose and can be simplified /// by first calling the `flatten` method on the `Iterator`. /// /// ### Example /// /// ```rust /// let x = vec![Some(1), Some(2), Some(3)]; /// for n in x { /// if let Some(n) = n { /// println!("{}", n); /// } /// } /// ``` /// Use instead: /// ```rust /// let x = vec![Some(1), Some(2), Some(3)]; /// for n in x.into_iter().flatten() { /// println!("{}", n); /// } /// ``` #[clippy::version = "1.52.0"] pub MANUAL_FLATTEN, complexity, "for loops over `Option`s or `Result`s with a single expression can be simplified" } declare_clippy_lint! { /// ### What it does /// Check for empty spin loops /// /// ### Why is this bad? /// The loop body should have something like `thread::park()` or at least /// `std::hint::spin_loop()` to avoid needlessly burning cycles and conserve /// energy. Perhaps even better use an actual lock, if possible. /// /// ### Known problems /// This lint doesn't currently trigger on `while let` or /// `loop { match .. { .. } }` loops, which would be considered idiomatic in /// combination with e.g. `AtomicBool::compare_exchange_weak`. /// /// ### Example /// /// ```ignore /// use core::sync::atomic::{AtomicBool, Ordering}; /// let b = AtomicBool::new(true); /// // give a ref to `b` to another thread,wait for it to become false /// while b.load(Ordering::Acquire) {}; /// ``` /// Use instead: /// ```rust,no_run ///# use core::sync::atomic::{AtomicBool, Ordering}; ///# let b = AtomicBool::new(true); /// while b.load(Ordering::Acquire) { /// std::hint::spin_loop() /// } /// ``` #[clippy::version = "1.61.0"] pub MISSING_SPIN_LOOP, perf, "An empty busy waiting loop" } declare_clippy_lint! { /// ### What it does /// Check for manual implementations of Iterator::find /// /// ### Why is this bad? /// It doesn't affect performance, but using `find` is shorter and easier to read. /// /// ### Example /// /// ```rust /// fn example(arr: Vec) -> Option { /// for el in arr { /// if el == 1 { /// return Some(el); /// } /// } /// None /// } /// ``` /// Use instead: /// ```rust /// fn example(arr: Vec) -> Option { /// arr.into_iter().find(|&el| el == 1) /// } /// ``` #[clippy::version = "1.64.0"] pub MANUAL_FIND, complexity, "manual implementation of `Iterator::find`" } declare_lint_pass!(Loops => [ MANUAL_MEMCPY, MANUAL_FLATTEN, NEEDLESS_RANGE_LOOP, EXPLICIT_ITER_LOOP, EXPLICIT_INTO_ITER_LOOP, ITER_NEXT_LOOP, WHILE_LET_LOOP, EXPLICIT_COUNTER_LOOP, EMPTY_LOOP, WHILE_LET_ON_ITERATOR, FOR_KV_MAP, NEVER_LOOP, MUT_RANGE_BOUND, WHILE_IMMUTABLE_CONDITION, SAME_ITEM_PUSH, SINGLE_ELEMENT_LOOP, MISSING_SPIN_LOOP, MANUAL_FIND, ]); impl<'tcx> LateLintPass<'tcx> for Loops { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let for_loop = higher::ForLoop::hir(expr); if let Some(higher::ForLoop { pat, arg, body, loop_id, span, }) = for_loop { // we don't want to check expanded macros // this check is not at the top of the function // since higher::for_loop expressions are marked as expansions if body.span.from_expansion() { return; } check_for_loop(cx, pat, arg, body, expr, span); if let ExprKind::Block(block, _) = body.kind { never_loop::check(cx, block, loop_id, span, for_loop.as_ref()); } } // we don't want to check expanded macros if expr.span.from_expansion() { return; } // check for never_loop if let ExprKind::Loop(block, ..) = expr.kind { never_loop::check(cx, block, expr.hir_id, expr.span, None); } // check for `loop { if let {} else break }` that could be `while let` // (also matches an explicit "match" instead of "if let") // (even if the "match" or "if let" is used for declaration) if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind { // also check for empty `loop {}` statements, skipping those in #[panic_handler] empty_loop::check(cx, expr, block); while_let_loop::check(cx, expr, block); } while_let_on_iterator::check(cx, expr); if let Some(higher::While { condition, body }) = higher::While::hir(expr) { while_immutable_condition::check(cx, condition, body); missing_spin_loop::check(cx, condition, body); } } } fn check_for_loop<'tcx>( cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>, body: &'tcx Expr<'_>, expr: &'tcx Expr<'_>, span: Span, ) { let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr); if !is_manual_memcpy_triggered { needless_range_loop::check(cx, pat, arg, body, expr); explicit_counter_loop::check(cx, pat, arg, body, expr); } check_for_loop_arg(cx, pat, arg); for_kv_map::check(cx, pat, arg, body); mut_range_bound::check(cx, arg, body); single_element_loop::check(cx, pat, arg, body, expr); same_item_push::check(cx, pat, arg, body, expr); manual_flatten::check(cx, pat, arg, body, span); manual_find::check(cx, pat, arg, body, span, expr); } fn check_for_loop_arg(cx: &LateContext<'_>, _: &Pat<'_>, arg: &Expr<'_>) { if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind { let method_name = method.ident.as_str(); // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x match method_name { "iter" | "iter_mut" => { explicit_iter_loop::check(cx, self_arg, arg, method_name); }, "into_iter" => { explicit_iter_loop::check(cx, self_arg, arg, method_name); explicit_into_iter_loop::check(cx, self_arg, arg); }, "next" => { iter_next_loop::check(cx, arg); }, _ => {}, } } }