Skip to content

Commit c38fe51

Browse files
committed
Add unnecessary_find_map lint
1 parent e511476 commit c38fe51

8 files changed

+120
-21
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3498,6 +3498,7 @@ Released 2018-09-13
34983498
[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
34993499
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
35003500
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
3501+
[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map
35013502
[`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold
35023503
[`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
35033504
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed

clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
189189
LintId::of(methods::SUSPICIOUS_SPLITN),
190190
LintId::of(methods::UNINIT_ASSUMED_INIT),
191191
LintId::of(methods::UNNECESSARY_FILTER_MAP),
192+
LintId::of(methods::UNNECESSARY_FIND_MAP),
192193
LintId::of(methods::UNNECESSARY_FOLD),
193194
LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS),
194195
LintId::of(methods::UNNECESSARY_TO_OWNED),

clippy_lints/src/lib.register_complexity.rs

+1
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
4949
LintId::of(methods::SEARCH_IS_SOME),
5050
LintId::of(methods::SKIP_WHILE_NEXT),
5151
LintId::of(methods::UNNECESSARY_FILTER_MAP),
52+
LintId::of(methods::UNNECESSARY_FIND_MAP),
5253
LintId::of(methods::USELESS_ASREF),
5354
LintId::of(misc::SHORT_CIRCUIT_STATEMENT),
5455
LintId::of(misc_early::UNNEEDED_WILDCARD_PATTERN),

clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ store.register_lints(&[
323323
methods::SUSPICIOUS_SPLITN,
324324
methods::UNINIT_ASSUMED_INIT,
325325
methods::UNNECESSARY_FILTER_MAP,
326+
methods::UNNECESSARY_FIND_MAP,
326327
methods::UNNECESSARY_FOLD,
327328
methods::UNNECESSARY_LAZY_EVALUATIONS,
328329
methods::UNNECESSARY_TO_OWNED,

clippy_lints/src/methods/mod.rs

+36-2
Original file line numberDiff line numberDiff line change
@@ -1309,7 +1309,7 @@ declare_clippy_lint! {
13091309

13101310
declare_clippy_lint! {
13111311
/// ### What it does
1312-
/// Checks for `filter_map` calls which could be replaced by `filter` or `map`.
1312+
/// Checks for `filter_map` calls that could be replaced by `filter` or `map`.
13131313
/// More specifically it checks if the closure provided is only performing one of the
13141314
/// filter or map operations and suggests the appropriate option.
13151315
///
@@ -1337,6 +1337,36 @@ declare_clippy_lint! {
13371337
"using `filter_map` when a more succinct alternative exists"
13381338
}
13391339

1340+
declare_clippy_lint! {
1341+
/// ### What it does
1342+
/// Checks for `find_map` calls that could be replaced by `find` or `map`. More
1343+
/// specifically it checks if the closure provided is only performing one of the
1344+
/// find or map operations and suggests the appropriate option.
1345+
///
1346+
/// ### Why is this bad?
1347+
/// Complexity. The intent is also clearer if only a single
1348+
/// operation is being performed.
1349+
///
1350+
/// ### Example
1351+
/// ```rust
1352+
/// let _ = (0..3).find_map(|x| if x > 2 { Some(x) } else { None });
1353+
///
1354+
/// // As there is no transformation of the argument this could be written as:
1355+
/// let _ = (0..3).find(|&x| x > 2);
1356+
/// ```
1357+
///
1358+
/// ```rust
1359+
/// let _ = (0..4).find_map(|x| Some(x + 1));
1360+
///
1361+
/// // As there is no conditional check on the argument this could be written as:
1362+
/// let _ = (0..4).map(|x| x + 1).next();
1363+
/// ```
1364+
#[clippy::version = "1.61.0"]
1365+
pub UNNECESSARY_FIND_MAP,
1366+
complexity,
1367+
"using `find_map` when a more succinct alternative exists"
1368+
}
1369+
13401370
declare_clippy_lint! {
13411371
/// ### What it does
13421372
/// Checks for `into_iter` calls on references which should be replaced by `iter`
@@ -2020,6 +2050,7 @@ impl_lint_pass!(Methods => [
20202050
USELESS_ASREF,
20212051
UNNECESSARY_FOLD,
20222052
UNNECESSARY_FILTER_MAP,
2053+
UNNECESSARY_FIND_MAP,
20232054
INTO_ITER_ON_REF,
20242055
SUSPICIOUS_MAP,
20252056
UNINIT_ASSUMED_INIT,
@@ -2305,9 +2336,12 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
23052336
extend_with_drain::check(cx, expr, recv, arg);
23062337
},
23072338
("filter_map", [arg]) => {
2308-
unnecessary_filter_map::check(cx, expr, arg);
2339+
unnecessary_filter_map::check(cx, expr, arg, name);
23092340
filter_map_identity::check(cx, expr, arg, span);
23102341
},
2342+
("find_map", [arg]) => {
2343+
unnecessary_filter_map::check(cx, expr, arg, name);
2344+
},
23112345
("flat_map", [arg]) => {
23122346
flat_map_identity::check(cx, expr, arg, span);
23132347
flat_map_option::check(cx, expr, arg, span);

clippy_lints/src/methods/unnecessary_filter_map.rs

+23-15
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ use rustc_middle::ty;
1111
use rustc_span::sym;
1212

1313
use super::UNNECESSARY_FILTER_MAP;
14+
use super::UNNECESSARY_FIND_MAP;
1415

15-
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
16+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) {
1617
if !is_trait_method(cx, expr, sym::Iterator) {
1718
return;
1819
}
@@ -32,23 +33,30 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<
3233
found_filtering |= return_visitor.found_filtering;
3334

3435
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
35-
let sugg = if !found_filtering {
36-
"map"
37-
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
38-
match cx.typeck_results().expr_ty(&body.value).kind() {
39-
ty::Adt(adt, subst) if cx.tcx.is_diagnostic_item(sym::Option, adt.did) && in_ty == subst.type_at(0) => {
40-
"filter"
41-
},
42-
_ => return,
43-
}
44-
} else {
45-
return;
46-
};
36+
let sugg =
37+
if !found_filtering {
38+
if name == "filter_map" { "map" } else { "map(..).next()" }
39+
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
40+
match cx.typeck_results().expr_ty(&body.value).kind() {
41+
ty::Adt(adt, subst)
42+
if cx.tcx.is_diagnostic_item(sym::Option, adt.did) && in_ty == subst.type_at(0) =>
43+
{
44+
if name == "filter_map" { "filter" } else { "find" }
45+
},
46+
_ => return,
47+
}
48+
} else {
49+
return;
50+
};
4751
span_lint(
4852
cx,
49-
UNNECESSARY_FILTER_MAP,
53+
if name == "filter_map" {
54+
UNNECESSARY_FILTER_MAP
55+
} else {
56+
UNNECESSARY_FIND_MAP
57+
},
5058
expr.span,
51-
&format!("this `.filter_map` can be written more simply using `.{}`", sugg),
59+
&format!("this `.{}` can be written more simply using `.{}`", name, sugg),
5260
);
5361
}
5462
}

tests/ui/unnecessary_filter_map.rs

+17
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,43 @@
22

33
fn main() {
44
let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
5+
let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
56
let _ = (0..4).filter_map(|x| {
67
if x > 1 {
78
return Some(x);
89
};
910
None
1011
});
12+
let _ = (0..4).find_map(|x| {
13+
if x > 1 {
14+
return Some(x);
15+
};
16+
None
17+
});
1118
let _ = (0..4).filter_map(|x| match x {
1219
0 | 1 => None,
1320
_ => Some(x),
1421
});
22+
let _ = (0..4).find_map(|x| match x {
23+
0 | 1 => None,
24+
_ => Some(x),
25+
});
1526

1627
let _ = (0..4).filter_map(|x| Some(x + 1));
28+
let _ = (0..4).find_map(|x| Some(x + 1));
1729

1830
let _ = (0..4).filter_map(i32::checked_abs);
31+
let _ = (0..4).find_map(i32::checked_abs);
1932
}
2033

2134
fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> {
2235
"".chars().filter_map(|_| None)
2336
}
2437

38+
fn find_map_none_changes_item_type() -> Option<bool> {
39+
"".chars().find_map(|_| None)
40+
}
41+
2542
// https://github.com/rust-lang/rust-clippy/issues/4433#issue-483920107
2643
mod comment_483920107 {
2744
enum Severity {

tests/ui/unnecessary_filter_map.stderr

+40-4
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,17 @@ LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
66
|
77
= note: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
88

9-
error: this `.filter_map` can be written more simply using `.filter`
9+
error: this `.find_map` can be written more simply using `.find`
1010
--> $DIR/unnecessary_filter_map.rs:5:13
1111
|
12+
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
|
15+
= note: `-D clippy::unnecessary-find-map` implied by `-D warnings`
16+
17+
error: this `.filter_map` can be written more simply using `.filter`
18+
--> $DIR/unnecessary_filter_map.rs:6:13
19+
|
1220
LL | let _ = (0..4).filter_map(|x| {
1321
| _____________^
1422
LL | | if x > 1 {
@@ -18,8 +26,20 @@ LL | | None
1826
LL | | });
1927
| |______^
2028

29+
error: this `.find_map` can be written more simply using `.find`
30+
--> $DIR/unnecessary_filter_map.rs:12:13
31+
|
32+
LL | let _ = (0..4).find_map(|x| {
33+
| _____________^
34+
LL | | if x > 1 {
35+
LL | | return Some(x);
36+
LL | | };
37+
LL | | None
38+
LL | | });
39+
| |______^
40+
2141
error: this `.filter_map` can be written more simply using `.filter`
22-
--> $DIR/unnecessary_filter_map.rs:11:13
42+
--> $DIR/unnecessary_filter_map.rs:18:13
2343
|
2444
LL | let _ = (0..4).filter_map(|x| match x {
2545
| _____________^
@@ -28,11 +48,27 @@ LL | | _ => Some(x),
2848
LL | | });
2949
| |______^
3050

51+
error: this `.find_map` can be written more simply using `.find`
52+
--> $DIR/unnecessary_filter_map.rs:22:13
53+
|
54+
LL | let _ = (0..4).find_map(|x| match x {
55+
| _____________^
56+
LL | | 0 | 1 => None,
57+
LL | | _ => Some(x),
58+
LL | | });
59+
| |______^
60+
3161
error: this `.filter_map` can be written more simply using `.map`
32-
--> $DIR/unnecessary_filter_map.rs:16:13
62+
--> $DIR/unnecessary_filter_map.rs:27:13
3363
|
3464
LL | let _ = (0..4).filter_map(|x| Some(x + 1));
3565
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3666

37-
error: aborting due to 4 previous errors
67+
error: this `.find_map` can be written more simply using `.map(..).next()`
68+
--> $DIR/unnecessary_filter_map.rs:28:13
69+
|
70+
LL | let _ = (0..4).find_map(|x| Some(x + 1));
71+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
72+
73+
error: aborting due to 8 previous errors
3874

0 commit comments

Comments
 (0)