From 659e7c1d5efa79394695c62375ef6860ab37aa67 Mon Sep 17 00:00:00 2001 From: Florian Hartwig Date: Tue, 20 Oct 2015 01:04:21 +0200 Subject: [PATCH] Don't suggest using a for loop if the iterator is used in the loop body Due to https://github.com/rust-lang/rust/issues/8372, we have to use while-let in these cases. --- src/loops.rs | 33 ++++++++++++++++++++++++++++++-- tests/compile-fail/while_loop.rs | 10 ++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/loops.rs b/src/loops.rs index 6b6fbf60c4c..04a2350db6b 100644 --- a/src/loops.rs +++ b/src/loops.rs @@ -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"); } @@ -314,6 +317,32 @@ impl<'v, 't> Visitor<'v> for VarVisitor<'v, 't> { } } +fn var_used(expr: &Expr, def_id: Option, 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 { diff --git a/tests/compile-fail/while_loop.rs b/tests/compile-fail/while_loop.rs index efd77a6d9c3..ae2f6995061 100755 --- a/tests/compile-fail/while_loop.rs +++ b/tests/compile-fail/while_loop.rs @@ -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)