diff options
| -rw-r--r-- | clippy_lints/src/loops.rs | 2 | ||||
| -rw-r--r-- | tests/ui/for_loop.stdout | 0 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.fixed | 304 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.rs (renamed from tests/ui/for_loop.rs) | 27 | ||||
| -rw-r--r-- | tests/ui/for_loop_fixable.stderr (renamed from tests/ui/for_loop.stderr) | 72 | ||||
| -rw-r--r-- | tests/ui/for_loop_unfixable.rs | 41 | ||||
| -rw-r--r-- | tests/ui/for_loop_unfixable.stderr | 9 |
7 files changed, 379 insertions, 76 deletions
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 2db8acc4b95..0032cfd1985 100644 --- a/clippy_lints/src/loops.rs +++ b/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 { ".." }; diff --git a/tests/ui/for_loop.stdout b/tests/ui/for_loop.stdout deleted file mode 100644 index e69de29bb2d..00000000000 --- a/tests/ui/for_loop.stdout +++ /dev/null diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed new file mode 100644 index 00000000000..3075638ef94 --- /dev/null +++ b/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!() + } +} diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop_fixable.rs index 5d367a62fc9..2201596fd6a 100644 --- a/tests/ui/for_loop.rs +++ b/tests/ui/for_loop_fixable.rs @@ -1,8 +1,8 @@ -use std::collections::*; -use std::rc::Rc; +// run-rustfix + +#![allow(dead_code, unused)] -static STATIC: [usize; 4] = [0, 1, 8, 16]; -const CONST: [usize; 4] = [0, 1, 8, 16]; +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; diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop_fixable.stderr index 0f84abf45ed..6d6fa3ac7af 100644 --- a/tests/ui/for_loop.stderr +++ b/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 diff --git a/tests/ui/for_loop_unfixable.rs b/tests/ui/for_loop_unfixable.rs new file mode 100644 index 00000000000..5d94647e0db --- /dev/null +++ b/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"); + } + } +} diff --git a/tests/ui/for_loop_unfixable.stderr b/tests/ui/for_loop_unfixable.stderr new file mode 100644 index 00000000000..e88bfffaae6 --- /dev/null +++ b/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`. |