for_loop: Split test into fixable/unfixable, make needless_range_loop use updated range syntax

clippy_lints/src/loops.rs

@@ -1272,7 +1272,7 @@ fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx
                     let start_snippet = snippet(cx, start.span, "_");
                     let end_snippet = snippet(cx, end.span, "_");
                     let dots = if limits == ast::RangeLimits::Closed {
-                        "..."
+                        "..="
                     } else {
                         ".."
                     };

tests/ui/for_loop_fixable.fixed

@@ -0,0 +1,304 @@
+// run-rustfix
+
+#![allow(dead_code, unused)]
+
+use std::collections::*;
+
+#[warn(clippy::all)]
+struct Unrelated(Vec<u8>);
+impl Unrelated {
+    fn next(&self) -> std::slice::Iter<u8> {
+        self.0.iter()
+    }
+
+    fn iter(&self) -> std::slice::Iter<u8> {
+        self.0.iter()
+    }
+}
+
+#[warn(
+    clippy::needless_range_loop,
+    clippy::explicit_iter_loop,
+    clippy::explicit_into_iter_loop,
+    clippy::iter_next_loop,
+    clippy::reverse_range_loop,
+    clippy::for_kv_map
+)]
+#[allow(
+    clippy::linkedlist,
+    clippy::shadow_unrelated,
+    clippy::unnecessary_mut_passed,
+    clippy::cognitive_complexity,
+    clippy::similar_names
+)]
+#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)]
+fn main() {
+    const MAX_LEN: usize = 42;
+    let mut vec = vec![1, 2, 3, 4];
+
+    for i in (0..10).rev() {
+        println!("{}", i);
+    }
+
+    for i in (0..=10).rev() {
+        println!("{}", i);
+    }
+
+    for i in (0..MAX_LEN).rev() {
+        println!("{}", i);
+    }
+
+    for i in 5..=5 {
+        // not an error, this is the range with only one element “5”
+        println!("{}", i);
+    }
+
+    for i in 0..10 {
+        // not an error, the start index is less than the end index
+        println!("{}", i);
+    }
+
+    for i in -10..0 {
+        // not an error
+        println!("{}", i);
+    }
+
+    for i in (10..0).map(|x| x * 2) {
+        // not an error, it can't be known what arbitrary methods do to a range
+        println!("{}", i);
+    }
+
+    // testing that the empty range lint folds constants
+    for i in (5 + 4..10).rev() {
+        println!("{}", i);
+    }
+
+    for i in ((3 - 1)..(5 + 2)).rev() {
+        println!("{}", i);
+    }
+
+    for i in (2 * 2)..(2 * 3) {
+        // no error, 4..6 is fine
+        println!("{}", i);
+    }
+
+    let x = 42;
+    for i in x..10 {
+        // no error, not constant-foldable
+        println!("{}", i);
+    }
+
+    // See #601
+    for i in 0..10 {
+        // no error, id_col does not exist outside the loop
+        let mut id_col = vec![0f64; 10];
+        id_col[i] = 1f64;
+    }
+
+    for _v in &vec {}
+
+    for _v in &mut vec {}
+
+    let out_vec = vec![1, 2, 3];
+    for _v in out_vec {}
+
+    let array = [1, 2, 3];
+    for _v in &array {}
+
+    for _v in &vec {} // these are fine
+    for _v in &mut vec {} // these are fine
+
+    for _v in &[1, 2, 3] {}
+
+    for _v in (&mut [1, 2, 3]).iter() {} // no error
+
+    for _v in &[0; 32] {}
+
+    for _v in [0; 33].iter() {} // no error
+
+    let ll: LinkedList<()> = LinkedList::new();
+    for _v in &ll {}
+
+    let vd: VecDeque<()> = VecDeque::new();
+    for _v in &vd {}
+
+    let bh: BinaryHeap<()> = BinaryHeap::new();
+    for _v in &bh {}
+
+    let hm: HashMap<(), ()> = HashMap::new();
+    for _v in &hm {}
+
+    let bt: BTreeMap<(), ()> = BTreeMap::new();
+    for _v in &bt {}
+
+    let hs: HashSet<()> = HashSet::new();
+    for _v in &hs {}
+
+    let bs: BTreeSet<()> = BTreeSet::new();
+    for _v in &bs {}
+
+    let u = Unrelated(vec![]);
+    for _v in u.next() {} // no error
+    for _v in u.iter() {} // no error
+
+    let mut out = vec![];
+    vec.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>();
+    let _y = vec.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>(); // this is fine
+
+    // Loop with explicit counter variable
+
+    // Potential false positives
+    let mut _index = 0;
+    _index = 1;
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut _index = 0;
+    _index += 1;
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut _index = 0;
+    if true {
+        _index = 1
+    }
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut _index = 0;
+    let mut _index = 1;
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut _index = 0;
+    for _v in &vec {
+        _index += 1;
+        _index += 1
+    }
+
+    let mut _index = 0;
+    for _v in &vec {
+        _index *= 2;
+        _index += 1
+    }
+
+    let mut _index = 0;
+    for _v in &vec {
+        _index = 1;
+        _index += 1
+    }
+
+    let mut _index = 0;
+
+    for _v in &vec {
+        let mut _index = 0;
+        _index += 1
+    }
+
+    let mut _index = 0;
+    for _v in &vec {
+        _index += 1;
+        _index = 0;
+    }
+
+    let mut _index = 0;
+    for _v in &vec {
+        for _x in 0..1 {
+            _index += 1;
+        }
+        _index += 1
+    }
+
+    let mut _index = 0;
+    for x in &vec {
+        if *x == 1 {
+            _index += 1
+        }
+    }
+
+    let mut _index = 0;
+    if true {
+        _index = 1
+    };
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut _index = 1;
+    if false {
+        _index = 0
+    };
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut index = 0;
+    {
+        let mut _x = &mut index;
+    }
+    for _v in &vec {
+        _index += 1
+    }
+
+    let mut index = 0;
+    for _v in &vec {
+        index += 1
+    }
+    println!("index: {}", index);
+
+    fn f<T>(_: &T, _: &T) -> bool {
+        unimplemented!()
+    }
+    fn g<T>(_: &mut [T], _: usize, _: usize) {
+        unimplemented!()
+    }
+    for i in 1..vec.len() {
+        if f(&vec[i - 1], &vec[i]) {
+            g(&mut vec, i - 1, i);
+        }
+    }
+
+    for mid in 1..vec.len() {
+        let (_, _) = vec.split_at(mid);
+    }
+}
+
+fn partition<T: PartialOrd + Send>(v: &mut [T]) -> usize {
+    let pivot = v.len() - 1;
+    let mut i = 0;
+    for j in 0..pivot {
+        if v[j] <= v[pivot] {
+            v.swap(i, j);
+            i += 1;
+        }
+    }
+    v.swap(i, pivot);
+    i
+}
+
+#[warn(clippy::needless_range_loop)]
+pub fn manual_copy_same_destination(dst: &mut [i32], d: usize, s: usize) {
+    // Same source and destination - don't trigger lint
+    for i in 0..dst.len() {
+        dst[d + i] = dst[s + i];
+    }
+}
+
+mod issue_2496 {
+    pub trait Handle {
+        fn new_for_index(index: usize) -> Self;
+        fn index(&self) -> usize;
+    }
+
+    pub fn test<H: Handle>() -> H {
+        for x in 0..5 {
+            let next_handle = H::new_for_index(x);
+            println!("{}", next_handle.index());
+        }
+        unimplemented!()
+    }
+}

