{flat_,}map_identity: recognize (tuple) struct de- and restructuring (#15261)
Follow-up of rust-lang/rust-clippy#15229, as described in https://github.com/rust-lang/rust-clippy/pull/15229#issuecomment-3050279790 -- 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
This commit is contained in:
Timo
GitHub
commit
ffcd12946e
4 changed files with 200 additions and 13 deletions
|
|
@ -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, 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)| is_expr_identity_of_pat(cx, pat, expr, by_hir))
|
||||
},
|
||||
(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,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue