Skip to content

Commit 860e800

Browse files
committed
[filter_map_bool_then]: peel as many refs as needed
1 parent f54275f commit 860e800

File tree

4 files changed

+40
-5
lines changed

4 files changed

+40
-5
lines changed

clippy_lints/src/methods/filter_map_bool_then.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ use super::FILTER_MAP_BOOL_THEN;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::paths::BOOL_THEN;
44
use clippy_utils::source::snippet_opt;
5-
use clippy_utils::ty::is_copy;
5+
use clippy_utils::ty::{is_copy, peel_mid_ty_refs_is_mutable};
66
use clippy_utils::{is_from_proc_macro, is_trait_method, match_def_path, peel_blocks};
77
use rustc_errors::Applicability;
88
use rustc_hir::{Expr, ExprKind};
99
use rustc_lint::{LateContext, LintContext};
1010
use rustc_middle::lint::in_external_macro;
11-
use rustc_middle::ty::{self, Binder};
11+
use rustc_middle::ty::Binder;
1212
use rustc_span::{sym, Span};
1313

1414
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &Expr<'_>, call_span: Span) {
@@ -36,7 +36,11 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &
3636
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(value.hir_id)
3737
&& match_def_path(cx, def_id, &BOOL_THEN)
3838
&& !is_from_proc_macro(cx, expr)
39-
&& let ref_bool = matches!(cx.typeck_results().expr_ty(recv).kind(), ty::Ref(..))
39+
// Peel all refs (e.g. `&&&&mut &&&bool` -> `bool`) and get its count so we can suggest the exact
40+
// amount of derefs to get to the bool in the filter.
41+
// `peel_mid_ty_refs` alone doesn't handle mutable reference, so we use `_is_mutable`
42+
// instead which counts them too and just ignore the resulting mutability
43+
&& let (_, needed_derefs, _) = peel_mid_ty_refs_is_mutable(cx.typeck_results().expr_ty(recv))
4044
&& let Some(param_snippet) = snippet_opt(cx, param.span)
4145
&& let Some(filter) = snippet_opt(cx, recv.span)
4246
&& let Some(map) = snippet_opt(cx, then_body.span)
@@ -47,7 +51,10 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, arg: &
4751
call_span,
4852
"usage of `bool::then` in `filter_map`",
4953
"use `filter` then `map` instead",
50-
format!("filter(|&{param_snippet}| {}{filter}).map(|{param_snippet}| {map})", if ref_bool { "*" } else { "" }),
54+
format!(
55+
"filter(|&{param_snippet}| {derefs}{filter}).map(|{param_snippet}| {map})",
56+
derefs="*".repeat(needed_derefs)
57+
),
5158
Applicability::MachineApplicable,
5259
);
5360
}

tests/ui/filter_map_bool_then.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,12 @@ fn issue11309<'a>(iter: impl Iterator<Item = (&'a str, &'a str)>) -> Vec<&'a str
5959
fn issue11503() {
6060
let bools: &[bool] = &[true, false, false, true];
6161
let _: Vec<usize> = bools.iter().enumerate().filter(|&(i, b)| *b).map(|(i, b)| i).collect();
62+
63+
// Need to insert multiple derefs if there is more than one layer of references
64+
let bools: &[&&bool] = &[&&true, &&false, &&false, &&true];
65+
let _: Vec<usize> = bools.iter().enumerate().filter(|&(i, b)| ***b).map(|(i, b)| i).collect();
66+
67+
// Should also suggest derefs when going through a mutable reference
68+
let bools: &[&mut bool] = &[&mut true];
69+
let _: Vec<usize> = bools.iter().enumerate().filter(|&(i, b)| **b).map(|(i, b)| i).collect();
6270
}

tests/ui/filter_map_bool_then.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,12 @@ fn issue11309<'a>(iter: impl Iterator<Item = (&'a str, &'a str)>) -> Vec<&'a str
5959
fn issue11503() {
6060
let bools: &[bool] = &[true, false, false, true];
6161
let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
62+
63+
// Need to insert multiple derefs if there is more than one layer of references
64+
let bools: &[&&bool] = &[&&true, &&false, &&false, &&true];
65+
let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
66+
67+
// Should also suggest derefs when going through a mutable reference
68+
let bools: &[&mut bool] = &[&mut true];
69+
let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
6270
}

tests/ui/filter_map_bool_then.stderr

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,17 @@ error: usage of `bool::then` in `filter_map`
4343
LL | let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
4444
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&(i, b)| *b).map(|(i, b)| i)`
4545

46-
error: aborting due to 7 previous errors
46+
error: usage of `bool::then` in `filter_map`
47+
--> $DIR/filter_map_bool_then.rs:65:50
48+
|
49+
LL | let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&(i, b)| ***b).map(|(i, b)| i)`
51+
52+
error: usage of `bool::then` in `filter_map`
53+
--> $DIR/filter_map_bool_then.rs:69:50
54+
|
55+
LL | let _: Vec<usize> = bools.iter().enumerate().filter_map(|(i, b)| b.then(|| i)).collect();
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use `filter` then `map` instead: `filter(|&(i, b)| **b).map(|(i, b)| i)`
57+
58+
error: aborting due to 9 previous errors
4759

0 commit comments

Comments
 (0)