From 5379fc1b2804b647946b4a5d485db6c9579e4f55 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fran=C3=A7ois=20Mockers?= <mockersf@gmail.com>
Date: Sun, 27 May 2018 23:59:07 +0200
Subject: [PATCH] better parsing of condition in while loop for mutability

allow condition to be a block: by calling visit_expr of the visitor directly on the condition instead of walk_expr on the whole expression, we bypass the match to ExprWhile that calls visit_expr on the condition and visit_block on the body. This allow to re-enable visit_block in the visitor, as it won't be called on the while body
allow condition to use static variables: maintain a list of static variables used, and if they are mutable
---
 clippy_lints/src/loops.rs              | 11 ++++++----
 tests/run-pass/issues_loop_mut_cond.rs | 28 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)
 create mode 100644 tests/run-pass/issues_loop_mut_cond.rs

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index b0f399376685..c8f1d3edaeb3 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -2154,9 +2154,10 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
     let mut mut_var_visitor = VarCollectorVisitor {
         cx,
         ids: HashMap::new(),
+        def_ids: HashMap::new(),
         skip: false,
     };
-    walk_expr(&mut mut_var_visitor, expr);
+    mut_var_visitor.visit_expr(cond);
     if mut_var_visitor.skip {
         return;
     }
@@ -2172,7 +2173,7 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
     if delegate.skip {
         return;
     }
-    if !delegate.used_mutably.iter().any(|(_, v)| *v) {
+    if !(delegate.used_mutably.iter().any(|(_, v)| *v) || mut_var_visitor.def_ids.iter().any(|(_, v)| *v)) {
         span_lint(
             cx,
             WHILE_IMMUTABLE_CONDITION,
@@ -2189,6 +2190,7 @@ fn check_infinite_loop<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, cond: &'tcx Expr, b
 struct VarCollectorVisitor<'a, 'tcx: 'a> {
     cx: &'a LateContext<'a, 'tcx>,
     ids: HashMap<NodeId, bool>,
+    def_ids: HashMap<def_id::DefId, bool>,
     skip: bool,
 }
 
@@ -2203,6 +2205,9 @@ impl<'a, 'tcx> VarCollectorVisitor<'a, 'tcx> {
                     Def::Local(node_id) | Def::Upvar(node_id, ..) => {
                         self.ids.insert(node_id, false);
                     },
+                    Def::Static(def_id, mutable) => {
+                        self.def_ids.insert(def_id, mutable);
+                    },
                     _ => {},
                 }
             }
@@ -2221,8 +2226,6 @@ impl<'a, 'tcx> Visitor<'tcx> for VarCollectorVisitor<'a, 'tcx> {
         }
     }
 
-    fn visit_block(&mut self, _b: &'tcx Block) {}
-
     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
         NestedVisitorMap::None
     }
diff --git a/tests/run-pass/issues_loop_mut_cond.rs b/tests/run-pass/issues_loop_mut_cond.rs
new file mode 100644
index 000000000000..6ecd40b99b1c
--- /dev/null
+++ b/tests/run-pass/issues_loop_mut_cond.rs
@@ -0,0 +1,28 @@
+#![allow(dead_code)]
+
+/// Issue: https://github.com/rust-lang-nursery/rust-clippy/issues/2596
+pub fn loop_on_block_condition(u: &mut isize) {
+    while { *u < 0 } {
+        *u += 1;
+    }
+}
+
+/// https://github.com/rust-lang-nursery/rust-clippy/issues/2584
+fn loop_with_unsafe_condition(ptr: *const u8) {
+    let mut len = 0;
+    while unsafe { *ptr.offset(len) } != 0 {
+        len += 1;
+    }
+}
+
+/// https://github.com/rust-lang-nursery/rust-clippy/issues/2710
+static mut RUNNING: bool = true;
+fn loop_on_static_condition() {
+    unsafe {
+        while RUNNING {
+            RUNNING = false;
+        }
+    }
+}
+
+fn main() {}