From 5a8aea9971d89292b8173405e056932486022be8 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 14:47:28 +0200 Subject: [PATCH 1/9] basic check --- clippy_utils/src/lib.rs | 5 +++++ tests/ui/f1.fixed | 14 ++++++++++++++ tests/ui/f1.rs | 14 ++++++++++++++ tests/ui/f1.stderr | 11 +++++++++++ 4 files changed, 44 insertions(+) create mode 100644 tests/ui/f1.fixed create mode 100644 tests/ui/f1.rs create mode 100644 tests/ui/f1.stderr diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8b9cd6a54dd6..8809033fd50b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1897,6 +1897,7 @@ 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 }` /// /// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead. fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { @@ -1929,6 +1930,10 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { .zip(arr) .all(|(pat, expr)| check_pat(cx, pat, expr)) }, + ( + PatKind::Struct(pat_ident, field_pats, false), + ExprKind::Struct(ident, fields, hir::StructTailExpr::None), + ) => field_pats.len() == fields.len(), _ => false, } } diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed new file mode 100644 index 000000000000..fb945633cc6b --- /dev/null +++ b/tests/ui/f1.fixed @@ -0,0 +1,14 @@ +#![allow(clippy::disallowed_names)] +#![warn(clippy::map_identity)] + +struct Foo { + foo: u8, + bar: u8, +} + +fn main() { + let x = [Foo { foo: 0, bar: 0 }]; + + let _ = x.into_iter(); + //~^ map_identity +} diff --git a/tests/ui/f1.rs b/tests/ui/f1.rs new file mode 100644 index 000000000000..9bc56db2ecea --- /dev/null +++ b/tests/ui/f1.rs @@ -0,0 +1,14 @@ +#![allow(clippy::disallowed_names)] +#![warn(clippy::map_identity)] + +struct Foo { + foo: u8, + bar: u8, +} + +fn main() { + let x = [Foo { foo: 0, bar: 0 }]; + + let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); + //~^ map_identity +} diff --git a/tests/ui/f1.stderr b/tests/ui/f1.stderr new file mode 100644 index 000000000000..57290f80b6fb --- /dev/null +++ b/tests/ui/f1.stderr @@ -0,0 +1,11 @@ +error: unnecessary map of the identity function + --> tests/ui/f1.rs:12:26 + | +LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` + | + = note: `-D clippy::map-identity` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` + +error: aborting due to 1 previous error + From 1621c01663107921dd8b6d4f2c4944c750b590f5 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 14:56:56 +0200 Subject: [PATCH 2/9] same fields different struct --- clippy_utils/src/lib.rs | 6 +++++- tests/ui/f1.fixed | 9 +++++++++ tests/ui/f1.rs | 9 +++++++++ tests/ui/f1.stderr | 2 +- 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8809033fd50b..c52def71fe50 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1933,7 +1933,11 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { ( PatKind::Struct(pat_ident, field_pats, false), ExprKind::Struct(ident, fields, hir::StructTailExpr::None), - ) => field_pats.len() == fields.len(), + ) => { + // NOTE: we're inside a (function) body, so this won't ICE + let qpath_res = |qpath, hir| cx.typeck_results().qpath_res(qpath, hir); + field_pats.len() == fields.len() && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id) + }, _ => false, } } diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed index fb945633cc6b..4c05bf944e2b 100644 --- a/tests/ui/f1.fixed +++ b/tests/ui/f1.fixed @@ -1,14 +1,23 @@ #![allow(clippy::disallowed_names)] #![warn(clippy::map_identity)] +#[derive(Clone, Copy)] struct Foo { foo: u8, bar: u8, } +struct Bar { + foo: u8, + bar: u8, +} + fn main() { let x = [Foo { foo: 0, bar: 0 }]; 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 }); } diff --git a/tests/ui/f1.rs b/tests/ui/f1.rs index 9bc56db2ecea..8765901ff9b5 100644 --- a/tests/ui/f1.rs +++ b/tests/ui/f1.rs @@ -1,14 +1,23 @@ #![allow(clippy::disallowed_names)] #![warn(clippy::map_identity)] +#[derive(Clone, Copy)] struct Foo { foo: u8, bar: u8, } +struct Bar { + foo: u8, + bar: u8, +} + fn main() { let x = [Foo { foo: 0, bar: 0 }]; let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); //~^ map_identity + + // don't lint: same fields but different structs + let _ = x.into_iter().map(|Foo { foo, bar }| Bar { foo, bar }); } diff --git a/tests/ui/f1.stderr b/tests/ui/f1.stderr index 57290f80b6fb..2ff8d1942c45 100644 --- a/tests/ui/f1.stderr +++ b/tests/ui/f1.stderr @@ -1,5 +1,5 @@ error: unnecessary map of the identity function - --> tests/ui/f1.rs:12:26 + --> tests/ui/f1.rs:18:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` From 8f11c8030eaac716e30efe59b075a966d3520614 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 15:18:39 +0200 Subject: [PATCH 3/9] reordered fields --- clippy_utils/src/lib.rs | 8 +++++++- tests/ui/f1.fixed | 12 ++++++++++++ tests/ui/f1.rs | 12 ++++++++++++ tests/ui/f1.stderr | 14 +++++++++++++- 4 files changed, 44 insertions(+), 2 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index c52def71fe50..5d49d6629687 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1936,7 +1936,13 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { ) => { // NOTE: we're inside a (function) body, so this won't ICE let qpath_res = |qpath, hir| cx.typeck_results().qpath_res(qpath, hir); - field_pats.len() == fields.len() && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id) + field_pats.len() == fields.len() + && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id) + && field_pats.iter().all(|field_pat| { + fields + .iter() + .any(|field| field_pat.ident == field.ident && check_pat(cx, field_pat.pat, field.expr)) + }) }, _ => false, } diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed index 4c05bf944e2b..8e59f350affc 100644 --- a/tests/ui/f1.fixed +++ b/tests/ui/f1.fixed @@ -20,4 +20,16 @@ fn main() { // 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 }); } diff --git a/tests/ui/f1.rs b/tests/ui/f1.rs index 8765901ff9b5..eb3c4271e49b 100644 --- a/tests/ui/f1.rs +++ b/tests/ui/f1.rs @@ -20,4 +20,16 @@ fn main() { // 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 }); } diff --git a/tests/ui/f1.stderr b/tests/ui/f1.stderr index 2ff8d1942c45..b23879152e50 100644 --- a/tests/ui/f1.stderr +++ b/tests/ui/f1.stderr @@ -7,5 +7,17 @@ LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); = note: `-D clippy::map-identity` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` -error: aborting due to 1 previous error +error: unnecessary map of the identity function + --> tests/ui/f1.rs:26: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/f1.rs:30:26 + | +LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` + +error: aborting due to 3 previous errors From af2dedf9ae5b740cb128b2aea627ceef0fb80038 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 15:36:08 +0200 Subject: [PATCH 4/9] different paths to the same struct --- tests/ui/f1.fixed | 15 +++++++++++---- tests/ui/f1.rs | 15 +++++++++++---- tests/ui/f1.stderr | 14 ++++++++++---- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed index 8e59f350affc..b5f67687e4a7 100644 --- a/tests/ui/f1.fixed +++ b/tests/ui/f1.fixed @@ -1,11 +1,14 @@ #![allow(clippy::disallowed_names)] #![warn(clippy::map_identity)] -#[derive(Clone, Copy)] -struct Foo { - foo: u8, - bar: u8, +mod foo { + #[derive(Clone, Copy)] + pub struct Foo { + pub foo: u8, + pub bar: u8, + } } +use foo::Foo; struct Bar { foo: u8, @@ -18,6 +21,10 @@ fn main() { 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 }); diff --git a/tests/ui/f1.rs b/tests/ui/f1.rs index eb3c4271e49b..d22575b508aa 100644 --- a/tests/ui/f1.rs +++ b/tests/ui/f1.rs @@ -1,11 +1,14 @@ #![allow(clippy::disallowed_names)] #![warn(clippy::map_identity)] -#[derive(Clone, Copy)] -struct Foo { - foo: u8, - bar: u8, +mod foo { + #[derive(Clone, Copy)] + pub struct Foo { + pub foo: u8, + pub bar: u8, + } } +use foo::Foo; struct Bar { foo: u8, @@ -18,6 +21,10 @@ fn main() { 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 }); diff --git a/tests/ui/f1.stderr b/tests/ui/f1.stderr index b23879152e50..32c0a23854a1 100644 --- a/tests/ui/f1.stderr +++ b/tests/ui/f1.stderr @@ -1,5 +1,5 @@ error: unnecessary map of the identity function - --> tests/ui/f1.rs:18:26 + --> tests/ui/f1.rs:21:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` @@ -8,16 +8,22 @@ LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` error: unnecessary map of the identity function - --> tests/ui/f1.rs:26:26 + --> tests/ui/f1.rs:25: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/f1.rs:33: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/f1.rs:30:26 + --> tests/ui/f1.rs:37:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors From 0e4b3a87933e32c61dd866651b3b607756870e2d Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 15:44:30 +0200 Subject: [PATCH 5/9] same for tuple structs --- clippy_utils/src/lib.rs | 16 ++++++++++++++-- tests/ui/f1.fixed | 26 ++++++++++++++++++++++++-- tests/ui/f1.rs | 26 ++++++++++++++++++++++++-- tests/ui/f1.stderr | 22 +++++++++++++++++----- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 5d49d6629687..a307d17b10a8 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1898,6 +1898,7 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { /// * `|(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 { @@ -1914,6 +1915,9 @@ 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() @@ -1930,12 +1934,20 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { .zip(arr) .all(|(pat, expr)| check_pat(cx, pat, expr)) }, + (PatKind::TupleStruct(pat_ident, field_pats, dotdot), ExprKind::Call(ident, fields)) + if dotdot.as_opt_usize().is_none() + && let ExprKind::Path(ident) = ident.kind => + { + field_pats.len() == fields.len() + && qpath_res(&pat_ident, pat.hir_id) == qpath_res(&ident, expr.hir_id) + && (field_pats.iter()) + .zip(fields) + .all(|(pat, expr)| check_pat(cx, pat, expr)) + }, ( PatKind::Struct(pat_ident, field_pats, false), ExprKind::Struct(ident, fields, hir::StructTailExpr::None), ) => { - // NOTE: we're inside a (function) body, so this won't ICE - let qpath_res = |qpath, hir| cx.typeck_results().qpath_res(qpath, hir); field_pats.len() == fields.len() && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id) && field_pats.iter().all(|field_pat| { diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed index b5f67687e4a7..28634c625c43 100644 --- a/tests/ui/f1.fixed +++ b/tests/ui/f1.fixed @@ -7,15 +7,20 @@ mod foo { pub foo: u8, pub bar: u8, } + + #[derive(Clone, Copy)] + pub struct Foo2(pub u8, pub u8); } -use foo::Foo; +use foo::{Foo, Foo2}; struct Bar { foo: u8, bar: u8, } -fn main() { +struct Bar2(u8, u8); + +fn structs() { let x = [Foo { foo: 0, bar: 0 }]; let _ = x.into_iter(); @@ -40,3 +45,20 @@ fn main() { // 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/f1.rs b/tests/ui/f1.rs index d22575b508aa..bf762974dbf5 100644 --- a/tests/ui/f1.rs +++ b/tests/ui/f1.rs @@ -7,15 +7,20 @@ mod foo { pub foo: u8, pub bar: u8, } + + #[derive(Clone, Copy)] + pub struct Foo2(pub u8, pub u8); } -use foo::Foo; +use foo::{Foo, Foo2}; struct Bar { foo: u8, bar: u8, } -fn main() { +struct Bar2(u8, u8); + +fn structs() { let x = [Foo { foo: 0, bar: 0 }]; let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); @@ -40,3 +45,20 @@ fn main() { // 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/f1.stderr b/tests/ui/f1.stderr index 32c0a23854a1..db5261d07d00 100644 --- a/tests/ui/f1.stderr +++ b/tests/ui/f1.stderr @@ -1,5 +1,5 @@ error: unnecessary map of the identity function - --> tests/ui/f1.rs:21:26 + --> tests/ui/f1.rs:26:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` @@ -8,22 +8,34 @@ LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` error: unnecessary map of the identity function - --> tests/ui/f1.rs:25:26 + --> tests/ui/f1.rs:30: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/f1.rs:33:26 + --> tests/ui/f1.rs:38: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/f1.rs:37:26 + --> tests/ui/f1.rs:42:26 | LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { bar, foo }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` -error: aborting due to 4 previous errors +error: unnecessary map of the identity function + --> tests/ui/f1.rs:52: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/f1.rs:56:26 + | +LL | let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` + +error: aborting due to 6 previous errors From 27bd316ec718beae0f319a2f36ab43a4c2cec75f Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 15:45:25 +0200 Subject: [PATCH 6/9] style: use `zip`-the-function all over the place makes the code a bit more concise by removing the need for explicit `.iter()` --- clippy_utils/src/lib.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index a307d17b10a8..d148be04bea0 100644 --- 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; @@ -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; } @@ -1925,14 +1925,12 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { (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, slice, after), ExprKind::Array(arr)) if slice.is_none() && before.len() + after.len() == arr.len() => { - (before.iter().chain(after)) - .zip(arr) - .all(|(pat, expr)| check_pat(cx, pat, expr)) + zip(before.iter().chain(after), arr).all(|(pat, expr)| check_pat(cx, pat, expr)) }, (PatKind::TupleStruct(pat_ident, field_pats, dotdot), ExprKind::Call(ident, fields)) if dotdot.as_opt_usize().is_none() @@ -1940,9 +1938,7 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { { field_pats.len() == fields.len() && qpath_res(&pat_ident, pat.hir_id) == qpath_res(&ident, expr.hir_id) - && (field_pats.iter()) - .zip(fields) - .all(|(pat, expr)| check_pat(cx, pat, expr)) + && zip(field_pats, fields).all(|(pat, expr)| check_pat(cx, pat, expr)) }, ( PatKind::Struct(pat_ident, field_pats, false), From 174d82c7d67db4a619618eb5e7078a84c49736a9 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 16:01:50 +0200 Subject: [PATCH 7/9] style: move precondition checking to the match guard the match arms above put the "sanity" checks in the guard, and call only `check_pat` in the body. With this commit, the (tuple) struct cases follow that convention as well. Well, almost -- I think the ident check belongs to the second category of checks, so I put it in the body as well --- clippy_utils/src/lib.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index d148be04bea0..bf6f6e77dc46 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1933,19 +1933,26 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { zip(before.iter().chain(after), arr).all(|(pat, expr)| check_pat(cx, pat, expr)) }, (PatKind::TupleStruct(pat_ident, field_pats, dotdot), ExprKind::Call(ident, fields)) - if dotdot.as_opt_usize().is_none() - && let ExprKind::Path(ident) = ident.kind => + if dotdot.as_opt_usize().is_none() && field_pats.len() == fields.len() => { - field_pats.len() == fields.len() - && qpath_res(&pat_ident, pat.hir_id) == qpath_res(&ident, expr.hir_id) + // 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), - ) => { - field_pats.len() == fields.len() - && qpath_res(&pat_ident, pat.hir_id) == qpath_res(ident, expr.hir_id) + ) 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() From e7c4121fd377d66b8f6cabd0501f652077a527d8 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 16:18:38 +0200 Subject: [PATCH 8/9] misc: use `None` in the pattern directly this'll probably be marginally faster thanks to the equality check being now structural --- clippy_utils/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index bf6f6e77dc46..49e7735cf2ed 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1927,9 +1927,7 @@ fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool { { zip(pats, tup).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::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::TupleStruct(pat_ident, field_pats, dotdot), ExprKind::Call(ident, fields)) From f73c961cd8acce7ee2687dd734ed91dd5d8579ea Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 12 Jul 2025 16:28:17 +0200 Subject: [PATCH 9/9] move the tests to the right file --- tests/ui/f1.fixed | 64 ------------------------------------ tests/ui/f1.rs | 64 ------------------------------------ tests/ui/f1.stderr | 41 ----------------------- tests/ui/map_identity.fixed | 64 +++++++++++++++++++++++++++++++++++- tests/ui/map_identity.rs | 64 +++++++++++++++++++++++++++++++++++- tests/ui/map_identity.stderr | 38 ++++++++++++++++++++- 6 files changed, 163 insertions(+), 172 deletions(-) delete mode 100644 tests/ui/f1.fixed delete mode 100644 tests/ui/f1.rs delete mode 100644 tests/ui/f1.stderr diff --git a/tests/ui/f1.fixed b/tests/ui/f1.fixed deleted file mode 100644 index 28634c625c43..000000000000 --- a/tests/ui/f1.fixed +++ /dev/null @@ -1,64 +0,0 @@ -#![allow(clippy::disallowed_names)] -#![warn(clippy::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/f1.rs b/tests/ui/f1.rs deleted file mode 100644 index bf762974dbf5..000000000000 --- a/tests/ui/f1.rs +++ /dev/null @@ -1,64 +0,0 @@ -#![allow(clippy::disallowed_names)] -#![warn(clippy::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/f1.stderr b/tests/ui/f1.stderr deleted file mode 100644 index db5261d07d00..000000000000 --- a/tests/ui/f1.stderr +++ /dev/null @@ -1,41 +0,0 @@ -error: unnecessary map of the identity function - --> tests/ui/f1.rs:26:26 - | -LL | let _ = x.into_iter().map(|Foo { foo, bar }| Foo { foo, bar }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` - | - = note: `-D clippy::map-identity` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::map_identity)]` - -error: unnecessary map of the identity function - --> tests/ui/f1.rs:30: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/f1.rs:38: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/f1.rs:42: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/f1.rs:52: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/f1.rs:56:26 - | -LL | let _ = x.into_iter().map(|Foo2(foo, bar)| foo::Foo2(foo, bar)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map` - -error: aborting due to 6 previous errors - diff --git a/tests/ui/map_identity.fixed b/tests/ui/map_identity.fixed index b82d3e6d9567..6c971ba63384 100644 --- 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 index c295bf872701..59dcfcda3b6e 100644 --- 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 index 9b624a0dc755..a50c0d6b87b5 100644 --- 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