diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 8a12530cb0d..6d9519911eb 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1942,14 +1942,16 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> { } } } else if is_loop(expr) { - self.states.clear(); - self.done = true; + walk_expr(self, expr); return; } else if is_conditional(expr) { self.depth += 1; walk_expr(self, expr); self.depth -= 1; return; + } else if let ExprKind::Continue(_) = expr.node { + self.done = true; + return; } walk_expr(self, expr); } diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop.rs index 39eee64883c..55060b0769d 100644 --- a/tests/ui/for_loop.rs +++ b/tests/ui/for_loop.rs @@ -571,3 +571,68 @@ mod issue_2496 { unimplemented!() } } + +mod issue_1219 { + #[warn(clippy::explicit_counter_loop)] + pub fn test() { + // should not trigger the lint because variable is used after the loop #473 + let vec = vec![1,2,3]; + let mut index = 0; + for _v in &vec { index += 1 } + println!("index: {}", index); + + // should not trigger the lint because the count is conditional #1219 + let text = "banana"; + let mut count = 0; + for ch in text.chars() { + if ch == 'a' { + continue; + } + count += 1; + println!("{}", count); + } + + // should not trigger the lint because the count is conditional + let text = "banana"; + let mut count = 0; + for ch in text.chars() { + if ch == 'a' { + count += 1; + } + println!("{}", count); + } + + // should trigger the lint because the count is not conditional + let text = "banana"; + let mut count = 0; + for ch in text.chars() { + count += 1; + if ch == 'a' { + continue; + } + println!("{}", count); + } + + // should trigger the lint because the count is not conditional + let text = "banana"; + let mut count = 0; + for ch in text.chars() { + count += 1; + for i in 0..2 { + let _ = 123; + } + println!("{}", count); + } + + // should not trigger the lint because the count is incremented multiple times + let text = "banana"; + let mut count = 0; + for ch in text.chars() { + count += 1; + for i in 0..2 { + count += 1; + } + println!("{}", count); + } + } +} diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index bab7bdc77a4..d829147541e 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop.stderr @@ -487,5 +487,17 @@ error: it looks like you're manually copying between slices 547 | for i in 0..src.len() { | ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])` -error: aborting due to 59 previous errors +error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators + --> $DIR/for_loop.rs:608:19 + | +608 | for ch in text.chars() { + | ^^^^^^^^^^^^ + +error: the variable `count` is used as a loop counter. Consider using `for (count, item) in text.chars().enumerate()` or similar iterators + --> $DIR/for_loop.rs:619:19 + | +619 | for ch in text.chars() { + | ^^^^^^^^^^^^ + +error: aborting due to 61 previous errors