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