Skip to content

Commit

Permalink
Fixes for code review comments
Browse files Browse the repository at this point in the history
* remove weird infinite loops from compile-tests
* remove call to Option::unwrap
* in the lint message, show while-let loop rewritten as for loop
  • Loading branch information
fhartwig committed Oct 22, 2015
1 parent 2d6ca62 commit 9cf7a73
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 19 deletions.
27 changes: 18 additions & 9 deletions src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,26 @@ impl LateLintPass for LoopsPass {
}
}
}
if let ExprMatch(ref expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
if let ExprMatch(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node {
let body = &arms[0].body;
let pat = &arms[0].pats[0].node;
if let (&PatEnum(ref path, _), &ExprMethodCall(method_name, _, ref args)) = (pat, &expr.node) {
let iterator_def_id = var_def_id(cx, &args[0]);
if method_name.node.as_str() == "next" &&
match_trait_method(cx, expr, &["core", "iter", "Iterator"]) &&
path.segments.last().unwrap().identifier.name.as_str() == "Some" &&
!var_used(body, iterator_def_id, cx) {
span_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
"this loop could be written as a `for` loop");
if let (&PatEnum(ref path, Some(ref pat_args)),
&ExprMethodCall(method_name, _, ref method_args)) =
(pat, &match_expr.node) {
let iterator_def_id = var_def_id(cx, &method_args[0]);
if let Some(lhs_constructor) = path.segments.last() {
if method_name.node.as_str() == "next" &&
match_trait_method(cx, match_expr, &["core", "iter", "Iterator"]) &&
lhs_constructor.identifier.name.as_str() == "Some" &&
!var_used(body, iterator_def_id, cx) {
let iterator = snippet(cx, method_args[0].span, "_");
let loop_var = snippet(cx, pat_args[0].span, "_");
span_help_and_lint(cx, WHILE_LET_ON_ITERATOR, expr.span,
"this loop could be written as a `for` loop",
&format!("try\nfor {} in {} {{...}}",
loop_var,
iterator));
}
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions tests/compile-fail/while_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,24 @@ fn main() {
println!("{}", x);
}


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

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

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

while let None = (1..20).next() {} // this is fine (if nonsensical)
let mut iter = 1..20;
while let None = iter.next() {} // this is fine (if nonsensical)

if let Some(x) = (1..20).next() { // also fine
let mut iter = 1..20;
if let Some(x) = iter.next() { // also fine
println!("{}", x)
}

Expand All @@ -76,10 +80,6 @@ fn main() {
while let Some(x) = iter.next() {
println!("next: {:?}", iter.next())
}

// but this should:
let mut iter2 = 1u32..20;
while let Some(x) = iter2.next() { } //~ERROR this loop could be written as a `for` loop
}

// regression test (#360)
Expand Down

0 comments on commit 9cf7a73

Please sign in to comment.