diff --git a/CHANGELOG.md b/CHANGELOG.md index c936e35dc1b04..1ab879ba9eea2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3499,6 +3499,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 diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 6bfbacad086c1..4484f6e196066 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -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), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index bd5ff613447cd..5d588ac66499f 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -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), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 59e94029eedd6..c34d8f46e09b6 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -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, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3021a40fae142..581f4c8b0e2e3 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -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); diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index 108e143f33737..9e48cd13b4cd9 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -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), ); } } diff --git a/tests/ui/unnecessary_find_map.rs b/tests/ui/unnecessary_find_map.rs new file mode 100644 index 0000000000000..a52390861b403 --- /dev/null +++ b/tests/ui/unnecessary_find_map.rs @@ -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 { + "".chars().find_map(|_| None) +} diff --git a/tests/ui/unnecessary_find_map.stderr b/tests/ui/unnecessary_find_map.stderr new file mode 100644 index 0000000000000..fb33c122fe337 --- /dev/null +++ b/tests/ui/unnecessary_find_map.stderr @@ -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 +