Allow filter_map_identity when the closure is typed

clippy_lints/src/methods/filter_map_identity.rs

@@ -1,22 +1,34 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::{is_expr_untyped_identity_function, is_trait_method};
+use clippy_utils::{is_expr_identity_function, is_expr_untyped_identity_function, is_trait_method};
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
 use rustc_span::{sym, Span};
 
 use super::FILTER_MAP_IDENTITY;
 
+fn is_identity(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option<Applicability> {
+    if is_expr_untyped_identity_function(cx, expr) {
+        return Some(Applicability::MachineApplicable);
+    }
+    if is_expr_identity_function(cx, expr) {
+        return Some(Applicability::Unspecified);
+    }
+    None
+}
+
 pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg: &hir::Expr<'_>, filter_map_span: Span) {
-    if is_trait_method(cx, expr, sym::Iterator) && is_expr_untyped_identity_function(cx, filter_map_arg) {
+    if is_trait_method(cx, expr, sym::Iterator)
+        && let Some(applicability) = is_identity(cx, filter_map_arg)
+    {
         span_lint_and_sugg(
             cx,
             FILTER_MAP_IDENTITY,
             filter_map_span.with_hi(expr.span.hi()),
             "use of `filter_map` with an identity function",
             "try",
             "flatten()".to_string(),
-            Applicability::MachineApplicable,
+            applicability,
         );
     }
 }

tests/ui/filter_map_identity.fixed

@@ -1,17 +1,83 @@
-#![allow(unused_imports, clippy::needless_return)]
+#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
 #![warn(clippy::filter_map_identity)]
+#![feature(stmt_expr_attributes)]
+
+use std::option::Option;
+struct NonCopy;
+use std::convert::identity;
+
+fn non_copy_vec() -> Vec<Option<NonCopy>> {
+    todo!()
+}
+
+fn copy_vec<T: Copy>() -> Vec<Option<T>> {
+    todo!()
+}
+
+fn copy_vec_non_inferred() -> Vec<Option<i32>> {
+    todo!()
+}
+
+fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
+    vec![Some(T::default())]
+}
 
 fn main() {
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.flatten();
+    {
+        // into_iter
+        copy_vec_non_inferred().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().flatten();
+        //~^ ERROR: use of
+
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
+
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().flatten();
+        //~^ ERROR: use of
 
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.flatten();
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
 
-    use std::convert::identity;
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.flatten();
+        // we are forced to pass the type in the call.
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        #[rustfmt::skip]
+            copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+        #[rustfmt::skip]
+            copy_vec::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
 
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.flatten();
+        // note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
+        // it behaves the same as copy_vec.
+        opaque::<i32>().into_iter().flatten();
+        //~^ ERROR: use of
+    }
 }

tests/ui/filter_map_identity.rs

@@ -1,17 +1,83 @@
-#![allow(unused_imports, clippy::needless_return)]
+#![allow(unused_imports, clippy::needless_return, clippy::useless_vec)]
 #![warn(clippy::filter_map_identity)]
+#![feature(stmt_expr_attributes)]
+
+use std::option::Option;
+struct NonCopy;
+use std::convert::identity;
+
+fn non_copy_vec() -> Vec<Option<NonCopy>> {
+    todo!()
+}
+
+fn copy_vec<T: Copy>() -> Vec<Option<T>> {
+    todo!()
+}
+
+fn copy_vec_non_inferred() -> Vec<Option<i32>> {
+    todo!()
+}
+
+fn opaque<T: Default>() -> impl IntoIterator<Item = Option<T>> {
+    vec![Some(T::default())]
+}
 
 fn main() {
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.filter_map(|x| x);
+    {
+        // into_iter
+        copy_vec_non_inferred().into_iter().filter_map(|x| x);
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().filter_map(identity);
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().filter_map(|x| return x);
+        //~^ ERROR: use of
+        copy_vec_non_inferred().into_iter().filter_map(|x| return x);
+        //~^ ERROR: use of
+
+        non_copy_vec().into_iter().filter_map(|x| x);
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().filter_map(|x| x);
+        //~^ ERROR: use of
+
+        non_copy_vec().into_iter().filter_map(std::convert::identity);
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().filter_map(identity);
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().filter_map(|x| return x);
+        //~^ ERROR: use of
+        non_copy_vec().into_iter().filter_map(|x| return x);
+        //~^ ERROR: use of
 
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.filter_map(std::convert::identity);
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
+        //~^ ERROR: use of
 
-    use std::convert::identity;
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.filter_map(identity);
+        // we are forced to pass the type in the call.
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
+        //~^ ERROR: use of
+        copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
+        //~^ ERROR: use of
+        #[rustfmt::skip]
+            copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
+        //~^ ERROR: use of
+        #[rustfmt::skip]
+            copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
+        //~^ ERROR: use of
 
-    let iterator = vec![Some(1), None, Some(2)].into_iter();
-    let _ = iterator.filter_map(|x| return x);
+        // note, the compiler requires that we pass the type to `opaque`. This is mostly for reference,
+        // it behaves the same as copy_vec.
+        opaque::<i32>().into_iter().filter_map(|x| x);
+        //~^ ERROR: use of
+    }
 }

tests/ui/filter_map_identity.stderr

@@ -1,29 +1,137 @@
 error: use of `filter_map` with an identity function
-  --> tests/ui/filter_map_identity.rs:6:22
+  --> tests/ui/filter_map_identity.rs:28:45
    |
-LL |     let _ = iterator.filter_map(|x| x);
-   |                      ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+LL |         copy_vec_non_inferred().into_iter().filter_map(|x| x);
+   |                                             ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
    |
    = note: `-D clippy::filter-map-identity` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::filter_map_identity)]`
 
 error: use of `filter_map` with an identity function
-  --> tests/ui/filter_map_identity.rs:9:22
+  --> tests/ui/filter_map_identity.rs:30:45
    |
-LL |     let _ = iterator.filter_map(std::convert::identity);
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+LL |         copy_vec_non_inferred().into_iter().filter_map(std::convert::identity);
+   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
 
 error: use of `filter_map` with an identity function
-  --> tests/ui/filter_map_identity.rs:13:22
+  --> tests/ui/filter_map_identity.rs:32:45
    |
-LL |     let _ = iterator.filter_map(identity);
-   |                      ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+LL |         copy_vec_non_inferred().into_iter().filter_map(identity);
+   |                                             ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
 
 error: use of `filter_map` with an identity function
-  --> tests/ui/filter_map_identity.rs:16:22
+  --> tests/ui/filter_map_identity.rs:34:45
    |
-LL |     let _ = iterator.filter_map(|x| return x);
-   |                      ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+LL |         copy_vec_non_inferred().into_iter().filter_map(|x| return x);
+   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
 
-error: aborting due to 4 previous errors
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:36:45
+   |
+LL |         copy_vec_non_inferred().into_iter().filter_map(|x| return x);
+   |                                             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:39:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(|x| x);
+   |                                    ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:41:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(|x| x);
+   |                                    ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:44:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(std::convert::identity);
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:46:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(identity);
+   |                                    ^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:48:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(|x| return x);
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:50:36
+   |
+LL |         non_copy_vec().into_iter().filter_map(|x| return x);
+   |                                    ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:53:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:55:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:57:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:59:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<_>| return x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:63:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:65:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:67:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:69:39
+   |
+LL |         copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| return x);
+   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:72:43
+   |
+LL |             copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ x }});
+   |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:75:43
+   |
+LL |             copy_vec::<i32>().into_iter().filter_map(|x: Option<i32>| -> Option<i32> {{ return x }});
+   |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: use of `filter_map` with an identity function
+  --> tests/ui/filter_map_identity.rs:80:37
+   |
+LL |         opaque::<i32>().into_iter().filter_map(|x| x);
+   |                                     ^^^^^^^^^^^^^^^^^ help: try: `flatten()`
+
+error: aborting due to 22 previous errors