From 88aa04dfa54069ae64313188b543a9d6a3fdeb45 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Fri, 23 Jun 2017 18:29:18 +0200 Subject: [PATCH] don't lint while_let_on_iterator on nested loops The problem is with a nested loop, the iterator may well be reused. This changeset introduces a false negative, when the iterator is initialized within the outer loop. A further PR could get rid of this false negative by checking if the iterator is indeed initialized within the outer loop. --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/loops.rs | 17 ++++++++++++++--- clippy_tests/examples/while_loop.rs | 12 ++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ec73e9d50d9..94a258977bf 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -250,7 +250,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box shadow::Pass); reg.register_late_lint_pass(box types::LetPass); reg.register_late_lint_pass(box types::UnitCmp); - reg.register_late_lint_pass(box loops::Pass); + reg.register_late_lint_pass(box loops::Pass::default()); reg.register_late_lint_pass(box lifetimes::LifetimePass); reg.register_late_lint_pass(box entry::HashMapLint); reg.register_late_lint_pass(box ranges::StepByZero); diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 8ef9c1d35f9..dca98d4a079 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -304,8 +304,10 @@ declare_lint! { "any loop that will always `break` or `return`" } -#[derive(Copy, Clone)] -pub struct Pass; +#[derive(Copy, Clone, Default)] +pub struct Pass { + loop_count : usize, +} impl LintPass for Pass { fn get_lints(&self) -> LintArray { @@ -327,6 +329,13 @@ impl LintPass for Pass { } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_expr_post(&mut self, _: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { + match expr.node { + ExprWhile(..) | ExprLoop(..) => { self.loop_count -= 1; } + _ => () + } + } + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let Some((pat, arg, body)) = higher::for_loop(expr) { check_for_loop(cx, pat, arg, body, expr); @@ -336,6 +345,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { match expr.node { ExprWhile(_, ref block, _) | ExprLoop(ref block, _, _) => { + self.loop_count += 1; if never_loop(block, &expr.id) { span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops"); } @@ -398,7 +408,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { &ExprMethodCall(method_name, _, ref method_args)) = (pat, &match_expr.node) { let iter_expr = &method_args[0]; let lhs_constructor = last_path_segment(qpath); - if method_name.node == "next" && match_trait_method(cx, match_expr, &paths::ITERATOR) && + if self.loop_count < 2 && method_name.node == "next" && + match_trait_method(cx, match_expr, &paths::ITERATOR) && lhs_constructor.name == "Some" && !is_refutable(cx, &pat_args[0]) && !is_iterator_used_after_while_let(cx, iter_expr) { let iterator = snippet(cx, method_args[0].span, "_"); diff --git a/clippy_tests/examples/while_loop.rs b/clippy_tests/examples/while_loop.rs index 42b9d75b084..09893b87f16 100644 --- a/clippy_tests/examples/while_loop.rs +++ b/clippy_tests/examples/while_loop.rs @@ -165,4 +165,16 @@ fn refutable() { for &(1, 2, 3) in b {} for &Option::None in b.next() {} // */ + + let x = a.iter(); + loop { // x is reused, so don't lint here + while let Some(v) = x.next() { + } + } + + let y = a.iter(); + for _ in 0..2 { + while let Some(v) = x.next() { + } + } }