fix: filter_map_bool_then suggest wrongly when contain return

Author: profetia

clippy_lints/src/methods/filter_map_bool_then.rs

@@ -1,8 +1,8 @@
 use super::FILTER_MAP_BOOL_THEN;
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::SpanRangeExt;
 use clippy_utils::ty::is_copy;
-use clippy_utils::{is_from_proc_macro, is_trait_method, peel_blocks};
+use clippy_utils::{contains_return, is_from_proc_macro, is_trait_method, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind};
 use rustc_lint::{LateContext, LintContext};
@@ -44,17 +44,26 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &
         && let Some(filter) = recv.span.get_source_text(cx)
         && let Some(map) = then_body.span.get_source_text(cx)
     {
-        span_lint_and_sugg(
+        span_lint_and_then(
             cx,
             FILTER_MAP_BOOL_THEN,
             call_span,
             "usage of `bool::then` in `filter_map`",
-            "use `filter` then `map` instead",
-            format!(
-                "filter(|&{param_snippet}| {derefs}{filter}).map(|{param_snippet}| {map})",
-                derefs = "*".repeat(needed_derefs)
-            ),
-            Applicability::MachineApplicable,
+            |diag| {
+                if contains_return(recv) {
+                    diag.help("consider using `filter` then `map` instead");
+                } else {
+                    diag.span_suggestion(
+                        call_span,
+                        "use `filter` then `map` instead",
+                        format!(
+                            "filter(|&{param_snippet}| {derefs}{filter}).map(|{param_snippet}| {map})",
+                            derefs = "*".repeat(needed_derefs)
+                        ),
+                        Applicability::MachineApplicable,
+                    );
+                }
+            },
         );
     }
 }

tests/ui/filter_map_bool_then_unfixable.rs

@@ -0,0 +1,52 @@
+#![allow(clippy::question_mark, unused)]
+#![warn(clippy::filter_map_bool_then)]
+//@no-rustfix
+
+mod issue14368 {
+
+    fn do_something(_: ()) -> bool {
+        true
+    }
+
+    fn option_with_early_return(x: &[Option<bool>]) {
+        let _ = x.iter().filter_map(|&x| x?.then(|| do_something(())));
+        //~^ filter_map_bool_then
+        let _ = x
+            .iter()
+            .filter_map(|&x| if let Some(x) = x { x } else { return None }.then(|| do_something(())));
+        //~^ filter_map_bool_then
+        let _ = x.iter().filter_map(|&x| {
+            //~^ filter_map_bool_then
+            match x {
+                Some(x) => x,
+                None => return None,
+            }
+            .then(|| do_something(()))
+        });
+    }
+
+    #[derive(Copy, Clone)]
+    enum Foo {
+        One(bool),
+        Two,
+        Three(Option<i32>),
+    }
+
+    fn nested_type_with_early_return(x: &[Foo]) {
+        let _ = x.iter().filter_map(|&x| {
+            //~^ filter_map_bool_then
+            match x {
+                Foo::One(x) => x,
+                Foo::Two => return None,
+                Foo::Three(inner) => {
+                    if inner? == 0 {
+                        return Some(false);
+                    } else {
+                        true
+                    }
+                },
+            }
+            .then(|| do_something(()))
+        });
+    }
+}

tests/ui/filter_map_bool_then_unfixable.stderr

@@ -0,0 +1,50 @@
+error: usage of `bool::then` in `filter_map`
+  --> tests/ui/filter_map_bool_then_unfixable.rs:12:26
+   |
+LL |         let _ = x.iter().filter_map(|&x| x?.then(|| do_something(())));
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `filter` then `map` instead
+   = note: `-D clippy::filter-map-bool-then` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::filter_map_bool_then)]`
+
+error: usage of `bool::then` in `filter_map`
+  --> tests/ui/filter_map_bool_then_unfixable.rs:16:14
+   |
+LL |             .filter_map(|&x| if let Some(x) = x { x } else { return None }.then(|| do_something(())));
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using `filter` then `map` instead
+
+error: usage of `bool::then` in `filter_map`
+  --> tests/ui/filter_map_bool_then_unfixable.rs:18:26
+   |
+LL |           let _ = x.iter().filter_map(|&x| {
+   |  __________________________^
+LL | |
+LL | |             match x {
+LL | |                 Some(x) => x,
+...  |
+LL | |             .then(|| do_something(()))
+LL | |         });
+   | |__________^
+   |
+   = help: consider using `filter` then `map` instead
+
+error: usage of `bool::then` in `filter_map`
+  --> tests/ui/filter_map_bool_then_unfixable.rs:36:26
+   |
+LL |           let _ = x.iter().filter_map(|&x| {
+   |  __________________________^
+LL | |
+LL | |             match x {
+LL | |                 Foo::One(x) => x,
+...  |
+LL | |             .then(|| do_something(()))
+LL | |         });
+   | |__________^
+   |
+   = help: consider using `filter` then `map` instead
+
+error: aborting due to 4 previous errors
+