lint: while loop: detect if no var from the condition is mutated

clippy_lints/src/loops.rs

@@ -2140,23 +2140,45 @@ fn path_name(e: &Expr) -> Option<Name> {
     None
 }
 
-fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, _block: &'tcx Block, _expr: &'tcx Expr) {
+fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, block: &'tcx Block, expr: &'tcx Expr) {
     let mut mut_var_visitor = MutableVarsVisitor {
         cx,
         ids: HashSet::new(),
         skip: false,
     };
-    walk_expr(&mut mut_var_visitor, cond);
+    walk_expr(&mut mut_var_visitor, expr);
-    if !mut_var_visitor.skip && mut_var_visitor.ids.len() == 0 {
+    if mut_var_visitor.skip {
+        return;
+    }
+
+    if mut_var_visitor.ids.len() == 0 {
         span_lint(
             cx,
             WHILE_IMMUTABLE_CONDITION,
             cond.span,
             "all variables in condition are immutable. This might lead to infinite loops.",
-        )
+        );
+        return;
+    }
+
+    let mut use_visitor = MutablyUsedVisitor {
+        cx,
+        ids: mut_var_visitor.ids,
+        any_used: false,
+    };
+    walk_block(&mut use_visitor, block);
+    if !use_visitor.any_used {
+        span_lint(
+            cx,
+            WHILE_IMMUTABLE_CONDITION,
+            expr.span,
+            "Variable in the condition are not mutated in the loop body. This might lead to infinite loops.",
+        );
     }
 }
 
+/// Collects the set of mutable variable in an expression
+/// Stops analysis if a function call is found
 struct MutableVarsVisitor<'a, 'tcx: 'a> {
     cx: &'a LateContext<'a, 'tcx>,
     ids: HashSet<NodeId>,
@@ -2171,12 +2193,46 @@ impl<'a, 'tcx> Visitor<'tcx> for MutableVarsVisitor<'a, 'tcx> {
             },
 
             // If there is any fuction/method call… we just stop analysis
-            ExprCall(_, _) | ExprMethodCall(_, _, _) => self.skip = true,
+            ExprCall(..) | ExprMethodCall(..) => self.skip = true,
 
             _ => walk_expr(self, ex),
         }
     }
 
+    fn visit_block(&mut self, _b: &'tcx Block) {}
+
+    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
+        NestedVisitorMap::None
+    }
+}
+
+/// checks within an expression/statement if any of the variables are used mutably
+struct MutablyUsedVisitor<'a, 'tcx: 'a> {
+    cx: &'a LateContext<'a, 'tcx>,
+    ids: HashSet<NodeId>,
+    any_used: bool,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for MutablyUsedVisitor<'a, 'tcx> {
+    fn visit_expr(&mut self, ex: &'tcx Expr) {
+        if self.any_used { return; }
+
+        match ex.node {
+            ExprAddrOf(MutMutable, ref p) | ExprAssign(ref p, _) | ExprAssignOp(_, ref p, _) =>
+                if let Some(id) = check_for_mutability(self.cx, p) {
+                    self.any_used = self.ids.contains(&id);
+                }
+            _ => walk_expr(self, ex)
+        }
+    }
+
+    fn visit_stmt(&mut self, s: &'tcx Stmt) {
+        match s.node {
+            StmtExpr(..) | StmtSemi (..) => walk_stmt(self, s),
+            _ => {}
+        }
+    }
+
     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
         NestedVisitorMap::None
     }

tests/ui/infinite_loop.rs

@@ -1,7 +1,8 @@
 fn fn_val(i: i32) -> i32 { unimplemented!() }
 fn fn_constref(i: &i32) -> i32 { unimplemented!() }
 fn fn_mutref(i: &mut i32) { unimplemented!() }
-fn foo() -> i32 { unimplemented!() }
+fn fooi() -> i32 { unimplemented!() }
+fn foob() -> bool { unimplemented!() }
 
 fn immutable_condition() {
     // Should warn when all vars mentionned are immutable
@@ -12,6 +13,8 @@ fn immutable_condition() {
 
     let x = 0;
     while y < 10 && x < 3 {
+        let mut k = 1;
+        k += 2;
         println!("KO - x and y immutable");
     }
 
@@ -32,7 +35,11 @@ fn immutable_condition() {
         println!("OK - mut_cond is mutable");
     }
 
-    while foo() < x {
+    while fooi() < x {
+        println!("OK - Fn call results may vary");
+    }
+
+    while foob() {
         println!("OK - Fn call results may vary");
     }
 
@@ -80,6 +87,11 @@ fn used_immutable() {
         println!("OK - passed by mutable reference");
         fn_mutref(&mut i)
     }
+
+    while i < 3 {
+        fn_mutref(&mut i);
+        println!("OK - passed by mutable reference");
+    }
 }
 
 fn main() {

tests/ui/infinite_loop.stderr

@@ -1,22 +1,67 @@
 error: all variables in condition are immutable. This might lead to infinite loops.
- --> $DIR/infinite_loop.rs:9:11
+  --> $DIR/infinite_loop.rs:10:11
-  |
+   |
-9 |     while y < 10 {
+10 |     while y < 10 {
-  |           ^^^^^^
+   |           ^^^^^^
-  |
+   |
-  = note: `-D while-immutable-condition` implied by `-D warnings`
+   = note: `-D while-immutable-condition` implied by `-D warnings`
 
 error: all variables in condition are immutable. This might lead to infinite loops.
-  --> $DIR/infinite_loop.rs:14:11
+  --> $DIR/infinite_loop.rs:15:11
    |
-14 |     while y < 10 && x < 3 {
+15 |     while y < 10 && x < 3 {
    |           ^^^^^^^^^^^^^^^
 
 error: all variables in condition are immutable. This might lead to infinite loops.
-  --> $DIR/infinite_loop.rs:19:11
+  --> $DIR/infinite_loop.rs:22:11
    |
-19 |     while !cond {
+22 |     while !cond {
    |           ^^^^^
 
-error: aborting due to 3 previous errors
+error: Variable in the condition are not mutated in the loop body. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:52:5
+   |
+52 | /     while i < 3 {
+53 | |         j = 3;
+54 | |         println!("KO - i not mentionned");
+55 | |     }
+   | |_____^
+
+error: Variable in the condition are not mutated in the loop body. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:57:5
+   |
+57 | /     while i < 3 && j > 0 {
+58 | |         println!("KO - i and j not mentionned");
+59 | |     }
+   | |_____^
+
+error: Variable in the condition are not mutated in the loop body. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:61:5
+   |
+61 | /     while i < 3 {
+62 | |         let mut i = 5;
+63 | |         fn_mutref(&mut i);
+64 | |         println!("KO - shadowed");
+65 | |     }
+   | |_____^
+
+error: Variable in the condition are not mutated in the loop body. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:76:5
+   |
+76 | /     while i < 3 {
+77 | |         fn_constref(&i);
+78 | |         println!("KO - const reference");
+79 | |     }
+   | |_____^
+
+error: Variable in the condition are not mutated in the loop body. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:81:5
+   |
+81 | /     while i < 3 {
+82 | |         fn_val(i);
+83 | |         println!("KO - passed by value");
+84 | |     }
+   | |_____^
+
+error: aborting due to 8 previous errors