unnecessary_{find,filter}_map: make diagnostic spans more precise

clippy_lints/src/methods/mod.rs

@@ -5162,13 +5162,13 @@ impl Methods {
                 },
                 (sym::filter_map, [arg]) => {
                     unused_enumerate_index::check(cx, expr, recv, arg);
-                    unnecessary_filter_map::check(cx, expr, arg, name);
+                    unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FilterMap);
                     filter_map_bool_then::check(cx, expr, arg, call_span);
                     filter_map_identity::check(cx, expr, arg, span);
                 },
                 (sym::find_map, [arg]) => {
                     unused_enumerate_index::check(cx, expr, recv, arg);
-                    unnecessary_filter_map::check(cx, expr, arg, name);
+                    unnecessary_filter_map::check(cx, expr, arg, call_span, unnecessary_filter_map::Kind::FindMap);
                 },
                 (sym::flat_map, [arg]) => {
                     unused_enumerate_index::check(cx, expr, recv, arg);

clippy_lints/src/methods/unnecessary_filter_map.rs

@@ -2,23 +2,24 @@ use super::utils::clone_or_copy_needed;
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath, MaybeTypeckRes};
 use clippy_utils::sym;
-use clippy_utils::ty::is_copy;
+use clippy_utils::ty::{is_copy, option_arg_ty};
 use clippy_utils::usage::mutated_variables;
 use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
 use core::ops::ControlFlow;
 use rustc_hir as hir;
 use rustc_hir::LangItem::{OptionNone, OptionSome};
 use rustc_lint::LateContext;
-use rustc_middle::ty;
-use rustc_span::Symbol;
+use rustc_span::Span;
+use std::fmt::Display;
 
 use super::{UNNECESSARY_FILTER_MAP, UNNECESSARY_FIND_MAP};
 
 pub(super) fn check<'tcx>(
     cx: &LateContext<'tcx>,
     expr: &'tcx hir::Expr<'tcx>,
     arg: &'tcx hir::Expr<'tcx>,
-    name: Symbol,
+    call_span: Span,
+    kind: Kind,
 ) {
     if !cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator) {
         return;
@@ -45,61 +46,88 @@ pub(super) fn check<'tcx>(
         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 == sym::filter_map
-                && let hir::ExprKind::Call(expr, args) = body.value.kind
+            if kind.is_filter_map()
+                && let hir::ExprKind::Call(expr, [arg]) = body.value.kind
                 && expr.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome)
-                && let hir::ExprKind::Path(_) = args[0].kind
+                && let hir::ExprKind::Path(_) = arg.kind
             {
                 span_lint(
                     cx,
                     UNNECESSARY_FILTER_MAP,
-                    expr.span,
+                    call_span,
                     String::from("this call to `.filter_map(..)` is unnecessary"),
                 );
                 return;
             }
-            if name == sym::filter_map {
-                "map(..)"
-            } else {
-                "map(..).next()"
+            match kind {
+                Kind::FilterMap => "map(..)",
+                Kind::FindMap => "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 == sym::filter_map {
-                        "filter(..)"
-                    } else {
-                        "find(..)"
-                    }
-                },
-                _ => return,
+            let ty = cx.typeck_results().expr_ty(body.value);
+            if option_arg_ty(cx, ty).is_some_and(|t| t == in_ty) {
+                match kind {
+                    Kind::FilterMap => "filter(..)",
+                    Kind::FindMap => "find(..)",
+                }
+            } else {
+                return;
             }
         } else {
             return;
         };
         span_lint(
             cx,
-            if name == sym::filter_map {
-                UNNECESSARY_FILTER_MAP
-            } else {
-                UNNECESSARY_FIND_MAP
+            match kind {
+                Kind::FilterMap => UNNECESSARY_FILTER_MAP,
+                Kind::FindMap => UNNECESSARY_FIND_MAP,
             },
-            expr.span,
-            format!("this `.{name}(..)` can be written more simply using `.{sugg}`"),
+            call_span,
+            format!("this `.{kind}(..)` can be written more simply using `.{sugg}`"),
         );
     }
 }
 
