lint: immutable only vars in while condition

clippy_lints/src/lib.rs

@@ -521,6 +521,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         loops::UNUSED_COLLECT,
         loops::WHILE_LET_LOOP,
         loops::WHILE_LET_ON_ITERATOR,
+        loops::WHILE_IMMUTABLE_CONDITION,
         map_clone::MAP_CLONE,
         matches::MATCH_AS_REF,
         matches::MATCH_BOOL,

clippy_lints/src/loops.rs

@@ -343,6 +343,27 @@ declare_lint! {
     "for loop over a range where one of the bounds is a mutable variable"
 }
 
+/// **What it does:** Checks whether variables used within while loop condition
+/// can be (and are) mutated in the body.
+///
+/// **Why is this bad?** If the condition is unchanged, entering the body of the loop
+/// will lead to an infinite loop.
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```rust
+/// let i = 0;
+/// while i > 10 {
+///    println!("let me loop forever!");
+/// }
+/// ```
+declare_lint! {
+    pub WHILE_IMMUTABLE_CONDITION,
+    Warn,
+    "variables used within while expression are not mutated in the body"
+}
+
 #[derive(Copy, Clone)]
 pub struct Pass;
 
@@ -364,7 +385,8 @@ impl LintPass for Pass {
             WHILE_LET_ON_ITERATOR,
             FOR_KV_MAP,
             NEVER_LOOP,
-            MUT_RANGE_BOUND
+            MUT_RANGE_BOUND,
+            WHILE_IMMUTABLE_CONDITION,
         )
     }
 }
@@ -469,6 +491,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                 }
             }
         }
+
+        // check for while loops which conditions never change
+        if let ExprWhile(ref cond, ref block, _) = expr.node {
+            check_infinite_loop(cx, cond, block, expr);
+        }
     }
 
     fn check_stmt(&mut self, cx: &LateContext<'a, 'tcx>, stmt: &'tcx Stmt) {
@@ -662,6 +689,46 @@ fn check_for_loop<'a, 'tcx>(
     detect_manual_memcpy(cx, pat, arg, body, expr);
 }
 
+fn search_mutable_vars<'a, 'tcx> (
+    cx: &LateContext<'a, 'tcx>,
+    ex: &'tcx Expr,
+    acc: &mut Vec<NodeId>,
+) -> bool {
+    match ex.node {
+        ExprBinary(_, ref a, ref b) =>
+            search_mutable_vars(cx, a, acc) && search_mutable_vars(cx, b, acc),
+
+        ExprUnary(_, ref a) => search_mutable_vars(cx, a, acc),
+        ExprPath(_) => {
+            if let Some(node_id) = check_for_mutability(cx, &ex) {
+                acc.push(node_id);
+            }
+            true
+        }
+        ExprLit(_) => true,
+
+        // Skip if any method or function call is encountered
+        _ => false
+    }
+}
+
+fn check_infinite_loop<'a, 'tcx>(
+    cx: &LateContext<'a, 'tcx>,
+    cond: &'tcx Expr,
+    _block: &'tcx Block,
+    _expr: &'tcx Expr,
+) {
+    let mut mutable_vars = Vec::new();
+    if search_mutable_vars(cx, cond, &mut mutable_vars) && mutable_vars.len() == 0 {
+        span_lint(
+            cx,
+            WHILE_IMMUTABLE_CONDITION,
+            cond.span,
+            "all variables in condition are immutable. This might lead to infinite loops."
+        )
+    }
+}
+
 fn same_var<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &Expr, var: ast::NodeId) -> bool {
     if_chain! {
         if let ExprPath(ref qpath) = expr.node;

tests/ui/infinite_loop.rs

@@ -0,0 +1,89 @@
+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 immutable_condition() {
+    // Should warn when all vars mentionned are immutable
+    let y = 0;
+    while y < 10 {
+        println!("KO - y is immutable");
+    }
+
+    let x = 0;
+    while y < 10 && x < 3 {
+        println!("KO - x and y immutable");
+    }
+
+    let cond = false;
+    while !cond {
+        println!("KO - cond immutable");
+    }
+
+    let mut i = 0;
+    while y < 10 && i < 3 {
+        i += 1;
+        println!("OK - i is mutable");
+    }
+
+    let mut mut_cond = false;
+    while !mut_cond || cond {
+        mut_cond = true;
+        println!("OK - mut_cond is mutable");
+    }
+
+    while foo() < x {
+        println!("OK - Fn call results may vary");
+    }
+
+}
+
+fn unused_var() {
+    // Should warn when a (mutable) var is not used in while body
+    let (mut i, mut j) = (0, 0);
+
+    while i < 3 {
+        j = 3;
+        println!("KO - i not mentionned");
+    }
+
+    while i < 3 && j > 0 {
+        println!("KO - i and j not mentionned");
+    }
+
+    while i < 3 {
+        let mut i = 5;
+        fn_mutref(&mut i);
+        println!("KO - shadowed");
+    }
+
+    while i < 3 && j > 0 {
+        i = 5;
+        println!("OK - i in cond and mentionned");
+    }
+}
+
+fn used_immutable() {
+    let mut i = 0;
+
+    while i < 3 {
+        fn_constref(&i);
+        println!("KO - const reference");
+    }
+
+    while i < 3 {
+        fn_val(i);
+        println!("KO - passed by value");
+    }
+
+    while i < 3 {
+        fn_mutref(&mut i);
+        println!("OK - passed by mutable reference");
+    }
+}
+
+fn main() {
+    immutable_condition();
+    unused_var();
+    used_immutable();
+}

tests/ui/infinite_loop.stderr

@@ -0,0 +1,22 @@
+error: all variables in condition are immutable. This might lead to infinite loops.
+ --> $DIR/infinite_loop.rs:9:11
+  |
+9 |     while y < 10 {
+  |           ^^^^^^
+  |
+  = 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
+   |
+14 |     while y < 10 && x < 3 {
+   |           ^^^^^^^^^^^^^^^
+
+error: all variables in condition are immutable. This might lead to infinite loops.
+  --> $DIR/infinite_loop.rs:19:11
+   |
+19 |     while !cond {
+   |           ^^^^^
+
+error: aborting due to 3 previous errors
+

tests/ui/never_loop.rs

@@ -1,6 +1,6 @@
 
 
-#![allow(single_match, unused_assignments, unused_variables)]
+#![allow(single_match, unused_assignments, unused_variables, while_immutable_condition)]
 
 fn test1() {
     let mut x = 0;