Merge pull request #3135 from JoshMcguigan/explicit_counter_loop-1219

Closes #1219 false positive for explicit_counter_loop

Author: flip1995

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);
     }

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);
+        }
+    }
+}

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