+#[derive(Clone, Copy)]
+pub(super) enum Kind {
+    FilterMap,
+    FindMap,
+}
+
+impl Kind {
+    fn is_filter_map(self) -> bool {
+        matches!(self, Self::FilterMap)
+    }
+}
+
+impl Display for Kind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::FilterMap => f.write_str("filter_map"),
+            Self::FindMap => f.write_str("find_map"),
+        }
+    }
+}
+
 // returns (found_mapping, found_filtering)
 fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) {
     match expr.kind {
+        hir::ExprKind::Path(ref path)
+            if cx
+                .qpath_res(path, expr.hir_id)
+                .ctor_parent(cx)
+                .is_lang_item(cx, OptionNone) =>
+        {
+            // None
+            (false, true)
+        },
         hir::ExprKind::Call(func, args) => {
             if func.res(cx).ctor_parent(cx).is_lang_item(cx, OptionSome) {
                 if args[0].res_local_id() == Some(arg_id) {
+                    // Some(arg_id)
                     return (false, false);
                 }
+                // Some(not arg_id)
                 return (true, false);
             }
             (true, true)
@@ -109,8 +137,10 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
                 && cx.typeck_results().expr_ty(recv).is_bool()
                 && arg.res_local_id() == Some(arg_id)
             {
+                // bool.then_some(arg_id)
                 (false, true)
             } else {
+                // bool.then_some(not arg_id)
                 (true, true)
             }
         },
@@ -134,14 +164,6 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc
             let else_check = check_expression(cx, arg_id, else_arm);
             (if_check.0 | else_check.0, if_check.1 | else_check.1)
         },
-        hir::ExprKind::Path(ref path)
-            if cx
-                .qpath_res(path, expr.hir_id)
-                .ctor_parent(cx)
-                .is_lang_item(cx, OptionNone) =>
-        {
-            (false, true)
-        },
         _ => (true, true),
     }
 }

tests/ui/unnecessary_filter_map.stderr

@@ -1,17 +1,17 @@
 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:4:20
    |
 LL |     let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None });
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = 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
+  --> tests/ui/unnecessary_filter_map.rs:7:20
    |
 LL |       let _ = (0..4).filter_map(|x| {
-   |  _____________^
+   |  ____________________^
 LL | |
 LL | |
 LL | |         if x > 1 {
@@ -21,33 +21,33 @@ LL | |     });
    | |______^
 
 error: this `.filter_map(..)` can be written more simply using `.filter(..)`
-  --> tests/ui/unnecessary_filter_map.rs:15:13
+  --> tests/ui/unnecessary_filter_map.rs:15:20
    |
 LL |       let _ = (0..4).filter_map(|x| match x {
-   |  _____________^
+   |  ____________________^
 LL | |
 LL | |         0 | 1 => None,
 LL | |         _ => Some(x),
 LL | |     });
    | |______^
 
 error: this `.filter_map(..)` can be written more simply using `.map(..)`
-  --> tests/ui/unnecessary_filter_map.rs:21:13
+  --> tests/ui/unnecessary_filter_map.rs:21:20
    |
 LL |     let _ = (0..4).filter_map(|x| Some(x + 1));
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this call to `.filter_map(..)` is unnecessary
-  --> tests/ui/unnecessary_filter_map.rs:28:61
+  --> tests/ui/unnecessary_filter_map.rs:28:46
    |
 LL |     let _ = vec![Some(10), None].into_iter().filter_map(|x| Some(x));
-   |                                                             ^^^^
+   |                                              ^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this `.filter_map(..)` can be written more simply using `.filter(..)`
-  --> tests/ui/unnecessary_filter_map.rs:166:14
+  --> tests/ui/unnecessary_filter_map.rs:166:33
    |
 LL |     let _x = std::iter::once(1).filter_map(|n| (n > 1).then_some(n));
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 6 previous errors
 

tests/ui/unnecessary_find_map.rs

@@ -1,5 +1,3 @@
-#![allow(dead_code)]
-
 fn main() {
     let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
     //~^ unnecessary_find_map

tests/ui/unnecessary_find_map.stderr

@@ -1,17 +1,17 @@
 error: this `.find_map(..)` can be written more simply using `.find(..)`
-  --> tests/ui/unnecessary_find_map.rs:4:13
+  --> tests/ui/unnecessary_find_map.rs:2:20
    |
 LL |     let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None });
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = 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
+  --> tests/ui/unnecessary_find_map.rs:5:20
    |
 LL |       let _ = (0..4).find_map(|x| {
-   |  _____________^
+   |  ____________________^
 LL | |
 LL | |
 LL | |         if x > 1 {
@@ -21,27 +21,27 @@ LL | |     });
    | |______^
 
 error: this `.find_map(..)` can be written more simply using `.find(..)`
-  --> tests/ui/unnecessary_find_map.rs:15:13
+  --> tests/ui/unnecessary_find_map.rs:13:20
    |
 LL |       let _ = (0..4).find_map(|x| match x {
-   |  _____________^
+   |  ____________________^
 LL | |
 LL | |         0 | 1 => None,
 LL | |         _ => Some(x),
 LL | |     });
    | |______^
 
 error: this `.find_map(..)` can be written more simply using `.map(..).next()`
-  --> tests/ui/unnecessary_find_map.rs:21:13
+  --> tests/ui/unnecessary_find_map.rs:19:20
    |
 LL |     let _ = (0..4).find_map(|x| Some(x + 1));
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: this `.find_map(..)` can be written more simply using `.find(..)`
-  --> tests/ui/unnecessary_find_map.rs:33:14
+  --> tests/ui/unnecessary_find_map.rs:31:33
    |
 LL |     let _x = std::iter::once(1).find_map(|n| (n > 1).then_some(n));
-   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 5 previous errors