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
! {
30 /// Checks for for-loops that manually copy items between
31 /// slices that could be optimized by having a memcpy.
33 /// ### Why is this bad?
34 /// It is not as fast as a memcpy.
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
! {
57 /// Checks for looping over the range of `0..len` of some
58 /// collection just to get the values by index.
60 /// ### Why is this bad?
61 /// Just iterating the collection itself makes the intent
62 /// more clear and is probably faster.
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
! {
85 /// Checks for loops on `x.iter()` where `&x` will do, and
86 /// suggests the latter.
88 /// ### Why is this bad?
91 /// ### Known problems
92 /// False negatives. We currently only warn on some known
97 /// // with `y` a `Vec` or slice:
98 /// # let y = vec![1];
99 /// for x in y.iter() {
103 /// can be rewritten to
105 /// # let y = vec![1];
110 pub EXPLICIT_ITER_LOOP
,
112 "for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do"
115 declare_clippy_lint
! {
117 /// Checks for loops on `y.into_iter()` where `y` will do, and
118 /// suggests the latter.
120 /// ### Why is this bad?
125 /// # let y = vec![1];
126 /// // with `y` a `Vec` or slice:
127 /// for x in y.into_iter() {
131 /// can be rewritten to
133 /// # let y = vec![1];
138 pub EXPLICIT_INTO_ITER_LOOP
,
140 "for-looping over `_.into_iter()` when `_` would do"
143 declare_clippy_lint
! {
145 /// Checks for loops on `x.next()`.
147 /// ### Why is this bad?
148 /// `next()` returns either `Some(value)` if there was a
149 /// value, or `None` otherwise. The insidious thing is that `Option<_>`
150 /// implements `IntoIterator`, so that possibly one value will be iterated,
151 /// leading to some hard to find bugs. No one will want to write such code
152 /// [except to win an Underhanded Rust
153 /// Contest](https://www.reddit.com/r/rust/comments/3hb0wm/underhanded_rust_contest/cu5yuhr).
157 /// for x in y.next() {
163 "for-looping over `_.next()` which is probably not intended"
166 declare_clippy_lint
! {
168 /// Checks for `for` loops over `Option` or `Result` values.
170 /// ### Why is this bad?
171 /// Readability. This is more clearly expressed as an `if
176 /// # let opt = Some(1);
184 /// if let Some(x) = opt {
192 /// # let res: Result<i32, std::io::Error> = Ok(1);
200 /// if let Ok(x) = res {
204 pub FOR_LOOPS_OVER_FALLIBLES
,
206 "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
209 declare_clippy_lint
! {
211 /// Detects `loop + match` combinations that are easier
212 /// written as a `while let` loop.
214 /// ### Why is this bad?
215 /// The `while let` loop is usually shorter and more
218 /// ### Known problems
219 /// Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)).
223 /// # let y = Some(1);
225 /// let x = match y {
229 /// // .. do something with x
231 /// // is easier written as
232 /// while let Some(x) = y {
233 /// // .. do something with x
238 "`loop { if let { ... } else break }`, which can be written as a `while let` loop"
241 declare_clippy_lint
! {
243 /// Checks for functions collecting an iterator when collect
246 /// ### Why is this bad?
247 /// `collect` causes the allocation of a new data structure,
248 /// when this allocation may not be needed.
252 /// # let iterator = vec![1].into_iter();
253 /// let len = iterator.clone().collect::<Vec<_>>().len();
255 /// let len = iterator.count();
257 pub NEEDLESS_COLLECT
,
259 "collecting an iterator when collect is not needed"
262 declare_clippy_lint
! {
264 /// Checks `for` loops over slices with an explicit counter
265 /// and suggests the use of `.enumerate()`.
267 /// ### Why is this bad?
268 /// Using `.enumerate()` makes the intent more clear,
269 /// declutters the code and may be faster in some instances.
273 /// # let v = vec![1];
274 /// # fn bar(bar: usize, baz: usize) {}
281 /// Could be written as
283 /// # let v = vec![1];
284 /// # fn bar(bar: usize, baz: usize) {}
285 /// for (i, item) in v.iter().enumerate() { bar(i, *item); }
287 pub EXPLICIT_COUNTER_LOOP
,
289 "for-looping with an explicit counter when `_.enumerate()` would do"
292 declare_clippy_lint
! {
294 /// Checks for empty `loop` expressions.
296 /// ### Why is this bad?
297 /// These busy loops burn CPU cycles without doing
298 /// anything. It is _almost always_ a better idea to `panic!` than to have
301 /// If panicking isn't possible, think of the environment and either:
302 /// - block on something
303 /// - sleep the thread for some microseconds
304 /// - yield or pause the thread
306 /// For `std` targets, this can be done with
307 /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
308 /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
310 /// For `no_std` targets, doing this is more complicated, especially because
311 /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
312 /// probably need to invoke some target-specific intrinsic. Examples include:
313 /// - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
314 /// - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
322 "empty `loop {}`, which should block or sleep"
325 declare_clippy_lint
! {
327 /// Checks for `while let` expressions on iterators.
329 /// ### Why is this bad?
330 /// Readability. A simple `for` loop is shorter and conveys
331 /// the intent better.
335 /// while let Some(val) = iter() {
339 pub WHILE_LET_ON_ITERATOR
,
341 "using a `while let` loop instead of a for loop on an iterator"
344 declare_clippy_lint
! {
346 /// Checks for iterating a map (`HashMap` or `BTreeMap`) and
347 /// ignoring either the keys or values.
349 /// ### Why is this bad?
350 /// Readability. There are `keys` and `values` methods that
351 /// can be used to express that don't need the values or keys.
355 /// for (k, _) in &map {
360 /// could be replaced by
363 /// for k in map.keys() {
369 "looping on a map using `iter` when `keys` or `values` would do"
372 declare_clippy_lint
! {
374 /// Checks for loops that will always `break`, `return` or
375 /// `continue` an outer loop.
377 /// ### Why is this bad?
378 /// This loop never loops, all it does is obfuscating the
390 "any loop that will always `break` or `return`"
393 declare_clippy_lint
! {
395 /// Checks for loops which have a range bound that is a mutable variable
397 /// ### Why is this bad?
398 /// One might think that modifying the mutable variable changes the loop bounds
402 /// let mut foo = 42;
403 /// for i in 0..foo {
405 /// println!("{}", i); // prints numbers from 0 to 42, not 0 to 21
410 "for loop over a range where one of the bounds is a mutable variable"
413 declare_clippy_lint
! {
415 /// Checks whether variables used within while loop condition
416 /// can be (and are) mutated in the body.
418 /// ### Why is this bad?
419 /// If the condition is unchanged, entering the body of the loop
420 /// will lead to an infinite loop.
422 /// ### Known problems
423 /// If the `while`-loop is in a closure, the check for mutation of the
424 /// condition variables in the body can cause false negatives. For example when only `Upvar` `a` is
425 /// in the condition and only `Upvar` `b` gets mutated in the body, the lint will not trigger.
431 /// println!("let me loop forever!");
434 pub WHILE_IMMUTABLE_CONDITION
,
436 "variables used within while expression are not mutated in the body"
439 declare_clippy_lint
! {
441 /// Checks whether a for loop is being used to push a constant
442 /// value into a Vec.
444 /// ### Why is this bad?
445 /// This kind of operation can be expressed more succinctly with
446 /// `vec![item;SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
447 /// have better performance.
453 /// let mut vec: Vec<u8> = Vec::new();
461 /// could be written as
465 /// let mut vec: Vec<u8> = vec![item1; 20];
466 /// vec.resize(20 + 30, item2);
470 "the same item is pushed inside of a for loop"
473 declare_clippy_lint
! {
475 /// Checks whether a for loop has a single element.
477 /// ### Why is this bad?
478 /// There is no reason to have a loop of a
484 /// for item in &[item1] {
485 /// println!("{}", item);
488 /// could be written as
491 /// let item = &item1;
492 /// println!("{}", item);
494 pub SINGLE_ELEMENT_LOOP
,
496 "there is no reason to have a single element loop"
499 declare_clippy_lint
! {
501 /// Check for unnecessary `if let` usage in a for loop
502 /// where only the `Some` or `Ok` variant of the iterator element is used.
504 /// ### Why is this bad?
505 /// It is verbose and can be simplified
506 /// by first calling the `flatten` method on the `Iterator`.
511 /// let x = vec![Some(1), Some(2), Some(3)];
513 /// if let Some(n) = n {
514 /// println!("{}", n);
520 /// let x = vec![Some(1), Some(2), Some(3)];
521 /// for n in x.into_iter().flatten() {
522 /// println!("{}", n);
527 "for loops over `Option`s or `Result`s with a single expression can be simplified"
530 declare_lint_pass
!(Loops
=> [
535 EXPLICIT_INTO_ITER_LOOP
,
537 FOR_LOOPS_OVER_FALLIBLES
,
540 EXPLICIT_COUNTER_LOOP
,
542 WHILE_LET_ON_ITERATOR
,
546 WHILE_IMMUTABLE_CONDITION
,
551 impl<'tcx
> LateLintPass
<'tcx
> for Loops
{
552 #[allow(clippy::too_many_lines)]
553 fn check_expr(&mut self, cx
: &LateContext
<'tcx
>, expr
: &'tcx Expr
<'_
>) {
554 if let Some(higher
::ForLoop { pat, arg, body, span }
) = higher
::ForLoop
::hir(expr
) {
555 // we don't want to check expanded macros
556 // this check is not at the top of the function
557 // since higher::for_loop expressions are marked as expansions
558 if body
.span
.from_expansion() {
561 check_for_loop(cx
, pat
, arg
, body
, expr
, span
);
564 // we don't want to check expanded macros
565 if expr
.span
.from_expansion() {
569 // check for never_loop
570 never_loop
::check(cx
, expr
);
572 // check for `loop { if let {} else break }` that could be `while let`
573 // (also matches an explicit "match" instead of "if let")
574 // (even if the "match" or "if let" is used for declaration)
575 if let ExprKind
::Loop(block
, _
, LoopSource
::Loop
, _
) = expr
.kind
{
576 // also check for empty `loop {}` statements, skipping those in #[panic_handler]
577 empty_loop
::check(cx
, expr
, block
);
578 while_let_loop
::check(cx
, expr
, block
);
581 while_let_on_iterator
::check(cx
, expr
);
583 if let Some(higher
::While { if_cond, if_then, .. }
) = higher
::While
::hir(&expr
) {
584 while_immutable_condition
::check(cx
, if_cond
, if_then
);
587 needless_collect
::check(expr
, cx
);
591 fn check_for_loop
<'tcx
>(
592 cx
: &LateContext
<'tcx
>,
595 body
: &'tcx Expr
<'_
>,
596 expr
: &'tcx Expr
<'_
>,
599 let is_manual_memcpy_triggered
= manual_memcpy
::check(cx
, pat
, arg
, body
, expr
);
600 if !is_manual_memcpy_triggered
{
601 needless_range_loop
::check(cx
, pat
, arg
, body
, expr
);
602 explicit_counter_loop
::check(cx
, pat
, arg
, body
, expr
);
604 check_for_loop_arg(cx
, pat
, arg
, expr
);
605 for_kv_map
::check(cx
, pat
, arg
, body
, expr
);
606 mut_range_bound
::check(cx
, arg
, body
);
607 single_element_loop
::check(cx
, pat
, arg
, body
, expr
);
608 same_item_push
::check(cx
, pat
, arg
, body
, expr
);
609 manual_flatten
::check(cx
, pat
, arg
, body
, span
);
612 fn check_for_loop_arg(cx
: &LateContext
<'_
>, pat
: &Pat
<'_
>, arg
: &Expr
<'_
>, expr
: &Expr
<'_
>) {
613 let mut next_loop_linted
= false; // whether or not ITER_NEXT_LOOP lint was used
615 if let ExprKind
::MethodCall(method
, _
, [self_arg
], _
) = arg
.kind
{
616 let method_name
= &*method
.ident
.as_str();
617 // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
619 "iter" | "iter_mut" => explicit_iter_loop
::check(cx
, self_arg
, arg
, method_name
),
621 explicit_iter_loop
::check(cx
, self_arg
, arg
, method_name
);
622 explicit_into_iter_loop
::check(cx
, self_arg
, arg
);
625 next_loop_linted
= iter_next_loop
::check(cx
, arg
, expr
);
631 if !next_loop_linted
{
632 for_loops_over_fallibles
::check(cx
, pat
, arg
);