From 749084512d80152b71af5b2d0e53ff1c435c76e5 Mon Sep 17 00:00:00 2001 From: Omer Shtivi Date: Mon, 6 May 2024 09:24:57 +0300 Subject: [PATCH 1/2] fix: unnecessary_filter_map return of Some arg --- .../src/methods/unnecessary_filter_map.rs | 19 +++++++++-- tests/ui/unnecessary_filter_map.rs | 3 ++ tests/ui/unnecessary_filter_map.stderr | 33 +++++++++++++++---- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index fabf3fa0c0c8..24c91d3860f0 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -3,7 +3,8 @@ use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::is_copy; use clippy_utils::usage::mutated_variables; use clippy_utils::visitors::{for_each_expr, Descend}; -use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id}; +use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id, MaybePath}; +use rustc_middle::query::Key; use core::ops::ControlFlow; use rustc_hir as hir; use rustc_hir::LangItem::{OptionNone, OptionSome}; @@ -17,7 +18,6 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a if !is_trait_method(cx, expr, sym::Iterator) { return; } - if let hir::ExprKind::Closure(&hir::Closure { body, .. }) = arg.kind { let body = cx.tcx.hir().body(body); let arg_id = body.params[0].pat.hir_id; @@ -36,9 +36,22 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a ControlFlow::Continue(Descend::Yes) } }); - let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); 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 { + if is_res_lang_ctor(cx, path_res(cx, expr), OptionSome) && arg_id.ty_def_id() == args[0].hir_id().ty_def_id(){ + if let hir::ExprKind::Path(_) = args[0].kind { + span_lint( + cx, + UNNECESSARY_FILTER_MAP, + arg.span, + &format!("{name} is unnecessary") + ) + } + + } + } 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() { diff --git a/tests/ui/unnecessary_filter_map.rs b/tests/ui/unnecessary_filter_map.rs index 1e0d7d12965c..65d83eac8b64 100644 --- a/tests/ui/unnecessary_filter_map.rs +++ b/tests/ui/unnecessary_filter_map.rs @@ -1,3 +1,4 @@ +//@no-rustfix #![allow(dead_code)] fn main() { @@ -21,6 +22,8 @@ fn main() { //~^ ERROR: this `.filter_map` can be written more simply using `.map` let _ = (0..4).filter_map(i32::checked_abs); + + let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x)); } fn filter_map_none_changes_item_type() -> impl Iterator { diff --git a/tests/ui/unnecessary_filter_map.stderr b/tests/ui/unnecessary_filter_map.stderr index f32d444ba9e0..fd6adc1fa4bd 100644 --- a/tests/ui/unnecessary_filter_map.stderr +++ b/tests/ui/unnecessary_filter_map.stderr @@ -1,5 +1,5 @@ error: this `.filter_map` can be written more simply using `.filter` - --> 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 }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); = help: to override `-D warnings` add `#[allow(clippy::unnecessary_filter_map)]` error: this `.filter_map` can be written more simply using `.filter` - --> tests/ui/unnecessary_filter_map.rs:7:13 + --> tests/ui/unnecessary_filter_map.rs:8:13 | LL | let _ = (0..4).filter_map(|x| { | _____________^ @@ -21,7 +21,7 @@ LL | | }); | |______^ error: this `.filter_map` can be written more simply using `.filter` - --> 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 { | _____________^ @@ -32,16 +32,37 @@ LL | | }); | |______^ error: this `.filter_map` can be written more simply using `.map` - --> 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)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +error: filter_map is unnecessary + --> tests/ui/unnecessary_filter_map.rs:26:57 + | +LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x)); + | ^^^^^^^^^^^ + +error: this `.filter_map` can be written more simply using `.map` + --> tests/ui/unnecessary_filter_map.rs:26:13 + | +LL | let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: redundant closure + --> tests/ui/unnecessary_filter_map.rs:26: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: this `.filter_map` can be written more simply using `.filter` - --> tests/ui/unnecessary_filter_map.rs:160:14 + --> tests/ui/unnecessary_filter_map.rs:163:14 | LL | let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: aborting due to 8 previous errors From 37e9deddaa11e81cc3542d2d2edad30c8e4c2d87 Mon Sep 17 00:00:00 2001 From: Omer Shtivi Date: Mon, 6 May 2024 09:31:48 +0300 Subject: [PATCH 2/2] cargo dev fmt --- .../src/methods/unnecessary_filter_map.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index 24c91d3860f0..559c675b3aa2 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -4,11 +4,11 @@ use clippy_utils::ty::is_copy; use clippy_utils::usage::mutated_variables; use clippy_utils::visitors::{for_each_expr, Descend}; use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id, MaybePath}; -use rustc_middle::query::Key; use core::ops::ControlFlow; use rustc_hir as hir; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_lint::LateContext; +use rustc_middle::query::Key; use rustc_middle::ty; use rustc_span::sym; @@ -39,17 +39,15 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); 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 { - if is_res_lang_ctor(cx, path_res(cx, expr), OptionSome) && arg_id.ty_def_id() == args[0].hir_id().ty_def_id(){ + if name == "filter_map" + && let hir::ExprKind::Call(expr, args) = body.value.kind + { + if is_res_lang_ctor(cx, path_res(cx, expr), OptionSome) + && arg_id.ty_def_id() == args[0].hir_id().ty_def_id() + { if let hir::ExprKind::Path(_) = args[0].kind { - span_lint( - cx, - UNNECESSARY_FILTER_MAP, - arg.span, - &format!("{name} is unnecessary") - ) + span_lint(cx, UNNECESSARY_FILTER_MAP, arg.span, &format!("{name} is unnecessary")) } - } } if name == "filter_map" { "map" } else { "map(..).next()" }