diff --git a/tests/ui/explicit_counter_loop.rs b/tests/ui/explicit_counter_loop.rs index 08e6de5dc19..e02b8f62b3d 100644 --- a/tests/ui/explicit_counter_loop.rs +++ b/tests/ui/explicit_counter_loop.rs @@ -23,6 +23,54 @@ fn main() { for _v in vec { _index += 1; } + + let vec = [1, 2, 3, 4]; + // 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; + 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; + if true { + _index = 1 + }; + for _v in &vec { + _index += 1 + } + + let mut _index = 1; + if false { + _index = 0 + }; + for _v in &vec { + _index += 1 + } } mod issue_1219 { diff --git a/tests/ui/explicit_counter_loop.stderr b/tests/ui/explicit_counter_loop.stderr index d3f3c626bbd..0677e4d78c8 100644 --- a/tests/ui/explicit_counter_loop.stderr +++ b/tests/ui/explicit_counter_loop.stderr @@ -25,31 +25,31 @@ LL | for _v in vec { | ^^^^^^^^^^^^^ help: consider using: `for (_index, _v) in vec.into_iter().enumerate()` error: the variable `count` is used as a loop counter - --> $DIR/explicit_counter_loop.rs:62:9 + --> $DIR/explicit_counter_loop.rs:110:9 | LL | for ch in text.chars() { | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()` error: the variable `count` is used as a loop counter - --> $DIR/explicit_counter_loop.rs:73:9 + --> $DIR/explicit_counter_loop.rs:121:9 | LL | for ch in text.chars() { | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `for (count, ch) in text.chars().enumerate()` error: the variable `count` is used as a loop counter - --> $DIR/explicit_counter_loop.rs:131:9 + --> $DIR/explicit_counter_loop.rs:179:9 | LL | for _i in 3..10 { | ^^^^^^^^^^^^^^^ help: consider using: `for (count, _i) in (3..10).enumerate()` error: the variable `idx_usize` is used as a loop counter - --> $DIR/explicit_counter_loop.rs:171:9 + --> $DIR/explicit_counter_loop.rs:219:9 | LL | for _item in slice { | ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_usize, _item) in slice.iter().enumerate()` error: the variable `idx_u32` is used as a loop counter - --> $DIR/explicit_counter_loop.rs:183:9 + --> $DIR/explicit_counter_loop.rs:231:9 | LL | for _item in slice { | ^^^^^^^^^^^^^^^^^^ help: consider using: `for (idx_u32, _item) in (0_u32..).zip(slice.iter())` diff --git a/tests/ui/explicit_into_iter_loop.fixed b/tests/ui/explicit_into_iter_loop.fixed new file mode 100644 index 00000000000..bc9ecb1e05a --- /dev/null +++ b/tests/ui/explicit_into_iter_loop.fixed @@ -0,0 +1,55 @@ +//@run-rustfix +#![warn(clippy::explicit_into_iter_loop)] + +fn main() { + // Issue #4958 + fn _takes_iterator(iterator: &T) + where + for<'a> &'a T: IntoIterator, + { + for i in iterator { + println!("{}", i); + } + } + + struct T; + impl IntoIterator for &T { + type Item = (); + type IntoIter = std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + vec![].into_iter() + } + } + + let t = T; + let r = &t; + let rr = &&t; + + // This case is handled by `explicit_iter_loop`. No idea why. + for _ in t.into_iter() {} + + for _ in r {} + + // No suggestion for this. + // We'd have to suggest `for _ in *rr {}` which is less clear. + for _ in rr.into_iter() {} + + // Issue #6900 + struct S; + impl S { + #[allow(clippy::should_implement_trait)] + pub fn into_iter(self) -> I { + unimplemented!() + } + } + + struct I(T); + impl Iterator for I { + type Item = T; + fn next(&mut self) -> Option { + unimplemented!() + } + } + + for _ in S.into_iter::() {} +} diff --git a/tests/ui/explicit_into_iter_loop.rs b/tests/ui/explicit_into_iter_loop.rs new file mode 100644 index 00000000000..9b610fe3d11 --- /dev/null +++ b/tests/ui/explicit_into_iter_loop.rs @@ -0,0 +1,55 @@ +//@run-rustfix +#![warn(clippy::explicit_into_iter_loop)] + +fn main() { + // Issue #4958 + fn _takes_iterator(iterator: &T) + where + for<'a> &'a T: IntoIterator, + { + for i in iterator.into_iter() { + println!("{}", i); + } + } + + struct T; + impl IntoIterator for &T { + type Item = (); + type IntoIter = std::vec::IntoIter; + fn into_iter(self) -> Self::IntoIter { + vec![].into_iter() + } + } + + let t = T; + let r = &t; + let rr = &&t; + + // This case is handled by `explicit_iter_loop`. No idea why. + for _ in t.into_iter() {} + + for _ in r.into_iter() {} + + // No suggestion for this. + // We'd have to suggest `for _ in *rr {}` which is less clear. + for _ in rr.into_iter() {} + + // Issue #6900 + struct S; + impl S { + #[allow(clippy::should_implement_trait)] + pub fn into_iter(self) -> I { + unimplemented!() + } + } + + struct I(T); + impl Iterator for I { + type Item = T; + fn next(&mut self) -> Option { + unimplemented!() + } + } + + for _ in S.into_iter::() {} +} diff --git a/tests/ui/explicit_into_iter_loop.stderr b/tests/ui/explicit_into_iter_loop.stderr new file mode 100644 index 00000000000..1bd2b38a0e7 --- /dev/null +++ b/tests/ui/explicit_into_iter_loop.stderr @@ -0,0 +1,16 @@ +error: it is more concise to loop over containers instead of using explicit iteration methods + --> $DIR/explicit_into_iter_loop.rs:10:18 + | +LL | for i in iterator.into_iter() { + | ^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `iterator` + | + = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings` + +error: it is more concise to loop over containers instead of using explicit iteration methods + --> $DIR/explicit_into_iter_loop.rs:31:14 + | +LL | for _ in r.into_iter() {} + | ^^^^^^^^^^^^^ help: to write this more concisely, try: `r` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/explicit_iter_loop.fixed b/tests/ui/explicit_iter_loop.fixed new file mode 100644 index 00000000000..b0de50ed928 --- /dev/null +++ b/tests/ui/explicit_iter_loop.fixed @@ -0,0 +1,140 @@ +//@run-rustfix +#![deny(clippy::explicit_iter_loop)] +#![allow( + clippy::linkedlist, + clippy::similar_names, + clippy::needless_borrow, + clippy::deref_addrof, + dead_code +)] + +use core::slice; +use std::collections::*; + +fn main() { + let mut vec = vec![1, 2, 3, 4]; + + for _ in &vec {} + for _ in &mut vec {} + + for _ in &vec {} // these are fine + for _ in &mut vec {} // these are fine + + for _ in &[1, 2, 3] {} + + for _ in &*(&mut [1, 2, 3]) {} + + for _ in &[0; 32] {} + for _ in &[0; 33] {} + + let ll: LinkedList<()> = LinkedList::new(); + for _ in &ll {} + + let vd: VecDeque<()> = VecDeque::new(); + for _ in &vd {} + + let bh: BinaryHeap<()> = BinaryHeap::new(); + for _ in &bh {} + + let hm: HashMap<(), ()> = HashMap::new(); + for _ in &hm {} + + let bt: BTreeMap<(), ()> = BTreeMap::new(); + for _ in &bt {} + + let hs: HashSet<()> = HashSet::new(); + for _ in &hs {} + + let bs: BTreeSet<()> = BTreeSet::new(); + for _ in &bs {} + + struct NoIntoIter(); + impl NoIntoIter { + fn iter(&self) -> slice::Iter { + unimplemented!() + } + + fn iter_mut(&mut self) -> slice::IterMut { + unimplemented!() + } + } + let mut x = NoIntoIter(); + for _ in x.iter() {} // no error + for _ in x.iter_mut() {} // no error + + struct IntoIterDiffTy; + impl IntoIterator for &'_ IntoIterDiffTy { + type Item = &'static (); + type IntoIter = core::slice::Iter<'static, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl IntoIterDiffTy { + fn iter(&self) -> core::slice::Iter<'static, i32> { + unimplemented!() + } + } + let x = IntoIterDiffTy; + for _ in x.iter() {} + + struct IntoIterDiffSig; + impl IntoIterator for &'_ IntoIterDiffSig { + type Item = &'static (); + type IntoIter = core::slice::Iter<'static, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl IntoIterDiffSig { + fn iter(&self, _: u32) -> core::slice::Iter<'static, ()> { + unimplemented!() + } + } + let x = IntoIterDiffSig; + for _ in x.iter(0) {} + + struct IntoIterDiffLt<'a>(&'a ()); + impl<'a> IntoIterator for &'a IntoIterDiffLt<'_> { + type Item = &'a (); + type IntoIter = core::slice::Iter<'a, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl<'a> IntoIterDiffLt<'a> { + fn iter(&self) -> core::slice::Iter<'a, ()> { + unimplemented!() + } + } + let x = IntoIterDiffLt(&()); + for _ in x.iter() {} + + struct CustomType; + impl<'a> IntoIterator for &'a CustomType { + type Item = &'a u32; + type IntoIter = core::slice::Iter<'a, u32>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl<'a> IntoIterator for &'a mut CustomType { + type Item = &'a mut u32; + type IntoIter = core::slice::IterMut<'a, u32>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl CustomType { + fn iter(&self) -> <&'_ Self as IntoIterator>::IntoIter { + panic!() + } + + fn iter_mut(&mut self) -> core::slice::IterMut<'_, u32> { + panic!() + } + } + let mut x = CustomType; + for _ in &x {} + for _ in &mut x {} +} diff --git a/tests/ui/explicit_iter_loop.rs b/tests/ui/explicit_iter_loop.rs new file mode 100644 index 00000000000..33d3bada6e2 --- /dev/null +++ b/tests/ui/explicit_iter_loop.rs @@ -0,0 +1,140 @@ +//@run-rustfix +#![deny(clippy::explicit_iter_loop)] +#![allow( + clippy::linkedlist, + clippy::similar_names, + clippy::needless_borrow, + clippy::deref_addrof, + dead_code +)] + +use core::slice; +use std::collections::*; + +fn main() { + let mut vec = vec![1, 2, 3, 4]; + + for _ in vec.iter() {} + for _ in vec.iter_mut() {} + + for _ in &vec {} // these are fine + for _ in &mut vec {} // these are fine + + for _ in [1, 2, 3].iter() {} + + for _ in (&mut [1, 2, 3]).iter() {} + + for _ in [0; 32].iter() {} + for _ in [0; 33].iter() {} + + let ll: LinkedList<()> = LinkedList::new(); + for _ in ll.iter() {} + + let vd: VecDeque<()> = VecDeque::new(); + for _ in vd.iter() {} + + let bh: BinaryHeap<()> = BinaryHeap::new(); + for _ in bh.iter() {} + + let hm: HashMap<(), ()> = HashMap::new(); + for _ in hm.iter() {} + + let bt: BTreeMap<(), ()> = BTreeMap::new(); + for _ in bt.iter() {} + + let hs: HashSet<()> = HashSet::new(); + for _ in hs.iter() {} + + let bs: BTreeSet<()> = BTreeSet::new(); + for _ in bs.iter() {} + + struct NoIntoIter(); + impl NoIntoIter { + fn iter(&self) -> slice::Iter { + unimplemented!() + } + + fn iter_mut(&mut self) -> slice::IterMut { + unimplemented!() + } + } + let mut x = NoIntoIter(); + for _ in x.iter() {} // no error + for _ in x.iter_mut() {} // no error + + struct IntoIterDiffTy; + impl IntoIterator for &'_ IntoIterDiffTy { + type Item = &'static (); + type IntoIter = core::slice::Iter<'static, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl IntoIterDiffTy { + fn iter(&self) -> core::slice::Iter<'static, i32> { + unimplemented!() + } + } + let x = IntoIterDiffTy; + for _ in x.iter() {} + + struct IntoIterDiffSig; + impl IntoIterator for &'_ IntoIterDiffSig { + type Item = &'static (); + type IntoIter = core::slice::Iter<'static, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl IntoIterDiffSig { + fn iter(&self, _: u32) -> core::slice::Iter<'static, ()> { + unimplemented!() + } + } + let x = IntoIterDiffSig; + for _ in x.iter(0) {} + + struct IntoIterDiffLt<'a>(&'a ()); + impl<'a> IntoIterator for &'a IntoIterDiffLt<'_> { + type Item = &'a (); + type IntoIter = core::slice::Iter<'a, ()>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl<'a> IntoIterDiffLt<'a> { + fn iter(&self) -> core::slice::Iter<'a, ()> { + unimplemented!() + } + } + let x = IntoIterDiffLt(&()); + for _ in x.iter() {} + + struct CustomType; + impl<'a> IntoIterator for &'a CustomType { + type Item = &'a u32; + type IntoIter = core::slice::Iter<'a, u32>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl<'a> IntoIterator for &'a mut CustomType { + type Item = &'a mut u32; + type IntoIter = core::slice::IterMut<'a, u32>; + fn into_iter(self) -> Self::IntoIter { + unimplemented!() + } + } + impl CustomType { + fn iter(&self) -> <&'_ Self as IntoIterator>::IntoIter { + panic!() + } + + fn iter_mut(&mut self) -> core::slice::IterMut<'_, u32> { + panic!() + } + } + let mut x = CustomType; + for _ in x.iter() {} + for _ in x.iter_mut() {} +} diff --git a/tests/ui/explicit_iter_loop.stderr b/tests/ui/explicit_iter_loop.stderr new file mode 100644 index 00000000000..1508080a243 --- /dev/null +++ b/tests/ui/explicit_iter_loop.stderr @@ -0,0 +1,106 @@ +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:17:14 + | +LL | for _ in vec.iter() {} + | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` + | +note: the lint level is defined here + --> $DIR/explicit_iter_loop.rs:2:9 + | +LL | #![deny(clippy::explicit_iter_loop)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:18:14 + | +LL | for _ in vec.iter_mut() {} + | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:23:14 + | +LL | for _ 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/explicit_iter_loop.rs:25:14 + | +LL | for _ in (&mut [1, 2, 3]).iter() {} + | ^^^^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&*(&mut [1, 2, 3])` + +error: the method `iter` doesn't need a mutable reference + --> $DIR/explicit_iter_loop.rs:25:14 + | +LL | for _ in (&mut [1, 2, 3]).iter() {} + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-mut-passed` implied by `-D warnings` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:27:14 + | +LL | for _ 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/explicit_iter_loop.rs:28:14 + | +LL | for _ in [0; 33].iter() {} + | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 33]` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:31:14 + | +LL | for _ 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/explicit_iter_loop.rs:34:14 + | +LL | for _ 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/explicit_iter_loop.rs:37:14 + | +LL | for _ 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/explicit_iter_loop.rs:40:14 + | +LL | for _ 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/explicit_iter_loop.rs:43:14 + | +LL | for _ 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/explicit_iter_loop.rs:46:14 + | +LL | for _ 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/explicit_iter_loop.rs:49:14 + | +LL | for _ in bs.iter() {} + | ^^^^^^^^^ help: to write this more concisely, try: `&bs` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:138:14 + | +LL | for _ in x.iter() {} + | ^^^^^^^^ help: to write this more concisely, try: `&x` + +error: it is more concise to loop over references to containers instead of using explicit iteration methods + --> $DIR/explicit_iter_loop.rs:139:14 + | +LL | for _ in x.iter_mut() {} + | ^^^^^^^^^^^^ help: to write this more concisely, try: `&mut x` + +error: aborting due to 16 previous errors + diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed deleted file mode 100644 index 99230856446..00000000000 --- a/tests/ui/for_loop_fixable.fixed +++ /dev/null @@ -1,392 +0,0 @@ -//@run-rustfix -#![allow(dead_code, unused)] -#![allow(clippy::uninlined_format_args, clippy::useless_vec, clippy::deref_addrof)] - -use std::collections::*; - -#[warn(clippy::all)] -struct Unrelated(Vec); -impl Unrelated { - fn next(&self) -> std::slice::Iter { - self.0.iter() - } - - fn iter(&self) -> std::slice::Iter { - self.0.iter() - } -} - -#[warn( - clippy::needless_range_loop, - clippy::explicit_iter_loop, - clippy::explicit_into_iter_loop, - clippy::iter_next_loop, - clippy::for_kv_map -)] -#[allow( - clippy::linkedlist, - clippy::unnecessary_mut_passed, - clippy::similar_names, - clippy::needless_borrow -)] -#[allow(unused_variables)] -fn main() { - let mut vec = vec![1, 2, 3, 4]; - - // 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 {} - - 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]) {} // no error - - for _v in &[0; 32] {} - - for _v in &[0; 33] {} // 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::>(); - let _y = vec.iter().cloned().map(|x| out.push(x)).collect::>(); // 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) -> bool { - unimplemented!() - } - fn g(_: &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(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 { - for x in 0..5 { - let next_handle = H::new_for_index(x); - println!("{}", next_handle.index()); - } - unimplemented!() - } -} - -// explicit_into_iter_loop bad suggestions -#[warn(clippy::explicit_into_iter_loop, clippy::explicit_iter_loop)] -mod issue_4958 { - fn takes_iterator(iterator: &T) - where - for<'a> &'a T: IntoIterator, - { - for i in iterator { - println!("{}", i); - } - } - - struct T; - impl IntoIterator for &T { - type Item = (); - type IntoIter = std::vec::IntoIter; - fn into_iter(self) -> Self::IntoIter { - vec![].into_iter() - } - } - - fn more_tests() { - let t = T; - let r = &t; - let rr = &&t; - - // This case is handled by `explicit_iter_loop`. No idea why. - for _ in t {} - - for _ in r {} - - // No suggestion for this. - // We'd have to suggest `for _ in *rr {}` which is less clear. - for _ in rr.into_iter() {} - } -} - -// explicit_into_iter_loop -#[warn(clippy::explicit_into_iter_loop)] -mod issue_6900 { - struct S; - impl S { - #[allow(clippy::should_implement_trait)] - pub fn into_iter(self) -> I { - unimplemented!() - } - } - - struct I(T); - impl Iterator for I { - type Item = T; - fn next(&mut self) -> Option { - unimplemented!() - } - } - - fn f() { - for _ in S.into_iter::() { - unimplemented!() - } - } -} - -struct IntoIterDiffTy; -impl IntoIterator for &'_ IntoIterDiffTy { - type Item = &'static (); - type IntoIter = core::slice::Iter<'static, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl IntoIterDiffTy { - fn iter(&self) -> core::slice::Iter<'static, i32> { - unimplemented!() - } -} - -struct IntoIterDiffSig; -impl IntoIterator for &'_ IntoIterDiffSig { - type Item = &'static (); - type IntoIter = core::slice::Iter<'static, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl IntoIterDiffSig { - fn iter(&self, _: u32) -> core::slice::Iter<'static, ()> { - unimplemented!() - } -} - -struct IntoIterDiffLt<'a>(&'a ()); -impl<'a> IntoIterator for &'a IntoIterDiffLt<'_> { - type Item = &'a (); - type IntoIter = core::slice::Iter<'a, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl<'a> IntoIterDiffLt<'a> { - fn iter(&self) -> core::slice::Iter<'a, ()> { - unimplemented!() - } -} - -struct CustomType; -impl<'a> IntoIterator for &'a CustomType { - type Item = &'a u32; - type IntoIter = core::slice::Iter<'a, u32>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl<'a> IntoIterator for &'a mut CustomType { - type Item = &'a mut u32; - type IntoIter = core::slice::IterMut<'a, u32>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl CustomType { - fn iter(&self) -> <&'_ Self as IntoIterator>::IntoIter { - panic!() - } - - fn iter_mut(&mut self) -> core::slice::IterMut<'_, u32> { - panic!() - } -} - -#[warn(clippy::explicit_iter_loop)] -fn _f() { - let x = IntoIterDiffTy; - for _ in x.iter() {} - - let x = IntoIterDiffSig; - for _ in x.iter(0) {} - - let x = IntoIterDiffLt(&()); - for _ in x.iter() {} - - let mut x = CustomType; - for _ in &x {} - for _ in &mut x {} -} diff --git a/tests/ui/for_loop_fixable.rs b/tests/ui/for_loop_fixable.rs deleted file mode 100644 index 0d7fe3c0314..00000000000 --- a/tests/ui/for_loop_fixable.rs +++ /dev/null @@ -1,392 +0,0 @@ -//@run-rustfix -#![allow(dead_code, unused)] -#![allow(clippy::uninlined_format_args, clippy::useless_vec, clippy::deref_addrof)] - -use std::collections::*; - -#[warn(clippy::all)] -struct Unrelated(Vec); -impl Unrelated { - fn next(&self) -> std::slice::Iter { - self.0.iter() - } - - fn iter(&self) -> std::slice::Iter { - self.0.iter() - } -} - -#[warn( - clippy::needless_range_loop, - clippy::explicit_iter_loop, - clippy::explicit_into_iter_loop, - clippy::iter_next_loop, - clippy::for_kv_map -)] -#[allow( - clippy::linkedlist, - clippy::unnecessary_mut_passed, - clippy::similar_names, - clippy::needless_borrow -)] -#[allow(unused_variables)] -fn main() { - let mut vec = vec![1, 2, 3, 4]; - - // 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.iter() {} - - for _v in vec.iter_mut() {} - - let out_vec = vec![1, 2, 3]; - for _v in out_vec.into_iter() {} - - for _v in &vec {} // these are fine - for _v in &mut vec {} // these are fine - - for _v in [1, 2, 3].iter() {} - - for _v in (&mut [1, 2, 3]).iter() {} // no error - - for _v in [0; 32].iter() {} - - for _v in [0; 33].iter() {} // no error - - let ll: LinkedList<()> = LinkedList::new(); - for _v in ll.iter() {} - - let vd: VecDeque<()> = VecDeque::new(); - for _v in vd.iter() {} - - let bh: BinaryHeap<()> = BinaryHeap::new(); - for _v in bh.iter() {} - - let hm: HashMap<(), ()> = HashMap::new(); - for _v in hm.iter() {} - - let bt: BTreeMap<(), ()> = BTreeMap::new(); - for _v in bt.iter() {} - - let hs: HashSet<()> = HashSet::new(); - for _v in hs.iter() {} - - let bs: BTreeSet<()> = BTreeSet::new(); - for _v in bs.iter() {} - - 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::>(); - let _y = vec.iter().cloned().map(|x| out.push(x)).collect::>(); // 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) -> bool { - unimplemented!() - } - fn g(_: &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(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 { - for x in 0..5 { - let next_handle = H::new_for_index(x); - println!("{}", next_handle.index()); - } - unimplemented!() - } -} - -// explicit_into_iter_loop bad suggestions -#[warn(clippy::explicit_into_iter_loop, clippy::explicit_iter_loop)] -mod issue_4958 { - fn takes_iterator(iterator: &T) - where - for<'a> &'a T: IntoIterator, - { - for i in iterator.into_iter() { - println!("{}", i); - } - } - - struct T; - impl IntoIterator for &T { - type Item = (); - type IntoIter = std::vec::IntoIter; - fn into_iter(self) -> Self::IntoIter { - vec![].into_iter() - } - } - - fn more_tests() { - let t = T; - let r = &t; - let rr = &&t; - - // This case is handled by `explicit_iter_loop`. No idea why. - for _ in t.into_iter() {} - - for _ in r.into_iter() {} - - // No suggestion for this. - // We'd have to suggest `for _ in *rr {}` which is less clear. - for _ in rr.into_iter() {} - } -} - -// explicit_into_iter_loop -#[warn(clippy::explicit_into_iter_loop)] -mod issue_6900 { - struct S; - impl S { - #[allow(clippy::should_implement_trait)] - pub fn into_iter(self) -> I { - unimplemented!() - } - } - - struct I(T); - impl Iterator for I { - type Item = T; - fn next(&mut self) -> Option { - unimplemented!() - } - } - - fn f() { - for _ in S.into_iter::() { - unimplemented!() - } - } -} - -struct IntoIterDiffTy; -impl IntoIterator for &'_ IntoIterDiffTy { - type Item = &'static (); - type IntoIter = core::slice::Iter<'static, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl IntoIterDiffTy { - fn iter(&self) -> core::slice::Iter<'static, i32> { - unimplemented!() - } -} - -struct IntoIterDiffSig; -impl IntoIterator for &'_ IntoIterDiffSig { - type Item = &'static (); - type IntoIter = core::slice::Iter<'static, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl IntoIterDiffSig { - fn iter(&self, _: u32) -> core::slice::Iter<'static, ()> { - unimplemented!() - } -} - -struct IntoIterDiffLt<'a>(&'a ()); -impl<'a> IntoIterator for &'a IntoIterDiffLt<'_> { - type Item = &'a (); - type IntoIter = core::slice::Iter<'a, ()>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl<'a> IntoIterDiffLt<'a> { - fn iter(&self) -> core::slice::Iter<'a, ()> { - unimplemented!() - } -} - -struct CustomType; -impl<'a> IntoIterator for &'a CustomType { - type Item = &'a u32; - type IntoIter = core::slice::Iter<'a, u32>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl<'a> IntoIterator for &'a mut CustomType { - type Item = &'a mut u32; - type IntoIter = core::slice::IterMut<'a, u32>; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -impl CustomType { - fn iter(&self) -> <&'_ Self as IntoIterator>::IntoIter { - panic!() - } - - fn iter_mut(&mut self) -> core::slice::IterMut<'_, u32> { - panic!() - } -} - -#[warn(clippy::explicit_iter_loop)] -fn _f() { - let x = IntoIterDiffTy; - for _ in x.iter() {} - - let x = IntoIterDiffSig; - for _ in x.iter(0) {} - - let x = IntoIterDiffLt(&()); - for _ in x.iter() {} - - let mut x = CustomType; - for _ in x.iter() {} - for _ in x.iter_mut() {} -} diff --git a/tests/ui/for_loop_fixable.stderr b/tests/ui/for_loop_fixable.stderr deleted file mode 100644 index 4ad23e0b9db..00000000000 --- a/tests/ui/for_loop_fixable.stderr +++ /dev/null @@ -1,120 +0,0 @@ -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:43:15 - | -LL | for _v in vec.iter() {} - | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` - | - = 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_fixable.rs:45: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_fixable.rs:48:15 - | -LL | for _v in out_vec.into_iter() {} - | ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec` - | - = 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_fixable.rs:53: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_fixable.rs:55:15 - | -LL | for _v in (&mut [1, 2, 3]).iter() {} // no error - | ^^^^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&*(&mut [1, 2, 3])` - -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:57: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_fixable.rs:59:15 - | -LL | for _v in [0; 33].iter() {} // no error - | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 33]` - -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:62: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_fixable.rs:65: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_fixable.rs:68: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_fixable.rs:71: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_fixable.rs:74: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_fixable.rs:77: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_fixable.rs:80:15 - | -LL | for _v in bs.iter() {} - | ^^^^^^^^^ help: to write this more concisely, try: `&bs` - -error: it is more concise to loop over containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:255:18 - | -LL | for i in iterator.into_iter() { - | ^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `iterator` - -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:275:18 - | -LL | for _ in t.into_iter() {} - | ^^^^^^^^^^^^^ help: to write this more concisely, try: `t` - -error: it is more concise to loop over containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:277:18 - | -LL | for _ in r.into_iter() {} - | ^^^^^^^^^^^^^ help: to write this more concisely, try: `r` - -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:390:14 - | -LL | for _ in x.iter() {} - | ^^^^^^^^ help: to write this more concisely, try: `&x` - -error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:391:14 - | -LL | for _ in x.iter_mut() {} - | ^^^^^^^^^^^^ help: to write this more concisely, try: `&mut x` - -error: aborting due to 19 previous errors - diff --git a/tests/ui/for_loop_unfixable.rs b/tests/ui/for_loop_unfixable.rs deleted file mode 100644 index 8e3fb99bc7c..00000000000 --- a/tests/ui/for_loop_unfixable.rs +++ /dev/null @@ -1,21 +0,0 @@ -// 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::for_kv_map -)] -#[allow( - clippy::linkedlist, - clippy::unnecessary_mut_passed, - clippy::similar_names, - clippy::useless_vec -)] -#[allow(for_loops_over_fallibles)] -fn main() { - let vec = vec![1, 2, 3, 4]; - - for _v in vec.iter().next() {} -} diff --git a/tests/ui/for_loop_unfixable.stderr b/tests/ui/for_loop_unfixable.stderr deleted file mode 100644 index 8b163a142a5..00000000000 --- a/tests/ui/for_loop_unfixable.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want - --> $DIR/for_loop_unfixable.rs:20:15 - | -LL | for _v in vec.iter().next() {} - | ^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::iter-next-loop` implied by `-D warnings` - -error: aborting due to previous error - diff --git a/tests/ui/iter_next_loop.rs b/tests/ui/iter_next_loop.rs new file mode 100644 index 00000000000..548b799de44 --- /dev/null +++ b/tests/ui/iter_next_loop.rs @@ -0,0 +1,16 @@ +#![allow(dead_code, unused, for_loops_over_fallibles)] +#![warn(clippy::iter_next_loop)] + +fn main() { + let x = [1, 2, 3, 4]; + for _ in vec.iter().next() {} + + struct Unrelated(&'static [u8]); + impl Unrelated { + fn next(&self) -> std::slice::Iter { + self.0.iter() + } + } + let u = Unrelated(&[0]); + for _v in u.next() {} // no error +} diff --git a/tests/ui/iter_next_loop.stderr b/tests/ui/iter_next_loop.stderr new file mode 100644 index 00000000000..5bba0e635bb --- /dev/null +++ b/tests/ui/iter_next_loop.stderr @@ -0,0 +1,9 @@ +error[E0423]: expected value, found macro `vec` + --> $DIR/iter_next_loop.rs:6:14 + | +LL | for _ in vec.iter().next() {} + | ^^^ not a value + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0423`. diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed index b7e80af5015..17a56cdfc2f 100644 --- a/tests/ui/needless_collect.fixed +++ b/tests/ui/needless_collect.fixed @@ -70,6 +70,11 @@ fn main() { bar((0..10).collect::>(), (0..10)); baz((0..10), (), ('a'..='z')) } + + let values = [1, 2, 3, 4]; + let mut out = vec![]; + values.iter().cloned().map(|x| out.push(x)).collect::>(); + let _y = values.iter().cloned().map(|x| out.push(x)).collect::>(); // this is fine } fn foo(_: impl IntoIterator) {} diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index 680b6fa5b55..8615899e979 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -70,6 +70,11 @@ fn from_iter>(iter: I) -> Self { bar((0..10).collect::>(), (0..10).collect::>()); baz((0..10), (), ('a'..='z').collect::>()) } + + let values = [1, 2, 3, 4]; + let mut out = vec![]; + values.iter().cloned().map(|x| out.push(x)).collect::>(); + let _y = values.iter().cloned().map(|x| out.push(x)).collect::>(); // this is fine } fn foo(_: impl IntoIterator) {} diff --git a/tests/ui/needless_range_loop.rs b/tests/ui/needless_range_loop.rs index 329cdc1f06b..1935ccb94bd 100644 --- a/tests/ui/needless_range_loop.rs +++ b/tests/ui/needless_range_loop.rs @@ -82,6 +82,29 @@ fn main() { for i in 0..2 { println!("{}", test[i]); } + + // See #601 + for i in 0..10 { + // no error, id_col does not exist outside the loop + let mut id_col = [0f64; 10]; + id_col[i] = 1f64; + } + + fn f(_: &T, _: &T) -> bool { + unimplemented!() + } + fn g(_: &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); + } } struct Test { @@ -94,3 +117,38 @@ fn index(&self, index: usize) -> &Self::Output { &self.inner[index] } } + +fn partition(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 +} + +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 { + for x in 0..5 { + let next_handle = H::new_for_index(x); + println!("{}", next_handle.index()); + } + unimplemented!() + } +}