Skip to content

Commit 007e4b8

Browse files
camsteffencamsteffen
committed
Replace find_map with manual_find_map
1 parent d469ee5 commit 007e4b8

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
@@ -2035,6 +2035,7 @@ Released 2018-09-13
20352035
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
20362036
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
20372037
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
2038+
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
20382039
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
20392040
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
20402041
[`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
@@ -744,6 +744,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
744744
&methods::ITER_NTH_ZERO,
745745
&methods::ITER_SKIP_NEXT,
746746
&methods::MANUAL_FILTER_MAP,
747+
&methods::MANUAL_FIND_MAP,
747748
&methods::MANUAL_SATURATING_ARITHMETIC,
748749
&methods::MAP_COLLECT_RESULT_UNIT,
749750
&methods::MAP_FLATTEN,
@@ -1523,6 +1524,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
15231524
LintId::of(&methods::ITER_NTH_ZERO),
15241525
LintId::of(&methods::ITER_SKIP_NEXT),
15251526
LintId::of(&methods::MANUAL_FILTER_MAP),
1527+
LintId::of(&methods::MANUAL_FIND_MAP),
15261528
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
15271529
LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
15281530
LintId::of(&methods::NEW_RET_NO_SELF),
@@ -1818,6 +1820,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
18181820
LintId::of(&methods::FILTER_NEXT),
18191821
LintId::of(&methods::FLAT_MAP_IDENTITY),
18201822
LintId::of(&methods::MANUAL_FILTER_MAP),
1823+
LintId::of(&methods::MANUAL_FIND_MAP),
18211824
LintId::of(&methods::OPTION_AS_REF_DEREF),
18221825
LintId::of(&methods::SEARCH_IS_SOME),
18231826
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
@@ -476,6 +476,32 @@ declare_clippy_lint! {
476476
"using `_.filter(_).map(_)` in a way that can be written more simply as `filter_map(_)`"
477477
}
478478

479+
declare_clippy_lint! {
480+
/// **What it does:** Checks for usage of `_.find(_).map(_)` that can be written more simply
481+
/// as `find_map(_)`.
482+
///
483+
/// **Why is this bad?** Redundant code in the `find` and `map` operations is poor style and
484+
/// less performant.
485+
///
486+
/// **Known problems:** None.
487+
///
488+
/// **Example:**
489+
/// Bad:
490+
/// ```rust
491+
/// (0_i32..10)
492+
/// .find(|n| n.checked_add(1).is_some())
493+
/// .map(|n| n.checked_add(1).unwrap());
494+
/// ```
495+
///
496+
/// Good:
497+
/// ```rust
498+
/// (0_i32..10).find_map(|n| n.checked_add(1));
499+
/// ```
500+
pub MANUAL_FIND_MAP,
501+
complexity,
502+
"using `_.find(_).map(_)` in a way that can be written more simply as `find_map(_)`"
503+
}
504+
479505
declare_clippy_lint! {
480506
/// **What it does:** Checks for usage of `_.filter_map(_).next()`.
481507
///
@@ -1470,6 +1496,7 @@ impl_lint_pass!(Methods => [
14701496
SKIP_WHILE_NEXT,
14711497
FILTER_MAP,
14721498
MANUAL_FILTER_MAP,
1499+
MANUAL_FIND_MAP,
14731500
FILTER_MAP_NEXT,
14741501
FLAT_MAP_IDENTITY,
14751502
FIND_MAP,
@@ -1536,10 +1563,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
15361563
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
15371564
["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
15381565
["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
1539-
["map", "filter"] => lint_filter_map(cx, expr),
1566+
["map", "filter"] => lint_filter_map(cx, expr, false),
15401567
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
15411568
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1], self.msrv.as_ref()),
1542-
["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]),
1569+
["map", "find"] => lint_filter_map(cx, expr, true),
15431570
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
15441571
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
15451572
["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]),
@@ -2983,12 +3010,12 @@ fn lint_skip_while_next<'tcx>(
29833010
}
29843011
}
29853012

2986-
/// lint use of `filter().map()` for `Iterators`
2987-
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
3013+
/// lint use of `filter().map()` or `find().map()` for `Iterators`
3014+
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool) {
29883015
if_chain! {
29893016
if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
29903017
if let ExprKind::MethodCall(_, _, [_, filter_arg], filter_span) = map_recv.kind;
2991-
if match_trait_method(cx, expr, &paths::ITERATOR);
3018+
if match_trait_method(cx, map_recv, &paths::ITERATOR);
29923019

29933020
// filter(|x| ...is_some())...
29943021
if let ExprKind::Closure(_, _, filter_body_id, ..) = filter_arg.kind;
@@ -3045,10 +3072,16 @@ fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
30453072
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
30463073
then {
30473074
let span = filter_span.to(map_span);
3048-
let msg = "`filter(..).map(..)` can be simplified as `filter_map(..)`";
3075+
let (filter_name, lint) = if is_find {
3076+
("find", MANUAL_FIND_MAP)
3077+
} else {
3078+
("filter", MANUAL_FILTER_MAP)
3079+
};
3080+
let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name);
30493081
let to_opt = if is_result { ".ok()" } else { "" };
3050-
let sugg = format!("filter_map(|{}| {}{})", map_param_ident, snippet(cx, map_arg.span, ".."), to_opt);
3051-
span_lint_and_sugg(cx, MANUAL_FILTER_MAP, span, msg, "try", sugg, Applicability::MachineApplicable);
3082+
let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident,
3083+
snippet(cx, map_arg.span, ".."), to_opt);
3084+
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
30523085
}
30533086
}
30543087
}
@@ -3087,21 +3120,6 @@ fn lint_filter_map_next<'tcx>(
30873120
}
30883121
}
30893122

3090-
/// lint use of `find().map()` for `Iterators`
3091-
fn lint_find_map<'tcx>(
3092-
cx: &LateContext<'tcx>,
3093-
expr: &'tcx hir::Expr<'_>,
3094-
_find_args: &'tcx [hir::Expr<'_>],
3095-
map_args: &'tcx [hir::Expr<'_>],
3096-
) {
3097-
// lint if caller of `.filter().map()` is an Iterator
3098-
if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
3099-
let msg = "called `find(..).map(..)` on an `Iterator`";
3100-
let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
3101-
span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint);
3102-
}
3103-
}
3104-
31053123
/// lint use of `filter_map().map()` for `Iterators`
31063124
fn lint_filter_map_map<'tcx>(
31073125
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)