Merge pull request #1229 from Manishearth/loops

Fix #1188

clippy_lints/src/loops.rs

@@ -14,9 +14,9 @@
 use syntax::ast;
 use utils::sugg;
 
-use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg, in_external_macro,
-            span_help_and_lint, is_integer_literal, get_enclosing_block, span_lint_and_then, higher,
-            walk_ptrs_ty};
+use utils::{snippet, span_lint, get_parent_expr, match_trait_method, match_type, multispan_sugg,
+            in_external_macro, is_refutable, span_help_and_lint, is_integer_literal,
+            get_enclosing_block, span_lint_and_then, higher, walk_ptrs_ty};
 use utils::paths;
 
 /// **What it does:** Checks for looping over the range of `0..len` of some
@@ -354,6 +354,7 @@ fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
                     if method_name.node.as_str() == "next" &&
                        match_trait_method(cx, match_expr, &paths::ITERATOR) &&
                        lhs_constructor.name.as_str() == "Some" &&
+                       !is_refutable(cx, &pat_args[0]) &&
                        !is_iterator_used_after_while_let(cx, iter_expr) {
                         let iterator = snippet(cx, method_args[0].span, "_");
                         let loop_var = snippet(cx, pat_args[0].span, "_");

clippy_lints/src/utils/mod.rs

@@ -721,7 +721,43 @@ pub fn type_is_unsafe_function(ty: ty::Ty) -> bool {
     }
 }
 
-pub fn is_copy<'a, 'ctx>(cx: &LateContext<'a, 'ctx>, ty: ty::Ty<'ctx>, env: NodeId) -> bool {
+pub fn is_copy<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, env: NodeId) -> bool {
     let env = ty::ParameterEnvironment::for_item(cx.tcx, env);
     !ty.subst(cx.tcx, env.free_substs).moves_by_default(cx.tcx.global_tcx(), &env, DUMMY_SP)
 }
+
+/// Return whether a pattern is refutable.
+pub fn is_refutable(cx: &LateContext, pat: &Pat) -> bool {
+    fn is_enum_variant(cx: &LateContext, did: NodeId) -> bool {
+        matches!(cx.tcx.def_map.borrow().get(&did).map(|d| d.full_def()), Some(def::Def::Variant(..)))
+    }
+
+    fn are_refutable<'a, I: Iterator<Item=&'a Pat>>(cx: &LateContext, mut i: I) -> bool {
+        i.any(|pat| is_refutable(cx, pat))
+    }
+
+    match pat.node {
+        PatKind::Binding(..) | PatKind::Wild => false,
+        PatKind::Box(ref pat) | PatKind::Ref(ref pat, _) => is_refutable(cx, pat),
+        PatKind::Lit(..) | PatKind::Range(..) => true,
+        PatKind::Path(..) => is_enum_variant(cx, pat.id),
+        PatKind::Tuple(ref pats, _) => are_refutable(cx, pats.iter().map(|pat| &**pat)),
+        PatKind::Struct(_, ref fields, _) => {
+            if is_enum_variant(cx, pat.id) {
+                true
+            } else {
+                are_refutable(cx, fields.iter().map(|field| &*field.node.pat))
+            }
+        }
+        PatKind::TupleStruct(_, ref pats, _) => {
+            if is_enum_variant(cx, pat.id) {
+                true
+            } else {
+                are_refutable(cx, pats.iter().map(|pat| &**pat))
+            }
+        }
+        PatKind::Vec(ref head, ref middle, ref tail) => {
+            are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat))
+        }
+    }
+}

tests/compile-fail/while_loop.rs

@@ -165,3 +165,31 @@ fn issue1017() {
         }
     }
 }
+
+// Issue #1188
+fn refutable() {
+    let a = [42, 1337];
+    let mut b = a.iter();
+
+    // consume all the 42s
+    while let Some(&42) = b.next() {
+    }
+
+    let a = [(1, 2, 3)];
+    let mut b = a.iter();
+
+    while let Some(&(1, 2, 3)) = b.next() {
+    }
+
+    let a = [Some(42)];
+    let mut b = a.iter();
+
+    while let Some(&None) = b.next() {
+    }
+
+    /* This gives “refutable pattern in `for` loop binding: `&_` not covered”
+    for &42 in b {}
+    for &(1, 2, 3) in b {}
+    for &Option::None in b.next() {}
+    // */
+}