Auto merge of #13548 - wowinter13:unnecessary_filter_map_filter_map_some, r=llogiq

fix: remove unnecessary filter_map usages

fixes https://github.com/rust-lang/rust-clippy/issues/12556

(Fixed version of https://github.com/rust-lang/rust-clippy/pull/12766)

changelog: [unnecessary_filter_map]: filter map improvements
This commit is contained in:
bors 2024-10-27 22:27:49 +00:00
commit 73bad368f2
5 changed files with 92 additions and 44 deletions

View file

@ -1,13 +1,15 @@
use super::utils::clone_or_copy_needed; use super::utils::clone_or_copy_needed;
use clippy_utils::diagnostics::span_lint; use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::ty::is_copy; use clippy_utils::ty::is_copy;
use clippy_utils::usage::mutated_variables; use clippy_utils::usage::mutated_variables;
use clippy_utils::visitors::{Descend, for_each_expr_without_closures}; use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id}; use clippy_utils::{MaybePath, is_res_lang_ctor, is_trait_method, path_res, path_to_local_id};
use core::ops::ControlFlow; use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir as hir; use rustc_hir as hir;
use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::LangItem::{OptionNone, OptionSome};
use rustc_lint::LateContext; use rustc_lint::LateContext;
use rustc_middle::query::Key;
use rustc_middle::ty; use rustc_middle::ty;
use rustc_span::sym; use rustc_span::sym;
@ -36,9 +38,25 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
ControlFlow::Continue(Descend::Yes) ControlFlow::Continue(Descend::Yes)
} }
}); });
let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); let in_ty = cx.typeck_results().node_type(body.params[0].hir_id);
let sugg = if !found_filtering { let sugg = if !found_filtering {
// Check if the closure is .filter_map(|x| Some(x))
if name == "filter_map"
&& let hir::ExprKind::Call(expr, args) = body.value.kind
&& is_res_lang_ctor(cx, path_res(cx, expr), OptionSome)
&& arg_id.ty_def_id() == args[0].hir_id().ty_def_id()
&& let hir::ExprKind::Path(_) = args[0].kind
{
span_lint_and_sugg(
cx,
UNNECESSARY_FILTER_MAP,
expr.span,
format!("{name} is unnecessary"),
"try removing the filter_map",
String::new(),
Applicability::MaybeIncorrect,
);
}
if name == "filter_map" { "map" } else { "map(..).next()" } if name == "filter_map" { "map" } else { "map(..).next()" }
} else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) { } else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
match cx.typeck_results().expr_ty(body.value).kind() { match cx.typeck_results().expr_ty(body.value).kind() {
@ -52,7 +70,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
} else { } else {
return; return;
}; };
span_lint( span_lint_and_sugg(
cx, cx,
if name == "filter_map" { if name == "filter_map" {
UNNECESSARY_FILTER_MAP UNNECESSARY_FILTER_MAP
@ -60,7 +78,10 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
UNNECESSARY_FIND_MAP UNNECESSARY_FIND_MAP
}, },
expr.span, expr.span,
format!("this `.{name}` can be written more simply using `.{sugg}`"), format!("this `.{name}` can be written more simply"),
"try instead",
sugg.to_string(),
Applicability::MaybeIncorrect,
); );
} }
} }

View file

