Auto merge of #8489 - smoelius:unnecessary-find-map, r=llogiq

Add `unnecessary_find_map` lint

This PR adds an `unnecessary_find_map` lint. It is essentially just a minor enhancement of `unnecessary_filter_map`.

Closes #8467

changelog: New lint `unnecessary_find_map`
This commit is contained in:
bors
2022-03-02 19:50:27 +00:00
8 changed files with 124 additions and 17 deletions

View File

@@ -190,6 +190,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

@@ -326,6 +326,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),
);
}
}