squiid/rust
1
0
Fork 0
Code Issues Pull requests Projects Activity

Replace find_map with manual_find_map

Browse source
This commit is contained in:
Cameron Steffen 2021-01-14 16:36:36 -06:00
parent c92bdc4dbb
commit a752d31e0a
7 changed files with 141 additions and 49 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -2036,6 +2036,7 @@ Released 2018-09-13
[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
[`manual_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_filter_map
[`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or

3
clippy_lints/src/lib.rs
View file

@ -746,6 +746,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::ITER_NTH_ZERO,
&methods::ITER_SKIP_NEXT,
&methods::MANUAL_FILTER_MAP,
&methods::MANUAL_FIND_MAP,
&methods::MANUAL_SATURATING_ARITHMETIC,
&methods::MAP_COLLECT_RESULT_UNIT,
&methods::MAP_FLATTEN,
@ -1528,6 +1529,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::ITER_NTH_ZERO),
LintId::of(&methods::ITER_SKIP_NEXT),
LintId::of(&methods::MANUAL_FILTER_MAP),
LintId::of(&methods::MANUAL_FIND_MAP),
LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
LintId::of(&methods::NEW_RET_NO_SELF),
@ -1826,6 +1828,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::FLAT_MAP_IDENTITY),
LintId::of(&methods::INSPECT_FOR_EACH),
LintId::of(&methods::MANUAL_FILTER_MAP),
LintId::of(&methods::MANUAL_FIND_MAP),
LintId::of(&methods::OPTION_AS_REF_DEREF),
LintId::of(&methods::SEARCH_IS_SOME),
LintId::of(&methods::SKIP_WHILE_NEXT),

64
clippy_lints/src/methods/mod.rs
View file

@ -477,6 +477,32 @@ declare_clippy_lint! {
"using `_.filter(_).map(_)` in a way that can be written more simply as `filter_map(_)`"
}
declare_clippy_lint! {
/// **What it does:** Checks for usage of `_.find(_).map(_)` that can be written more simply
/// as `find_map(_)`.
///
/// **Why is this bad?** Redundant code in the `find` and `map` operations is poor style and
/// less performant.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// (0_i32..10)
/// .find(|n| n.checked_add(1).is_some())
/// .map(|n| n.checked_add(1).unwrap());
/// ```
///
/// Good:
/// ```rust
/// (0_i32..10).find_map(|n| n.checked_add(1));
/// ```
pub MANUAL_FIND_MAP,
complexity,
"using `_.find(_).map(_)` in a way that can be written more simply as `find_map(_)`"
}
declare_clippy_lint! {
/// **What it does:** Checks for usage of `_.filter_map(_).next()`.
///
@ -1501,6 +1527,7 @@ impl_lint_pass!(Methods => [
SKIP_WHILE_NEXT,
FILTER_MAP,
MANUAL_FILTER_MAP,
MANUAL_FIND_MAP,
FILTER_MAP_NEXT,
FLAT_MAP_IDENTITY,
FIND_MAP,
@ -1568,10 +1595,10 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]),
["next", "iter"] => lint_iter_next(cx, expr, arg_lists[1]),
["map", "filter"] => lint_filter_map(cx, expr),
["map", "filter"] => lint_filter_map(cx, expr, false),
["map", "filter_map"] => lint_filter_map_map(cx, expr, arg_lists[1], arg_lists[0]),
["next", "filter_map"] => lint_filter_map_next(cx, expr, arg_lists[1], self.msrv.as_ref()),
["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]),
["map", "find"] => lint_filter_map(cx, expr, true),
["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]),
["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]),
@ -3016,12 +3043,12 @@ fn lint_skip_while_next<'tcx>(
}
}
/// lint use of `filter().map()` for `Iterators`
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) {
/// lint use of `filter().map()` or `find().map()` for `Iterators`
fn lint_filter_map<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, is_find: bool) {
if_chain! {
if let ExprKind::MethodCall(_, _, [map_recv, map_arg], map_span) = expr.kind;
if let ExprKind::MethodCall(_, _, [_, filter_arg], filter_span) = map_recv.kind;
if match_trait_method(cx, expr, &paths::ITERATOR);
if match_trait_method(cx, map_recv, &paths::ITERATOR);
// filter(|x| ...is_some())...
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<'_>) {
if SpanlessEq::new(cx).expr_fallback(eq_fallback).eq_expr(filter_arg, map_arg);
then {
let span = filter_span.to(map_span);
let msg = "`filter(..).map(..)` can be simplified as `filter_map(..)`";
let (filter_name, lint) = if is_find {
("find", MANUAL_FIND_MAP)
} else {
("filter", MANUAL_FILTER_MAP)
};
let msg = format!("`{}(..).map(..)` can be simplified as `{0}_map(..)`", filter_name);
let to_opt = if is_result { ".ok()" } else { "" };
let sugg = format!("filter_map(|{}| {}{})", map_param_ident, snippet(cx, map_arg.span, ".."), to_opt);
span_lint_and_sugg(cx, MANUAL_FILTER_MAP, span, msg, "try", sugg, Applicability::MachineApplicable);
let sugg = format!("{}_map(|{}| {}{})", filter_name, map_param_ident,
snippet(cx, map_arg.span, ".."), to_opt);
span_lint_and_sugg(cx, lint, span, &msg, "try", sugg, Applicability::MachineApplicable);
}
}
}
@ -3120,21 +3153,6 @@ fn lint_filter_map_next<'tcx>(
}
}
/// lint use of `find().map()` for `Iterators`
fn lint_find_map<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_find_args: &'tcx [hir::Expr<'_>],
map_args: &'tcx [hir::Expr<'_>],
) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, &map_args[0], &paths::ITERATOR) {
let msg = "called `find(..).map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by calling `.find_map(..)` instead";
span_lint_and_help(cx, FIND_MAP, expr.span, msg, None, hint);
}
}
/// lint use of `filter_map().map()` for `Iterators`
fn lint_filter_map_map<'tcx>(
cx: &LateContext<'tcx>,

