Replace find_map with manual_find_map

7 files changed, 141 insertions, 49 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8feb1a148af..7f2de888d35 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2036,6 +2036,7 @@ Released 2018-09-13
 [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
 [`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
 [`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
+[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
 [`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
 [`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index bde9c630c6c..b22ddfacf86 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -746,6 +746,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::ITER_NTH_ZERO,
         &methods::ITER_SKIP_NEXT,
         &methods::MANUAL_FILTER_MAP,
+        &methods::MANUAL_FIND_MAP,
         &methods::MANUAL_SATURATING_ARITHMETIC,
         &methods::MAP_COLLECT_RESULT_UNIT,
         &methods::MAP_FLATTEN,
@@ -1528,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::ITER_NTH_ZERO),
         LintId::of(&methods::ITER_SKIP_NEXT),
         LintId::of(&methods::MANUAL_FILTER_MAP),
+        LintId::of(&methods::MANUAL_FIND_MAP),
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
         LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(&methods::NEW_RET_NO_SELF),
@@ -1826,6 +1828,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::FLAT_MAP_IDENTITY),
         LintId::of(&methods::INSPECT_FOR_EACH),
         LintId::of(&methods::MANUAL_FILTER_MAP),
+        LintId::of(&methods::MANUAL_FIND_MAP),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SKIP_WHILE_NEXT),
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 518d2e67ad1..9d078588746 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -478,6 +478,32 @@ declare_clippy_lint! {
 }
 
 declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `_.find(_).map(_)` that can be written more simply
+    /// as `find_map(_)`.
+    ///
+    /// **Why is this bad?** Redundant code in the `find` and `map` operations is poor style and
+    /// less performant.
+    ///
+    /// **Known problems:** None.
+    ///
+     /// **Example:**
+    /// Bad:
+    /// ```rust
+    /// (0_i32..10)
+    ///     .find(|n| n.checked_add(1).is_some())
+    ///     .map(|n| n.checked_add(1).unwrap());
+    /// ```
+    ///
+    /// Good:
+    /// ```rust
+    /// (0_i32..10).find_map(|n| n.checked_add(1));
+    /// ```
+    pub MANUAL_FIND_MAP,
+    complexity,
+    "using `_.find(_).map(_)` in a way that can be written more simply as `find_map(_)`"
+}
+
+declare_clippy_lint! {
     /// **What it does:** Checks for usage of `_.filter_map(_).next()`.
     ///
     /// **Why is this bad?** Readability, this can be written more concisely as
@@ -1501,6 +1527,7 @@ impl_lint_pass!(Methods => [
     SKIP_WHILE_NEXT,
     FILTER_MAP,
     MANUAL_FILTER_MAP,
+    MANUAL_FIND_MAP,
     FILTER_MAP_NEXT,
     FLAT_MAP_IDENTITY,
     FIND_MAP,
@@ -1568,10 +1595,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
             ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
             ["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
-            ["map", "filter"] => lint_filter_map(cx, expr),
+            ["map", "filter"] => lint_filter_map(cx, expr, false),
             ["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1], self.msrv.as_ref()),
-            ["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]),
+            ["map", "find"] => lint_filter_map(cx, expr, true),
             ["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
             ["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]),
@@ -3016,12 +3043,12 @@ fn lint_skip_while_next<'tcx>(
     }
 }
 
-/// lint use of `filter().map()` for `Iterators`
-fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
+/// lint use of `filter().map()` or `find().map()` for `Iterators`
+fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool) {
     if_chain! {
         if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
         if let ExprKind::MethodCall(_, _, [_, filter_arg], filter_span) = map_recv.kind;
-        if match_trait_method(cx, expr, &paths::ITERATOR);
+        if match_trait_method(cx, map_recv, &paths::ITERATOR);
 
         // filter(|x| ...is_some())...
         if let ExprKind::Closure(_, _, filter_body_id, ..) = filter_arg.kind;
@@ -3078,10 +3105,16 @@ fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
         if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
         then {
             let span = filter_span.to(map_span);
-            let msg = "`filter(..).map(..)` can be simplified as `filter_map(..)`";
+            let (filter_name, lint) = if is_find {
+                ("find", MANUAL_FIND_MAP)
+            } else {
+                ("filter", MANUAL_FILTER_MAP)
+            };
+            let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name);
             let to_opt = if is_result { ".ok()" } else { "" };
-            let sugg = format!("filter_map(|{}| {}{})", map_param_ident, snippet(cx, map_arg.span, ".."), to_opt);
-            span_lint_and_sugg(cx, MANUAL_FILTER_MAP, span, msg, "try", sugg, Applicability::MachineApplicable);
+            let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident,
+                snippet(cx, map_arg.span, ".."), to_opt);
+            span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
         }
     }
 }
@@ -3120,21 +3153,6 @@ fn lint_filter_map_next<'tcx>(
     }
 }
 
-/// lint use of `find().map()` for `Iterators`
-fn lint_find_map<'tcx>(
-    cx: &LateContext<'tcx>,
-    expr: &'tcx hir::Expr<'_>,
-    _find_args: &'tcx [hir::Expr<'_>],
-    map_args: &'tcx [hir::Expr<'_>],
-) {
-    // lint if caller of `.filter().map()` is an Iterator
-    if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
-        let msg = "called `find(..).map(..)` on an `Iterator`";
-        let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
-        span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint);
-    }
-}
-
 /// lint use of `filter_map().map()` for `Iterators`
 fn lint_filter_map_map<'tcx>(
     cx: &LateContext<'tcx>,
