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

y21 · web-flow · commit ffcd12946e2d · 2025-08-02T11:37:12.000Z
Follow-up of #15229, as described in #15229 (comment) -- it turned out to be not that difficult after all! changelog: [`map_identity`,`flat_map_identity`]: also recognize (tuple) struct de- and resctructuring r? @y21
diff --git a/clippy_utils/src/lib.rs b/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;
@@ -582,7 +582,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;
         }
@@ -1898,6 +1898,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 {
@@ -1942,6 +1944,8 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
 /// * `x` is the identity representation of `x`
 /// * `(x, y)` is the identity representation of `(x, y)`
 /// * `[x, y]` is the identity representation of `[x, y]`
+/// * `Foo(bar, baz)` is the identity representation of `Foo(bar, baz)`
+/// * `Foo { bar, baz }` is the identity representation of `Foo { bar, baz }`
 ///
 /// Note that `by_hir` is used to determine bindings are checked by their `HirId` or by their name.
 /// This can be useful when checking patterns in `let` bindings or `match` arms.
@@ -1958,6 +1962,9 @@ pub fn is_expr_identity_of_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<
         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, _, _), _) if by_hir => {
             path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()
@@ -1968,16 +1975,36 @@ pub fn is_expr_identity_of_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<
         (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)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
+            zip(pats, tup).all(|(pat, expr)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
+        },
+        (PatKind::Slice(before, None, after), ExprKind::Array(arr)) if before.len() + after.len() == arr.len() => {
+            zip(before.iter().chain(after), arr).all(|(pat, expr)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
         },
-        (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)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
+            // 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)| is_expr_identity_of_pat(cx, pat, expr,by_hir))
+            {
+                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 && is_expr_identity_of_pat(cx, field_pat.pat, field.expr, by_hir)
+                    })
+                })
         },
         _ => false,
     }
diff --git a/tests/ui/map_identity.fixed b/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));
+}
diff --git a/tests/ui/map_identity.rs b/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));
+}
diff --git a/tests/ui/map_identity.stderr b/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
 
