Skip to content

Commit 78ef0f2

Browse files
nahuakangnahuakang
committed
Add additional test cases and improve span lint
1 parent 0f5e71f commit 78ef0f2

File tree

3 files changed

+108
-49
lines changed

3 files changed

+108
-49
lines changed

‎clippy_lints/src/loops.rs

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2034,42 +2034,27 @@ fn check_manual_flatten<'tcx>(
20342034

20352035
// Prepare the help message
20362036
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 arg_ty = cx.typeck_results().expr_ty(arg);
2045-
let hint = if arg_ty.is_ref() {
2046-
if has_iter_method(cx, arg_ty).is_none() {
2047-
return;
2048-
} else if let ExprKind::AddrOf(_, _, arg_expr) = arg.kind {
2049-
format!("{}.iter().flatten()", snippet(cx, arg_expr.span, ".."))
2050-
} else {
2051-
return;
2052-
}
2053-
} else if let Some(id) = get_trait_def_id(cx, &paths::ITERATOR) {
2054-
let is_iterator = implements_trait(cx, arg_ty, id, &[]);
2055-
if is_iterator {
2056-
format!("{}.flatten()", arg_snippet)
2057-
} else {
2058-
format!("{}.into_iter().flatten()", arg_snippet)
2059-
}
2060-
} else {
2061-
return
2062-
};
2037+
let arg_snippet = make_iterator_snippet(cx, arg, &mut applicability);
20632038

2064-
span_lint_and_sugg(
2039+
span_lint_and_then(
20652040
cx,
20662041
MANUAL_FLATTEN,
20672042
span,
20682043
&msg,
2069-
"try",
2070-
hint,
2071-
applicability,
2072-
)
2044+
|diag| {
2045+
let sugg = format!("{}.flatten()", arg_snippet);
2046+
diag.span_suggestion(
2047+
arg.span,
2048+
"try",
2049+
sugg,
2050+
Applicability::MaybeIncorrect,
2051+
);
2052+
diag.span_help(
2053+
inner_expr.span,
2054+
"also remove the `if let` statement in the for loop",
2055+
);
2056+
}
2057+
);
20732058
}
20742059
}
20752060
}

‎tests/ui/manual_flatten.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ fn main() {
44
// Test for loop over implicitly adjusted `Iterator` with `if let` expression
55
let x = vec![Some(1), Some(2), Some(3)];
66
for n in x {
7-
if let Some(n) = n {
8-
println!("{}", n);
7+
if let Some(y) = n {
8+
println!("{}", y);
99
}
1010
}
1111

@@ -24,12 +24,22 @@ fn main() {
2424
}
2525
}
2626

27+
// Test for loop over an implicit reference
28+
// Note: If `clippy::manual_flatten` is made autofixable, this case will
29+
// lead to a follow-up lint `clippy::into_iter_on_ref`
30+
let z = &y;
31+
for n in z {
32+
if let Ok(n) = n {
33+
println!("{}", n);
34+
}
35+
}
36+
2737
// Test for loop over `Iterator` with `if let` expression
2838
let z = vec![Some(1), Some(2), Some(3)];
2939
let z = z.iter();
3040
for n in z {
31-
if let Some(n) = n {
32-
println!("{}", n);
41+
if let Some(m) = n {
42+
println!("{}", m);
3343
}
3444
}
3545

‎tests/ui/manual_flatten.stderr

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,108 @@
11
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
22
--> $DIR/manual_flatten.rs:6:5
33
|
4-
LL | / for n in x {
5-
LL | | if let Some(n) = n {
6-
LL | | println!("{}", n);
4+
LL | for n in x {
5+
| ^ - help: try: `x.into_iter().flatten()`
6+
| _____|
7+
| |
8+
LL | | if let Some(y) = n {
9+
LL | | println!("{}", y);
710
LL | | }
811
LL | | }
9-
| |_____^ help: try: `x.into_iter().flatten()`
12+
| |_____^
1013
|
1114
= note: `-D clippy::manual-flatten` implied by `-D warnings`
15+
help: also remove the `if let` statement in the for loop
16+
--> $DIR/manual_flatten.rs:7:9
17+
|
18+
LL | / if let Some(y) = n {
19+
LL | | println!("{}", y);
20+
LL | | }
21+
| |_________^
1222

1323
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
1424
--> $DIR/manual_flatten.rs:14:5
1525
|
16-
LL | / for n in y.clone() {
26+
LL | for n in y.clone() {
27+
| ^ --------- help: try: `y.clone().into_iter().flatten()`
28+
| _____|
29+
| |
1730
LL | | if let Ok(n) = n {
1831
LL | | println!("{}", n);
1932
LL | | };
2033
LL | | }
21-
| |_____^ help: try: `y.clone().into_iter().flatten()`
34+
| |_____^
35+
|
36+
help: also remove the `if let` statement in the for loop
37+
--> $DIR/manual_flatten.rs:15:9
38+
|
39+
LL | / if let Ok(n) = n {
40+
LL | | println!("{}", n);
41+
LL | | };
42+
| |_________^
2243

2344
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
2445
--> $DIR/manual_flatten.rs:21:5
2546
|
26-
LL | / for n in &y {
47+
LL | for n in &y {
48+
| ^ -- help: try: `y.iter().flatten()`
49+
| _____|
50+
| |
2751
LL | | if let Ok(n) = n {
2852
LL | | println!("{}", n);
2953
LL | | }
3054
LL | | }
31-
| |_____^ help: try: `y.iter().flatten()`
55+
| |_____^
56+
|
57+
help: also remove the `if let` statement in the for loop
58+
--> $DIR/manual_flatten.rs:22:9
59+
|
60+
LL | / if let Ok(n) = n {
61+
LL | | println!("{}", n);
62+
LL | | }
63+
| |_________^
3264

33-
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
34-
--> $DIR/manual_flatten.rs:30:5
65+
error: unnecessary `if let` since only the `Ok` variant of the iterator element is used
66+
--> $DIR/manual_flatten.rs:31:5
3567
|
36-
LL | / for n in z {
37-
LL | | if let Some(n) = n {
68+
LL | for n in z {
69+
| ^ - help: try: `z.into_iter().flatten()`
70+
| _____|
71+
| |
72+
LL | | if let Ok(n) = n {
3873
LL | | println!("{}", n);
3974
LL | | }
4075
LL | | }
41-
| |_____^ help: try: `z.flatten()`
76+
| |_____^
77+
|
78+
help: also remove the `if let` statement in the for loop
79+
--> $DIR/manual_flatten.rs:32:9
80+
|
81+
LL | / if let Ok(n) = n {
82+
LL | | println!("{}", n);
83+
LL | | }
84+
| |_________^
85+
86+
error: unnecessary `if let` since only the `Some` variant of the iterator element is used
87+
--> $DIR/manual_flatten.rs:40:5
88+
|
89+
LL | for n in z {
90+
| ^ - help: try: `z.flatten()`
91+
| _____|
92+
| |
93+
LL | | if let Some(m) = n {
94+
LL | | println!("{}", m);
95+
LL | | }
96+
LL | | }
97+
| |_____^
98+
|
99+
help: also remove the `if let` statement in the for loop
100+
--> $DIR/manual_flatten.rs:41:9
101+
|
102+
LL | / if let Some(m) = n {
103+
LL | | println!("{}", m);
104+
LL | | }
105+
| |_________^
42106

43-
error: aborting due to 4 previous errors
107+
error: aborting due to 5 previous errors
44108

0 commit comments

Comments
 (0)