Skip to content

Commit e07cd5b

Browse files
nahuakangnahuakang
committed
Enhance manual flatten
1 parent b87e189 commit e07cd5b

File tree

3 files changed

+101
-68
lines changed

3 files changed

+101
-68
lines changed

‎clippy_lints/src/loops.rs‎

Lines changed: 71 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,52 +1996,82 @@ fn check_manual_flatten<'tcx>(
19961996
body: &'tcx Expr<'_>,
19971997
span: Span,
19981998
) {
1999-
if_chain! {
2000-
// Ensure the `if let` statement is the only expression in the for-loop
2001-
if let ExprKind::Block(ref block, _) = body.kind;
2002-
if block.stmts.is_empty();
2003-
if let Some(inner_expr) = block.expr;
2004-
if let ExprKind::Match(
2005-
ref match_expr, ref match_arms, MatchSource::IfLetDesugar{ contains_else_clause: false }
2006-
) = inner_expr.kind;
2007-
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
2008-
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
2009-
if let ExprKind::Path(QPath::Resolved(None, match_expr_path)) = match_expr.kind;
2010-
if let Res::Local(match_expr_path_id) = match_expr_path.res;
2011-
if pat_hir_id == match_expr_path_id;
2012-
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
2013-
if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
2014-
if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);
2015-
let if_let_type = if is_some_ctor(cx, path.res) {
2016-
"Some"
1999+
if let ExprKind::Block(ref block, _) = body.kind {
2000+
// Ensure the `if let` statement is the only expression or statement in the for-loop
2001+
let inner_expr = if block.stmts.len() == 1 && block.expr.is_none() {
2002+
let match_stmt = &block.stmts[0];
2003+
if let StmtKind::Semi(inner_expr) = match_stmt.kind {
2004+
Some(inner_expr)
2005+
} else {
2006+
None
2007+
}
2008+
} else if block.stmts.is_empty() {
2009+
block.expr
20172010
} else {
2018-
"Ok"
2011+
None
20192012
};
2020-
// Determine if `arg` is `Iterator` or implicitly calls `into_iter`
2021-
let arg_ty = cx.typeck_results().expr_ty(arg);
2022-
if let Some(id) = get_trait_def_id(cx, &paths::ITERATOR);
2023-
if let is_iterator = implements_trait(cx, arg_ty, id, &[]);
20242013

2025-
then {
2026-
// Prepare the error message
2027-
let msg = format!("Unnecessary `if let` since only the `{}` variant of the iterator element is used.", if_let_type);
2014+
if_chain! {
2015+
if let Some(inner_expr) = inner_expr;
2016+
if let ExprKind::Match(
2017+
ref match_expr, ref match_arms, MatchSource::IfLetDesugar{ contains_else_clause: false }
2018+
) = inner_expr.kind;
2019+
// Ensure match_expr in `if let` statement is the same as the pat from the for-loop
2020+
if let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind;
2021+
if let ExprKind::Path(QPath::Resolved(None, match_expr_path)) = match_expr.kind;
2022+
if let Res::Local(match_expr_path_id) = match_expr_path.res;
2023+
if pat_hir_id == match_expr_path_id;
2024+
// Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result`
2025+
if let PatKind::TupleStruct(QPath::Resolved(None, path), _, _) = match_arms[0].pat.kind;
2026+
let some_ctor = is_some_ctor(cx, path.res);
2027+
let ok_ctor = is_ok_ctor(cx, path.res);
2028+
if some_ctor || ok_ctor;
2029+
let if_let_type = if some_ctor { "Some" } else { "Ok" };
20282030

2029-
// Prepare the help message
2030-
let arg_snippet = snippet(cx, arg.span, "..");
2031-
let hint = if is_iterator {
2032-
format!("try: `{}.flatten()`", arg_snippet)
2033-
} else {
2034-
format!("try: `{}.into_iter().flatten()`", arg_snippet)
2035-
};
2031+
then {
2032+
// Prepare the error message
2033+
let msg = format!("unnecessary `if let` since only the `{}` variant of the iterator element is used", if_let_type);
20362034

2037-
span_lint_and_help(
2038-
cx,
2039-
MANUAL_FLATTEN,
2040-
span,
2041-
&msg,
2042-
Some(arg.span),
2043-
&hint,
2044-
);
2035+
// Prepare the help message
2036+
let mut applicability = Applicability::MaybeIncorrect;
2037+
let arg_snippet = snippet_with_applicability(
2038+
cx,
2039+
arg.span,
2040+
"..",
2041+
&mut applicability,
2042+
);
2043+
// Determine if `arg` is by reference, an `Iterator`, or implicitly adjusted with `into_iter`
2044+
let hint = match arg.kind {
2045+
ExprKind::AddrOf(_, _, arg_expr) => {
2046+
format!("{}.iter().flatten()", snippet(cx, arg_expr.span, ".."))
2047+
},
2048+
ExprKind::MethodCall(_, _, _, _) | ExprKind::Path(QPath::Resolved(None, _)) => {
2049+
// Determine if `arg` is `Iterator` or implicitly calls `into_iter`
2050+
let arg_ty = cx.typeck_results().expr_ty(arg);
2051+
if let Some(id) = get_trait_def_id(cx, &paths::ITERATOR) {
2052+
let is_iterator = implements_trait(cx, arg_ty, id, &[]);
2053+
if is_iterator {
2054+
format!("{}.flatten()", arg_snippet)
2055+
} else {
2056+
format!("{}.into_iter().flatten()", arg_snippet)
2057+
}
2058+
} else {
2059+
return
2060+
}
2061+
},
2062+
_ => return,
2063+
};
2064+
2065+
span_lint_and_sugg(
2066+
cx,
2067+
MANUAL_FLATTEN,
2068+
span,
2069+
&msg,
2070+
"try",
2071+
hint,
2072+
applicability,
2073+
)
2074+
}
20452075
}
20462076
}
20472077
}

‎tests/ui/manual_flatten.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,30 @@
11
#![warn(clippy::manual_flatten)]
22

33
fn main() {
4+
// Test for loop over implicitly adjusted `Iterator` with `if let` expression
45
let x = vec![Some(1), Some(2), Some(3)];
56
for n in x {
67
if let Some(n) = n {
78
println!("{}", n);
89
}
910
}
1011

12+
// Test for loop over implicitly implicitly adjusted `Iterator` with `if let` statement
1113
let y: Vec<Result<i32, i32>> = vec![];
1214
for n in y.clone() {
15+
if let Ok(n) = n {
16+
println!("{}", n);
17+
};
18+
}
19+
20+
// Test for loop over by reference
21+
for n in &y {
1322
if let Ok(n) = n {
1423
println!("{}", n);
1524
}
1625
}
1726

27+
// Test for loop over `Iterator` with `if let` expression
1828
let z = vec![Some(1), Some(2), Some(3)];
1929
let z = z.iter();
2030
for n in z {

‎tests/ui/manual_flatten.stderr‎

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,44 @@
1-
error: Unnecessary `if let` since only the `Some` variant of the iterator element is used.
2-
--> $DIR/manual_flatten.rs:5:5
1+
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
2+
--> $DIR/manual_flatten.rs:6:5
33
|
44
LL | / for n in x {
55
LL | | if let Some(n) = n {
66
LL | | println!("{}", n);
77
LL | | }
88
LL | | }
9-
| |_____^
9+
| |_____^ help: try: `x.into_iter().flatten()`
1010
|
1111
= note: `-D clippy::manual-flatten` implied by `-D warnings`
12-
help: try: `x.into_iter().flatten()`
13-
--> $DIR/manual_flatten.rs:5:14
14-
|
15-
LL | for n in x {
16-
| ^
1712

18-
error: Unnecessary `if let` since only the `Ok` variant of the iterator element is used.
19-
--> $DIR/manual_flatten.rs:12:5
13+
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
14+
--> $DIR/manual_flatten.rs:14:5
2015
|
2116
LL | / for n in y.clone() {
2217
LL | | if let Ok(n) = n {
2318
LL | | println!("{}", n);
24-
LL | | }
19+
LL | | };
2520
LL | | }
26-
| |_____^
27-
|
28-
help: try: `y.clone().into_iter().flatten()`
29-
--> $DIR/manual_flatten.rs:12:14
21+
| |_____^ help: try: `y.clone().into_iter().flatten()`
22+
23+
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
24+
--> $DIR/manual_flatten.rs:21:5
3025
|
31-
LL | for n in y.clone() {
32-
| ^^^^^^^^^
26+
LL | / for n in &y {
27+
LL | | if let Ok(n) = n {
28+
LL | | println!("{}", n);
29+
LL | | }
30+
LL | | }
31+
| |_____^ help: try: `y.iter().flatten()`
3332

34-
error: Unnecessary `if let` since only the `Some` variant of the iterator element is used.
35-
--> $DIR/manual_flatten.rs:20:5
33+
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
34+
--> $DIR/manual_flatten.rs:30:5
3635
|
3736
LL | / for n in z {
3837
LL | | if let Some(n) = n {
3938
LL | | println!("{}", n);
4039
LL | | }
4140
LL | | }
42-
| |_____^
43-
|
44-
help: try: `z.flatten()`
45-
--> $DIR/manual_flatten.rs:20:14
46-
|
47-
LL | for n in z {
48-
| ^
41+
| |_____^ help: try: `z.flatten()`
4942

50-
error: aborting due to 3 previous errors
43+
error: aborting due to 4 previous errors
5144

0 commit comments

Comments
 (0)