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