2 mod explicit_counter_loop;
3 mod explicit_into_iter_loop;
4 mod explicit_iter_loop;
6 mod for_loops_over_fallibles;
12 mod needless_range_loop;
15 mod single_element_loop;
17 mod while_immutable_condition;
19 mod while_let_on_iterator;
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};
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.
32 /// **Why is this bad?** It is not as fast as a memcpy.
34 /// **Known problems:** None.
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];
44 /// Could be written as:
46 /// # let src = vec![1];
47 /// # let mut dst = vec![0; 65];
48 /// dst[64..(src.len() + 64)].clone_from_slice(&src[..]);
52 "manually copying items between slices"
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.
59 /// **Why is this bad?** Just iterating the collection itself makes the intent
60 /// more clear and is probably faster.
62 /// **Known problems:** None.
66 /// let vec = vec!['a', 'b', 'c'];
67 /// for i in 0..vec.len() {
68 /// println!("{}", vec[i]);
71 /// Could be written as:
73 /// let vec = vec!['a', 'b', 'c'];
75 /// println!("{}", i);
78 pub NEEDLESS_RANGE_LOOP,
80 "for-looping over a range of indices where an iterator over items would do"
83 declare_clippy_lint! {
84 /// **What it does:** Checks for loops on `x.iter()` where `&x` will do, and
85 /// suggests the latter.
87 /// **Why is this bad?** Readability.
89 /// **Known problems:** False negatives. We currently only warn on some known
94 /// // with `y` a `Vec` or slice:
95 /// # let y = vec![1];
96 /// for x in y.iter() {
100 /// can be rewritten to
102 /// # let y = vec![1];
107 pub EXPLICIT_ITER_LOOP,
109 "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
112 declare_clippy_lint! {
113 /// **What it does:** Checks for loops on `y.into_iter()` where `y` will do, and
114 /// suggests the latter.
116 /// **Why is this bad?** Readability.
118 /// **Known problems:** None
122 /// # let y = vec![1];
123 /// // with `y` a `Vec` or slice:
124 /// for x in y.into_iter() {
128 /// can be rewritten to
130 /// # let y = vec![1];
135 pub EXPLICIT_INTO_ITER_LOOP,
137 "for-looping over `_.into_iter()` when `_` would do"
140 declare_clippy_lint! {
141 /// **What it does:** Checks for loops on `x.next()`.
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).
150 /// **Known problems:** None.
154 /// for x in y.next() {
160 "for-looping over `_.next()` which is probably not intended"
163 declare_clippy_lint! {
164 /// **What it does:** Checks for `for` loops over `Option` or `Result` values.
166 /// **Why is this bad?** Readability. This is more clearly expressed as an `if
169 /// **Known problems:** None.
173 /// # let opt = Some(1);
181 /// if let Some(x) = opt {
189 /// # let res: Result<i32, std::io::Error> = Ok(1);
197 /// if let Ok(x) = res {
201 pub FOR_LOOPS_OVER_FALLIBLES,
203 "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
206 declare_clippy_lint! {
207 /// **What it does:** Detects `loop + match` combinations that are easier
208 /// written as a `while let` loop.
210 /// **Why is this bad?** The `while let` loop is usually shorter and more
213 /// **Known problems:** Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)).
217 /// # let y = Some(1);
219 /// let x = match y {
223 /// // .. do something with x
225 /// // is easier written as
226 /// while let Some(x) = y {
227 /// // .. do something with x
232 "`loop { if let { ... } else break }`, which can be written as a `while let` loop"
235 declare_clippy_lint! {
236 /// **What it does:** Checks for functions collecting an iterator when collect
239 /// **Why is this bad?** `collect` causes the allocation of a new data structure,
240 /// when this allocation may not be needed.
242 /// **Known problems:**
247 /// # let iterator = vec![1].into_iter();
248 /// let len = iterator.clone().collect::<Vec<_>>().len();
250 /// let len = iterator.count();
252 pub NEEDLESS_COLLECT,
254 "collecting an iterator when collect is not needed"
257 declare_clippy_lint! {
258 /// **What it does:** Checks `for` loops over slices with an explicit counter
259 /// and suggests the use of `.enumerate()`.
261 /// **Why is it bad?** Using `.enumerate()` makes the intent more clear,
262 /// declutters the code and may be faster in some instances.
264 /// **Known problems:** None.
268 /// # let v = vec![1];
269 /// # fn bar(bar: usize, baz: usize) {}
276 /// Could be written as
278 /// # let v = vec![1];
279 /// # fn bar(bar: usize, baz: usize) {}
280 /// for (i, item) in v.iter().enumerate() { bar(i, *item); }
282 pub EXPLICIT_COUNTER_LOOP,
284 "for-looping with an explicit counter when `_.enumerate()` would do"
287 declare_clippy_lint! {
288 /// **What it does:** Checks for empty `loop` expressions.
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
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
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).
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)
309 /// **Known problems:** None.
317 "empty `loop {}`, which should block or sleep"
320 declare_clippy_lint! {
321 /// **What it does:** Checks for `while let` expressions on iterators.
323 /// **Why is this bad?** Readability. A simple `for` loop is shorter and conveys
324 /// the intent better.
326 /// **Known problems:** None.
330 /// while let Some(val) = iter() {
334 pub WHILE_LET_ON_ITERATOR,
336 "using a `while let` loop instead of a for loop on an iterator"
339 declare_clippy_lint! {
340 /// **What it does:** Checks for iterating a map (`HashMap` or `BTreeMap`) and
341 /// ignoring either the keys or values.
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.
346 /// **Known problems:** None.
350 /// for (k, _) in &map {
355 /// could be replaced by
358 /// for k in map.keys() {
364 "looping on a map using `iter` when `keys` or `values` would do"
367 declare_clippy_lint! {
368 /// **What it does:** Checks for loops that will always `break`, `return` or
369 /// `continue` an outer loop.
371 /// **Why is this bad?** This loop never loops, all it does is obfuscating the
374 /// **Known problems:** None
385 "any loop that will always `break` or `return`"
388 declare_clippy_lint! {
389 /// **What it does:** Checks for loops which have a range bound that is a mutable variable
391 /// **Why is this bad?** One might think that modifying the mutable variable changes the loop bounds
393 /// **Known problems:** None
397 /// let mut foo = 42;
398 /// for i in 0..foo {
400 /// println!("{}", i); // prints numbers from 0 to 42, not 0 to 21
405 "for loop over a range where one of the bounds is a mutable variable"
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.
412 /// **Why is this bad?** If the condition is unchanged, entering the body of the loop
413 /// will lead to an infinite loop.
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.
423 /// println!("let me loop forever!");
426 pub WHILE_IMMUTABLE_CONDITION,
428 "variables used within while expression are not mutated in the body"
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.
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
444 /// let mut vec: Vec<u8> = Vec::new();
452 /// could be written as
456 /// let mut vec: Vec<u8> = vec![item1; 20];
457 /// vec.resize(20 + 30, item2);
461 "the same item is pushed inside of a for loop"
464 declare_clippy_lint! {
465 /// **What it does:** Checks whether a for loop has a single element.
467 /// **Why is this bad?** There is no reason to have a loop of a
469 /// **Known problems:** None
474 /// for item in &[item1] {
475 /// println!("{}", item);
478 /// could be written as
481 /// let item = &item1;
482 /// println!("{}", item);
484 pub SINGLE_ELEMENT_LOOP,
486 "there is no reason to have a single element loop"
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.
493 /// **Why is this bad?** It is verbose and can be simplified
494 /// by first calling the `flatten` method on the `Iterator`.
496 /// **Known problems:** None.
501 /// let x = vec![Some(1), Some(2), Some(3)];
503 /// if let Some(n) = n {
504 /// println!("{}", n);
510 /// let x = vec![Some(1), Some(2), Some(3)];
511 /// for n in x.into_iter().flatten() {
512 /// println!("{}", n);
517 "for loops over `Option`s or `Result`s with a single expression can be simplified"
520 declare_lint_pass!(Loops => [
525 EXPLICIT_INTO_ITER_LOOP,
527 FOR_LOOPS_OVER_FALLIBLES,
530 EXPLICIT_COUNTER_LOOP,
532 WHILE_LET_ON_ITERATOR,
536 WHILE_IMMUTABLE_CONDITION,
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() {
551 check_for_loop(cx, pat, arg, body, expr, span);
554 // we don't want to check expanded macros
555 if expr.span.from_expansion() {
559 // check for never_loop
560 never_loop::check(cx, expr);
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);
571 while_let_on_iterator::check(cx, expr);
573 if let Some((cond, body)) = higher::while_loop(&expr) {
574 while_immutable_condition::check(cx, cond, body);
577 needless_collect::check(expr, cx);
581 fn check_for_loop<'tcx>(
582 cx: &LateContext<'tcx>,
585 body: &'tcx Expr<'_>,
586 expr: &'tcx Expr<'_>,
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);
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);
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
605 if let ExprKind::MethodCall(ref method, _, ref args, _) = arg.kind {
606 // just the receiver, no arguments
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
611 "iter" | "iter_mut" => explicit_iter_loop::check(cx, args, arg, method_name),
613 explicit_iter_loop::check(cx, args, arg, method_name);
614 explicit_into_iter_loop::check(cx, args, arg);
617 next_loop_linted = iter_next_loop::check(cx, arg, expr);
624 if !next_loop_linted {
625 for_loops_over_fallibles::check(cx, pat, arg);