`while_let_loop` doesn't take into account break-with-value #1948

diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 0ed8debfa1e0db8ae72c4ad7c005ae5ce78d9cf1..4452cee8613d20d237612255e28c1c2f18d552a5 100644 (file)
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -392,7 +392,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
                         MatchSource::Normal | MatchSource::IfLetDesugar { .. } => {
                             if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
                                 arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
-                                is_break_expr(&arms[1].body)
+                                is_simple_break_expr(&arms[1].body)
                             {
                                 if in_external_macro(cx, expr.span) {
                                     return;
@@ -1500,13 +1500,16 @@ fn extract_first_expr(block: &Block) -> Option<&Expr> {
     }
 }
 
-/// Return true if expr contains a single break expr (maybe within a block).
-fn is_break_expr(expr: &Expr) -> bool {
+/// Return true if expr contains a single break expr without destination label and
+/// passed expression. The expression may be within a block.
+fn is_simple_break_expr(expr: &Expr) -> bool {
     match expr.node {
-        ExprBreak(dest, _) if dest.ident.is_none() => true,
-        ExprBlock(ref b) => match extract_first_expr(b) {
-            Some(subexpr) => is_break_expr(subexpr),
-            None => false,
+        ExprBreak(dest, ref passed_expr) if dest.ident.is_none() && passed_expr.is_none() => true,
+        ExprBlock(ref b) => {
+            match extract_first_expr(b) {
+                Some(subexpr) => is_simple_break_expr(subexpr),
+                None => false,
+            }
         },
         _ => false,
     }

diff --git a/tests/ui/while_loop.rs b/tests/ui/while_loop.rs
index 848735826098bc37e6632dfea66f6d1e9ddcdd6e..b67c41621e64ba403f598668e2f40a50eae5e4e6 100644 (file)
--- a/tests/ui/while_loop.rs
+++ b/tests/ui/while_loop.rs
@@ -183,4 +183,14 @@ fn refutable() {
         while let Some(v) = y.next() { // use a for loop here
         }
     }
+
+    //should not trigger while_let_loop lint because break passes an expression
+    let a = Some(10);
+    let b = loop {
+        if let Some(c) = a {
+            break Some(c);
+        } else {
+            break None;
+        }
+    };
 }