about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2020-09-24 13:45:24 +0000
committerbors <bors@rust-lang.org>2020-09-24 13:45:24 +0000
commit019c0d5f7f90f959ff92684a04b0d766b22527a5 (patch)
tree08ccf7ed491edf68bf61a4d936b31b4be0e0f870
parente636b88aa180e8cab9e28802aac90adbc984234d (diff)
parent5e393c7747d081c45414060f81016e9ea3cb961f (diff)
downloadrust-019c0d5f7f90f959ff92684a04b0d766b22527a5.tar.gz
rust-019c0d5f7f90f959ff92684a04b0d766b22527a5.zip
Auto merge of #6076 - rail-rain:fix_fp_explicit_counter_loop, r=matthiaskrgr
Fix a FP in `explicit_counter_loop`

Fixes #4677 and #6074

Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented, adjust the test so that counters are incremented at the end of the loop and add the test for this false positive.

---

changelog: Fix a false positive in `explicit_counter_loop` where the loop counter is used after incremented
-rw-r--r--clippy_lints/src/loops.rs6
-rw-r--r--tests/ui/explicit_counter_loop.rs29
2 files changed, 26 insertions, 9 deletions
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 3410341a1e3..7f998c63f49 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2134,7 +2134,7 @@ enum VarState {
     DontWarn,
 }
 
-/// Scan a for loop for variables that are incremented exactly once.
+/// Scan a for loop for variables that are incremented exactly once and not used after that.
 struct IncrementVisitor<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,          // context reference
     states: FxHashMap<HirId, VarState>, // incremented variables
@@ -2154,6 +2154,10 @@ impl<'a, 'tcx> Visitor<'tcx> for IncrementVisitor<'a, 'tcx> {
         if let Some(def_id) = var_def_id(self.cx, expr) {
             if let Some(parent) = get_parent_expr(self.cx, expr) {
                 let state = self.states.entry(def_id).or_insert(VarState::Initial);
+                if *state == VarState::IncrOnce {
+                    *state = VarState::DontWarn;
+                    return;
+                }
 
                 match parent.kind {
                     ExprKind::AssignOp(op, ref lhs, ref rhs) => {
diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs
index aa6ef162fe4..81d8221bd13 100644
--- a/tests/ui/explicit_counter_loop.rs
+++ b/tests/ui/explicit_counter_loop.rs
@@ -38,54 +38,54 @@ mod issue_1219 {
         let text = "banana";
         let mut count = 0;
         for ch in text.chars() {
+            println!("{}", count);
             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() {
+            println!("{}", count);
             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() {
+            println!("{}", count);
             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() {
+            println!("{}", count);
             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() {
+            println!("{}", count);
             count += 1;
             for i in 0..2 {
                 count += 1;
             }
-            println!("{}", count);
         }
     }
 }
@@ -96,30 +96,30 @@ mod issue_3308 {
         let mut skips = 0;
         let erasures = vec![];
         for i in 0..10 {
+            println!("{}", skips);
             while erasures.contains(&(i + skips)) {
                 skips += 1;
             }
-            println!("{}", skips);
         }
 
         // should not trigger the lint because the count is incremented multiple times
         let mut skips = 0;
         for i in 0..10 {
+            println!("{}", skips);
             let mut j = 0;
             while j < 5 {
                 skips += 1;
                 j += 1;
             }
-            println!("{}", skips);
         }
 
         // should not trigger the lint because the count is incremented multiple times
         let mut skips = 0;
         for i in 0..10 {
+            println!("{}", skips);
             for j in 0..5 {
                 skips += 1;
             }
-            println!("{}", skips);
         }
     }
 }
@@ -145,3 +145,16 @@ mod issue_4732 {
         let _closure = || println!("index: {}", index);
     }
 }
+
+mod issue_4677 {
+    pub fn test() {
+        let slice = &[1, 2, 3];
+
+        // should not trigger the lint because the count is used after incremented
+        let mut count = 0;
+        for _i in slice {
+            count += 1;
+            println!("{}", count);
+        }
+    }
+}