From 28cd1b8faff37ab981dd805aa00263f267263742 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 18 Feb 2019 13:11:58 -0800 Subject: [PATCH 1/3] Fix invalid while_let_on_iterator suggestion Slice patterns without .. are refutable. --- clippy_lints/src/utils/mod.rs | 6 +++++- tests/ui/while_loop.rs | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 0816c209a42d..a2a061cfb437 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -958,7 +958,11 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool { } }, PatKind::Slice(ref head, ref middle, ref tail) => { - are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) + if middle.is_none() { + true + } else { + are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) + } }, } } diff --git a/tests/ui/while_loop.rs b/tests/ui/while_loop.rs index 8c3bf1cc674b..a3e09b082e6a 100644 --- a/tests/ui/while_loop.rs +++ b/tests/ui/while_loop.rs @@ -226,4 +226,14 @@ fn refutable() { let _ = elem.or_else(|| *iter.next()?); } } + + // Issue 3780 + { + let v = vec![1, 2, 3]; + let mut it = v.windows(2); + while let Some([x, y]) = it.next() { + println!("x: {}", x); + println!("y: {}", y); + } + } } From 813f99d881506c6de85d8547ce2dd61587de10fb Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 19 Feb 2019 20:05:44 -0800 Subject: [PATCH 2/3] Add additional refutable cases --- clippy_lints/src/utils/mod.rs | 7 ++----- tests/ui/while_loop.rs | 14 ++++++++++++++ tests/ui/while_loop.stderr | 32 +++++++++++++++++++------------- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index a2a061cfb437..f24c4b92364a 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -958,11 +958,8 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool { } }, PatKind::Slice(ref head, ref middle, ref tail) => { - if middle.is_none() { - true - } else { - are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) - } + // When matching slices, [..] is the only irrefutable slice pattern. + !head.is_empty() || middle.is_none() || !tail.is_empty() }, } } diff --git a/tests/ui/while_loop.rs b/tests/ui/while_loop.rs index a3e09b082e6a..d26ef1f64740 100644 --- a/tests/ui/while_loop.rs +++ b/tests/ui/while_loop.rs @@ -1,5 +1,6 @@ #![warn(clippy::while_let_loop, clippy::empty_loop, clippy::while_let_on_iterator)] #![allow(dead_code, clippy::never_loop, unused, clippy::cyclomatic_complexity)] +#![feature(slice_patterns)] fn main() { let y = Some(true); @@ -235,5 +236,18 @@ fn refutable() { println!("x: {}", x); println!("y: {}", y); } + + let mut it = v.windows(2); + while let Some([x, ..]) = it.next() { + println!("x: {}", x); + } + + let mut it = v.windows(2); + while let Some([.., y]) = it.next() { + println!("y: {}", y); + } + + let mut it = v.windows(2); + while let Some([..]) = it.next() {} } } diff --git a/tests/ui/while_loop.stderr b/tests/ui/while_loop.stderr index dde98da46c33..00f29bf45c68 100644 --- a/tests/ui/while_loop.stderr +++ b/tests/ui/while_loop.stderr @@ -1,5 +1,5 @@ error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:6:5 + --> $DIR/while_loop.rs:7:5 | LL | / loop { LL | | if let Some(_x) = y { @@ -13,7 +13,7 @@ LL | | } = note: `-D clippy::while-let-loop` implied by `-D warnings` error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:20:5 + --> $DIR/while_loop.rs:21:5 | LL | / loop { LL | | match y { @@ -24,7 +24,7 @@ LL | | } | |_____^ help: try: `while let Some(_x) = y { .. }` error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:26:5 + --> $DIR/while_loop.rs:27:5 | LL | / loop { LL | | let x = match y { @@ -36,7 +36,7 @@ LL | | } | |_____^ help: try: `while let Some(x) = y { .. }` error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:34:5 + --> $DIR/while_loop.rs:35:5 | LL | / loop { LL | | let x = match y { @@ -48,7 +48,7 @@ LL | | } | |_____^ help: try: `while let Some(x) = y { .. }` error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:62:5 + --> $DIR/while_loop.rs:63:5 | LL | / loop { LL | | let (e, l) = match "".split_whitespace().next() { @@ -60,7 +60,7 @@ LL | | } | |_____^ help: try: `while let Some(word) = "".split_whitespace().next() { .. }` error: this loop could be written as a `for` loop - --> $DIR/while_loop.rs:72:33 + --> $DIR/while_loop.rs:73:33 | LL | while let Option::Some(x) = iter.next() { | ^^^^^^^^^^^ help: try: `for x in iter { .. }` @@ -68,19 +68,19 @@ LL | while let Option::Some(x) = iter.next() { = note: `-D clippy::while-let-on-iterator` implied by `-D warnings` error: this loop could be written as a `for` loop - --> $DIR/while_loop.rs:77:25 + --> $DIR/while_loop.rs:78:25 | LL | while let Some(x) = iter.next() { | ^^^^^^^^^^^ help: try: `for x in iter { .. }` error: this loop could be written as a `for` loop - --> $DIR/while_loop.rs:82:25 + --> $DIR/while_loop.rs:83:25 | LL | while let Some(_) = iter.next() {} | ^^^^^^^^^^^ help: try: `for _ in iter { .. }` error: this loop could be written as a `while let` loop - --> $DIR/while_loop.rs:125:5 + --> $DIR/while_loop.rs:126:5 | LL | / loop { LL | | let _ = match iter.next() { @@ -92,7 +92,7 @@ LL | | } | |_____^ help: try: `while let Some(ele) = iter.next() { .. }` error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body. - --> $DIR/while_loop.rs:130:9 + --> $DIR/while_loop.rs:131:9 | LL | loop {} | ^^^^^^^ @@ -100,16 +100,22 @@ LL | loop {} = note: `-D clippy::empty-loop` implied by `-D warnings` error: this loop could be written as a `for` loop - --> $DIR/while_loop.rs:188:29 + --> $DIR/while_loop.rs:189:29 | LL | while let Some(v) = y.next() { | ^^^^^^^^ help: try: `for v in y { .. }` error: this loop could be written as a `for` loop - --> $DIR/while_loop.rs:216:26 + --> $DIR/while_loop.rs:217:26 | LL | while let Some(..) = values.iter().next() { | ^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in values.iter() { .. }` -error: aborting due to 12 previous errors +error: this loop could be written as a `for` loop + --> $DIR/while_loop.rs:251:32 + | +LL | while let Some([..]) = it.next() {} + | ^^^^^^^^^ help: try: `for [..] in it { .. }` + +error: aborting due to 13 previous errors From ff37d0a198c84198aec2c788c44d97ccc09e2092 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 24 Feb 2019 18:57:43 -0800 Subject: [PATCH 3/3] Handle slice pattern matching arrays --- clippy_lints/src/utils/mod.rs | 12 ++++++++++-- tests/ui/while_loop.rs | 7 +++++++ tests/ui/while_loop.stderr | 8 +++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index f24c4b92364a..3e2b77178ea4 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -958,8 +958,16 @@ pub fn is_refutable(cx: &LateContext<'_, '_>, pat: &Pat) -> bool { } }, PatKind::Slice(ref head, ref middle, ref tail) => { - // When matching slices, [..] is the only irrefutable slice pattern. - !head.is_empty() || middle.is_none() || !tail.is_empty() + match &cx.tables.node_type(pat.hir_id).sty { + ty::Slice(..) => { + // [..] is the only irrefutable slice pattern. + !head.is_empty() || middle.is_none() || !tail.is_empty() + }, + _ => { + // Assume it's an array. + are_refutable(cx, head.iter().chain(middle).chain(tail.iter()).map(|pat| &**pat)) + }, + } }, } } diff --git a/tests/ui/while_loop.rs b/tests/ui/while_loop.rs index d26ef1f64740..d9769baaf433 100644 --- a/tests/ui/while_loop.rs +++ b/tests/ui/while_loop.rs @@ -249,5 +249,12 @@ fn refutable() { let mut it = v.windows(2); while let Some([..]) = it.next() {} + + let v = vec![[1], [2], [3]]; + let mut it = v.iter(); + while let Some([1]) = it.next() {} + + let mut it = v.iter(); + while let Some([x]) = it.next() {} } } diff --git a/tests/ui/while_loop.stderr b/tests/ui/while_loop.stderr index 00f29bf45c68..72a56c3f7064 100644 --- a/tests/ui/while_loop.stderr +++ b/tests/ui/while_loop.stderr @@ -117,5 +117,11 @@ error: this loop could be written as a `for` loop LL | while let Some([..]) = it.next() {} | ^^^^^^^^^ help: try: `for [..] in it { .. }` -error: aborting due to 13 previous errors +error: this loop could be written as a `for` loop + --> $DIR/while_loop.rs:258:31 + | +LL | while let Some([x]) = it.next() {} + | ^^^^^^^^^ help: try: `for [x] in it { .. }` + +error: aborting due to 14 previous errors