tests/ui/for_loop_fixable.rs

@@ -1,8 +1,8 @@
-use std::collections::*;
-use std::rc::Rc;
+// run-rustfix
 
-static STATIC: [usize; 4] = [0, 1, 8, 16];
-const CONST: [usize; 4] = [0, 1, 8, 16];
+#![allow(dead_code, unused)]
+
+use std::collections::*;
 
 #[warn(clippy::all)]
 struct Unrelated(Vec<u8>);
@@ -48,10 +48,6 @@ fn main() {
         println!("{}", i);
     }
 
-    for i in 5..5 {
-        println!("{}", i);
-    }
-
     for i in 5..=5 {
         // not an error, this is the range with only one element “5”
         println!("{}", i);
@@ -81,10 +77,6 @@ fn main() {
         println!("{}", i);
     }
 
-    for i in (5 + 2)..(8 - 1) {
-        println!("{}", i);
-    }
-
     for i in (2 * 2)..(2 * 3) {
         // no error, 4..6 is fine
         println!("{}", i);
@@ -145,8 +137,6 @@ fn main() {
     let bs: BTreeSet<()> = BTreeSet::new();
     for _v in bs.iter() {}
 
-    for _v in vec.iter().next() {}
-
     let u = Unrelated(vec![]);
     for _v in u.next() {} // no error
     for _v in u.iter() {} // no error
@@ -275,17 +265,8 @@ fn main() {
     for mid in 1..vec.len() {
         let (_, _) = vec.split_at(mid);
     }
-
-    const ZERO: usize = 0;
-
-    for i in ZERO..vec.len() {
-        if f(&vec[i], &vec[i]) {
-            panic!("at the disco");
-        }
-    }
 }
 
-#[allow(dead_code)]
 fn partition<T: PartialOrd + Send>(v: &mut [T]) -> usize {
     let pivot = v.len() - 1;
     let mut i = 0;

tests/ui/for_loop_fixable.stderr

@@ -1,5 +1,5 @@
 error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:39:14
+  --> $DIR/for_loop_fixable.rs:39:14
    |
 LL |     for i in 10..0 {
    |              ^^^^^
@@ -11,17 +11,17 @@ LL |     for i in (0..10).rev() {
    |              ^^^^^^^^^^^^^
 
 error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:43:14
+  --> $DIR/for_loop_fixable.rs:43:14
    |
 LL |     for i in 10..=0 {
    |              ^^^^^^
 help: consider using the following if you are attempting to iterate over this range in reverse
    |
-LL |     for i in (0...10).rev() {
+LL |     for i in (0..=10).rev() {
    |              ^^^^^^^^^^^^^^
 
 error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:47:14
+  --> $DIR/for_loop_fixable.rs:47:14
    |
 LL |     for i in MAX_LEN..0 {
    |              ^^^^^^^^^^
@@ -31,13 +31,7 @@ LL |     for i in (0..MAX_LEN).rev() {
    |              ^^^^^^^^^^^^^^^^^^
 
 error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:51:14
-   |
-LL |     for i in 5..5 {
-   |              ^^^^
-
-error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:76:14
+  --> $DIR/for_loop_fixable.rs:72:14
    |
 LL |     for i in 10..5 + 4 {
    |              ^^^^^^^^^
@@ -47,7 +41,7 @@ LL |     for i in (5 + 4..10).rev() {
    |              ^^^^^^^^^^^^^^^^^
 
 error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:80:14
+  --> $DIR/for_loop_fixable.rs:76:14
    |
 LL |     for i in (5 + 2)..(3 - 1) {
    |              ^^^^^^^^^^^^^^^^
@@ -56,14 +50,8 @@ help: consider using the following if you are attempting to iterate over this ra
 LL |     for i in ((3 - 1)..(5 + 2)).rev() {
    |              ^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: this range is empty so this for loop will never run
-  --> $DIR/for_loop.rs:84:14
-   |
-LL |     for i in (5 + 2)..(8 - 1) {
-   |              ^^^^^^^^^^^^^^^^
-
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:106:15
+  --> $DIR/for_loop_fixable.rs:98:15
    |
 LL |     for _v in vec.iter() {}
    |               ^^^^^^^^^^ help: to write this more concisely, try: `&vec`
@@ -71,13 +59,13 @@ LL |     for _v in vec.iter() {}
    = note: `-D clippy::explicit-iter-loop` implied by `-D warnings`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:108:15
+  --> $DIR/for_loop_fixable.rs:100:15
    |
 LL |     for _v in vec.iter_mut() {}
    |               ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec`
 
 error: it is more concise to loop over containers instead of using explicit iteration methods`
-  --> $DIR/for_loop.rs:111:15
+  --> $DIR/for_loop_fixable.rs:103:15
    |
 LL |     for _v in out_vec.into_iter() {}
    |               ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec`
@@ -85,84 +73,64 @@ LL |     for _v in out_vec.into_iter() {}
    = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:114:15
+  --> $DIR/for_loop_fixable.rs:106:15
    |
 LL |     for _v in array.into_iter() {}
    |               ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:119:15
+  --> $DIR/for_loop_fixable.rs:111:15
    |
 LL |     for _v in [1, 2, 3].iter() {}
    |               ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:123:15
+  --> $DIR/for_loop_fixable.rs:115:15
    |
 LL |     for _v in [0; 32].iter() {}
    |               ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:128:15
+  --> $DIR/for_loop_fixable.rs:120:15
    |
 LL |     for _v in ll.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&ll`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:131:15
+  --> $DIR/for_loop_fixable.rs:123:15
    |
 LL |     for _v in vd.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&vd`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:134:15
+  --> $DIR/for_loop_fixable.rs:126:15
    |
 LL |     for _v in bh.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&bh`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:137:15
+  --> $DIR/for_loop_fixable.rs:129:15
    |
 LL |     for _v in hm.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&hm`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:140:15
+  --> $DIR/for_loop_fixable.rs:132:15
    |
 LL |     for _v in bt.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&bt`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:143:15
+  --> $DIR/for_loop_fixable.rs:135:15
    |
 LL |     for _v in hs.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&hs`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/for_loop.rs:146:15
+  --> $DIR/for_loop_fixable.rs:138:15
    |
 LL |     for _v in bs.iter() {}
    |               ^^^^^^^^^ help: to write this more concisely, try: `&bs`
 
-error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want
-  --> $DIR/for_loop.rs:148:15
-   |
-LL |     for _v in vec.iter().next() {}
-   |               ^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::iter-next-loop` implied by `-D warnings`
-
-error: the loop variable `i` is only used to index `vec`.
-  --> $DIR/for_loop.rs:281:14
-   |
-LL |     for i in ZERO..vec.len() {
-   |              ^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::needless-range-loop` implied by `-D warnings`
-help: consider using an iterator
-   |
-LL |     for <item> in &vec {
-   |         ^^^^^^    ^^^^
-
-error: aborting due to 22 previous errors
+error: aborting due to 18 previous errors
 

tests/ui/for_loop_unfixable.rs

@@ -0,0 +1,41 @@
+// Tests from for_loop.rs that don't have suggestions
+
+#[warn(
+    clippy::needless_range_loop,
+    clippy::explicit_iter_loop,
+    clippy::explicit_into_iter_loop,
+    clippy::iter_next_loop,
+    clippy::reverse_range_loop,
+    clippy::for_kv_map
+)]
+#[allow(
+    clippy::linkedlist,
+    clippy::shadow_unrelated,
+    clippy::unnecessary_mut_passed,
+    clippy::cognitive_complexity,
+    clippy::similar_names,
+    unused,
+    dead_code
+)]
+#[allow(clippy::many_single_char_names, unused_variables, clippy::into_iter_on_array)]
+fn main() {
+    for i in 5..5 {
+        println!("{}", i);
+    }
+
+    let vec = vec![1, 2, 3, 4];
+
+    for _v in vec.iter().next() {}
+
+    for i in (5 + 2)..(8 - 1) {
+        println!("{}", i);
+    }
+
+    const ZERO: usize = 0;
+
+    for i in ZERO..vec.len() {
+        if f(&vec[i], &vec[i]) {
+            panic!("at the disco");
+        }
+    }
+}

tests/ui/for_loop_unfixable.stderr

@@ -0,0 +1,9 @@
+error[E0425]: cannot find function `f` in this scope
+  --> $DIR/for_loop_unfixable.rs:37:12
+   |
+LL |         if f(&vec[i], &vec[i]) {
+   |            ^ help: a local variable with a similar name exists: `i`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0425`.