From c8986b46765cedc1bb67bed2c8d4f66f8c6bebe9 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Fri, 16 Sep 2016 15:45:44 +0200
Subject: [PATCH] Fix FP with `WHILE_LET_ON_ITERATOR` and refutable pats
---
clippy_lints/src/loops.rs | 7 ++++---
clippy_lints/src/utils/mod.rs | 36 ++++++++++++++++++++++++++++++++
tests/compile-fail/while_loop.rs | 28 +++++++++++++++++++++++++
3 files changed, 68 insertions(+), 3 deletions(-)
diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs
index 8707d3be153..66ae404221b 100644
--- a/clippy_lints/src/loops.rs
+++ b/clippy_lints/src/loops.rs
@@ -14,9 +14,9 @@ use std::collections::HashMap;
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 @@ impl LateLintPass for Pass {
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, "_");
diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index e6e29fbc3e2..2a86d0af380 100644
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -725,3 +725,39 @@ pub fn is_copy<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, env: Node
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))
+ }
+ }
+}
diff --git a/tests/compile-fail/while_loop.rs b/tests/compile-fail/while_loop.rs
index d8cea42e20b..7b0fb43575e 100644
--- a/tests/compile-fail/while_loop.rs
+++ b/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() {}
+ // */
+}