fix: remove unnecessary unfilter_map usages

rust-lang · Oct 14, 2024 · 11162d1 · 11162d1
1 parent 1f8f982
commit 11162d1
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 44 deletions.
diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs
@@ -1,13 +1,15 @@
 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::usage::mutated_variables;
 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 rustc_errors::Applicability;
 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;
 
@@ -36,9 +38,25 @@ 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
+                && 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()" }
         } else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) {
             match cx.typeck_results().expr_ty(body.value).kind() {
@@ -52,15 +70,18 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, a
         } else {
             return;
         };
-        span_lint(
+        span_lint_and_sugg(
             cx,
             if name == "filter_map" {
                 UNNECESSARY_FILTER_MAP
             } else {
                 UNNECESSARY_FIND_MAP
             },
             expr.span,
-            format!("this `.{name}` can be written more simply using `.{sugg}`"),
+            format!("this `.{name}` can be written more simply"),
+            format!("try replacing with `.{sugg}`"),
+            sugg.to_string(),
+            Applicability::MaybeIncorrect,
         );
     }
 }

diff --git a/tests/ui/unnecessary_filter_map.rs b/tests/ui/unnecessary_filter_map.rs
@@ -1,26 +1,31 @@
+//@no-rustfix
 #![allow(dead_code)]
 
 fn main() {
     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`
     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 {
             return Some(x);
         };
         None
     });
     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,
         _ => Some(x),
     });
 
     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(Some);
+
+    let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
 }
 
 fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> {

diff --git a/tests/ui/unnecessary_filter_map.stderr b/tests/ui/unnecessary_filter_map.stderr
@@ -1,14 +1,14 @@
-error: this `.filter_map` can be written more simply using `.filter`
-  --> tests/ui/unnecessary_filter_map.rs:4:13
+error: this `.filter_map` can be written more simply
+  --> tests/ui/unnecessary_filter_map.rs:5:13
    |
 LL |     let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing with `.filter`: `filter`
    |
    = note: `-D clippy::unnecessary-filter-map` implied by `-D warnings`
    = 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
+error: this `.filter_map` can be written more simply
+  --> tests/ui/unnecessary_filter_map.rs:8:13
    |
 LL |       let _ = (0..4).filter_map(|x| {
    |  _____________^
@@ -18,30 +18,51 @@ LL | |             return Some(x);
 LL | |         };
 LL | |         None
 LL | |     });
-   | |______^
+   | |______^ help: try replacing with `.filter`: `filter`
 
-error: this `.filter_map` can be written more simply using `.filter`
-  --> tests/ui/unnecessary_filter_map.rs:14:13
+error: this `.filter_map` can be written more simply
+  --> tests/ui/unnecessary_filter_map.rs:15:13
    |
 LL |       let _ = (0..4).filter_map(|x| match x {
    |  _____________^
 LL | |
 LL | |         0 | 1 => None,
 LL | |         _ => Some(x),
 LL | |     });
-   | |______^
+   | |______^ help: try replacing with `.filter`: `filter`
 
-error: this `.filter_map` can be written more simply using `.map`
-  --> tests/ui/unnecessary_filter_map.rs:20:13
+error: this `.filter_map` can be written more simply
+  --> tests/ui/unnecessary_filter_map.rs:21:13
    |
 LL |     let _ = (0..4).filter_map(|x| Some(x + 1));
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing with `.map`: `map`
 
-error: this `.filter_map` can be written more simply using `.filter`
-  --> tests/ui/unnecessary_filter_map.rs:160:14
+error: redundant closure
+  --> 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 replacing with `.map`: `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));
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing with `.filter`: `filter`
 
-error: aborting due to 5 previous errors
+error: aborting due to 8 previous errors
 
diff --git a/tests/ui/unnecessary_find_map.rs b/tests/ui/unnecessary_find_map.rs
@@ -1,24 +1,25 @@
+//@no-rustfix
 #![allow(dead_code)]
 
 fn main() {
     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`
     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 {
             return Some(x);
         };
         None
     });
     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,
         _ => Some(x),
     });
 
     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);
 }

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