fix: filter_map_bool_then: suggests wrongly when mut capture in then

Author: profetia

clippy_lints/src/methods/filter_map_bool_then.rs

@@ -2,9 +2,13 @@ use super::FILTER_MAP_BOOL_THEN;
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::SpanRangeExt;
 use clippy_utils::ty::is_copy;
-use clippy_utils::{contains_return, is_from_proc_macro, is_trait_method, peel_blocks};
+use clippy_utils::{
+    CaptureKind, can_move_expr_to_closure, contains_return, is_from_proc_macro, is_trait_method, peel_blocks,
+};
+use rustc_ast::Mutability;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind};
+use rustc_hir::{Expr, ExprKind, HirId, Param, Pat};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::ty::Binder;
 use rustc_middle::ty::adjustment::Adjust;
@@ -50,9 +54,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &
             call_span,
             "usage of `bool::then` in `filter_map`",
             |diag| {
-                if contains_return(recv) {
-                    diag.help("consider using `filter` then `map` instead");
-                } else {
+                if can_filter_and_then_move_to_closure(cx, &param, recv, then_body) {
                     diag.span_suggestion(
                         call_span,
                         "use `filter` then `map` instead",
@@ -62,8 +64,53 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &
                         ),
                         Applicability::MachineApplicable,
                     );
+                } else {
+                    diag.help("consider using `filter` then `map` instead");
                 }
             },
         );
     }
 }
+
+/// Returns a set of all bindings found in the given pattern.
+fn find_bindings_from_pat(pat: &Pat<'_>) -> FxHashSet<HirId> {
+    let mut bindings = FxHashSet::default();
+    pat.walk(|p| {
+        if let rustc_hir::PatKind::Binding(_, hir_id, _, _) = p.kind {
+            bindings.insert(hir_id);
+        }
+        true
+    });
+    bindings
+}
+
+/// Returns true if we can take a closure parameter and have it in both the `filter` function and
+/// the`map` function. This is not the case if:
+///
+/// - The `filter` would contain an early return,
+/// - `filter` and `then` contain captures, and any of those are &mut
+fn can_filter_and_then_move_to_closure<'tcx>(
+    cx: &LateContext<'tcx>,
+    param: &Param<'tcx>,
+    filter: &'tcx Expr<'tcx>,
+    then: &'tcx Expr<'tcx>,
+) -> bool {
+    if contains_return(filter) {
+        return false;
+    }
+
+    let Some(filter_captures) = can_move_expr_to_closure(cx, filter) else {
+        return true;
+    };
+    let Some(then_captures) = can_move_expr_to_closure(cx, then) else {
+        return true;
+    };
+
+    let param_bindings = find_bindings_from_pat(param.pat);
+    filter_captures.iter().all(|(hir_id, filter_cap)| {
+        param_bindings.contains(hir_id)
+            || !then_captures
+                .get(hir_id)
+                .is_some_and(|then_cap| matches!(*filter_cap | *then_cap, CaptureKind::Ref(Mutability::Mut)))
+    })
+}

tests/ui/filter_map_bool_then_unfixable.rs

@@ -2,6 +2,17 @@
 #![warn(clippy::filter_map_bool_then)]
 //@no-rustfix
 
+fn issue11617() {
+    let mut x: Vec<usize> = vec![0; 10];
+    let _ = (0..x.len()).zip(x.clone().iter()).filter_map(|(i, v)| {
+        //~^ filter_map_bool_then
+        (x[i] != *v).then(|| {
+            x[i] = i;
+            i
+        })
+    });
+}
+
 mod issue14368 {
 
     fn do_something(_: ()) -> bool {

tests/ui/filter_map_bool_then_unfixable.stderr

@@ -1,23 +1,38 @@
 error: usage of `bool::then` in `filter_map`
-  --> tests/ui/filter_map_bool_then_unfixable.rs:12:26
+  --> tests/ui/filter_map_bool_then_unfixable.rs:7:48
    |
-LL |         let _ = x.iter().filter_map(|&x| x?.then(|| do_something(())));
-   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |       let _ = (0..x.len()).zip(x.clone().iter()).filter_map(|(i, v)| {
+   |  ________________________________________________^
+LL | |
+LL | |         (x[i] != *v).then(|| {
+LL | |             x[i] = i;
+LL | |             i
+LL | |         })
+LL | |     });
+   | |______^
    |
    = 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
+  --> tests/ui/filter_map_bool_then_unfixable.rs:23:26
+   |
+LL |         let _ = x.iter().filter_map(|&x| x?.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:27: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
+  --> tests/ui/filter_map_bool_then_unfixable.rs:29:26
    |
 LL |           let _ = x.iter().filter_map(|&x| {
    |  __________________________^
@@ -32,7 +47,7 @@ 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
+  --> tests/ui/filter_map_bool_then_unfixable.rs:47:26
    |
 LL |           let _ = x.iter().filter_map(|&x| {
    |  __________________________^
@@ -46,5 +61,5 @@ LL | |         });
    |
    = help: consider using `filter` then `map` instead
 
-error: aborting due to 4 previous errors
+error: aborting due to 5 previous errors