Skip to content

Commit 8a00f41

Browse files
committed
changelog: Fix [manual_flatten] with nested Some or Ok pattern
1 parent da7b678 commit 8a00f41

File tree

3 files changed

+66
-8
lines changed

3 files changed

+66
-8
lines changed

clippy_lints/src/loops/manual_flatten.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ pub(super) fn check<'tcx>(
2828
&& let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind
2929
&& path_to_local_id(let_expr, pat_hir_id)
3030
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
31-
&& let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind
31+
&& let PatKind::TupleStruct(ref qpath, pats, _) = let_pat.kind
3232
&& let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id)
3333
&& let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
3434
&& let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id)
@@ -39,9 +39,13 @@ pub(super) fn check<'tcx>(
3939
&& msrv.meets(cx, msrvs::ITER_FLATTEN)
4040
{
4141
let if_let_type = if some_ctor { "Some" } else { "Ok" };
42+
let has_nested_tuple_struct = contains_nested_tuple_struct(pats);
4243
// Prepare the error message
43-
let msg =
44-
format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used");
44+
let msg = if has_nested_tuple_struct {
45+
"manual flattening detected".to_string()
46+
} else {
47+
format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used")
48+
};
4549

4650
// Prepare the help message
4751
let mut applicability = Applicability::MaybeIncorrect;
@@ -60,9 +64,15 @@ pub(super) fn check<'tcx>(
6064
// case, it will be shown in the extra `help` message at the end, which is why the first
6165
// `help_msg` needs to refer to the correct relative position of the suggestion.
6266
let help_msg = if sugg.contains('\n') {
63-
"remove the `if let` statement in the for loop and then..."
67+
if has_nested_tuple_struct {
68+
format!("remove the outer `{if_let_type}` variant in the for loop and then...")
69+
} else {
70+
"remove the `if let` statement in the for loop and then...".to_string()
71+
}
72+
} else if has_nested_tuple_struct {
73+
format!("...and remove the outer `{if_let_type}` variant in the for loop")
6474
} else {
65-
"...and remove the `if let` statement in the for loop"
75+
"...and remove the `if let` statement in the for loop".to_string()
6676
};
6777

6878
span_lint_and_then(cx, MANUAL_FLATTEN, span, msg, |diag| {
@@ -71,3 +81,19 @@ pub(super) fn check<'tcx>(
7181
});
7282
}
7383
}
84+
85+
fn contains_nested_tuple_struct(pats: &[Pat<'_>]) -> bool {
86+
for pat in pats {
87+
match pat.kind {
88+
PatKind::TupleStruct(_, _, _) => {
89+
return true;
90+
},
91+
PatKind::Tuple(pats, _) => {
92+
return contains_nested_tuple_struct(pats);
93+
},
94+
_ => {},
95+
}
96+
}
97+
98+
false
99+
}

tests/ui/manual_flatten.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,16 @@ fn main() {
123123
println!("{}", n);
124124
}
125125

126+
// Test for loop with nested Some pattern with `if let` expression
127+
let z = vec![Some((0, Some(0)))];
128+
for n in z {
129+
//~^ manual_flatten
130+
131+
if let Some((_, Some(n))) = n {
132+
println!("{}", n);
133+
}
134+
}
135+
126136
run_unformatted_tests();
127137
}
128138

tests/ui/manual_flatten.stderr

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,30 @@ LL | | println!("{:?}", n);
177177
LL | | }
178178
| |_________^
179179

180+
error: manual flattening detected
181+
--> tests/ui/manual_flatten.rs:128:5
182+
|
183+
LL | for n in z {
184+
| ^ - help: try: `z.into_iter().flatten()`
185+
| _____|
186+
| |
187+
LL | |
188+
LL | |
189+
LL | | if let Some((_, Some(n))) = n {
190+
... |
191+
LL | | }
192+
| |_____^
193+
|
194+
help: ...and remove the outer `Some` variant in the for loop
195+
--> tests/ui/manual_flatten.rs:131:9
196+
|
197+
LL | / if let Some((_, Some(n))) = n {
198+
LL | | println!("{}", n);
199+
LL | | }
200+
| |_________^
201+
180202
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
181-
--> tests/ui/manual_flatten.rs:132:5
203+
--> tests/ui/manual_flatten.rs:142:5
182204
|
183205
LL | / for n in vec![
184206
LL | |
@@ -189,7 +211,7 @@ LL | | }
189211
| |_____^
190212
|
191213
help: remove the `if let` statement in the for loop and then...
192-
--> tests/ui/manual_flatten.rs:139:9
214+
--> tests/ui/manual_flatten.rs:149:9
193215
|
194216
LL | / if let Some(n) = n {
195217
LL | | println!("{:?}", n);
@@ -203,5 +225,5 @@ LL | Some(3)
203225
LL ~ ].iter().flatten() {
204226
|
205227

206-
error: aborting due to 9 previous errors
228+
error: aborting due to 10 previous errors
207229

0 commit comments

Comments
 (0)