From 007fae10ed8c8fa51b396bcc02071fe792f2c288 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Thu, 2 Jun 2022 00:57:08 +0900 Subject: [PATCH 1/3] fix(manual_find_map and manual_filter_map): check clone method --- clippy_lints/src/methods/filter_map.rs | 10 +++++++++- tests/ui/manual_filter_map.fixed | 22 ++++++++++++++++++++++ tests/ui/manual_filter_map.rs | 23 +++++++++++++++++++++++ tests/ui/manual_filter_map.stderr | 16 +++++++++++++++- tests/ui/manual_find_map.fixed | 20 ++++++++++++++++++++ tests/ui/manual_find_map.rs | 20 ++++++++++++++++++++ tests/ui/manual_find_map.stderr | 14 +++++++++++++- 7 files changed, 122 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 558cb6bd64e..04ca295dc69 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -155,7 +155,15 @@ pub(super) fn check<'tcx>( } false }; - if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg); + + if match map_arg.kind { + ExprKind::MethodCall(clone, [original_arg], _) => { + clone.ident.name == sym::clone + && SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg) + }, + _ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg) + }; + then { let span = filter_span.with_hi(expr.span.hi()); let (filter_name, lint) = if is_find { diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index fc8f58f8ea5..c88a4f43de2 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -35,3 +35,25 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct OptionFoo { + field: Option, +} + +struct ResultFoo { + field: Result, +} + +fn issue_8920() { + let vec = vec![OptionFoo { + field: Some(String::from("str")), + }]; + let _ = vec + .iter() + .filter_map(|f| f.field.clone()); + + let vec = vec![ResultFoo { + field: Ok(String::from("str")), + }]; + let _ = vec.iter().filter_map(|f| f.field.clone().ok()); +} diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index 3af4bbee3bf..bb859ebe315 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -35,3 +35,26 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct OptionFoo { + field: Option, +} + +struct ResultFoo { + field: Result, +} + +fn issue_8920() { + let vec = vec![OptionFoo { + field: Some(String::from("str")), + }]; + let _ = vec + .iter() + .filter(|f| f.field.is_some()) + .map(|f| f.field.clone().unwrap()); + + let vec = vec![ResultFoo { + field: Ok(String::from("str")), + }]; + let _ = vec.iter().filter(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); +} diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index 4d4e2d5c12f..a78343e882f 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -18,5 +18,19 @@ error: `filter(..).map(..)` can be simplified as `filter_map(..)` LL | let _ = (0..).filter(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `filter_map(|a| to_res(a).ok())` -error: aborting due to 3 previous errors +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:53:10 + | +LL | .filter(|f| f.field.is_some()) + | __________^ +LL | | .map(|f| f.field.clone().unwrap()); + | |__________________________________________^ help: try: `filter_map(|f| f.field.clone())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:59:24 + | +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())` + +error: aborting due to 5 previous errors diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index 95e97c4fd1f..d78d72d8e8f 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -35,3 +35,23 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct OptionFoo { + field: Option, +} + +struct ResultFoo { + field: Result, +} + +fn issue_8920() { + let vec = vec![OptionFoo { + field: Some(String::from("str")), + }]; + let _ = vec.iter().find_map(|f| f.field.clone()); + + let vec = vec![ResultFoo { + field: Ok(String::from("str")), + }]; + let _ = vec.iter().find_map(|f| f.field.clone().ok()); +} diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index cd3c82e3b25..74e8e52ed16 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -35,3 +35,23 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct OptionFoo { + field: Option, +} + +struct ResultFoo { + field: Result, +} + +fn issue_8920() { + let vec = vec![OptionFoo { + field: Some(String::from("str")), + }]; + let _ = vec.iter().find(|f| f.field.is_some()).map(|f| f.field.clone().unwrap()); + + let vec = vec![ResultFoo { + field: Ok(String::from("str")), + }]; + let _ = vec.iter().find(|f| f.field.is_ok()).map(|f| f.field.clone().unwrap()); +} diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 9e7f798df45..0b1465a8422 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -18,5 +18,17 @@ error: `find(..).map(..)` can be simplified as `find_map(..)` LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())` -error: aborting due to 3 previous errors +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:51:24 + | +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: aborting due to 5 previous errors From 990f8bf5a6354d98a7e0b628f9f1b810644c3ad8 Mon Sep 17 00:00:00 2001 From: kyoto7250 <50972773+kyoto7250@users.noreply.github.com> Date: Sat, 4 Jun 2022 16:55:33 +0900 Subject: [PATCH 2/3] refactor: Add some methods --- clippy_lints/src/methods/filter_map.rs | 19 ++++++++++-- tests/ui/manual_filter_map.fixed | 22 +++++++++++++- tests/ui/manual_filter_map.rs | 27 ++++++++++++++++- tests/ui/manual_filter_map.stderr | 42 +++++++++++++++++++++++++- tests/ui/manual_find_map.fixed | 20 +++++++++++- tests/ui/manual_find_map.rs | 24 ++++++++++++++- tests/ui/manual_find_map.stderr | 40 +++++++++++++++++++++++- 7 files changed, 185 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 04ca295dc69..6cabe74fdef 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -6,7 +6,7 @@ use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::Res; -use rustc_hir::{Expr, ExprKind, PatKind, QPath, UnOp}; +use rustc_hir::{Expr, ExprKind, PatKind, PathSegment, QPath, UnOp}; use rustc_lint::LateContext; use rustc_span::source_map::Span; use rustc_span::symbol::{sym, Symbol}; @@ -157,8 +157,8 @@ pub(super) fn check<'tcx>( }; if match map_arg.kind { - ExprKind::MethodCall(clone, [original_arg], _) => { - clone.ident.name == sym::clone + ExprKind::MethodCall(method, [original_arg], _) => { + acceptable_methods(method) && SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, original_arg) }, _ => SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg) @@ -179,3 +179,16 @@ pub(super) fn check<'tcx>( } } } + +fn acceptable_methods(method: &PathSegment<'_>) -> bool { + let methods: [Symbol; 6] = [ + sym::clone, + sym::as_ref, + sym!(as_deref), + sym!(as_mut), + sym!(as_deref_mut), + sym!(to_owned), + ]; + + methods.contains(&method.ident.name) +} diff --git a/tests/ui/manual_filter_map.fixed b/tests/ui/manual_filter_map.fixed index c88a4f43de2..9de7040000a 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -52,8 +52,28 @@ fn issue_8920() { .iter() .filter_map(|f| f.field.clone()); - let vec = vec![ResultFoo { + 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()); + + let _ = vec + .iter() + .filter_map(|f| f.field.as_deref().ok()); + + let _ = vec + .iter_mut() + .filter_map(|f| f.field.as_mut().ok()); + + let _ = vec + .iter_mut() + .filter_map(|f| f.field.as_deref_mut().ok()); + + let _ = vec + .iter() + .filter_map(|f| f.field.to_owned().ok()); } diff --git a/tests/ui/manual_filter_map.rs b/tests/ui/manual_filter_map.rs index bb859ebe315..6766078d694 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -53,8 +53,33 @@ fn issue_8920() { .filter(|f| f.field.is_some()) .map(|f| f.field.clone().unwrap()); - let vec = vec![ResultFoo { + 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()); + + let _ = vec + .iter() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_deref().unwrap()); + + let _ = vec + .iter_mut() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_mut().unwrap()); + + let _ = vec + .iter_mut() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.as_deref_mut().unwrap()); + + let _ = vec + .iter() + .filter(|f| f.field.is_ok()) + .map(|f| f.field.to_owned().unwrap()); } diff --git a/tests/ui/manual_filter_map.stderr b/tests/ui/manual_filter_map.stderr index a78343e882f..5e67ca348fa 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -32,5 +32,45 @@ error: `filter(..).map(..)` can be simplified as `filter_map(..)` 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())` -error: aborting due to 5 previous errors +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:63:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_ref().unwrap()); + | |___________________________________________^ help: try: `filter_map(|f| f.field.as_ref().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:68:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref().unwrap()); + | |_____________________________________________^ help: try: `filter_map(|f| f.field.as_deref().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:73:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_mut().unwrap()); + | |___________________________________________^ help: try: `filter_map(|f| f.field.as_mut().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:78:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref_mut().unwrap()); + | |_________________________________________________^ help: try: `filter_map(|f| f.field.as_deref_mut().ok())` + +error: `filter(..).map(..)` can be simplified as `filter_map(..)` + --> $DIR/manual_filter_map.rs:83:10 + | +LL | .filter(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.to_owned().unwrap()); + | |_____________________________________________^ help: try: `filter_map(|f| f.field.to_owned().ok())` + +error: aborting due to 10 previous errors diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed index d78d72d8e8f..e7b5081ea59 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -50,8 +50,26 @@ fn issue_8920() { }]; let _ = vec.iter().find_map(|f| f.field.clone()); - let vec = vec![ResultFoo { + 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()); + + let _ = vec + .iter() + .find_map(|f| f.field.to_owned().ok()); } diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs index 74e8e52ed16..46badbb9a18 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -50,8 +50,30 @@ fn issue_8920() { }]; let _ = vec.iter().find(|f| f.field.is_some()).map(|f| f.field.clone().unwrap()); - let vec = vec![ResultFoo { + 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()); + + let _ = vec + .iter() + .find(|f| f.field.is_ok()) + .map(|f| f.field.to_owned().unwrap()); } diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr index 0b1465a8422..2b6955a17df 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -30,5 +30,43 @@ error: `find(..).map(..)` can be simplified as `find_map(..)` 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: aborting due to 5 previous errors +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 | | .map(|f| f.field.as_deref().unwrap()); + | |_____________________________________________^ help: try: `find_map(|f| f.field.as_deref().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:67:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_mut().unwrap()); + | |___________________________________________^ help: try: `find_map(|f| f.field.as_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:72:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.as_deref_mut().unwrap()); + | |_________________________________________________^ help: try: `find_map(|f| f.field.as_deref_mut().ok())` + +error: `find(..).map(..)` can be simplified as `find_map(..)` + --> $DIR/manual_find_map.rs:77:10 + | +LL | .find(|f| f.field.is_ok()) + | __________^ +LL | | .map(|f| f.field.to_owned().unwrap()); + | |_____________________________________________^ help: try: `find_map(|f| f.field.to_owned().ok())` + +error: aborting due to 10 previous errors 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 3/3] 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