@ -1,26 +1,31 @@
//@no-rustfix
#![allow(dead_code)] #![allow(dead_code)]
fn main() { fn main() {
let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
//~^ ERROR: this `.filter_map` can be written more simply using `.filter` //~^ ERROR: this `.filter_map` can be written more simply
//~| NOTE: `-D clippy::unnecessary-filter-map` implied by `-D warnings` //~| NOTE: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
let _ = (0..4).filter_map(|x| { let _ = (0..4).filter_map(|x| {
//~^ ERROR: this `.filter_map` can be written more simply using `.filter` //~^ ERROR: this `.filter_map` can be written more simply
if x > 1 { if x > 1 {
return Some(x); return Some(x);
}; };
None None
}); });
let _ = (0..4).filter_map(|x| match x { let _ = (0..4).filter_map(|x| match x {
//~^ ERROR: this `.filter_map` can be written more simply using `.filter` //~^ ERROR: this `.filter_map` can be written more simply
0 | 1 => None, 0 | 1 => None,
_ => Some(x), _ => Some(x),
}); });
let _ = (0..4).filter_map(|x| Some(x + 1)); let _ = (0..4).filter_map(|x| Some(x + 1));
//~^ ERROR: this `.filter_map` can be written more simply using `.map` //~^ ERROR: this `.filter_map` can be written more simply
let _ = (0..4).filter_map(i32::checked_abs); let _ = (0..4).filter_map(i32::checked_abs);
let _ = (0..4).filter_map(Some);
let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
} }
fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> { fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> {

View file

@ -1,14 +1,14 @@
error: this `.filter_map` can be written more simply using `.filter` error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:4:13 --> tests/ui/unnecessary_filter_map.rs:5:13
| |
LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
| |
= note: `-D clippy::unnecessary-filter-map` implied by `-D warnings` = note: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_filter_map)]` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_filter_map)]`
error: this `.filter_map` can be written more simply using `.filter` error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:7:13 --> tests/ui/unnecessary_filter_map.rs:8:13
| |
LL | let _ = (0..4).filter_map(|x| { LL | let _ = (0..4).filter_map(|x| {
| _____________^ | _____________^
@ -18,10 +18,10 @@ LL | | return Some(x);
LL | | }; LL | | };
LL | | None LL | | None
LL | | }); LL | | });
| |______^ | |______^ help: try instead: `filter`
error: this `.filter_map` can be written more simply using `.filter` error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:14:13 --> tests/ui/unnecessary_filter_map.rs:15:13
| |
LL | let _ = (0..4).filter_map(|x| match x { LL | let _ = (0..4).filter_map(|x| match x {
| _____________^ | _____________^
@ -29,19 +29,40 @@ LL | |
LL | | 0 | 1 => None, LL | | 0 | 1 => None,
LL | | _ => Some(x), LL | | _ => Some(x),
LL | | }); LL | | });
| |______^ | |______^ help: try instead: `filter`
error: this `.filter_map` can be written more simply using `.map` error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:20:13 --> tests/ui/unnecessary_filter_map.rs:21:13
| |
LL | let _ = (0..4).filter_map(|x| Some(x + 1)); LL | let _ = (0..4).filter_map(|x| Some(x + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map`
error: this `.filter_map` can be written more simply using `.filter` error: redundant closure
--> tests/ui/unnecessary_filter_map.rs:160:14 --> tests/ui/unnecessary_filter_map.rs:28:57
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^^^^^^^^ help: replace the closure with the function itself: `Some`
|
= note: `-D clippy::redundant-closure` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]`
error: filter_map is unnecessary
--> tests/ui/unnecessary_filter_map.rs:28:61
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^ help: try removing the filter_map
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:28:13
|
LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map`
error: this `.filter_map` can be written more simply
--> tests/ui/unnecessary_filter_map.rs:165:14
| |
LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n)); LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `filter`
error: aborting due to 5 previous errors error: aborting due to 8 previous errors

View file

@ -1,24 +1,25 @@
//@no-rustfix
#![allow(dead_code)] #![allow(dead_code)]
fn main() { fn main() {
let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None }); let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
//~^ ERROR: this `.find_map` can be written more simply using `.find` //~^ ERROR: this `.find_map` can be written more simply
//~| NOTE: `-D clippy::unnecessary-find-map` implied by `-D warnings` //~| NOTE: `-D clippy::unnecessary-find-map` implied by `-D warnings`
let _ = (0..4).find_map(|x| { let _ = (0..4).find_map(|x| {
//~^ ERROR: this `.find_map` can be written more simply using `.find` //~^ ERROR: this `.find_map` can be written more simply
if x > 1 { if x > 1 {
return Some(x); return Some(x);
}; };
None None
}); });
let _ = (0..4).find_map(|x| match x { let _ = (0..4).find_map(|x| match x {
//~^ ERROR: this `.find_map` can be written more simply using `.find` //~^ ERROR: this `.find_map` can be written more simply
0 | 1 => None, 0 | 1 => None,
_ => Some(x), _ => Some(x),
}); });
let _ = (0..4).find_map(|x| Some(x + 1)); let _ = (0..4).find_map(|x| Some(x + 1));
//~^ ERROR: this `.find_map` can be written more simply using `.map(..).next()` //~^ ERROR: this `.find_map` can be written more simply
let _ = (0..4).find_map(i32::checked_abs); let _ = (0..4).find_map(i32::checked_abs);
} }

View file

@ -1,14 +1,14 @@
error: this `.find_map` can be written more simply using `.find` error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:4:13 --> tests/ui/unnecessary_find_map.rs:5:13
| |
LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None }); LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `find`
| |
= note: `-D clippy::unnecessary-find-map` implied by `-D warnings` = note: `-D clippy::unnecessary-find-map` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_find_map)]` = help: to override `-D warnings` add `#[allow(clippy::unnecessary_find_map)]`
error: this `.find_map` can be written more simply using `.find` error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:7:13 --> tests/ui/unnecessary_find_map.rs:8:13
| |
LL | let _ = (0..4).find_map(|x| { LL | let _ = (0..4).find_map(|x| {
| _____________^ | _____________^
@ -18,10 +18,10 @@ LL | | return Some(x);
LL | | }; LL | | };
LL | | None LL | | None
LL | | }); LL | | });
| |______^ | |______^ help: try instead: `find`
error: this `.find_map` can be written more simply using `.find` error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:14:13 --> tests/ui/unnecessary_find_map.rs:15:13
| |
LL | let _ = (0..4).find_map(|x| match x { LL | let _ = (0..4).find_map(|x| match x {
| _____________^ | _____________^
@ -29,19 +29,19 @@ LL | |
LL | | 0 | 1 => None, LL | | 0 | 1 => None,
LL | | _ => Some(x), LL | | _ => Some(x),
LL | | }); LL | | });
| |______^ | |______^ help: try instead: `find`
error: this `.find_map` can be written more simply using `.map(..).next()` error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:20:13 --> tests/ui/unnecessary_find_map.rs:21:13
| |
LL | let _ = (0..4).find_map(|x| Some(x + 1)); LL | let _ = (0..4).find_map(|x| Some(x + 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `map(..).next()`
error: this `.find_map` can be written more simply using `.find` error: this `.find_map` can be written more simply
--> tests/ui/unnecessary_find_map.rs:32:14 --> tests/ui/unnecessary_find_map.rs:33:14
| |
LL | let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n)); LL | let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try instead: `find`
error: aborting due to 5 previous errors error: aborting due to 5 previous errors