Change unnecessary_iter_cloned to use multipart_suggestion
This commit is contained in:
Scott Gerring
parent
0f9cc8d58b
commit
8fe39b276f
5 changed files with 226 additions and 44 deletions
|
|
@ -6,6 +6,7 @@ use clippy_utils::ty::{get_iterator_item_ty, implements_trait};
|
|||
use clippy_utils::visitors::for_each_expr_without_closures;
|
||||
use clippy_utils::{can_mut_borrow_both, fn_def_id, get_parent_expr, path_to_local};
|
||||
use core::ops::ControlFlow;
|
||||
use itertools::Itertools;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def_id::DefId;
|
||||
use rustc_hir::{BindingMode, Expr, ExprKind, Node, PatKind};
|
||||
|
|
@ -122,14 +123,13 @@ pub fn check_for_loop_iter(
|
|||
} else {
|
||||
Applicability::MachineApplicable
|
||||
};
|
||||
diag.span_suggestion(expr.span, "use", snippet.to_owned(), applicability);
|
||||
if !references_to_binding.is_empty() {
|
||||
diag.multipart_suggestion(
|
||||
"remove any references to the binding",
|
||||
references_to_binding,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
|
||||
let combined = references_to_binding
|
||||
.into_iter()
|
||||
.chain(vec![(expr.span, snippet.to_owned())])
|
||||
.collect_vec();
|
||||
|
||||
diag.multipart_suggestion("remove any references to the binding", combined, applicability);
|
||||
},
|
||||
);
|
||||
return true;
|
||||
|
|
|
|||
201
tests/ui/unnecessary_iter_cloned.fixed
Normal file
201
tests/ui/unnecessary_iter_cloned.fixed
Normal file
|
|
@ -0,0 +1,201 @@
|
|||
#![allow(unused_assignments)]
|
||||
#![warn(clippy::unnecessary_to_owned)]
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(Clone, Copy)]
|
||||
enum FileType {
|
||||
Account,
|
||||
PrivateKey,
|
||||
Certificate,
|
||||
}
|
||||
|
||||
fn main() {
|
||||
let path = std::path::Path::new("x");
|
||||
|
||||
let _ = check_files(&[(FileType::Account, path)]);
|
||||
let _ = check_files_vec(vec![(FileType::Account, path)]);
|
||||
|
||||
// negative tests
|
||||
let _ = check_files_ref(&[(FileType::Account, path)]);
|
||||
let _ = check_files_mut(&[(FileType::Account, path)]);
|
||||
let _ = check_files_ref_mut(&[(FileType::Account, path)]);
|
||||
let _ = check_files_self_and_arg(&[(FileType::Account, path)]);
|
||||
let _ = check_files_mut_path_buf(&[(FileType::Account, std::path::PathBuf::new())]);
|
||||
|
||||
check_mut_iteratee_and_modify_inner_variable();
|
||||
}
|
||||
|
||||
// `check_files` and its variants are based on:
|
||||
// https://github.com/breard-r/acmed/blob/1f0dcc32aadbc5e52de6d23b9703554c0f925113/acmed/src/storage.rs#L262
|
||||
fn check_files(files: &[(FileType, &std::path::Path)]) -> bool {
|
||||
for (t, path) in files {
|
||||
let other = match get_file_path(t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn check_files_vec(files: Vec<(FileType, &std::path::Path)>) -> bool {
|
||||
for (t, path) in files.iter() {
|
||||
let other = match get_file_path(t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn check_files_ref(files: &[(FileType, &std::path::Path)]) -> bool {
|
||||
for (ref t, path) in files.iter().copied() {
|
||||
let other = match get_file_path(t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
#[allow(unused_assignments)]
|
||||
fn check_files_mut(files: &[(FileType, &std::path::Path)]) -> bool {
|
||||
for (mut t, path) in files.iter().copied() {
|
||||
t = FileType::PrivateKey;
|
||||
let other = match get_file_path(&t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn check_files_ref_mut(files: &[(FileType, &std::path::Path)]) -> bool {
|
||||
for (ref mut t, path) in files.iter().copied() {
|
||||
*t = FileType::PrivateKey;
|
||||
let other = match get_file_path(t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn check_files_self_and_arg(files: &[(FileType, &std::path::Path)]) -> bool {
|
||||
for (t, path) in files.iter().copied() {
|
||||
let other = match get_file_path(&t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.join(path).is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
#[allow(unused_assignments)]
|
||||
fn check_files_mut_path_buf(files: &[(FileType, std::path::PathBuf)]) -> bool {
|
||||
for (mut t, path) in files.iter().cloned() {
|
||||
t = FileType::PrivateKey;
|
||||
let other = match get_file_path(&t) {
|
||||
Ok(p) => p,
|
||||
Err(_) => {
|
||||
return false;
|
||||
},
|
||||
};
|
||||
if !path.is_file() || !other.is_file() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
fn get_file_path(_file_type: &FileType) -> Result<std::path::PathBuf, std::io::Error> {
|
||||
Ok(std::path::PathBuf::new())
|
||||
}
|
||||
|
||||
// Issue 12098
|
||||
// https://github.com/rust-lang/rust-clippy/issues/12098
|
||||
// no message emits
|
||||
fn check_mut_iteratee_and_modify_inner_variable() {
|
||||
struct Test {
|
||||
list: Vec<String>,
|
||||
mut_this: bool,
|
||||
}
|
||||
|
||||
impl Test {
|
||||
fn list(&self) -> &[String] {
|
||||
&self.list
|
||||
}
|
||||
}
|
||||
|
||||
let mut test = Test {
|
||||
list: vec![String::from("foo"), String::from("bar")],
|
||||
mut_this: false,
|
||||
};
|
||||
|
||||
for _item in test.list().to_vec() {
|
||||
println!("{}", _item);
|
||||
|
||||
test.mut_this = true;
|
||||
{
|
||||
test.mut_this = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod issue_12821 {
|
||||
fn foo() {
|
||||
let v: Vec<_> = "hello".chars().collect();
|
||||
for c in v.iter() {
|
||||
//~^ ERROR: unnecessary use of `cloned`
|
||||
println!("{c}"); // should not suggest to remove `&`
|
||||
}
|
||||
}
|
||||
|
||||
fn bar() {
|
||||
let v: Vec<_> = "hello".chars().collect();
|
||||
for c in v.iter() {
|
||||
//~^ ERROR: unnecessary use of `cloned`
|
||||
let ref_c = c; //~ HELP: remove any references to the binding
|
||||
println!("{ref_c}");
|
||||
}
|
||||
}
|
||||
|
||||
fn baz() {
|
||||
let v: Vec<_> = "hello".chars().enumerate().collect();
|
||||
for (i, c) in v.iter() {
|
||||
//~^ ERROR: unnecessary use of `cloned`
|
||||
let ref_c = c; //~ HELP: remove any references to the binding
|
||||
let ref_i = i;
|
||||
println!("{i} {ref_c}"); // should not suggest to remove `&` from `i`
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,8 +1,6 @@
|
|||
#![allow(unused_assignments)]
|
||||
#![warn(clippy::unnecessary_to_owned)]
|
||||
|
||||
//@no-rustfix: need to change the suggestion to a multipart suggestion
|
||||
|
||||
#[allow(dead_code)]
|
||||
#[derive(Clone, Copy)]
|
||||
enum FileType {
|
||||
|
|
|
|||
|
|
@ -1,71 +1,58 @@
|
|||
error: unnecessary use of `copied`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:33:22
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:31:22
|
||||
|
|
||||
LL | for (t, path) in files.iter().copied() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::unnecessary-to-owned` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_to_owned)]`
|
||||
help: use
|
||||
|
|
||||
LL | for (t, path) in files {
|
||||
| ~~~~~
|
||||
help: remove any references to the binding
|
||||
|
|
||||
LL - let other = match get_file_path(&t) {
|
||||
LL + let other = match get_file_path(t) {
|
||||
LL ~ for (t, path) in files {
|
||||
LL ~ let other = match get_file_path(t) {
|
||||
|
|
||||
|
||||
error: unnecessary use of `copied`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:48:22
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:46:22
|
||||
|
|
||||
LL | for (t, path) in files.iter().copied() {
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use
|
||||
|
|
||||
LL | for (t, path) in files.iter() {
|
||||
| ~~~~~~~~~~~~
|
||||
help: remove any references to the binding
|
||||
|
|
||||
LL - let other = match get_file_path(&t) {
|
||||
LL + let other = match get_file_path(t) {
|
||||
LL ~ for (t, path) in files.iter() {
|
||||
LL ~ let other = match get_file_path(t) {
|
||||
|
|
||||
|
||||
error: unnecessary use of `cloned`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:179:18
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:177:18
|
||||
|
|
||||
LL | for c in v.iter().cloned() {
|
||||
| ^^^^^^^^^^^^^^^^^ help: use: `v.iter()`
|
||||
| ^^^^^^^^^^^^^^^^^ help: remove any references to the binding: `v.iter()`
|
||||
|
||||
error: unnecessary use of `cloned`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:187:18
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:185:18
|
||||
|
|
||||
LL | for c in v.iter().cloned() {
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use
|
||||
|
|
||||
LL | for c in v.iter() {
|
||||
| ~~~~~~~~
|
||||
help: remove any references to the binding
|
||||
|
|
||||
LL - let ref_c = &c;
|
||||
LL + let ref_c = c;
|
||||
LL ~ for c in v.iter() {
|
||||
LL |
|
||||
LL ~ let ref_c = c;
|
||||
|
|
||||
|
||||
error: unnecessary use of `cloned`
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:196:23
|
||||
--> tests/ui/unnecessary_iter_cloned.rs:194:23
|
||||
|
|
||||
LL | for (i, c) in v.iter().cloned() {
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use
|
||||
|
|
||||
LL | for (i, c) in v.iter() {
|
||||
| ~~~~~~~~
|
||||
help: remove any references to the binding
|
||||
|
|
||||
LL ~ for (i, c) in v.iter() {
|
||||
LL |
|
||||
LL ~ let ref_c = c;
|
||||
LL ~ let ref_i = i;
|
||||
|
|
||||
|
|
|
|||
|
|
@ -519,14 +519,10 @@ error: unnecessary use of `to_vec`
|
|||
LL | for t in file_types.to_vec() {
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
help: use
|
||||
|
|
||||
LL | for t in file_types {
|
||||
| ~~~~~~~~~~
|
||||
help: remove any references to the binding
|
||||
|
|
||||
LL - let path = match get_file_path(&t) {
|
||||
LL + let path = match get_file_path(t) {
|
||||
LL ~ for t in file_types {
|
||||
LL ~ let path = match get_file_path(t) {
|
||||
|
|
||||
|
||||
error: unnecessary use of `to_vec`
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue