Auto merge of #12562 - m-rph:12501, r=y21

Allow `filter_map_identity` when the closure is typed

This extends the `filter_map_identity` lint to support typed closures.

For untyped closures, we know that the program compiles, and therefore we can safely suggest using flatten.

For typed closures, they may participate in type resolution. In this case we use `Applicability::MaybeIncorrect`.

Details:
https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Should.20.60filter_map_identity.60.20lint.20when.20closures.20are.20typed.3F

changelog: `filter_map_identity` will now suggest using flatten for typed closures.

r? `@y21` && `@Centri3`

4 files changed, 288 insertions, 36 deletions

diff --git a/clippy_lints/src/methods/filter_map_identity.rs b/clippy_lints/src/methods/filter_map_identity.rs
index 8291c373f37..999df875c75 100644
--- a/clippy_lints/src/methods/filter_map_identity.rs
+++ b/clippy_lints/src/methods/filter_map_identity.rs
@@ -1,5 +1,5 @@
 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;
@@ -7,8 +7,20 @@ 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,
@@ -16,7 +28,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_map_arg:
             "use of `filter_map` with an identity function",
             "try",
             "flatten()".to_string(),
-            Applicability::MachineApplicable,
+            applicability,
         );
     }
 }
diff --git a/tests/ui/filter_map_identity.fixed b/tests/ui/filter_map_identity.fixed
index ad438afaca7..f3f6848e5f9 100644
--- a/tests/ui/filter_map_identity.fixed
+++ b/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
+    }
 }
diff --git a/tests/ui/filter_map_identity.rs b/tests/ui/filter_map_identity.rs
index d7423276872..b9aa9c05be8 100644
--- a/tests/ui/filter_map_identity.rs
+++ b/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
+    }
 }
diff --git a/tests/ui/filter_map_identity.stderr b/tests/ui/filter_map_identity.stderr
index 5aa46ad6d23..55068db4e9d 100644
--- a/tests/ui/filter_map_identity.stderr
+++ b/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