From 42cf98553a72f4b6cdac2c4c6378121d30f3bf5f Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Sat, 4 Jun 2022 21:20:41 +0900 Subject: [PATCH] refactor: check copied and cloned --- clippy_lints/src/methods/filter_map.rs | 4 +- tests/ui/manual_filter_map.fixed | 50 ++++++++------ tests/ui/manual_filter_map.rs | 65 ++++++++++-------- tests/ui/manual_filter_map.stderr | 74 +++++++++++++-------- tests/ui/manual_find_map.fixed | 64 ++++++++++-------- tests/ui/manual_find_map.rs | 77 ++++++++++++--------- tests/ui/manual_find_map.stderr | 92 ++++++++++++++++---------- 7 files changed, 258 insertions(+), 168 deletions(-) diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 6cabe74fdef..7127d8242d8 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -181,9 +181,11 @@ pub(super) fn check<'tcx>( } fn acceptable_methods(method: &PathSegment<'_>) -> bool { - let methods: [Symbol; 6] = [ + let methods: [Symbol; 8] = [ sym::clone, sym::as_ref, + sym!(copied), + sym!(cloned), sym!(as_deref), sym!(as_mut), sym!(as_deref_mut), diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index 9de7040000a..de0d8614889 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -36,44 +36,52 @@ fn to_res(_: T) -> Result { unimplemented!() } -struct OptionFoo { - field: Option, -} - -struct ResultFoo { - field: Result, +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, } fn issue_8920() { - let vec = vec![OptionFoo { - field: Some(String::from("str")), + let mut vec = vec![Issue8920 { + option_field: Some(String::from("str")), + result_field: Ok(String::from("str")), + ref_field: Some(&1), }]; - let _ = vec - .iter() - .filter_map(|f| f.field.clone()); - - let mut vec = vec![ResultFoo { - field: Ok(String::from("str")), - }]; - let _ = vec.iter().filter_map(|f| f.field.clone().ok()); let _ = vec .iter() - .filter_map(|f| f.field.as_ref().ok()); + .filter_map(|f| f.option_field.clone()); let _ = vec .iter() - .filter_map(|f| f.field.as_deref().ok()); + .filter_map(|f| f.ref_field.cloned()); + + let _ = vec + .iter() + .filter_map(|f| f.ref_field.copied()); + + let _ = vec + .iter() + .filter_map(|f| f.result_field.clone().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.result_field.as_ref().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.result_field.as_deref().ok()); let _ = vec .iter_mut() - .filter_map(|f| f.field.as_mut().ok()); + .filter_map(|f| f.result_field.as_mut().ok()); let _ = vec .iter_mut() - .filter_map(|f| f.field.as_deref_mut().ok()); + .filter_map(|f| f.result_field.as_deref_mut().ok()); let _ = vec .iter() - .filter_map(|f| f.field.to_owned().ok()); + .filter_map(|f| f.result_field.to_owned().ok()); } diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index 6766078d694..bd6516f038b 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -36,50 +36,61 @@ fn to_res(_: T) -> Result { unimplemented!() } -struct OptionFoo { - field: Option, -} - -struct ResultFoo { - field: Result, +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, } fn issue_8920() { - let vec = vec![OptionFoo { - field: Some(String::from("str")), + let mut vec = vec![Issue8920 { + option_field: Some(String::from("str")), + result_field: Ok(String::from("str")), + ref_field: Some(&1), }]; - let _ = vec - .iter() - .filter(|f| f.field.is_some()) - .map(|f| f.field.clone().unwrap()); - - let mut vec = vec![ResultFoo { - field: Ok(String::from("str")), - }]; - let _ = vec.iter().filter(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); let _ = vec .iter() - .filter(|f| f.field.is_ok()) - .map(|f| f.field.as_ref().unwrap()); + .filter(|f| f.option_field.is_some()) + .map(|f| f.option_field.clone().unwrap()); let _ = vec .iter() - .filter(|f| f.field.is_ok()) - .map(|f| f.field.as_deref().unwrap()); + .filter(|f| f.ref_field.is_some()) + .map(|f| f.ref_field.cloned().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.ref_field.is_some()) + .map(|f| f.ref_field.copied().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.clone().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_ref().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_deref().unwrap()); let _ = vec .iter_mut() - .filter(|f| f.field.is_ok()) - .map(|f| f.field.as_mut().unwrap()); + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_mut().unwrap()); let _ = vec .iter_mut() - .filter(|f| f.field.is_ok()) - .map(|f| f.field.as_deref_mut().unwrap()); + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_deref_mut().unwrap()); let _ = vec .iter() - .filter(|f| f.field.is_ok()) - .map(|f| f.field.to_owned().unwrap()); + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.to_owned().unwrap()); } diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index 5e67ca348fa..465f1b19110 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -19,58 +19,76 @@ LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_o | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_res(a).ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:53:10 + --> $DIR/manual_filter_map.rs:54:10 | -LL | .filter(|f| f.field.is_some()) +LL | .filter(|f| f.option_field.is_some()) | __________^ -LL | | .map(|f| f.field.clone().unwrap()); - | |__________________________________________^ help: try: `filter_map(|f| f.field.clone())` +LL | | .map(|f| f.option_field.clone().unwrap()); + | |_________________________________________________^ help: try: `filter_map(|f| f.option_field.clone())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:59:24 + --> $DIR/manual_filter_map.rs:59:10 | -LL | let _ = vec.iter().filter(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|f| f.field.clone().ok())` +LL | .filter(|f| f.ref_field.is_some()) + | __________^ +LL | | .map(|f| f.ref_field.cloned().unwrap()); + | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.cloned())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:63:10 + --> $DIR/manual_filter_map.rs:64:10 | -LL | .filter(|f| f.field.is_ok()) +LL | .filter(|f| f.ref_field.is_some()) | __________^ -LL | | .map(|f| f.field.as_ref().unwrap()); - | |___________________________________________^ help: try: `filter_map(|f| f.field.as_ref().ok())` +LL | | .map(|f| f.ref_field.copied().unwrap()); + | |_______________________________________________^ help: try: `filter_map(|f| f.ref_field.copied())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:68:10 + --> $DIR/manual_filter_map.rs:69:10 | -LL | .filter(|f| f.field.is_ok()) +LL | .filter(|f| f.result_field.is_ok()) | __________^ -LL | | .map(|f| f.field.as_deref().unwrap()); - | |_____________________________________________^ help: try: `filter_map(|f| f.field.as_deref().ok())` +LL | | .map(|f| f.result_field.clone().unwrap()); + | |_________________________________________________^ help: try: `filter_map(|f| f.result_field.clone().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:73:10 + --> $DIR/manual_filter_map.rs:74:10 | -LL | .filter(|f| f.field.is_ok()) +LL | .filter(|f| f.result_field.is_ok()) | __________^ -LL | | .map(|f| f.field.as_mut().unwrap()); - | |___________________________________________^ help: try: `filter_map(|f| f.field.as_mut().ok())` +LL | | .map(|f| f.result_field.as_ref().unwrap()); + | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_ref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:78:10 + --> $DIR/manual_filter_map.rs:79:10 | -LL | .filter(|f| f.field.is_ok()) +LL | .filter(|f| f.result_field.is_ok()) | __________^ -LL | | .map(|f| f.field.as_deref_mut().unwrap()); - | |_________________________________________________^ help: try: `filter_map(|f| f.field.as_deref_mut().ok())` +LL | | .map(|f| f.result_field.as_deref().unwrap()); + | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref().ok())` error: `filter(..).map(..)` can be simplified as `filter_map(..)` - --> $DIR/manual_filter_map.rs:83:10 + --> $DIR/manual_filter_map.rs:84:10 | -LL | .filter(|f| f.field.is_ok()) +LL | .filter(|f| f.result_field.is_ok()) | __________^ -LL | | .map(|f| f.field.to_owned().unwrap()); - | |_____________________________________________^ help: try: `filter_map(|f| f.field.to_owned().ok())` +LL | | .map(|f| f.result_field.as_mut().unwrap()); + | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_mut().ok())` -error: aborting due to 10 previous errors +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:89:10 + | +LL | .filter(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); + | |________________________________________________________^ help: try: `filter_map(|f| f.result_field.as_deref_mut().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:94:10 + | +LL | .filter(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.to_owned().unwrap()); + | |____________________________________________________^ help: try: `filter_map(|f| f.result_field.to_owned().ok())` + +error: aborting due to 12 previous errors diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index e7b5081ea59..d69b6c1dcf3 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -36,40 +36,52 @@ fn to_res(_: T) -> Result { unimplemented!() } -struct OptionFoo { - field: Option, -} - -struct ResultFoo { - field: Result, +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, } fn issue_8920() { - let vec = vec![OptionFoo { - field: Some(String::from("str")), + let mut vec = vec![Issue8920 { + option_field: Some(String::from("str")), + result_field: Ok(String::from("str")), + ref_field: Some(&1), }]; - let _ = vec.iter().find_map(|f| f.field.clone()); - - let mut vec = vec![ResultFoo { - field: Ok(String::from("str")), - }]; - let _ = vec.iter().find_map(|f| f.field.clone().ok()); - - let _ = vec.iter().find_map(|f| f.field.as_ref().ok()); let _ = vec .iter() - .find_map(|f| f.field.as_deref().ok()); - - let _ = vec - .iter_mut() - .find_map(|f| f.field.as_mut().ok()); - - let _ = vec - .iter_mut() - .find_map(|f| f.field.as_deref_mut().ok()); + .find_map(|f| f.option_field.clone()); let _ = vec .iter() - .find_map(|f| f.field.to_owned().ok()); + .find_map(|f| f.ref_field.cloned()); + + let _ = vec + .iter() + .find_map(|f| f.ref_field.copied()); + + let _ = vec + .iter() + .find_map(|f| f.result_field.clone().ok()); + + let _ = vec + .iter() + .find_map(|f| f.result_field.as_ref().ok()); + + let _ = vec + .iter() + .find_map(|f| f.result_field.as_deref().ok()); + + let _ = vec + .iter_mut() + .find_map(|f| f.result_field.as_mut().ok()); + + let _ = vec + .iter_mut() + .find_map(|f| f.result_field.as_deref_mut().ok()); + + let _ = vec + .iter() + .find_map(|f| f.result_field.to_owned().ok()); } diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index 46badbb9a18..1c4e18e31c8 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -36,44 +36,61 @@ fn to_res(_: T) -> Result { unimplemented!() } -struct OptionFoo { - field: Option, -} - -struct ResultFoo { - field: Result, +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, } fn issue_8920() { - let vec = vec![OptionFoo { - field: Some(String::from("str")), + let mut vec = vec![Issue8920 { + option_field: Some(String::from("str")), + result_field: Ok(String::from("str")), + ref_field: Some(&1), }]; - let _ = vec.iter().find(|f| f.field.is_some()).map(|f| f.field.clone().unwrap()); - - let mut vec = vec![ResultFoo { - field: Ok(String::from("str")), - }]; - let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); - - let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.as_ref().unwrap()); let _ = vec .iter() - .find(|f| f.field.is_ok()) - .map(|f| f.field.as_deref().unwrap()); - - let _ = vec - .iter_mut() - .find(|f| f.field.is_ok()) - .map(|f| f.field.as_mut().unwrap()); - - let _ = vec - .iter_mut() - .find(|f| f.field.is_ok()) - .map(|f| f.field.as_deref_mut().unwrap()); + .find(|f| f.option_field.is_some()) + .map(|f| f.option_field.clone().unwrap()); let _ = vec .iter() - .find(|f| f.field.is_ok()) - .map(|f| f.field.to_owned().unwrap()); + .find(|f| f.ref_field.is_some()) + .map(|f| f.ref_field.cloned().unwrap()); + + let _ = vec + .iter() + .find(|f| f.ref_field.is_some()) + .map(|f| f.ref_field.copied().unwrap()); + + let _ = vec + .iter() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.clone().unwrap()); + + let _ = vec + .iter() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_ref().unwrap()); + + let _ = vec + .iter() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_deref().unwrap()); + + let _ = vec + .iter_mut() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_mut().unwrap()); + + let _ = vec + .iter_mut() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_deref_mut().unwrap()); + + let _ = vec + .iter() + .find(|f| f.result_field.is_ok()) + .map(|f| f.result_field.to_owned().unwrap()); } diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 2b6955a17df..9dea42b7686 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -19,54 +19,76 @@ LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:51:24 + --> $DIR/manual_find_map.rs:54:10 | -LL | let _ = vec.iter().find(|f| f.field.is_some()).map(|f| f.field.clone().unwrap()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|f| f.field.clone())` - -error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:56:24 - | -LL | let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|f| f.field.clone().ok())` - -error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:58:24 - | -LL | let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.as_ref().unwrap()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|f| f.field.as_ref().ok())` - -error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:62:10 - | -LL | .find(|f| f.field.is_ok()) +LL | .find(|f| f.option_field.is_some()) | __________^ -LL | | .map(|f| f.field.as_deref().unwrap()); - | |_____________________________________________^ help: try: `find_map(|f| f.field.as_deref().ok())` +LL | | .map(|f| f.option_field.clone().unwrap()); + | |_________________________________________________^ help: try: `find_map(|f| f.option_field.clone())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:67:10 + --> $DIR/manual_find_map.rs:59:10 | -LL | .find(|f| f.field.is_ok()) +LL | .find(|f| f.ref_field.is_some()) | __________^ -LL | | .map(|f| f.field.as_mut().unwrap()); - | |___________________________________________^ help: try: `find_map(|f| f.field.as_mut().ok())` +LL | | .map(|f| f.ref_field.cloned().unwrap()); + | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.cloned())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:72:10 + --> $DIR/manual_find_map.rs:64:10 | -LL | .find(|f| f.field.is_ok()) +LL | .find(|f| f.ref_field.is_some()) | __________^ -LL | | .map(|f| f.field.as_deref_mut().unwrap()); - | |_________________________________________________^ help: try: `find_map(|f| f.field.as_deref_mut().ok())` +LL | | .map(|f| f.ref_field.copied().unwrap()); + | |_______________________________________________^ help: try: `find_map(|f| f.ref_field.copied())` error: `find(..).map(..)` can be simplified as `find_map(..)` - --> $DIR/manual_find_map.rs:77:10 + --> $DIR/manual_find_map.rs:69:10 | -LL | .find(|f| f.field.is_ok()) +LL | .find(|f| f.result_field.is_ok()) | __________^ -LL | | .map(|f| f.field.to_owned().unwrap()); - | |_____________________________________________^ help: try: `find_map(|f| f.field.to_owned().ok())` +LL | | .map(|f| f.result_field.clone().unwrap()); + | |_________________________________________________^ help: try: `find_map(|f| f.result_field.clone().ok())` -error: aborting due to 10 previous errors +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:74:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_ref().unwrap()); + | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_ref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:79:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_deref().unwrap()); + | |____________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:84:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_mut().unwrap()); + | |__________________________________________________^ help: try: `find_map(|f| f.result_field.as_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:89:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_deref_mut().unwrap()); + | |________________________________________________________^ help: try: `find_map(|f| f.result_field.as_deref_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:94:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.to_owned().unwrap()); + | |____________________________________________________^ help: try: `find_map(|f| f.result_field.to_owned().ok())` + +error: aborting due to 12 previous errors