]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/loops/mod.rs
391de922e1e1a71b6d22b4ac05fcca89abcf8fa6
[rust.git] / 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 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     /// Checks for `for` loops over `Option` or `Result` values.
178     ///
179     /// ### Why is this bad?
180     /// Readability. This is more clearly expressed as an `if
181     /// let`.
182     ///
183     /// ### Example
184     /// ```rust
185     /// # let opt = Some(1);
186     /// # let res: Result<i32, std::io::Error> = Ok(1);
187     /// for x in opt {
188     ///     // ..
189     /// }
190     ///
191     /// for x in &res {
192     ///     // ..
193     /// }
194     ///
195     /// for x in res.iter() {
196     ///     // ..
197     /// }
198     /// ```
199     ///
200     /// Use instead:
201     /// ```rust
202     /// # let opt = Some(1);
203     /// # let res: Result<i32, std::io::Error> = Ok(1);
204     /// if let Some(x) = opt {
205     ///     // ..
206     /// }
207     ///
208     /// if let Ok(x) = res {
209     ///     // ..
210     /// }
211     /// ```
212     #[clippy::version = "1.45.0"]
213     pub FOR_LOOPS_OVER_FALLIBLES,
214     suspicious,
215     "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
216 }
217
218 declare_clippy_lint! {
219     /// ### What it does
220     /// Detects `loop + match` combinations that are easier
221     /// written as a `while let` loop.
222     ///
223     /// ### Why is this bad?
224     /// The `while let` loop is usually shorter and more
225     /// readable.
226     ///
227     /// ### Known problems
228     /// Sometimes the wrong binding is displayed ([#383](https://github.com/rust-lang/rust-clippy/issues/383)).
229     ///
230     /// ### Example
231     /// ```rust,no_run
232     /// # let y = Some(1);
233     /// loop {
234     ///     let x = match y {
235     ///         Some(x) => x,
236     ///         None => break,
237     ///     };
238     ///     // .. do something with x
239     /// }
240     /// // is easier written as
241     /// while let Some(x) = y {
242     ///     // .. do something with x
243     /// };
244     /// ```
245     #[clippy::version = "pre 1.29.0"]
246     pub WHILE_LET_LOOP,
247     complexity,
248     "`loop { if let { ... } else break }`, which can be written as a `while let` loop"
249 }
250
251 declare_clippy_lint! {
252     /// ### What it does
253     /// Checks for functions collecting an iterator when collect
254     /// is not needed.
255     ///
256     /// ### Why is this bad?
257     /// `collect` causes the allocation of a new data structure,
258     /// when this allocation may not be needed.
259     ///
260     /// ### Example
261     /// ```rust
262     /// # let iterator = vec![1].into_iter();
263     /// let len = iterator.clone().collect::<Vec<_>>().len();
264     /// // should be
265     /// let len = iterator.count();
266     /// ```
267     #[clippy::version = "1.30.0"]
268     pub NEEDLESS_COLLECT,
269     perf,
270     "collecting an iterator when collect is not needed"
271 }
272
273 declare_clippy_lint! {
274     /// ### What it does
275     /// Checks `for` loops over slices with an explicit counter
276     /// and suggests the use of `.enumerate()`.
277     ///
278     /// ### Why is this bad?
279     /// Using `.enumerate()` makes the intent more clear,
280     /// declutters the code and may be faster in some instances.
281     ///
282     /// ### Example
283     /// ```rust
284     /// # let v = vec![1];
285     /// # fn bar(bar: usize, baz: usize) {}
286     /// let mut i = 0;
287     /// for item in &v {
288     ///     bar(i, *item);
289     ///     i += 1;
290     /// }
291     /// ```
292     ///
293     /// Use instead:
294     /// ```rust
295     /// # let v = vec![1];
296     /// # fn bar(bar: usize, baz: usize) {}
297     /// for (i, item) in v.iter().enumerate() { bar(i, *item); }
298     /// ```
299     #[clippy::version = "pre 1.29.0"]
300     pub EXPLICIT_COUNTER_LOOP,
301     complexity,
302     "for-looping with an explicit counter when `_.enumerate()` would do"
303 }
304
305 declare_clippy_lint! {
306     /// ### What it does
307     /// Checks for empty `loop` expressions.
308     ///
309     /// ### Why is this bad?
310     /// These busy loops burn CPU cycles without doing
311     /// anything. It is _almost always_ a better idea to `panic!` than to have
312     /// a busy loop.
313     ///
314     /// If panicking isn't possible, think of the environment and either:
315     ///   - block on something
316     ///   - sleep the thread for some microseconds
317     ///   - yield or pause the thread
318     ///
319     /// For `std` targets, this can be done with
320     /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
321     /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
322     ///
323     /// For `no_std` targets, doing this is more complicated, especially because
324     /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
325     /// probably need to invoke some target-specific intrinsic. Examples include:
326     ///   - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
327     ///   - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
328     ///
329     /// ### Example
330     /// ```no_run
331     /// loop {}
332     /// ```
333     #[clippy::version = "pre 1.29.0"]
334     pub EMPTY_LOOP,
335     suspicious,
336     "empty `loop {}`, which should block or sleep"
337 }
338
339 declare_clippy_lint! {
340     /// ### What it does
341     /// Checks for `while let` expressions on iterators.
342     ///
343     /// ### Why is this bad?
344     /// Readability. A simple `for` loop is shorter and conveys
345     /// the intent better.
346     ///
347     /// ### Example
348     /// ```ignore
349     /// while let Some(val) = iter() {
350     ///     ..
351     /// }
352     /// ```
353     #[clippy::version = "pre 1.29.0"]
354     pub WHILE_LET_ON_ITERATOR,
355     style,
356     "using a `while let` loop instead of a for loop on an iterator"
357 }
358
359 declare_clippy_lint! {
360     /// ### What it does
361     /// Checks for iterating a map (`HashMap` or `BTreeMap`) and
362     /// ignoring either the keys or values.
363     ///
364     /// ### Why is this bad?
365     /// Readability. There are `keys` and `values` methods that
366     /// can be used to express that don't need the values or keys.
367     ///
368     /// ### Example
369     /// ```ignore
370     /// for (k, _) in &map {
371     ///     ..
372     /// }
373     /// ```
374     ///
375     /// could be replaced by
376     ///
377     /// ```ignore
378     /// for k in map.keys() {
379     ///     ..
380     /// }
381     /// ```
382     #[clippy::version = "pre 1.29.0"]
383     pub FOR_KV_MAP,
384     style,
385     "looping on a map using `iter` when `keys` or `values` would do"
386 }
387
388 declare_clippy_lint! {
389     /// ### What it does
390     /// Checks for loops that will always `break`, `return` or
391     /// `continue` an outer loop.
392     ///
393     /// ### Why is this bad?
394     /// This loop never loops, all it does is obfuscating the
395     /// code.
396     ///
397     /// ### Example
398     /// ```rust
399     /// loop {
400     ///     ..;
401     ///     break;
402     /// }
403     /// ```
404     #[clippy::version = "pre 1.29.0"]
405     pub NEVER_LOOP,
406     correctness,
407     "any loop that will always `break` or `return`"
408 }
409
410 declare_clippy_lint! {
411     /// ### What it does
412     /// Checks for loops which have a range bound that is a mutable variable
413     ///
414     /// ### Why is this bad?
415     /// One might think that modifying the mutable variable changes the loop bounds
416     ///
417     /// ### Known problems
418     /// False positive when mutation is followed by a `break`, but the `break` is not immediately
419     /// after the mutation:
420     ///
421     /// ```rust
422     /// let mut x = 5;
423     /// for _ in 0..x {
424     ///     x += 1; // x is a range bound that is mutated
425     ///     ..; // some other expression
426     ///     break; // leaves the loop, so mutation is not an issue
427     /// }
428     /// ```
429     ///
430     /// False positive on nested loops ([#6072](https://github.com/rust-lang/rust-clippy/issues/6072))
431     ///
432     /// ### Example
433     /// ```rust
434     /// let mut foo = 42;
435     /// for i in 0..foo {
436     ///     foo -= 1;
437     ///     println!("{}", i); // prints numbers from 0 to 42, not 0 to 21
438     /// }
439     /// ```
440     #[clippy::version = "pre 1.29.0"]
441     pub MUT_RANGE_BOUND,
442     suspicious,
443     "for loop over a range where one of the bounds is a mutable variable"
444 }
445
446 declare_clippy_lint! {
447     /// ### What it does
448     /// Checks whether variables used within while loop condition
449     /// can be (and are) mutated in the body.
450     ///
451     /// ### Why is this bad?
452     /// If the condition is unchanged, entering the body of the loop
453     /// will lead to an infinite loop.
454     ///
455     /// ### Known problems
456     /// If the `while`-loop is in a closure, the check for mutation of the
457     /// condition variables in the body can cause false negatives. For example when only `Upvar` `a` is
458     /// in the condition and only `Upvar` `b` gets mutated in the body, the lint will not trigger.
459     ///
460     /// ### Example
461     /// ```rust
462     /// let i = 0;
463     /// while i > 10 {
464     ///     println!("let me loop forever!");
465     /// }
466     /// ```
467     #[clippy::version = "pre 1.29.0"]
468     pub WHILE_IMMUTABLE_CONDITION,
469     correctness,
470     "variables used within while expression are not mutated in the body"
471 }
472
473 declare_clippy_lint! {
474     /// ### What it does
475     /// Checks whether a for loop is being used to push a constant
476     /// value into a Vec.
477     ///
478     /// ### Why is this bad?
479     /// This kind of operation can be expressed more succinctly with
480     /// `vec![item; SIZE]` or `vec.resize(NEW_SIZE, item)` and using these alternatives may also
481     /// have better performance.
482     ///
483     /// ### Example
484     /// ```rust
485     /// let item1 = 2;
486     /// let item2 = 3;
487     /// let mut vec: Vec<u8> = Vec::new();
488     /// for _ in 0..20 {
489     ///    vec.push(item1);
490     /// }
491     /// for _ in 0..30 {
492     ///     vec.push(item2);
493     /// }
494     /// ```
495     ///
496     /// Use instead:
497     /// ```rust
498     /// let item1 = 2;
499     /// let item2 = 3;
500     /// let mut vec: Vec<u8> = vec![item1; 20];
501     /// vec.resize(20 + 30, item2);
502     /// ```
503     #[clippy::version = "1.47.0"]
504     pub SAME_ITEM_PUSH,
505     style,
506     "the same item is pushed inside of a for loop"
507 }
508
509 declare_clippy_lint! {
510     /// ### What it does
511     /// Checks whether a for loop has a single element.
512     ///
513     /// ### Why is this bad?
514     /// There is no reason to have a loop of a
515     /// single element.
516     ///
517     /// ### Example
518     /// ```rust
519     /// let item1 = 2;
520     /// for item in &[item1] {
521     ///     println!("{}", item);
522     /// }
523     /// ```
524     ///
525     /// Use instead:
526     /// ```rust
527     /// let item1 = 2;
528     /// let item = &item1;
529     /// println!("{}", item);
530     /// ```
531     #[clippy::version = "1.49.0"]
532     pub SINGLE_ELEMENT_LOOP,
533     complexity,
534     "there is no reason to have a single element loop"
535 }
536
537 declare_clippy_lint! {
538     /// ### What it does
539     /// Check for unnecessary `if let` usage in a for loop
540     /// where only the `Some` or `Ok` variant of the iterator element is used.
541     ///
542     /// ### Why is this bad?
543     /// It is verbose and can be simplified
544     /// by first calling the `flatten` method on the `Iterator`.
545     ///
546     /// ### Example
547     ///
548     /// ```rust
549     /// let x = vec![Some(1), Some(2), Some(3)];
550     /// for n in x {
551     ///     if let Some(n) = n {
552     ///         println!("{}", n);
553     ///     }
554     /// }
555     /// ```
556     /// Use instead:
557     /// ```rust
558     /// let x = vec![Some(1), Some(2), Some(3)];
559     /// for n in x.into_iter().flatten() {
560     ///     println!("{}", n);
561     /// }
562     /// ```
563     #[clippy::version = "1.52.0"]
564     pub MANUAL_FLATTEN,
565     complexity,
566     "for loops over `Option`s or `Result`s with a single expression can be simplified"
567 }
568
569 declare_clippy_lint! {
570     /// ### What it does
571     /// Check for empty spin loops
572     ///
573     /// ### Why is this bad?
574     /// The loop body should have something like `thread::park()` or at least
575     /// `std::hint::spin_loop()` to avoid needlessly burning cycles and conserve
576     /// energy. Perhaps even better use an actual lock, if possible.
577     ///
578     /// ### Known problems
579     /// This lint doesn't currently trigger on `while let` or
580     /// `loop { match .. { .. } }` loops, which would be considered idiomatic in
581     /// combination with e.g. `AtomicBool::compare_exchange_weak`.
582     ///
583     /// ### Example
584     ///
585     /// ```ignore
586     /// use core::sync::atomic::{AtomicBool, Ordering};
587     /// let b = AtomicBool::new(true);
588     /// // give a ref to `b` to another thread,wait for it to become false
589     /// while b.load(Ordering::Acquire) {};
590     /// ```
591     /// Use instead:
592     /// ```rust,no_run
593     ///# use core::sync::atomic::{AtomicBool, Ordering};
594     ///# let b = AtomicBool::new(true);
595     /// while b.load(Ordering::Acquire) {
596     ///     std::hint::spin_loop()
597     /// }
598     /// ```
599     #[clippy::version = "1.61.0"]
600     pub MISSING_SPIN_LOOP,
601     perf,
602     "An empty busy waiting loop"
603 }
604
605 declare_lint_pass!(Loops => [
606     MANUAL_MEMCPY,
607     MANUAL_FLATTEN,
608     NEEDLESS_RANGE_LOOP,
609     EXPLICIT_ITER_LOOP,
610     EXPLICIT_INTO_ITER_LOOP,
611     ITER_NEXT_LOOP,
612     FOR_LOOPS_OVER_FALLIBLES,
613     WHILE_LET_LOOP,
614     NEEDLESS_COLLECT,
615     EXPLICIT_COUNTER_LOOP,
616     EMPTY_LOOP,
617     WHILE_LET_ON_ITERATOR,
618     FOR_KV_MAP,
619     NEVER_LOOP,
620     MUT_RANGE_BOUND,
621     WHILE_IMMUTABLE_CONDITION,
622     SAME_ITEM_PUSH,
623     SINGLE_ELEMENT_LOOP,
624     MISSING_SPIN_LOOP,
625 ]);
626
627 impl<'tcx> LateLintPass<'tcx> for Loops {
628     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
629         let for_loop = higher::ForLoop::hir(expr);
630         if let Some(higher::ForLoop {
631             pat,
632             arg,
633             body,
634             loop_id,
635             span,
636         }) = for_loop
637         {
638             // we don't want to check expanded macros
639             // this check is not at the top of the function
640             // since higher::for_loop expressions are marked as expansions
641             if body.span.from_expansion() {
642                 return;
643             }
644             check_for_loop(cx, pat, arg, body, expr, span);
645             if let ExprKind::Block(block, _) = body.kind {
646                 never_loop::check(cx, block, loop_id, span, for_loop.as_ref());
647             }
648         }
649
650         // we don't want to check expanded macros
651         if expr.span.from_expansion() {
652             return;
653         }
654
655         // check for never_loop
656         if let ExprKind::Loop(block, ..) = expr.kind {
657             never_loop::check(cx, block, expr.hir_id, expr.span, None);
658         }
659
660         // check for `loop { if let {} else break }` that could be `while let`
661         // (also matches an explicit "match" instead of "if let")
662         // (even if the "match" or "if let" is used for declaration)
663         if let ExprKind::Loop(block, _, LoopSource::Loop, _) = expr.kind {
664             // also check for empty `loop {}` statements, skipping those in #[panic_handler]
665             empty_loop::check(cx, expr, block);
666             while_let_loop::check(cx, expr, block);
667         }
668
669         while_let_on_iterator::check(cx, expr);
670
671         if let Some(higher::While { condition, body }) = higher::While::hir(expr) {
672             while_immutable_condition::check(cx, condition, body);
673             missing_spin_loop::check(cx, condition, body);
674         }
675
676         needless_collect::check(expr, cx);
677     }
678 }
679
680 fn check_for_loop<'tcx>(
681     cx: &LateContext<'tcx>,
682     pat: &'tcx Pat<'_>,
683     arg: &'tcx Expr<'_>,
684     body: &'tcx Expr<'_>,
685     expr: &'tcx Expr<'_>,
686     span: Span,
687 ) {
688     let is_manual_memcpy_triggered = manual_memcpy::check(cx, pat, arg, body, expr);
689     if !is_manual_memcpy_triggered {
690         needless_range_loop::check(cx, pat, arg, body, expr);
691         explicit_counter_loop::check(cx, pat, arg, body, expr);
692     }
693     check_for_loop_arg(cx, pat, arg);
694     for_kv_map::check(cx, pat, arg, body);
695     mut_range_bound::check(cx, arg, body);
696     single_element_loop::check(cx, pat, arg, body, expr);
697     same_item_push::check(cx, pat, arg, body, expr);
698     manual_flatten::check(cx, pat, arg, body, span);
699 }
700
701 fn check_for_loop_arg(cx: &LateContext<'_>, pat: &Pat<'_>, arg: &Expr<'_>) {
702     let mut next_loop_linted = false; // whether or not ITER_NEXT_LOOP lint was used
703
704     if let ExprKind::MethodCall(method, [self_arg], _) = arg.kind {
705         let method_name = method.ident.as_str();
706         // check for looping over x.iter() or x.iter_mut(), could use &x or &mut x
707         match method_name {
708             "iter" | "iter_mut" => {
709                 explicit_iter_loop::check(cx, self_arg, arg, method_name);
710                 for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
711             },
712             "into_iter" => {
713                 explicit_iter_loop::check(cx, self_arg, arg, method_name);
714                 explicit_into_iter_loop::check(cx, self_arg, arg);
715                 for_loops_over_fallibles::check(cx, pat, self_arg, Some(method_name));
716             },
717             "next" => {
718                 next_loop_linted = iter_next_loop::check(cx, arg);
719             },
720             _ => {},
721         }
722     }
723
724     if !next_loop_linted {
725         for_loops_over_fallibles::check(cx, pat, arg, None);
726     }
727 }