diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 558cb6bd64e..7127d8242d8 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -6,7 +6,7 @@ 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}; @@ -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(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) + }; + then { let span = filter_span.with_hi(expr.span.hi()); let (filter_name, lint) = if is_find { @@ -171,3 +179,18 @@ pub(super) fn check<'tcx>( } } } + +fn acceptable_methods(method: &PathSegment<'_>) -> bool { + let methods: [Symbol; 8] = [ + sym::clone, + sym::as_ref, + sym!(copied), + sym!(cloned), + 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 fc8f58f8ea5..de0d8614889 100644 --- a/tests/ui/manual_filter_map.fixed +++ b/tests/ui/manual_filter_map.fixed @@ -35,3 +35,53 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, +} + +fn issue_8920() { + 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.option_field.clone()); + + let _ = vec + .iter() + .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.result_field.as_mut().ok()); + + let _ = vec + .iter_mut() + .filter_map(|f| f.result_field.as_deref_mut().ok()); + + let _ = vec + .iter() + .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 3af4bbee3bf..bd6516f038b 100644 --- a/tests/ui/manual_filter_map.rs +++ b/tests/ui/manual_filter_map.rs @@ -35,3 +35,62 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, +} + +fn issue_8920() { + 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.option_field.is_some()) + .map(|f| f.option_field.clone().unwrap()); + + let _ = vec + .iter() + .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.result_field.is_ok()) + .map(|f| f.result_field.as_mut().unwrap()); + + let _ = vec + .iter_mut() + .filter(|f| f.result_field.is_ok()) + .map(|f| f.result_field.as_deref_mut().unwrap()); + + let _ = vec + .iter() + .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 4d4e2d5c12f..465f1b19110 100644 --- a/tests/ui/manual_filter_map.stderr +++ b/tests/ui/manual_filter_map.stderr @@ -18,5 +18,77 @@ 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:54:10 + | +LL | .filter(|f| f.option_field.is_some()) + | __________^ +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:10 + | +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:64:10 + | +LL | .filter(|f| f.ref_field.is_some()) + | __________^ +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:69:10 + | +LL | .filter(|f| f.result_field.is_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:74:10 + | +LL | .filter(|f| f.result_field.is_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:79:10 + | +LL | .filter(|f| f.result_field.is_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:84:10 + | +LL | .filter(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.as_mut().unwrap()); + | |__________________________________________________^ help: try: `filter_map(|f| f.result_field.as_mut().ok())` + +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 95e97c4fd1f..d69b6c1dcf3 100644 --- a/tests/ui/manual_find_map.fixed +++ b/tests/ui/manual_find_map.fixed @@ -35,3 +35,53 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, +} + +fn issue_8920() { + 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.option_field.clone()); + + let _ = vec + .iter() + .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 cd3c82e3b25..1c4e18e31c8 100644 --- a/tests/ui/manual_find_map.rs +++ b/tests/ui/manual_find_map.rs @@ -35,3 +35,62 @@ fn to_opt(_: T) -> Option { fn to_res(_: T) -> Result { unimplemented!() } + +struct Issue8920<'a> { + option_field: Option, + result_field: Result, + ref_field: Option<&'a usize>, +} + +fn issue_8920() { + 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.option_field.is_some()) + .map(|f| f.option_field.clone().unwrap()); + + let _ = vec + .iter() + .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 9e7f798df45..9dea42b7686 100644 --- a/tests/ui/manual_find_map.stderr +++ b/tests/ui/manual_find_map.stderr @@ -18,5 +18,77 @@ 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:54:10 + | +LL | .find(|f| f.option_field.is_some()) + | __________^ +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:59:10 + | +LL | .find(|f| f.ref_field.is_some()) + | __________^ +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:64:10 + | +LL | .find(|f| f.ref_field.is_some()) + | __________^ +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:69:10 + | +LL | .find(|f| f.result_field.is_ok()) + | __________^ +LL | | .map(|f| f.result_field.clone().unwrap()); + | |_________________________________________________^ help: try: `find_map(|f| f.result_field.clone().ok())` + +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