Skip to content

Commit 599ae33

Browse files
committed
Fixes for code review comments
* remove weird infinite loops from compile-tests * remove call to Option::unwrap * in the lint message, show while-let loop rewritten as for loop
1 parent eecc709 commit 599ae33

File tree

2 files changed

+28
-19
lines changed

2 files changed

+28
-19
lines changed

src/loops.rs

+18-9
Original file line numberDiff line numberDiff line change
@@ -231,17 +231,26 @@ impl LateLintPass for LoopsPass {
231231
}
232232
}
233233
}
234-
if let ExprMatch(ref expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
234+
if let ExprMatch(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
235235
let body = &arms[0].body;
236236
let pat = &arms[0].pats[0].node;
237-
if let (&PatEnum(ref path, _), &ExprMethodCall(method_name, _, ref args)) = (pat, &expr.node) {
238-
let iterator_def_id = var_def_id(cx, &args[0]);
239-
if method_name.node.as_str() == "next" &&
240-
match_trait_method(cx, expr, &["core", "iter", "Iterator"]) &&
241-
path.segments.last().unwrap().identifier.name.as_str() == "Some" &&
242-
!var_used(body, iterator_def_id, cx) {
243-
span_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
244-
"this loop could be written as a `for` loop");
237+
if let (&PatEnum(ref path, Some(ref pat_args)),
238+
&ExprMethodCall(method_name, _, ref method_args)) =
239+
(pat, &match_expr.node) {
240+
let iterator_def_id = var_def_id(cx, &method_args[0]);
241+
if let Some(lhs_constructor) = path.segments.last() {
242+
if method_name.node.as_str() == "next" &&
243+
match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) &&
244+
lhs_constructor.identifier.name.as_str() == "Some" &&
245+
!var_used(body, iterator_def_id, cx) {
246+
let iterator = snippet(cx, method_args[0].span, "_");
247+
let loop_var = snippet(cx, pat_args[0].span, "_");
248+
span_help_and_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
249+
"this loop could be written as a `for` loop",
250+
&format!("try\nfor {} in {} {{...}}",
251+
loop_var,
252+
iterator));
253+
}
245254
}
246255
}
247256
}

tests/compile-fail/while_loop.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,24 @@ fn main() {
5454
println!("{}", x);
5555
}
5656

57-
58-
while let Option::Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
57+
let mut iter = 1..20;
58+
while let Option::Some(x) = iter.next() { //~ERROR this loop could be written as a `for` loop
5959
println!("{}", x);
6060
}
6161

62-
while let Some(x) = (1..20).next() { //~ERROR this loop could be written as a `for` loop
62+
let mut iter = 1..20;
63+
while let Some(x) = iter.next() { //~ERROR this loop could be written as a `for` loop
6364
println!("{}", x);
6465
}
6566

66-
while let Some(_) = (1..20).next() {} //~ERROR this loop could be written as a `for` loop
67+
let mut iter = 1..20;
68+
while let Some(_) = iter.next() {} //~ERROR this loop could be written as a `for` loop
6769

68-
while let None = (1..20).next() {} // this is fine (if nonsensical)
70+
let mut iter = 1..20;
71+
while let None = iter.next() {} // this is fine (if nonsensical)
6972

70-
if let Some(x) = (1..20).next() { // also fine
73+
let mut iter = 1..20;
74+
if let Some(x) = iter.next() { // also fine
7175
println!("{}", x)
7276
}
7377

@@ -76,10 +80,6 @@ fn main() {
7680
while let Some(x) = iter.next() {
7781
println!("next: {:?}", iter.next())
7882
}
79-
80-
// but this should:
81-
let mut iter2 = 1u32..20;
82-
while let Some(x) = iter2.next() { } //~ERROR this loop could be written as a `for` loop
8383
}
8484

8585
// regression test (#360)

0 commit comments

Comments
 (0)