{flat_,}map_identity: recognize (tuple) struct de- and restructuring

clippy_utils/src/lib.rs

@@ -84,7 +84,7 @@ pub use self::hir_utils::{
 use core::mem;
 use core::ops::ControlFlow;
 use std::collections::hash_map::Entry;
-use std::iter::{once, repeat_n};
+use std::iter::{once, repeat_n, zip};
 use std::sync::{Mutex, MutexGuard, OnceLock};
 
 use itertools::Itertools;
@@ -581,7 +581,7 @@ pub fn can_mut_borrow_both(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>) -
         return false;
     }
 
-    for (x1, x2) in s1.iter().zip(s2.iter()) {
+    for (x1, x2) in zip(&s1, &s2) {
         if expr_custom_deref_adjustment(cx, x1).is_some() || expr_custom_deref_adjustment(cx, x2).is_some() {
             return false;
         }
@@ -1897,6 +1897,8 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 /// * `|x| { return x; }`
 /// * `|(x, y)| (x, y)`
 /// * `|[x, y]| [x, y]`
+/// * `|Foo { bar, baz }| Foo { bar, baz }`
+/// * `|Foo(bar, baz)| Foo(bar, baz)`
 ///
 /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
 fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
@@ -1913,21 +1915,47 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
             return false;
         }
 
+        // NOTE: we're inside a (function) body, so this won't ICE
+        let qpath_res = |qpath, hir| cx.typeck_results().qpath_res(qpath, hir);
+
         match (pat.kind, expr.kind) {
             (PatKind::Binding(_, id, _, _), _) => {
                 path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()
             },
             (PatKind::Tuple(pats, dotdot), ExprKind::Tup(tup))
                 if dotdot.as_opt_usize().is_none() && pats.len() == tup.len() =>
             {
-                pats.iter().zip(tup).all(|(pat, expr)| check_pat(cx, pat, expr))
+                zip(pats, tup).all(|(pat, expr)| check_pat(cx, pat, expr))
+            },
+            (PatKind::Slice(before, None, after), ExprKind::Array(arr)) if before.len() + after.len() == arr.len() => {
+                zip(before.iter().chain(after), arr).all(|(pat, expr)| check_pat(cx, pat, expr))
             },
-            (PatKind::Slice(before, slice, after), ExprKind::Array(arr))
-                if slice.is_none() && before.len() + after.len() == arr.len() =>
+            (PatKind::TupleStruct(pat_ident, field_pats, dotdot), ExprKind::Call(ident, fields))
+                if dotdot.as_opt_usize().is_none() && field_pats.len() == fields.len() =>
             {
-                (before.iter().chain(after))
-                    .zip(arr)
-                    .all(|(pat, expr)| check_pat(cx, pat, expr))
+                // check ident
+                if let ExprKind::Path(ident) = &ident.kind
+                    && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id)
+                    // check fields
+                    && zip(field_pats, fields).all(|(pat, expr)| check_pat(cx, pat, expr))
+                {
+                    true
+                } else {
+                    false
+                }
+            },
+            (
+                PatKind::Struct(pat_ident, field_pats, false),
+                ExprKind::Struct(ident, fields, hir::StructTailExpr::None),
+            ) if field_pats.len() == fields.len() => {
+                // check ident
+                qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id)
+                    // check fields
+                    && field_pats.iter().all(|field_pat| {
+                        fields
+                            .iter()
+                            .any(|field| field_pat.ident == field.ident && check_pat(cx, field_pat.pat, field.expr))
+                    })
             },
             _ => false,
         }

tests/ui/map_identity.fixed

@@ -1,5 +1,5 @@
 #![warn(clippy::map_identity)]
