Skip to content

Commit a752d31

Browse files
camsteffencamsteffen
committed
Replace find_map with manual_find_map
1 parent c92bdc4 commit a752d31

File tree

7 files changed

+141
-49
lines changed

7 files changed

+141
-49
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2036,6 +2036,7 @@ Released 2018-09-13
20362036
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
20372037
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
20382038
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
2039+
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
20392040
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
20402041
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
20412042
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
746746
&methods::ITER_NTH_ZERO,
747747
&methods::ITER_SKIP_NEXT,
748748
&methods::MANUAL_FILTER_MAP,
749+
&methods::MANUAL_FIND_MAP,
749750
&methods::MANUAL_SATURATING_ARITHMETIC,
750751
&methods::MAP_COLLECT_RESULT_UNIT,
751752
&methods::MAP_FLATTEN,
@@ -1528,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15281529
LintId::of(&methods::ITER_NTH_ZERO),
15291530
LintId::of(&methods::ITER_SKIP_NEXT),
15301531
LintId::of(&methods::MANUAL_FILTER_MAP),
1532+
LintId::of(&methods::MANUAL_FIND_MAP),
15311533
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
15321534
LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
15331535
LintId::of(&methods::NEW_RET_NO_SELF),
@@ -1826,6 +1828,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
18261828
LintId::of(&methods::FLAT_MAP_IDENTITY),
18271829
LintId::of(&methods::INSPECT_FOR_EACH),
18281830
LintId::of(&methods::MANUAL_FILTER_MAP),
1831+
LintId::of(&methods::MANUAL_FIND_MAP),
18291832
LintId::of(&methods::OPTION_AS_REF_DEREF),
18301833
LintId::of(&methods::SEARCH_IS_SOME),
18311834
LintId::of(&methods::SKIP_WHILE_NEXT),

‎clippy_lints/src/methods/mod.rs

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,32 @@ declare_clippy_lint! {
477477
"using `_.filter(_).map(_)` in a way that can be written more simply as `filter_map(_)`"
478478
}
479479

480+
declare_clippy_lint! {
481+
/// **What it does:** Checks for usage of `_.find(_).map(_)` that can be written more simply
482+
/// as `find_map(_)`.
483+
///
484+
/// **Why is this bad?** Redundant code in the `find` and `map` operations is poor style and
485+
/// less performant.
486+
///
487+
/// **Known problems:** None.
488+
///
489+
/// **Example:**
490+
/// Bad:
491+
/// ```rust
492+
/// (0_i32..10)
493+
/// .find(|n| n.checked_add(1).is_some())
494+
/// .map(|n| n.checked_add(1).unwrap());
495+
/// ```
496+
///
497+
/// Good:
498+
/// ```rust
499+
/// (0_i32..10).find_map(|n| n.checked_add(1));
500+
/// ```
501+
pub MANUAL_FIND_MAP,
502+
complexity,
503+
"using `_.find(_).map(_)` in a way that can be written more simply as `find_map(_)`"
504+
}
505+
480506
declare_clippy_lint! {
481507
/// **What it does:** Checks for usage of `_.filter_map(_).next()`.
482508
///
@@ -1501,6 +1527,7 @@ impl_lint_pass!(Methods => [
15011527
SKIP_WHILE_NEXT,
15021528
FILTER_MAP,
15031529
MANUAL_FILTER_MAP,
1530+
MANUAL_FIND_MAP,
15041531
FILTER_MAP_NEXT,
15051532
FLAT_MAP_IDENTITY,
15061533
FIND_MAP,
@@ -1568,10 +1595,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
15681595
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
15691596
["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
15701597
["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
1571-
["map", "filter"] => lint_filter_map(cx, expr),
1598+
["map", "filter"] => lint_filter_map(cx, expr, false),
15721599
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
15731600
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1], self.msrv.as_ref()),
1574-
["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]),
1601+
["map", "find"] => lint_filter_map(cx, expr, true),
15751602
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
15761603
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
15771604
["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]),
@@ -3016,12 +3043,12 @@ fn lint_skip_while_next<'tcx>(
30163043
}
30173044
}
30183045

3019-
/// lint use of `filter().map()` for `Iterators`
3020-
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
3046+
/// lint use of `filter().map()` or `find().map()` for `Iterators`
3047+
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool) {
30213048
if_chain! {
30223049
if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
30233050
if let ExprKind::MethodCall(_, _, [_, filter_arg], filter_span) = map_recv.kind;
3024-
if match_trait_method(cx, expr, &paths::ITERATOR);
3051+
if match_trait_method(cx, map_recv, &paths::ITERATOR);
30253052

30263053
// filter(|x| ...is_some())...
30273054
if let ExprKind::Closure(_, _, filter_body_id, ..) = filter_arg.kind;
@@ -3078,10 +3105,16 @@ fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
30783105
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
30793106
then {
30803107
let span = filter_span.to(map_span);
3081-
let msg = "`filter(..).map(..)` can be simplified as `filter_map(..)`";
3108+
let (filter_name, lint) = if is_find {
3109+
("find", MANUAL_FIND_MAP)
3110+
} else {
3111+
("filter", MANUAL_FILTER_MAP)
3112+
};
3113+
let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name);
30823114
let to_opt = if is_result { ".ok()" } else { "" };
3083-
let sugg = format!("filter_map(|{}| {}{})", map_param_ident, snippet(cx, map_arg.span, ".."), to_opt);
3084-
span_lint_and_sugg(cx, MANUAL_FILTER_MAP, span, msg, "try", sugg, Applicability::MachineApplicable);
3115+
let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident,
3116+
snippet(cx, map_arg.span, ".."), to_opt);
3117+
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
30853118
}
30863119
}
30873120
}
@@ -3120,21 +3153,6 @@ fn lint_filter_map_next<'tcx>(
31203153
}
31213154
}
31223155