26
tests/ui/find_map.stderr
View file

@ -1,26 +0,0 @@
error: called `find(..).map(..)` on an `Iterator`
--> $DIR/find_map.rs:20:26
|
LL | let _: Option<i32> = a.iter().find(|s| s.parse::<i32>().is_ok()).map(|s| s.parse().unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::find-map` implied by `-D warnings`
= help: this is more succinctly expressed by calling `.find_map(..)` instead
error: called `find(..).map(..)` on an `Iterator`
--> $DIR/find_map.rs:23:29
|
LL | let _: Option<Flavor> = desserts_of_the_week
| _____________________________^
LL | | .iter()
LL | | .find(|dessert| match *dessert {
LL | | Dessert::Cake(_) => true,
... |
LL | | _ => unreachable!(),
LL | | });
| |__________^
|
= help: this is more succinctly expressed by calling `.find_map(..)` instead
error: aborting due to 2 previous errors

37
tests/ui/manual_find_map.fixed Normal file
View file

@ -0,0 +1,37 @@
// run-rustfix
#![allow(dead_code)]
#![warn(clippy::manual_find_map)]
#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
fn main() {
// is_some(), unwrap()
let _ = (0..).find_map(|a| to_opt(a));
// ref pattern, expect()
let _ = (0..).find_map(|a| to_opt(a));
// is_ok(), unwrap_or()
let _ = (0..).find_map(|a| to_res(a).ok());
}
fn no_lint() {
// no shared code
let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
// very close but different since filter() provides a reference
let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
// similar but different
let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
let _ = (0..)
.find(|n| to_opt(n).map(|n| n + 1).is_some())
.map(|a| to_opt(a).unwrap());
}
fn to_opt<T>(_: T) -> Option<T> {
unimplemented!()
}
fn to_res<T>(_: T) -> Result<T, ()> {
unimplemented!()
}

37
tests/ui/manual_find_map.rs Normal file
View file

@ -0,0 +1,37 @@
// run-rustfix
#![allow(dead_code)]
#![warn(clippy::manual_find_map)]
#![allow(clippy::redundant_closure)] // FIXME suggestion may have redundant closure
fn main() {
// is_some(), unwrap()
let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
// ref pattern, expect()
let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
// is_ok(), unwrap_or()
let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
}
fn no_lint() {
// no shared code
let _ = (0..).filter(|n| *n > 1).map(|n| n + 1);
// very close but different since filter() provides a reference
let _ = (0..).find(|n| to_opt(n).is_some()).map(|a| to_opt(a).unwrap());
// similar but different
let _ = (0..).find(|n| to_opt(n).is_some()).map(|n| to_res(n).unwrap());
let _ = (0..)
.find(|n| to_opt(n).map(|n| n + 1).is_some())
.map(|a| to_opt(a).unwrap());
}
fn to_opt<T>(_: T) -> Option<T> {
unimplemented!()
}
fn to_res<T>(_: T) -> Result<T, ()> {
unimplemented!()
}

22
tests/ui/manual_find_map.stderr Normal file
View file

@ -0,0 +1,22 @@
error: `find(..).map(..)` can be simplified as `find_map(..)`
--> $DIR/manual_find_map.rs:8:19
|
LL | let _ = (0..).find(|n| to_opt(*n).is_some()).map(|a| to_opt(a).unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
|
= note: `-D clippy::manual-find-map` implied by `-D warnings`
error: `find(..).map(..)` can be simplified as `find_map(..)`
--> $DIR/manual_find_map.rs:11:19
|
LL | let _ = (0..).find(|&n| to_opt(n).is_some()).map(|a| to_opt(a).expect("hi"));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_opt(a))`
error: `find(..).map(..)` can be simplified as `find_map(..)`
--> $DIR/manual_find_map.rs:14:19
|
LL | let _ = (0..).find(|&n| to_res(n).is_ok()).map(|a| to_res(a).unwrap_or(1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `find_map(|a| to_res(a).ok())`
error: aborting due to 3 previous errors