Auto merge of #3611 - phansch:some_for_loop_cleanup, r=flip1995

UI test cleanup: Extract for_kv_map lint tests

cc #2038

4 files changed, 122 insertions, 106 deletions

diff --git a/tests/ui/for_kv_map.rs b/tests/ui/for_kv_map.rs
new file mode 100644
index 00000000000..d79ea4bebeb
--- /dev/null
+++ b/tests/ui/for_kv_map.rs
@@ -0,0 +1,56 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![warn(clippy::for_kv_map)]
+#![allow(clippy::used_underscore_binding)]
+
+use std::collections::*;
+use std::rc::Rc;
+
+fn main() {
+    let m: HashMap<u64, u64> = HashMap::new();
+    for (_, v) in &m {
+        let _v = v;
+    }
+
+    let m: Rc<HashMap<u64, u64>> = Rc::new(HashMap::new());
+    for (_, v) in &*m {
+        let _v = v;
+        // Here the `*` is not actually necessary, but the test tests that we don't
+        // suggest
+        // `in *m.values()` as we used to
+    }
+
+    let mut m: HashMap<u64, u64> = HashMap::new();
+    for (_, v) in &mut m {
+        let _v = v;
+    }
+
+    let m: &mut HashMap<u64, u64> = &mut HashMap::new();
+    for (_, v) in &mut *m {
+        let _v = v;
+    }
+
+    let m: HashMap<u64, u64> = HashMap::new();
+    let rm = &m;
+    for (k, _value) in rm {
+        let _k = k;
+    }
+    test_for_kv_map();
+}
+
+fn test_for_kv_map() {
+    let m: HashMap<u64, u64> = HashMap::new();
+
+    // No error, _value is actually used
+    for (k, _value) in &m {
+        let _ = _value;
+        let _k = k;
+    }
+}
diff --git a/tests/ui/for_kv_map.stderr b/tests/ui/for_kv_map.stderr
new file mode 100644
index 00000000000..7b65c58f58d
--- /dev/null
+++ b/tests/ui/for_kv_map.stderr
@@ -0,0 +1,54 @@
+error: you seem to want to iterate on a map's values
+  --> $DIR/for_kv_map.rs:18:19
+   |
+LL |     for (_, v) in &m {
+   |                   ^^
+   |
+   = note: `-D clippy::for-kv-map` implied by `-D warnings`
+help: use the corresponding method
+   |
+LL |     for v in m.values() {
+   |         ^    ^^^^^^^^^^
+
+error: you seem to want to iterate on a map's values
+  --> $DIR/for_kv_map.rs:23:19
+   |
+LL |     for (_, v) in &*m {
+   |                   ^^^
+help: use the corresponding method
+   |
+LL |     for v in (*m).values() {
+   |         ^    ^^^^^^^^^^^^^
+
+error: you seem to want to iterate on a map's values
+  --> $DIR/for_kv_map.rs:31:19
+   |
+LL |     for (_, v) in &mut m {
+   |                   ^^^^^^
+help: use the corresponding method
+   |
+LL |     for v in m.values_mut() {
+   |         ^    ^^^^^^^^^^^^^^
+
+error: you seem to want to iterate on a map's values
+  --> $DIR/for_kv_map.rs:36:19
+   |
+LL |     for (_, v) in &mut *m {
+   |                   ^^^^^^^
+help: use the corresponding method
+   |
+LL |     for v in (*m).values_mut() {
+   |         ^    ^^^^^^^^^^^^^^^^^
+
+error: you seem to want to iterate on a map's keys
+  --> $DIR/for_kv_map.rs:42:24
+   |
+LL |     for (k, _value) in rm {
+   |                        ^^
+help: use the corresponding method
+   |
+LL |     for k in rm.keys() {
+   |         ^    ^^^^^^^^^
+
+error: aborting due to 5 previous errors
+
diff --git a/tests/ui/for_loop.rs b/tests/ui/for_loop.rs
index 4747269bccd..c172b0b3b77 100644
--- a/tests/ui/for_loop.rs
+++ b/tests/ui/for_loop.rs
@@ -333,37 +333,6 @@ fn main() {
     }
     println!("index: {}", index);
 
-    let m: HashMap<u64, u64> = HashMap::new();
-    for (_, v) in &m {
-        let _v = v;
-    }
-
-    let m: Rc<HashMap<u64, u64>> = Rc::new(HashMap::new());
-    for (_, v) in &*m {
-        let _v = v;
-        // Here the `*` is not actually necessary, but the test tests that we don't
-        // suggest
-        // `in *m.values()` as we used to
-    }
-
-    let mut m: HashMap<u64, u64> = HashMap::new();
-    for (_, v) in &mut m {
-        let _v = v;
-    }
-
-    let m: &mut HashMap<u64, u64> = &mut HashMap::new();
-    for (_, v) in &mut *m {
-        let _v = v;
-    }
-
-    let m: HashMap<u64, u64> = HashMap::new();
-    let rm = &m;
-    for (k, _value) in rm {
-        let _k = k;
-    }
-
-    test_for_kv_map();
-
     fn f<T>(_: &T, _: &T) -> bool {
         unimplemented!()
     }
@@ -381,17 +350,6 @@ fn main() {
     }
 }
 
-#[allow(clippy::used_underscore_binding)]
-fn test_for_kv_map() {
-    let m: HashMap<u64, u64> = HashMap::new();
-
-    // No error, _value is actually used
-    for (k, _value) in &m {
-        let _ = _value;
-        let _k = k;
-    }
-}
-
 #[allow(dead_code)]
 fn partition<T: PartialOrd + Send>(v: &mut [T]) -> usize {
     let pivot = v.len() - 1;
diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr
index 937bef9f8a6..4ded425b321 100644
--- a/tests/ui/for_loop.stderr
+++ b/tests/ui/for_loop.stderr
@@ -292,60 +292,8 @@ LL |     vec.iter().cloned().map(|x| out.push(x)).collect::<Vec<_>>();
    |
    = note: `-D clippy::unused-collect` implied by `-D warnings`
 
-error: you seem to want to iterate on a map's values
-  --> $DIR/for_loop.rs:337:19
-   |
-LL |     for (_, v) in &m {
-   |                   ^^
-   |
-   = note: `-D clippy::for-kv-map` implied by `-D warnings`
-help: use the corresponding method
-   |
-LL |     for v in m.values() {
-   |         ^    ^^^^^^^^^^
-
-error: you seem to want to iterate on a map's values
-  --> $DIR/for_loop.rs:342:19
-   |
-LL |     for (_, v) in &*m {
-   |                   ^^^
-help: use the corresponding method
-   |
-LL |     for v in (*m).values() {
-   |         ^    ^^^^^^^^^^^^^
-
-error: you seem to want to iterate on a map's values
-  --> $DIR/for_loop.rs:350:19
-   |
-LL |     for (_, v) in &mut m {
-   |                   ^^^^^^
-help: use the corresponding method
-   |
-LL |     for v in m.values_mut() {
-   |         ^    ^^^^^^^^^^^^^^
-
-error: you seem to want to iterate on a map's values
-  --> $DIR/for_loop.rs:355:19
-   |
-LL |     for (_, v) in &mut *m {
-   |                   ^^^^^^^
-help: use the corresponding method
-   |
-LL |     for v in (*m).values_mut() {
-   |         ^    ^^^^^^^^^^^^^^^^^
-
-error: you seem to want to iterate on a map's keys
-  --> $DIR/for_loop.rs:361:24
-   |
-LL |     for (k, _value) in rm {
-   |                        ^^
-help: use the corresponding method
-   |
-LL |     for k in rm.keys() {
-   |         ^    ^^^^^^^^^
-
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:414:14
+  --> $DIR/for_loop.rs:372:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
@@ -353,31 +301,31 @@ LL |     for i in 0..src.len() {
    = note: `-D clippy::manual-memcpy` implied by `-D warnings`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:419:14
+  --> $DIR/for_loop.rs:377:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[10..(src.len() + 10)].clone_from_slice(&src[..])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:424:14
+  --> $DIR/for_loop.rs:382:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[10..])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:429:14
+  --> $DIR/for_loop.rs:387:14
    |
 LL |     for i in 11..src.len() {
    |              ^^^^^^^^^^^^^ help: try replacing the loop by: `dst[11..src.len()].clone_from_slice(&src[(11 - 10)..(src.len() - 10)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:434:14
+  --> $DIR/for_loop.rs:392:14
    |
 LL |     for i in 0..dst.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst.clone_from_slice(&src[..dst.len()])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:447:14
+  --> $DIR/for_loop.rs:405:14
    |
 LL |     for i in 10..256 {
    |              ^^^^^^^
@@ -388,34 +336,34 @@ LL |     dst2[(10 + 500)..(256 + 500)].clone_from_slice(&src[10..256]) {
    |
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:459:14
+  --> $DIR/for_loop.rs:417:14
    |
 LL |     for i in 10..LOOP_OFFSET {
    |              ^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[(10 + LOOP_OFFSET)..(LOOP_OFFSET + LOOP_OFFSET)].clone_from_slice(&src[(10 - some_var)..(LOOP_OFFSET - some_var)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:472:14
+  --> $DIR/for_loop.rs:430:14
    |
 LL |     for i in 0..src_vec.len() {
    |              ^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst_vec[..src_vec.len()].clone_from_slice(&src_vec[..])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:501:14
+  --> $DIR/for_loop.rs:459:14
    |
 LL |     for i in from..from + src.len() {
    |              ^^^^^^^^^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + src.len()].clone_from_slice(&src[0..(from + src.len() - from)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:505:14
+  --> $DIR/for_loop.rs:463:14
    |
 LL |     for i in from..from + 3 {
    |              ^^^^^^^^^^^^^^ help: try replacing the loop by: `dst[from..from + 3].clone_from_slice(&src[0..(from + 3 - from)])`
 
 error: it looks like you're manually copying between slices
-  --> $DIR/for_loop.rs:512:14
+  --> $DIR/for_loop.rs:470:14
    |
 LL |     for i in 0..src.len() {
    |              ^^^^^^^^^^^^ help: try replacing the loop by: `dst[..src.len()].clone_from_slice(&src[..])`
 
-error: aborting due to 51 previous errors
+error: aborting due to 46 previous errors