3123-
/// lint use of `find().map()` for `Iterators`
3124-
fn lint_find_map<'tcx>(
3125-
cx: &LateContext<'tcx>,
3126-
expr: &'tcx hir::Expr<'_>,
3127-
_find_args: &'tcx [hir::Expr<'_>],
3128-
map_args: &'tcx [hir::Expr<'_>],
3129-
) {
3130-
// lint if caller of `.filter().map()` is an Iterator
3131-
if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
3132-
let msg = "called `find(..).map(..)` on an `Iterator`";
3133-
let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
3134-
span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint);
3135-
}
3136-
}
3137-
31383156
/// lint use of `filter_map().map()` for `Iterators`
31393157
fn lint_filter_map_map<'tcx>(
31403158
cx: &LateContext<'tcx>,

‎tests/ui/find_map.stderr

Lines changed: 0 additions & 26 deletions
This file was deleted.

‎tests/ui/manual_find_map.fixed

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// run-rustfix
2+
#![allow(dead_code)]
3+
#![warn(clippy::manual_find_map)]
4+
#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
5+
6+
fn main() {
7+
// is_some(), unwrap()
8+
let _ = (0..).find_map(|a| to_opt(a));
9+
10+
// ref pattern, expect()
11+
let _ = (0..).find_map(|a| to_opt(a));
12+
13+
// is_ok(), unwrap_or()
14+
let _ = (0..).find_map(|a| to_res(a).ok());
15+
}
16+
17+
fn no_lint() {
18+
// no shared code
19+
let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
20+
21+
// very close but different since filter() provides a reference
22+
let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
23+
24+
// similar but different
25+
let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
26+
let _ = (0..)
27+
.find(|n| to_opt(n).map(|n| n + 1).is_some())
28+
.map(|a| to_opt(a).unwrap());
29+
}
30+
31+
fn to_opt<T>(_: T) -> Option<T> {
32+
unimplemented!()
33+
}
34+
35+
fn to_res<T>(_: T) -> Result<T, ()> {
36+
unimplemented!()
37+
}

‎tests/ui/manual_find_map.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// run-rustfix
2+
#![allow(dead_code)]
3+
#![warn(clippy::manual_find_map)]
4+
#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
5+
6+
fn main() {
7+
// is_some(), unwrap()
8+
let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
9+
10+
// ref pattern, expect()
11+
let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
12+
13+
// is_ok(), unwrap_or()
14+
let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
15+
}
16+
17+
fn no_lint() {
18+
// no shared code
19+
let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
20+
21+
// very close but different since filter() provides a reference
22+
let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
23+
24+
// similar but different
25+
let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
26+
let _ = (0..)
27+
.find(|n| to_opt(n).map(|n| n + 1).is_some())
28+
.map(|a| to_opt(a).unwrap());
29+
}
30+
31+
fn to_opt<T>(_: T) -> Option<T> {
32+
unimplemented!()
33+
}
34+
35+
fn to_res<T>(_: T) -> Result<T, ()> {
36+
unimplemented!()
37+
}

‎tests/ui/manual_find_map.stderr

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error: `find(..).map(..)` can be simplified as `find_map(..)`
2+
--> $DIR/manual_find_map.rs:8:19
3+
|
4+
LL | let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
6+
|
7+
= note: `-D clippy::manual-find-map` implied by `-D warnings`
8+
9+
error: `find(..).map(..)` can be simplified as `find_map(..)`
10+
--> $DIR/manual_find_map.rs:11:19
11+
|
12+
LL | let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
14+
15+
error: `find(..).map(..)` can be simplified as `find_map(..)`
16+
--> $DIR/manual_find_map.rs:14:19
17+
|
18+
LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())`
20+
21+
error: aborting due to 3 previous errors
22+

0 commit comments

Comments
 (0)