From b38a2d7ce9698a85a1874ed1c92386f0070c4fc9 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 2 Jan 2019 07:42:04 +0100 Subject: [PATCH] UI test cleanup: Extract for_kv_map lint tests --- tests/ui/for_kv_map.rs | 56 ++++++++++++++++++++++++++++ tests/ui/for_kv_map.stderr | 54 +++++++++++++++++++++++++++ tests/ui/for_loop.rs | 42 --------------------- tests/ui/for_loop.stderr | 76 ++++++-------------------------------- 4 files changed, 122 insertions(+), 106 deletions(-) create mode 100644 tests/ui/for_kv_map.rs create mode 100644 tests/ui/for_kv_map.stderr diff --git a/tests/ui/for_kv_map.rs b/tests/ui/for_kv_map.rs new file mode 100644 index 000000000000..d79ea4bebeb5 --- /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 or the MIT license +// , 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 = HashMap::new(); + for (_, v) in &m { + let _v = v; + } + + let m: Rc> = 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 = HashMap::new(); + for (_, v) in &mut m { + let _v = v; + } + + let m: &mut HashMap = &mut HashMap::new(); + for (_, v) in &mut *m { + let _v = v; + } + + let m: HashMap = 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 = 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 000000000000..7b65c58f58d5 --- /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 4747269bccdd..c172b0b3b770 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 = HashMap::new(); - for (_, v) in &m { - let _v = v; - } - - let m: Rc> = 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 = HashMap::new(); - for (_, v) in &mut m { - let _v = v; - } - - let m: &mut HashMap = &mut HashMap::new(); - for (_, v) in &mut *m { - let _v = v; - } - - let m: HashMap = HashMap::new(); - let rm = &m; - for (k, _value) in rm { - let _k = k; - } - - test_for_kv_map(); - fn f(_: &T, _: &T) -> bool { unimplemented!() } @@ -381,17 +350,6 @@ fn main() { } } -#[allow(clippy::used_underscore_binding)] -fn test_for_kv_map() { - let m: HashMap = HashMap::new(); - - // No error, _value is actually used - for (k, _value) in &m { - let _ = _value; - let _k = k; - } -} - #[allow(dead_code)] fn partition(v: &mut [T]) -> usize { let pivot = v.len() - 1; diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index 937bef9f8a6f..4ded425b3210 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::>(); | = 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