Skip to content

[manual_flatten]: Fix with nested Some or Ok pattern #14846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions clippy_lints/src/loops/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub(super) fn check<'tcx>(
&& let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind
&& path_to_local_id(let_expr, pat_hir_id)
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
&& let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind
&& let PatKind::TupleStruct(ref qpath, pats, _) = let_pat.kind
&& let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id)
&& let Some(variant_id) = cx.tcx.opt_parent(ctor_id)
&& let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id)
Expand All @@ -39,9 +39,13 @@ pub(super) fn check<'tcx>(
&& msrv.meets(cx, msrvs::ITER_FLATTEN)
{
let if_let_type = if some_ctor { "Some" } else { "Ok" };
let has_nested_tuple_struct = contains_nested_tuple_struct(pats);
// Prepare the error message
let msg =
format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used");
let msg = if has_nested_tuple_struct {
"manual flattening detected".to_string()
} else {
format!("unnecessary `if let` since only the `{if_let_type}` variant of the iterator element is used")
};

// Prepare the help message
let mut applicability = Applicability::MaybeIncorrect;
Expand All @@ -60,9 +64,15 @@ pub(super) fn check<'tcx>(
// case, it will be shown in the extra `help` message at the end, which is why the first
// `help_msg` needs to refer to the correct relative position of the suggestion.
let help_msg = if sugg.contains('\n') {
"remove the `if let` statement in the for loop and then..."
if has_nested_tuple_struct {
format!("remove the outer `{if_let_type}` variant in the for loop and then...")
} else {
"remove the `if let` statement in the for loop and then...".to_string()
}
} else if has_nested_tuple_struct {
format!("...and remove the outer `{if_let_type}` variant in the for loop")
} else {
"...and remove the `if let` statement in the for loop"
"...and remove the `if let` statement in the for loop".to_string()
};

span_lint_and_then(cx, MANUAL_FLATTEN, span, msg, |diag| {
Expand All @@ -71,3 +81,10 @@ pub(super) fn check<'tcx>(
});
}
}

fn contains_nested_tuple_struct(pats: &[Pat<'_>]) -> bool {
pats.iter().any(|pat| {
matches!(pat.kind, PatKind::TupleStruct(..))
|| matches!(pat.kind, PatKind::Tuple(pats, _) if contains_nested_tuple_struct(pats))
})
}
10 changes: 10 additions & 0 deletions tests/ui/manual_flatten.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,16 @@ fn main() {
println!("{}", n);
}

// Test for loop with nested Some pattern with `if let` expression
let z = vec![Some((0, Some(0)))];
for n in z {
//~^ manual_flatten

if let Some((_, Some(n))) = n {
println!("{}", n);
}
}

run_unformatted_tests();
}

Expand Down
28 changes: 25 additions & 3 deletions tests/ui/manual_flatten.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,30 @@ LL | | println!("{:?}", n);
LL | | }
| |_________^

error: manual flattening detected
--> tests/ui/manual_flatten.rs:128:5
|
LL | for n in z {
| ^ - help: try: `z.into_iter().flatten()`
| _____|
| |
LL | |
LL | |
LL | | if let Some((_, Some(n))) = n {
... |
LL | | }
| |_____^
|
help: ...and remove the outer `Some` variant in the for loop
--> tests/ui/manual_flatten.rs:131:9
|
LL | / if let Some((_, Some(n))) = n {
LL | | println!("{}", n);
LL | | }
| |_________^

error: unnecessary `if let` since only the `Some` variant of the iterator element is used
--> tests/ui/manual_flatten.rs:132:5
--> tests/ui/manual_flatten.rs:142:5
|
LL | / for n in vec![
LL | |
Expand All @@ -189,7 +211,7 @@ LL | | }
| |_____^
|
help: remove the `if let` statement in the for loop and then...
--> tests/ui/manual_flatten.rs:139:9
--> tests/ui/manual_flatten.rs:149:9
|
LL | / if let Some(n) = n {
LL | | println!("{:?}", n);
Expand All @@ -203,5 +225,5 @@ LL | Some(3)
LL ~ ].iter().flatten() {
|

error: aborting due to 9 previous errors
error: aborting due to 10 previous errors