Auto merge of #4730 - yerke:fix-check_infinite_loop, r=flip1995

Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body

changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes #4648

Author: bors

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,

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();
 }

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