-#![allow(clippy::needless_return)]
+#![allow(clippy::needless_return, clippy::disallowed_names)]
 
 fn main() {
     let x: [u16; 3] = [1, 2, 3];
@@ -99,3 +99,65 @@ fn issue15198() {
     let _ = x.iter().copied();
     //~^ map_identity
 }
+
+mod foo {
+    #[derive(Clone, Copy)]
+    pub struct Foo {
+        pub foo: u8,
+        pub bar: u8,
+    }
+
+    #[derive(Clone, Copy)]
+    pub struct Foo2(pub u8, pub u8);
+}
+use foo::{Foo, Foo2};
+
+struct Bar {
+    foo: u8,
+    bar: u8,
+}
+
+struct Bar2(u8, u8);
+
+fn structs() {
+    let x = [Foo { foo: 0, bar: 0 }];
+
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // still lint when different paths are used for the same struct
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // don't lint: same fields but different structs
+    let _ = x.into_iter().map(|Foo { foo, bar }| Bar { foo, bar });
+
+    // still lint with redundant field names
+    #[allow(clippy::redundant_field_names)]
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // still lint with field order change
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // don't lint: switched field assignment
+    let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: bar, bar: foo });
+}
+
+fn tuple_structs() {
+    let x = [Foo2(0, 0)];
+
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // still lint when different paths are used for the same struct
+    let _ = x.into_iter();
+    //~^ map_identity
+
+    // don't lint: same fields but different structs
+    let _ = x.into_iter().map(|Foo2(foo, bar)| Bar2(foo, bar));
+
+    // don't lint: switched field assignment
+    let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(bar, foo));
+}

tests/ui/map_identity.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::map_identity)]
-#![allow(clippy::needless_return)]
+#![allow(clippy::needless_return, clippy::disallowed_names)]
 
 fn main() {
     let x: [u16; 3] = [1, 2, 3];
@@ -105,3 +105,65 @@ fn issue15198() {
     let _ = x.iter().copied().map(|[x, y]| [x, y]);
     //~^ map_identity
 }
+
+mod foo {
+    #[derive(Clone, Copy)]
+    pub struct Foo {
+        pub foo: u8,
+        pub bar: u8,
+    }
+
+    #[derive(Clone, Copy)]
+    pub struct Foo2(pub u8, pub u8);
+}
+use foo::{Foo, Foo2};
+
+struct Bar {
+    foo: u8,
+    bar: u8,
+}
+
+struct Bar2(u8, u8);
+
+fn structs() {
+    let x = [Foo { foo: 0, bar: 0 }];
+
+    let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar });
+    //~^ map_identity
+
+    // still lint when different paths are used for the same struct
+    let _ = x.into_iter().map(|Foo { foo, bar }| foo::Foo { foo, bar });
+    //~^ map_identity
+
+    // don't lint: same fields but different structs
+    let _ = x.into_iter().map(|Foo { foo, bar }| Bar { foo, bar });
+
+    // still lint with redundant field names
+    #[allow(clippy::redundant_field_names)]
+    let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: foo, bar: bar });
+    //~^ map_identity
+
+    // still lint with field order change
+    let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo });
+    //~^ map_identity
+
+    // don't lint: switched field assignment
+    let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: bar, bar: foo });
+}
+
+fn tuple_structs() {
+    let x = [Foo2(0, 0)];
+
+    let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(foo, bar));
+    //~^ map_identity
+
+    // still lint when different paths are used for the same struct
+    let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar));
+    //~^ map_identity
+
+    // don't lint: same fields but different structs
+    let _ = x.into_iter().map(|Foo2(foo, bar)| Bar2(foo, bar));
+
+    // don't lint: switched field assignment
+    let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(bar, foo));
+}

tests/ui/map_identity.stderr

@@ -93,5 +93,41 @@ error: unnecessary map of the identity function
 LL |     let _ = x.iter().copied().map(|[x, y]| [x, y]);
    |                              ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
 
-error: aborting due to 14 previous errors
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:131:26
+   |
+LL |     let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar });
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:135:26
+   |
+LL |     let _ = x.into_iter().map(|Foo { foo, bar }| foo::Foo { foo, bar });
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:143:26
+   |
+LL |     let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo: foo, bar: bar });
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:147:26
+   |
+LL |     let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo });
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:157:26
+   |
+LL |     let _ = x.into_iter().map(|Foo2(foo, bar)| Foo2(foo, bar));
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: unnecessary map of the identity function
+  --> tests/ui/map_identity.rs:161:26
+   |
+LL |     let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar));
+   |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
+
+error: aborting due to 20 previous errors