Skip to content

Commit

Permalink
Don't suggest using a for loop if the iterator is used in the loop body
Browse files Browse the repository at this point in the history
Due to rust-lang/rust#8372, we have to use while-let
in these cases.
  • Loading branch information
fhartwig committed Oct 26, 2015
1 parent f6163fc commit 659e7c1
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
33 changes: 31 additions & 2 deletions src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,14 @@ impl LateLintPass for LoopsPass {
}
}
if let ExprMatch(ref 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, _, _)) = (pat, &expr.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" {
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");
}
Expand Down Expand Up @@ -314,6 +317,32 @@ impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> {
}
}

fn var_used(expr: &Expr, def_id: Option<NodeId>, cx: &LateContext) -> bool {
match def_id {
None => false,
Some(def_id) => {
let mut visitor = VarUsedVisitor{ def_id: def_id, found: false, cx: cx };
walk_expr(&mut visitor, expr);
visitor.found
}
}
}

struct VarUsedVisitor<'v, 't: 'v> {
cx: &'v LateContext<'v, 't>,
def_id: NodeId,
found: bool
}

impl<'v, 't> Visitor<'v> for VarUsedVisitor<'v, 't> {
fn visit_expr(&mut self, expr: &'v Expr) {
if Some(self.def_id) == var_def_id(self.cx, expr) {
self.found = true;
}
walk_expr(self, expr);
}
}

/// Return true if the type of expr is one that provides IntoIterator impls
/// for &T and &mut T, such as Vec.
fn is_ref_iterable_type(cx: &LateContext, e: &Expr) -> bool {
Expand Down
10 changes: 10 additions & 0 deletions tests/compile-fail/while_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ fn main() {
if let Some(x) = (1..20).next() { // also fine
println!("{}", x)
}

// the following shouldn't warn because it can't be written with a for loop
let mut iter = 1u32..20;
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 659e7c1

Please sign in to comment.