From 1cba0c9f7dab7ebd238ab4fa247644136ac1d729 Mon Sep 17 00:00:00 2001 From: Yerkebulan Tulibergenov Date: Thu, 24 Oct 2019 23:46:25 -0700 Subject: [PATCH] fix check_infinite_loop by checking for break or return inside loop body --- clippy_lints/src/loops.rs | 46 ++++++++++++++++++++++++++-- tests/ui/infinite_loop.rs | 19 ++++++++++++ tests/ui/infinite_loop.stderr | 57 +++++++++++++++++++++++++++++------ 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 75d540b38e5d..ca4aa1deaed5 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -2367,17 +2367,57 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, e return; }; let mutable_static_in_cond = var_visitor.def_ids.iter().any(|(_, v)| *v); + + let mut has_break_or_return_visitor = HasBreakOrReturnVisitor { + has_break_or_return: false, + }; + has_break_or_return_visitor.visit_expr(expr); + let has_break_or_return = has_break_or_return_visitor.has_break_or_return; + if no_cond_variable_mutated && !mutable_static_in_cond { - span_lint( + span_lint_and_then( cx, WHILE_IMMUTABLE_CONDITION, cond.span, - "Variable in the condition are not mutated in the loop body. \ - This either leads to an infinite or to a never running loop.", + "variables in the condition are not mutated in the loop body", + |db| { + db.note("this may lead to an infinite or to a never running loop"); + + if has_break_or_return { + db.note("this loop contains `return`s or `break`s"); + db.help("rewrite it as `if cond { loop { } }`"); + } + }, ); } } +struct HasBreakOrReturnVisitor { + has_break_or_return: bool, +} + +impl<'a, 'tcx> Visitor<'tcx> for HasBreakOrReturnVisitor { + fn visit_expr(&mut self, expr: &'tcx Expr) { + if self.has_break_or_return { + return; + } + + match expr.kind { + ExprKind::Ret(_) | ExprKind::Break(_, _) => { + self.has_break_or_return = true; + return; + }, + _ => {}, + } + + walk_expr(self, expr); + } + + fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { + NestedVisitorMap::None + } +} + /// Collects the set of variables in an expression /// Stops analysis if a function call is found /// Note: In some cases such as `self`, there are no mutable annotation, diff --git a/tests/ui/infinite_loop.rs b/tests/ui/infinite_loop.rs index 4df218aa4f31..09f47adc46e6 100644 --- a/tests/ui/infinite_loop.rs +++ b/tests/ui/infinite_loop.rs @@ -177,6 +177,23 @@ impl Counter { } } +fn while_loop_with_break_and_return() { + let y = 0; + while y < 10 { + if y == 0 { + break; + } + println!("KO - loop contains break"); + } + + while y < 10 { + if y == 0 { + return; + } + println!("KO - loop contains return"); + } +} + fn main() { immutable_condition(); unused_var(); @@ -186,4 +203,6 @@ fn main() { let mut c = Counter { count: 0 }; c.inc_n(5); c.print_n(2); + + while_loop_with_break_and_return(); } diff --git a/tests/ui/infinite_loop.stderr b/tests/ui/infinite_loop.stderr index 6f0332fa8c4a..2736753c14b6 100644 --- a/tests/ui/infinite_loop.stderr +++ b/tests/ui/infinite_loop.stderr @@ -1,58 +1,95 @@ -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:23:11 | LL | while y < 10 { | ^^^^^^ | = note: `#[deny(clippy::while_immutable_condition)]` on by default + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:28:11 | LL | while y < 10 && x < 3 { | ^^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:35:11 | LL | while !cond { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:79:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:84:11 | LL | while i < 3 && j > 0 { | ^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:88:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:103:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:108:11 | LL | while i < 3 { | ^^^^^ + | + = note: this may lead to an infinite or to a never running loop -error: Variable in the condition are not mutated in the loop body. This either leads to an infinite or to a never running loop. +error: variables in the condition are not mutated in the loop body --> $DIR/infinite_loop.rs:174:15 | LL | while self.count < n { | ^^^^^^^^^^^^^^ + | + = note: this may lead to an infinite or to a never running loop + +error: variables in the condition are not mutated in the loop body + --> $DIR/infinite_loop.rs:182:11 + | +LL | while y < 10 { + | ^^^^^^ + | + = note: this may lead to an infinite or to a never running loop + = note: this loop contains `return`s or `break`s + = help: rewrite it as `if cond { loop { } }` + +error: variables in the condition are not mutated in the loop body + --> $DIR/infinite_loop.rs:189:11 + | +LL | while y < 10 { + | ^^^^^^ + | + = note: this may lead to an infinite or to a never running loop + = note: this loop contains `return`s or `break`s + = help: rewrite it as `if cond { loop { } }` -error: aborting due to 9 previous errors +error: aborting due to 11 previous errors