Auto merge of #8930 - kyoto7250:issue_8920, r=Alexendoo

fix(manual_find_map and manual_filter_map): check clone method

close #8920

Added conditional branching when the clone method is used.

Thank you in advance.

---

changelog: check `clone()` and other variant preserving methods in [`manual_find_map`] and [`manual_filter_map`]

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};
@@ -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)
+}

tests/ui/manual_filter_map.fixed

@@ -35,3 +35,53 @@ fn to_opt<T>(_: T) -> Option<T> {
 fn to_res<T>(_: T) -> Result<T, ()> {
     unimplemented!()
 }
+
+struct Issue8920<'a> {
+    option_field: Option<String>,
+    result_field: Result<String, ()>,
+    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());
+}

tests/ui/manual_filter_map.rs

@@ -35,3 +35,62 @@ fn to_opt<T>(_: T) -> Option<T> {
 fn to_res<T>(_: T) -> Result<T, ()> {
     unimplemented!()
 }
+
+struct Issue8920<'a> {
+    option_field: Option<String>,
+    result_field: Result<String, ()>,
+    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());
+}

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
 

tests/ui/manual_find_map.fixed

@@ -35,3 +35,53 @@ fn to_opt<T>(_: T) -> Option<T> {
 fn to_res<T>(_: T) -> Result<T, ()> {
     unimplemented!()
 }
+
+struct Issue8920<'a> {
+    option_field: Option<String>,
+    result_field: Result<String, ()>,
+    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());
+}

tests/ui/manual_find_map.rs

@@ -35,3 +35,62 @@ fn to_opt<T>(_: T) -> Option<T> {
 fn to_res<T>(_: T) -> Result<T, ()> {
     unimplemented!()
 }
+
+struct Issue8920<'a> {
+    option_field: Option<String>,
+    result_field: Result<String, ()>,
+    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());
+}

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