Add unnecessary_find_map lint

This commit is contained in:
Samuel E. Moelius III 2022-02-28 19:17:45 -05:00
parent e511476f24
commit 2a588d810f
8 changed files with 124 additions and 17 deletions

View File

@ -3498,6 +3498,7 @@ Released 2018-09-13
[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map
[`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold
[`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed

View File

@ -189,6 +189,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
LintId::of(methods::SUSPICIOUS_SPLITN),
LintId::of(methods::UNINIT_ASSUMED_INIT),
LintId::of(methods::UNNECESSARY_FILTER_MAP),
LintId::of(methods::UNNECESSARY_FIND_MAP),
LintId::of(methods::UNNECESSARY_FOLD),
LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS),
LintId::of(methods::UNNECESSARY_TO_OWNED),

View File

@ -49,6 +49,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
LintId::of(methods::SEARCH_IS_SOME),
LintId::of(methods::SKIP_WHILE_NEXT),
LintId::of(methods::UNNECESSARY_FILTER_MAP),
LintId::of(methods::UNNECESSARY_FIND_MAP),
LintId::of(methods::USELESS_ASREF),
LintId::of(misc::SHORT_CIRCUIT_STATEMENT),
LintId::of(misc_early::UNNEEDED_WILDCARD_PATTERN),

View File

@ -323,6 +323,7 @@ store.register_lints(&[
methods::SUSPICIOUS_SPLITN,
methods::UNINIT_ASSUMED_INIT,
methods::UNNECESSARY_FILTER_MAP,
methods::UNNECESSARY_FIND_MAP,
methods::UNNECESSARY_FOLD,
methods::UNNECESSARY_LAZY_EVALUATIONS,
methods::UNNECESSARY_TO_OWNED,

View File

@ -1309,7 +1309,7 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Checks for `filter_map` calls which could be replaced by `filter` or `map`.
/// Checks for `filter_map` calls that could be replaced by `filter` or `map`.
/// More specifically it checks if the closure provided is only performing one of the
/// filter or map operations and suggests the appropriate option.
///
@ -1337,6 +1337,36 @@ declare_clippy_lint! {
"using `filter_map` when a more succinct alternative exists"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `find_map` calls that could be replaced by `find` or `map`. More
/// specifically it checks if the closure provided is only performing one of the
/// find or map operations and suggests the appropriate option.
///
/// ### Why is this bad?
/// Complexity. The intent is also clearer if only a single
/// operation is being performed.
///
/// ### Example
/// ```rust
/// let _ = (0..3).find_map(|x| if x > 2 { Some(x) } else { None });
///
/// // As there is no transformation of the argument this could be written as:
/// let _ = (0..3).find(|&x| x > 2);
/// ```
///
/// ```rust
/// let _ = (0..4).find_map(|x| Some(x + 1));
///
/// // As there is no conditional check on the argument this could be written as:
/// let _ = (0..4).map(|x| x + 1).next();
/// ```
#[clippy::version = "1.61.0"]
pub UNNECESSARY_FIND_MAP,
complexity,
"using `find_map` when a more succinct alternative exists"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for `into_iter` calls on references which should be replaced by `iter`
@ -2020,6 +2050,7 @@ impl_lint_pass!(Methods => [
USELESS_ASREF,
UNNECESSARY_FOLD,
UNNECESSARY_FILTER_MAP,
UNNECESSARY_FIND_MAP,
INTO_ITER_ON_REF,
SUSPICIOUS_MAP,
UNINIT_ASSUMED_INIT,
@ -2305,9 +2336,12 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
extend_with_drain::check(cx, expr, recv, arg);
},
("filter_map", [arg]) => {
unnecessary_filter_map::check(cx, expr, arg);
unnecessary_filter_map::check(cx, expr, arg, name);
filter_map_identity::check(cx, expr, arg, span);
},
("find_map", [arg]) => {
unnecessary_filter_map::check(cx, expr, arg, name);
},
("flat_map", [arg]) => {
flat_map_identity::check(cx, expr, arg, span);
flat_map_option::check(cx, expr, arg, span);

View File

@ -11,8 +11,9 @@ use rustc_middle::ty;
use rustc_span::sym;
use super::UNNECESSARY_FILTER_MAP;
use super::UNNECESSARY_FIND_MAP;
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) {
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) {
if !is_trait_method(cx, expr, sym::Iterator) {
return;
}
@ -32,23 +33,30 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<
found_filtering |= return_visitor.found_filtering;
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
let sugg = if !found_filtering {
"map"
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
match cx.typeck_results().expr_ty(&body.value).kind() {
ty::Adt(adt, subst) if cx.tcx.is_diagnostic_item(sym::Option, adt.did) && in_ty == subst.type_at(0) => {
"filter"
},
_ => return,
}
} else {
return;
};
let sugg =
if !found_filtering {
if name == "filter_map" { "map" } else { "map(..).next()" }
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
match cx.typeck_results().expr_ty(&body.value).kind() {
ty::Adt(adt, subst)
if cx.tcx.is_diagnostic_item(sym::Option, adt.did) && in_ty == subst.type_at(0) =>
{
if name == "filter_map" { "filter" } else { "find" }
},
_ => return,
}
} else {
return;
};
span_lint(
cx,
UNNECESSARY_FILTER_MAP,
if name == "filter_map" {
UNNECESSARY_FILTER_MAP
} else {
UNNECESSARY_FIND_MAP
},
expr.span,
&format!("this `.filter_map` can be written more simply using `.{}`", sugg),
&format!("this `.{}` can be written more simply using `.{}`", name, sugg),
);
}
}

View File

@ -0,0 +1,23 @@
#![allow(dead_code)]
fn main() {
let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
let _ = (0..4).find_map(|x| {
if x > 1 {
return Some(x);
};
None
});
let _ = (0..4).find_map(|x| match x {
0 | 1 => None,
_ => Some(x),
});
let _ = (0..4).find_map(|x| Some(x + 1));
let _ = (0..4).find_map(i32::checked_abs);
}
fn find_map_none_changes_item_type() -> Option<bool> {
"".chars().find_map(|_| None)
}

View File

@ -0,0 +1,38 @@
error: this `.find_map` can be written more simply using `.find`
--> $DIR/unnecessary_find_map.rs:4:13
|
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unnecessary-find-map` implied by `-D warnings`
error: this `.find_map` can be written more simply using `.find`
--> $DIR/unnecessary_find_map.rs:5:13
|
LL | let _ = (0..4).find_map(|x| {
| _____________^
LL | | if x > 1 {
LL | | return Some(x);
LL | | };
LL | | None
LL | | });
| |______^
error: this `.find_map` can be written more simply using `.find`
--> $DIR/unnecessary_find_map.rs:11:13
|
LL | let _ = (0..4).find_map(|x| match x {
| _____________^
LL | | 0 | 1 => None,
LL | | _ => Some(x),
LL | | });
| |______^
error: this `.find_map` can be written more simply using `.map(..).next()`
--> $DIR/unnecessary_find_map.rs:16:13
|
LL | let _ = (0..4).find_map(|x| Some(x + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 4 previous errors