diff options
| -rw-r--r-- | clippy_lints/src/methods/filter_map_bool_then.rs | 57 | ||||
| -rw-r--r-- | tests/ui/filter_map_bool_then_unfixable.rs | 11 | ||||
| -rw-r--r-- | tests/ui/filter_map_bool_then_unfixable.stderr | 29 |
3 files changed, 85 insertions, 12 deletions
diff --git a/clippy_lints/src/methods/filter_map_bool_then.rs b/clippy_lints/src/methods/filter_map_bool_then.rs index 3461f539b09..965993808f6 100644 --- a/clippy_lints/src/methods/filter_map_bool_then.rs +++ b/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, ¶m, 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))) + }) +} diff --git a/tests/ui/filter_map_bool_then_unfixable.rs b/tests/ui/filter_map_bool_then_unfixable.rs index f65b0b66f83..68294292502 100644 --- a/tests/ui/filter_map_bool_then_unfixable.rs +++ b/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 { diff --git a/tests/ui/filter_map_bool_then_unfixable.stderr b/tests/ui/filter_map_bool_then_unfixable.stderr index f72bcac8a75..2025958136b 100644 --- a/tests/ui/filter_map_bool_then_unfixable.stderr +++ b/tests/ui/filter_map_bool_then_unfixable.stderr @@ -1,15 +1,30 @@ 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(()))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +32,7 @@ LL | .filter_map(|&x| if let Some(x) = x { x } else { return None }. = 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 |