diff --git a/tests/ui/find_map.stderr b/tests/ui/find_map.stderr
deleted file mode 100644
index aea3cc62afc..00000000000
--- a/tests/ui/find_map.stderr
+++ /dev/null
@@ -1,26 +0,0 @@
-error: called `find(..).map(..)` on an `Iterator`
-  --> $DIR/find_map.rs:20:26
-   |
-LL |     let _: Option<i32> = a.iter().find(|s| s.parse::<i32>().is_ok()).map(|s| s.parse().unwrap());
-   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::find-map` implied by `-D warnings`
-   = help: this is more succinctly expressed by calling `.find_map(..)` instead
-
-error: called `find(..).map(..)` on an `Iterator`
-  --> $DIR/find_map.rs:23:29
-   |
-LL |       let _: Option<Flavor> = desserts_of_the_week
-   |  _____________________________^
-LL | |         .iter()
-LL | |         .find(|dessert| match *dessert {
-LL | |             Dessert::Cake(_) => true,
-...  |
-LL | |             _ => unreachable!(),
-LL | |         });
-   | |__________^
-   |
-   = help: this is more succinctly expressed by calling `.find_map(..)` instead
-
-error: aborting due to 2 previous errors
-
diff --git a/tests/ui/manual_find_map.fixed b/tests/ui/manual_find_map.fixed
new file mode 100644
index 00000000000..95e97c4fd1f
--- /dev/null
+++ b/tests/ui/manual_find_map.fixed
@@ -0,0 +1,37 @@
+// run-rustfix
+#![allow(dead_code)]
+#![warn(clippy::manual_find_map)]
+#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
+
+fn main() {
+    // is_some(), unwrap()
+    let _ = (0..).find_map(|a| to_opt(a));
+
+    // ref pattern, expect()
+    let _ = (0..).find_map(|a| to_opt(a));
+
+    // is_ok(), unwrap_or()
+    let _ = (0..).find_map(|a| to_res(a).ok());
+}
+
+fn no_lint() {
+    // no shared code
+    let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
+
+    // very close but different since filter() provides a reference
+    let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
+
+    // similar but different
+    let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
+    let _ = (0..)
+        .find(|n| to_opt(n).map(|n| n + 1).is_some())
+        .map(|a| to_opt(a).unwrap());
+}
+
+fn to_opt<T>(_: T) -> Option<T> {
+    unimplemented!()
+}
+
+fn to_res<T>(_: T) -> Result<T, ()> {
+    unimplemented!()
+}
diff --git a/tests/ui/manual_find_map.rs b/tests/ui/manual_find_map.rs
new file mode 100644
index 00000000000..cd3c82e3b25
--- /dev/null
+++ b/tests/ui/manual_find_map.rs
@@ -0,0 +1,37 @@
+// run-rustfix
+#![allow(dead_code)]
+#![warn(clippy::manual_find_map)]
+#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
+
+fn main() {
+    // is_some(), unwrap()
+    let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
+
+    // ref pattern, expect()
+    let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
+
+    // is_ok(), unwrap_or()
+    let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
+}
+
+fn no_lint() {
+    // no shared code
+    let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
+
+    // very close but different since filter() provides a reference
+    let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
+
+    // similar but different
+    let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
+    let _ = (0..)
+        .find(|n| to_opt(n).map(|n| n + 1).is_some())
+        .map(|a| to_opt(a).unwrap());
+}
+
+fn to_opt<T>(_: T) -> Option<T> {
+    unimplemented!()
+}
+
+fn to_res<T>(_: T) -> Result<T, ()> {
+    unimplemented!()
+}
diff --git a/tests/ui/manual_find_map.stderr b/tests/ui/manual_find_map.stderr
new file mode 100644
index 00000000000..9e7f798df45
--- /dev/null
+++ b/tests/ui/manual_find_map.stderr
@@ -0,0 +1,22 @@
+error: `find(..).map(..)` can be simplified as `find_map(..)`
+  --> $DIR/manual_find_map.rs:8:19
+   |
+LL |     let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
+   |
+   = note: `-D clippy::manual-find-map` implied by `-D warnings`
+
+error: `find(..).map(..)` can be simplified as `find_map(..)`
+  --> $DIR/manual_find_map.rs:11:19
+   |
+LL |     let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
+   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
+
+error: `find(..).map(..)` can be simplified as `find_map(..)`
+  --> $DIR/manual_find_map.rs:14:19
+   |
+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
+