Skip to content

Commit d487579

Browse files
borsbors
committed
Auto merge of #11792 - y21:issue11764, r=llogiq
[`map_identity`]: respect match ergonomics Fixes #11764 Note: the original tests before this were slightly wrong themselves already and had to be changed. They were calling `map` on an iterator of `&(i32, i32)`s, so this PR would stop linting there, but they were meant to test something else unrelated to binding modes, so I just changed them to remove the references so that it iterates over owned values and they all bind by value. This way they continue to test what they checked for before: the identity function for tuple patterns. changelog: [`map_identity`]: respect match ergonomics
2 parents 4a0c36d + b2cf8f7 commit d487579

File tree

4 files changed

+85
-43
lines changed

4 files changed

+85
-43
lines changed

‎clippy_utils/src/lib.rs

+12
Original file line numberDiff line numberDiff line change
@@ -2023,6 +2023,18 @@ pub fn is_must_use_func_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
20232023
/// Consider calling [`is_expr_untyped_identity_function`] or [`is_expr_identity_function`] instead.
20242024
fn is_body_identity_function(cx: &LateContext<'_>, func: &Body<'_>) -> bool {
20252025
fn check_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
2026+
if cx
2027+
.typeck_results()
2028+
.pat_binding_modes()
2029+
.get(pat.hir_id)
2030+
.is_some_and(|mode| matches!(mode, BindingMode::BindByReference(_)))
2031+
{
2032+
// If a tuple `(x, y)` is of type `&(i32, i32)`, then due to match ergonomics,
2033+
// the inner patterns become references. Don't consider this the identity function
2034+
// as that changes types.
2035+
return false;
2036+
}
2037+
20262038
match (pat.kind, expr.kind) {
20272039
(PatKind::Binding(_, id, _, _), _) => {
20282040
path_to_local_id(expr, id) && cx.typeck_results().expr_adjustments(expr).is_empty()

‎tests/ui/map_identity.fixed

+27-15
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,40 @@ fn main() {
2424

2525
fn issue7189() {
2626
// should lint
27-
let x = [(1, 2), (3, 4)];
28-
let _ = x.iter();
29-
let _ = x.iter();
30-
let _ = x.iter();
27+
let x = [(1, 2), (3, 4)].iter().copied();
28+
let _ = x.clone();
29+
let _ = x.clone();
30+
let _ = x.clone();
3131

32-
let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))];
33-
let _ = y.iter();
32+
let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))].iter().copied();
33+
let _ = y.clone();
3434

