filter_map_next: clean-up, overhaul suggestions (#17237)

changelog: [`filter_map_next`]: overhaul suggestions

Author: llogiq

clippy_lints/src/methods/filter_map_next.rs

@@ -1,42 +1,33 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::msrvs::{self, Msrv};
 use clippy_utils::res::{MaybeDef, MaybeTypeckRes};
-use clippy_utils::source::snippet;
+use clippy_utils::source::snippet_with_applicability;
 use rustc_errors::Applicability;
-use rustc_hir as hir;
+use rustc_hir::Expr;
 use rustc_lint::LateContext;
 use rustc_span::sym;
 
 use super::FILTER_MAP_NEXT;
 
-pub(super) fn check<'tcx>(
-    cx: &LateContext<'tcx>,
-    expr: &'tcx hir::Expr<'_>,
-    recv: &'tcx hir::Expr<'_>,
-    arg: &'tcx hir::Expr<'_>,
-    msrv: Msrv,
-) {
-    if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
-        if !msrv.meets(cx, msrvs::ITERATOR_FIND_MAP) {
-            return;
-        }
-
-        let msg = "called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling \
-                   `.find_map(..)` instead";
-        let filter_snippet = snippet(cx, arg.span, "..");
-        if filter_snippet.lines().count() <= 1 {
-            let iter_snippet = snippet(cx, recv.span, "..");
-            span_lint_and_sugg(
-                cx,
-                FILTER_MAP_NEXT,
-                expr.span,
-                msg,
-                "try",
-                format!("{iter_snippet}.find_map({filter_snippet})"),
-                Applicability::MachineApplicable,
-            );
-        } else {
-            span_lint(cx, FILTER_MAP_NEXT, expr.span, msg);
-        }
+pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>, msrv: Msrv) {
+    if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) && msrv.meets(cx, msrvs::ITERATOR_FIND_MAP)
+    {
+        span_lint_and_then(
+            cx,
+            FILTER_MAP_NEXT,
+            expr.span,
+            "called `filter_map(..).next()` on an `Iterator`",
+            |diag| {
+                let mut app = Applicability::MachineApplicable;
+                let iter_snippet = snippet_with_applicability(cx, recv.span, "_", &mut app);
+                let filter_snippet = snippet_with_applicability(cx, arg.span, "..", &mut app);
+                diag.span_suggestion_verbose(
+                    expr.span,
+                    "use `.find_map(..)` instead",
+                    format!("{iter_snippet}.find_map({filter_snippet})"),
+                    app,
+                );
+            },
+        );
     }
 }

tests/ui/filter_map_next_fixable.fixed tests/ui/filter_map_next.fixedtests/ui/filter_map_next_fixable.fixed renamed to tests/ui/filter_map_next.fixed

@@ -6,6 +6,27 @@ fn main() {
     let element: Option<i32> = a.iter().find_map(|s| s.parse().ok());
     //~^ filter_map_next
     assert_eq!(element, Some(1));
+
+    #[rustfmt::skip]
+    let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
+    //~^ filter_map_next
+
+
+        .into_iter().find_map(|x| {
+            if x == 2 {
+                Some(x * 2)
+            } else {
+                None
+            }
+        });
+
+    let element: Option<i32> = a
+        // very important comment -- don't delete!
+        .iter().find_map(|s| {
+            // another extremely important comment
+            s.parse().ok()
+        });
+    //~^^^^^^^^ filter_map_next
 }
 
 #[clippy::msrv = "1.29"]

tests/ui/filter_map_next.rs

@@ -3,6 +3,10 @@
 fn main() {
     let a = ["1", "lol", "3", "NaN", "5"];
 
+    let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+    //~^ filter_map_next
+    assert_eq!(element, Some(1));
+
     #[rustfmt::skip]
     let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
     //~^ filter_map_next
@@ -17,4 +21,27 @@ fn main() {
             }
         })
         .next();
+
+    let element: Option<i32> = a
+        // very important comment -- don't delete!
+        .iter()
+        .filter_map(|s| {
+            // another extremely important comment
+            s.parse().ok()
+        })
+        .next();
+    //~^^^^^^^^ filter_map_next
+}
+
+#[clippy::msrv = "1.29"]
+fn msrv_1_29() {
+    let a = ["1", "lol", "3", "NaN", "5"];
+    let _: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+}
+
+#[clippy::msrv = "1.30"]
+fn msrv_1_30() {
+    let a = ["1", "lol", "3", "NaN", "5"];
+    let _: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+    //~^ filter_map_next
 }

tests/ui/filter_map_next.stderr

@@ -1,5 +1,19 @@
-error: called `filter_map(..).next()` on an `Iterator`. This is more succinctly expressed by calling `.find_map(..)` instead
-  --> tests/ui/filter_map_next.rs:7:26
+error: called `filter_map(..).next()` on an `Iterator`
+  --> tests/ui/filter_map_next.rs:6:32
+   |
+LL |     let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::filter-map-next` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::filter_map_next)]`
+help: use `.find_map(..)` instead
+   |
+LL -     let element: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+LL +     let element: Option<i32> = a.iter().find_map(|s| s.parse().ok());
+   |
+
+error: called `filter_map(..).next()` on an `Iterator`
+  --> tests/ui/filter_map_next.rs:11:26
    |
 LL |       let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
    |  __________________________^
@@ -8,8 +22,55 @@ LL | |         })
 LL | |         .next();
    | |_______________^
    |
-   = note: `-D clippy::filter-map-next` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::filter_map_next)]`
+help: use `.find_map(..)` instead
+   |
+LL ~     let _: Option<u32> = vec![1, 2, 3, 4, 5, 6]
+LL +
+LL + 
+LL + 
+LL +         .into_iter().find_map(|x| {
+LL +             if x == 2 {
+LL +                 Some(x * 2)
+LL +             } else {
+LL +                 None
+LL +             }
+LL ~         });
+   |
+
+error: called `filter_map(..).next()` on an `Iterator`
+  --> tests/ui/filter_map_next.rs:25:32
+   |
+LL |       let element: Option<i32> = a
+   |  ________________________________^
+LL | |         // very important comment -- don't delete!
+LL | |         .iter()
+LL | |         .filter_map(|s| {
+...  |
+LL | |         })
+LL | |         .next();
+   | |_______________^
+   |
+help: use `.find_map(..)` instead
+   |
+LL ~     let element: Option<i32> = a
+LL +         // very important comment -- don't delete!
+LL +         .iter().find_map(|s| {
+LL +             // another extremely important comment
+LL +             s.parse().ok()
+LL ~         });
+   |
+
+error: called `filter_map(..).next()` on an `Iterator`
+  --> tests/ui/filter_map_next.rs:45:26
+   |
+LL |     let _: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `.find_map(..)` instead
+   |
+LL -     let _: Option<i32> = a.iter().filter_map(|s| s.parse().ok()).next();
+LL +     let _: Option<i32> = a.iter().find_map(|s| s.parse().ok());
+   |
 
-error: aborting due to 1 previous error
+error: aborting due to 4 previous errors