add tests and fixes

camsteffen · camsteffen · commit a73edc09448a · 2017-06-01T00:06:45.000-05:00
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
@@ -330,6 +330,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
         if let Some((pat, arg, body)) = higher::for_loop(expr) {
             check_for_loop(cx, pat, arg, body, expr);
         }
+
+        // check for never_loop
+        match expr.node {
+            ExprWhile(_, ref block, _) |
+            ExprLoop(ref block, _, _) => {
+                if never_loop(block, &expr.id) {
+                    span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops");
+                }
+            },
+            _ => (),
+        }
+
         // check for `loop { if let {} else break }` that could be `while let`
         // (also matches an explicit "match" instead of "if let")
         // (even if the "match" or "if let" is used for declaration)
@@ -342,9 +354,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
                           "empty `loop {}` detected. You may want to either use `panic!()` or add \
                            `std::thread::sleep(..);` to the loop body.");
             }
-            if never_loop(block, &expr.id) {
-                span_lint(cx, NEVER_LOOP, expr.span, "this loop never actually loops");
-            }
 
             // extract the expression from the first statement (if any) in a block
             let inner_stmt_expr = extract_expr_from_first_stmt(block);
@@ -456,14 +465,17 @@ fn contains_continue_expr(expr: &Expr, dest: &NodeId) -> bool {
         ExprTupField(ref e, _) |
         ExprAddrOf(_, ref e) |
         ExprRepeat(ref e, _) => contains_continue_expr(e, dest),
+        ExprArray(ref es) |
+        ExprMethodCall(_, _, ref es) |
+        ExprTup(ref es) => es.iter().any(|e| contains_continue_expr(e, dest)),
+        ExprCall(ref e, ref es) => contains_continue_expr(e, dest) || es.iter().any(|e| contains_continue_expr(e, dest)),
         ExprBinary(_, ref e1, ref e2) |
         ExprAssign(ref e1, ref e2) |
         ExprAssignOp(_, ref e1, ref e2) |
         ExprIndex(ref e1, ref e2) => [e1, e2].iter().any(|e| contains_continue_expr(e, dest)),
-        ExprArray(ref es) |
-        ExprTup(ref es) |
-        ExprMethodCall(_, _, ref es) => es.iter().any(|e| contains_continue_expr(e, dest)),
-        ExprCall(ref e, ref es) => contains_continue_expr(e, dest) || es.iter().any(|e| contains_continue_expr(e, dest)),
+        ExprIf(ref e, ref e2, ref e3) => [e, e2].iter().chain(e3.as_ref().iter()).any(|e| contains_continue_expr(e, dest)),
+        ExprWhile(ref e, ref b, _) => contains_continue_expr(e, dest) || contains_continue_block(b, dest),
+        ExprMatch(ref e, ref arms, _) => contains_continue_expr(e, dest) || arms.iter().any(|a| contains_continue_expr(&a.body, dest)),
         ExprBlock(ref block) => contains_continue_block(block, dest),
         ExprStruct(_, _, ref base) => base.as_ref().map_or(false, |e| contains_continue_expr(e, dest)),
         ExprAgain(d) => d.target_id.opt_id().map_or(false, |id| id == *dest),
@@ -501,8 +513,8 @@ fn loop_exit_expr(expr: &Expr) -> bool {
         ExprTupField(ref e, _) |
         ExprAddrOf(_, ref e) |
         ExprRepeat(ref e, _) => loop_exit_expr(e),
-        ExprMethodCall(_, _, ref es) => es.iter().any(|e| loop_exit_expr(e)),
         ExprArray(ref es) |
+        ExprMethodCall(_, _, ref es) |
         ExprTup(ref es) => es.iter().any(|e| loop_exit_expr(e)),
         ExprCall(ref e, ref es) => loop_exit_expr(e) || es.iter().any(|e| loop_exit_expr(e)),
         ExprBinary(_, ref e1, ref e2) |
diff --git a/clippy_tests/examples/for_loop.stderr b/clippy_tests/examples/for_loop.stderr
@@ -53,6 +53,28 @@ error: for loop over `v.iter().next().ok_or("x not found")`, which is a `Result`
    = note: `-D for-loop-over-result` implied by `-D warnings`
    = help: consider replacing `for x in v.iter().next().ok_or("x not found")` with `if let Ok(x) = v.iter().next().ok_or("x not found")`
 
+error: this loop never actually loops
+  --> for_loop.rs:52:5
+   |
+52 | /     while let Some(x) = option {
+53 | |         println!("{}", x);
+54 | |         break;
+55 | |     }
+   | |_____^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+  --> for_loop.rs:58:5
+   |
+58 | /     while let Ok(x) = result {
+59 | |         println!("{}", x);
+60 | |         break;
+61 | |     }
+   | |_____^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
 error: the loop variable `i` is only used to index `vec`.
   --> for_loop.rs:84:5
    |
diff --git a/clippy_tests/examples/never_loop.rs b/clippy_tests/examples/never_loop.rs
@@ -1,57 +1,118 @@
 #![feature(plugin)]
 #![plugin(clippy)]
+#![allow(single_match, unused_assignments, unused_variables)]
 
-#![warn(never_loop)]
-#![allow(single_match, while_true)]
-
-fn break_stmt() {
-    loop {
+fn test1() {
+    let mut x = 0;
+    loop { // never_loop
+        x += 1;
+        if x == 1 {
+            return
+        }
         break;
     }
 }
 
-fn conditional_break() {
-    let mut x = 5;
+fn test2() {
+    let mut x = 0;
     loop {
-        x -= 1;
+        x += 1;
         if x == 1 {
             break
         }
     }
 }
 
-fn nested_loop() {
-    loop {
-        while true {
-            break
-        }
+fn test3() {
+    let mut x = 0;
+    loop { // never loops
+        x += 1;
         break
     }
 }
 
-fn if_false() {
-    let x = 1;
+fn test4() {
+    let mut x = 1;
     loop {
-        if x == 1 {
-            return
+        x += 1;
+        match x {
+            5 => return,
+            _ => (),
         }
     }
 }
 
-fn match_false() {
-    let x = 1;
+fn test5() {
+    let i = 0;
+	loop { // never loops
+        while i == 0 { // never loops
+            break
+        }
+        return
+	}
+}
+
+fn test6() {
+    let mut x = 0;
+    'outer: loop { // never loops
+        x += 1;
+		loop { // never loops
+            if x == 5 { break }
+			continue 'outer
+		}
+		return
+	}
+}
+
+fn test7() {
+    let mut x = 0;
     loop {
+        x += 1;
         match x {
-            1 => return,
+            1 => continue,
             _ => (),
         }
+        return
+    }
+}
+
+fn test8() {
+    let mut x = 0;
+    loop {
+        x += 1;
+        match x {
+            5 => return,
+            _ => continue,
+        }
+    }
+}
+
+fn test9() {
+    let x = Some(1);
+    while let Some(y) = x { // never loops
+        return
+    }
+}
+
+fn test10() {
+    for x in 0..10 { // never loops
+        match x {
+            1 => break,
+            _ => return,
+        }
     }
 }
 
 fn main() {
-    break_stmt();
-    conditional_break();
-    nested_loop();
-    if_false();
-    match_false();
+    test1();
+    test2();
+    test3();
+    test4();
+    test5();
+    test6();
+    test7();
+    test8();
+    test9();
+    test10();
 }
+
diff --git a/clippy_tests/examples/never_loop.stderr b/clippy_tests/examples/never_loop.stderr
@@ -1,26 +1,99 @@
 error: this loop never actually loops
-  --> never_loop.rs:8:5
+  --> never_loop.rs:7:5
    |
-8  | /     loop {
-9  | |         break;
-10 | |     }
+7  | /     loop { // never_loop
+8  | |         x += 1;
+9  | |         if x == 1 {
+10 | |             return
+11 | |         }
+12 | |         break;
+13 | |     }
    | |_____^
    |
    = note: `-D never-loop` implied by `-D warnings`
 
 error: this loop never actually loops
-  --> never_loop.rs:24:5
+  --> never_loop.rs:28:5
    |
-24 | /     loop {
-25 | |         while true {
-26 | |             break
-27 | |         }
-28 | |         break
-29 | |     }
+28 | /     loop { // never loops
+29 | |         x += 1;
+30 | |         break
+31 | |     }
    | |_____^
    |
    = note: `-D never-loop` implied by `-D warnings`
 
+error: this loop never actually loops
+  --> never_loop.rs:47:2
+   |
+47 | / 	loop { // never loops
+48 | | 	       while i == 0 { // never loops
+49 | | 	           break
+50 | | 	       }
+51 | | 	       return
+52 | | 	}
+   | |__^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+  --> never_loop.rs:48:9
+   |
+48 | /         while i == 0 { // never loops
+49 | |             break
+50 | |         }
+   | |_________^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+  --> never_loop.rs:57:5
+   |
+57 | /     'outer: loop { // never loops
+58 | |         x += 1;
+59 | | 		loop { // never loops
+60 | |             if x == 5 { break }
+...  |
+63 | | 		return
+64 | | 	}
+   | |__^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+  --> never_loop.rs:59:3
+   |
+59 | / 		loop { // never loops
+60 | | 		          if x == 5 { break }
+61 | | 			continue 'outer
+62 | | 		}
+   | |___^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+  --> never_loop.rs:92:5
+   |
+92 | /     while let Some(y) = x { // never loops
+93 | |         return
+94 | |     }
+   | |_____^
+   |
+   = note: `-D never-loop` implied by `-D warnings`
+
+error: this loop never actually loops
+   --> never_loop.rs:98:5
+    |
+98  | /     for x in 0..10 { // never loops
+99  | |         match x {
+100 | |             1 => break,
+101 | |             _ => return,
+102 | |         }
+103 | |     }
+    | |_____^
+    |
+    = note: `-D never-loop` implied by `-D warnings`
+
 error: aborting due to previous error(s)
 
 error: Could not compile `clippy_tests`.