3535
// should not lint
36-
let _ = x.iter().map(|(x, y)| (x, y, y));
37-
let _ = x.iter().map(|(x, _y)| (x,));
38-
let _ = x.iter().map(|(x, _)| (x,));
39-
let _ = x.iter().map(|(x, ..)| (x,));
40-
let _ = y.iter().map(|(x, y, (z, _))| (x, y, (z, z)));
36+
let _ = x.clone().map(|(x, y)| (x, y, y));
37+
let _ = x.clone().map(|(x, _y)| (x,));
38+
let _ = x.clone().map(|(x, _)| (x,));
39+
let _ = x.clone().map(|(x, ..)| (x,));
40+
let _ = y.clone().map(|(x, y, (z, _))| (x, y, (z, z)));
4141
let _ = y
42-
.iter()
43-
.map(|(x, y, (z, _)): &(i32, i32, (i32, (i32,)))| (x, y, (z, z)));
42+
.clone()
43+
.map(|(x, y, (z, _)): (i32, i32, (i32, (i32,)))| (x, y, (z, z)));
4444
let _ = y
45-
.iter()
46-
.map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
45+
.clone()
46+
.map(|(x, y, (z, (w,))): (i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
4747
}
4848

4949
fn not_identity(x: &u16) -> u16 {
5050
*x
5151
}
52+
53+
fn issue11764() {
54+
let x = [(1, 2), (3, 4)];
55+
// don't lint: this is an `Iterator<Item = &(i32, i32)>`
56+
// match ergonomics makes the binding patterns into references
57+
// so that its type changes to `Iterator<Item = (&i32, &i32)>`
58+
let _ = x.iter().map(|(x, y)| (x, y));
59+
let _ = x.iter().map(|x| (x.0,)).map(|(x,)| x);
60+
61+
// no match ergonomics for `(i32, i32)`
62+
let _ = x.iter().copied();
63+
}

‎tests/ui/map_identity.rs

+27-15
Original file line numberDiff line numberDiff line change
@@ -26,30 +26,42 @@ fn main() {
2626

2727
fn issue7189() {
2828
// should lint
29-
let x = [(1, 2), (3, 4)];
30-
let _ = x.iter().map(|(x, y)| (x, y));
31-
let _ = x.iter().map(|(x, y)| {
29+
let x = [(1, 2), (3, 4)].iter().copied();
30+
let _ = x.clone().map(|(x, y)| (x, y));
31+
let _ = x.clone().map(|(x, y)| {
3232
return (x, y);
3333
});
34-
let _ = x.iter().map(|(x, y)| return (x, y));
34+
let _ = x.clone().map(|(x, y)| return (x, y));
3535

36-
let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))];
37-
let _ = y.iter().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
36+
let y = [(1, 2, (3, (4,))), (5, 6, (7, (8,)))].iter().copied();
37+
let _ = y.clone().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
3838

3939
// should not lint
40-
let _ = x.iter().map(|(x, y)| (x, y, y));
41-
let _ = x.iter().map(|(x, _y)| (x,));
42-
let _ = x.iter().map(|(x, _)| (x,));
43-
let _ = x.iter().map(|(x, ..)| (x,));
44-
let _ = y.iter().map(|(x, y, (z, _))| (x, y, (z, z)));
40+
let _ = x.clone().map(|(x, y)| (x, y, y));
41+
let _ = x.clone().map(|(x, _y)| (x,));
42+
let _ = x.clone().map(|(x, _)| (x,));
43+
let _ = x.clone().map(|(x, ..)| (x,));
44+
let _ = y.clone().map(|(x, y, (z, _))| (x, y, (z, z)));
4545
let _ = y
46-
.iter()
47-
.map(|(x, y, (z, _)): &(i32, i32, (i32, (i32,)))| (x, y, (z, z)));
46+
.clone()
47+
.map(|(x, y, (z, _)): (i32, i32, (i32, (i32,)))| (x, y, (z, z)));
4848
let _ = y
49-
.iter()
50-
.map(|(x, y, (z, (w,))): &(i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
49+
.clone()
50+
.map(|(x, y, (z, (w,))): (i32, i32, (i32, (i32,)))| (x, y, (z, (w,))));
5151
}
5252

5353
fn not_identity(x: &u16) -> u16 {
5454
*x
5555
}
56+
57+
fn issue11764() {
58+
let x = [(1, 2), (3, 4)];
59+
// don't lint: this is an `Iterator<Item = &(i32, i32)>`
60+
// match ergonomics makes the binding patterns into references
61+
// so that its type changes to `Iterator<Item = (&i32, &i32)>`
62+
let _ = x.iter().map(|(x, y)| (x, y));
63+
let _ = x.iter().map(|x| (x.0,)).map(|(x,)| x);
64+
65+
// no match ergonomics for `(i32, i32)`
66+
let _ = x.iter().copied().map(|(x, y)| (x, y));
67+
}

‎tests/ui/map_identity.stderr

+19-13
Original file line numberDiff line numberDiff line change
@@ -41,31 +41,37 @@ LL | let _: Result<u32, u32> = Ok(1).map_err(|a| a);
4141
| ^^^^^^^^^^^^^^^ help: remove the call to `map_err`
4242

4343
error: unnecessary map of the identity function
44-
--> $DIR/map_identity.rs:30:21
44+
--> $DIR/map_identity.rs:30:22
4545
|
46-
LL | let _ = x.iter().map(|(x, y)| (x, y));
47-
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
46+
LL | let _ = x.clone().map(|(x, y)| (x, y));
47+
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
4848

4949
error: unnecessary map of the identity function
50-
--> $DIR/map_identity.rs:31:21
50+
--> $DIR/map_identity.rs:31:22
5151
|
52-
LL | let _ = x.iter().map(|(x, y)| {
53-
| _____________________^
52+
LL | let _ = x.clone().map(|(x, y)| {
53+
| ______________________^
5454
LL | | return (x, y);
5555
LL | | });
5656
| |______^ help: remove the call to `map`
5757

5858
error: unnecessary map of the identity function
59-
--> $DIR/map_identity.rs:34:21
59+
--> $DIR/map_identity.rs:34:22
6060
|
61-
LL | let _ = x.iter().map(|(x, y)| return (x, y));
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
61+
LL | let _ = x.clone().map(|(x, y)| return (x, y));
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
6363

6464
error: unnecessary map of the identity function
65-
--> $DIR/map_identity.rs:37:21
65+
--> $DIR/map_identity.rs:37:22
6666
|
67-
LL | let _ = y.iter().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
68-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
67+
LL | let _ = y.clone().map(|(x, y, (z, (w,)))| (x, y, (z, (w,))));
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
6969

70-
error: aborting due to 10 previous errors
70+
error: unnecessary map of the identity function
71+
--> $DIR/map_identity.rs:66:30
72+
|
73+
LL | let _ = x.iter().copied().map(|(x, y)| (x, y));
74+
| ^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `map`
75+
76+
error: aborting due to 11 previous errors
7177

0 commit comments

Comments
 (0)