From 0d85d7e60f393563d717d86fbef05f7309923a08 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 7 Aug 2019 20:34:23 +0200 Subject: [PATCH 1/3] Fix suggestions for redundant_pattern_matching Fixes the problem displayed in https://github.com/rust-lang/rust-clippy/issues/4344#issuecomment-519206388. We now append `{}` to the suggestion so that the conditional has the correct syntax again. (If we were to _remove_ the `if` instead, it would trigger the `unused_must_use` warning for `#[must_use]` types. --- clippy_lints/src/redundant_pattern_matching.rs | 6 +++--- tests/ui/redundant_pattern_matching.stderr | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index ce33ebf87ba..68862f838cb 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -90,8 +90,8 @@ fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, db.span_suggestion( span, "try this", - format!("if {}.{}", snippet(cx, op.span, "_"), good_method), - Applicability::MachineApplicable, // snippet + format!("{}.{}", snippet(cx, op.span, "_"), good_method), + Applicability::MaybeIncorrect, // snippet ); }, ); @@ -154,7 +154,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, o span, "try this", format!("{}.{}", snippet(cx, op.span, "_"), good_method), - Applicability::MachineApplicable, // snippet + Applicability::MaybeIncorrect, // snippet ); }, ); diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index baed95d6f2e..a904c5ceb9d 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -2,7 +2,7 @@ error: redundant pattern matching, consider using `is_ok()` --> $DIR/redundant_pattern_matching.rs:5:12 | LL | if let Ok(_) = Ok::(42) {} - | -------^^^^^------------------------ help: try this: `if Ok::(42).is_ok()` + | -------^^^^^------------------------ help: try this: `Ok::(42).is_ok()` | = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` @@ -10,19 +10,19 @@ error: redundant pattern matching, consider using `is_err()` --> $DIR/redundant_pattern_matching.rs:7:12 | LL | if let Err(_) = Err::(42) {} - | -------^^^^^^------------------------- help: try this: `if Err::(42).is_err()` + | -------^^^^^^------------------------- help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_none()` --> $DIR/redundant_pattern_matching.rs:9:12 | LL | if let None = None::<()> {} - | -------^^^^---------------- help: try this: `if None::<()>.is_none()` + | -------^^^^---------------- help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` --> $DIR/redundant_pattern_matching.rs:11:12 | LL | if let Some(_) = Some(42) {} - | -------^^^^^^^-------------- help: try this: `if Some(42).is_some()` + | -------^^^^^^^-------------- help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_ok()` --> $DIR/redundant_pattern_matching.rs:25:5 From 84716e49f0a93c0ddbab8701a44835a0041e6ffc Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Thu, 8 Aug 2019 21:27:50 +0200 Subject: [PATCH 2/3] Add more testcases for redundant_pattern_matching These should make sure that, when the suggestions are fixed, they are fixed for all these cases. --- tests/ui/redundant_pattern_matching.rs | 26 +++++++++ tests/ui/redundant_pattern_matching.stderr | 68 ++++++++++++++++++---- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 8e8d4b59ba4..74dd12edfb6 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -51,4 +51,30 @@ fn main() { Some(_) => false, None => true, }; + + let _ = match None::<()> { + Some(_) => false, + None => true, + }; + + let _ = if let Ok(_) = Ok::(4) { true } else { false }; + + let _ = does_something(); + let _ = returns_unit(); +} + +fn does_something() -> bool { + if let Ok(_) = Ok::(4) { + true + } else { + false + } +} + +fn returns_unit() { + if let Ok(_) = Ok::(4) { + true + } else { + false + }; } diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index a904c5ceb9d..e3ba152d58a 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -1,5 +1,5 @@ error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:5:12 + --> $DIR/redundant_pattern_matching.rs:6:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^------------------------ help: try this: `Ok::(42).is_ok()` @@ -7,25 +7,25 @@ LL | if let Ok(_) = Ok::(42) {} = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:7:12 + --> $DIR/redundant_pattern_matching.rs:8:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^------------------------- help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:9:12 + --> $DIR/redundant_pattern_matching.rs:10:12 | LL | if let None = None::<()> {} | -------^^^^---------------- help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:11:12 + --> $DIR/redundant_pattern_matching.rs:12:12 | LL | if let Some(_) = Some(42) {} | -------^^^^^^^-------------- help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:25:5 + --> $DIR/redundant_pattern_matching.rs:26:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -34,7 +34,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:30:5 + --> $DIR/redundant_pattern_matching.rs:31:5 | LL | / match Ok::(42) { LL | | Ok(_) => false, @@ -43,7 +43,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_err()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:35:5 + --> $DIR/redundant_pattern_matching.rs:36:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -52,7 +52,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:40:5 + --> $DIR/redundant_pattern_matching.rs:41:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -61,7 +61,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:45:5 + --> $DIR/redundant_pattern_matching.rs:46:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -70,7 +70,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:50:5 + --> $DIR/redundant_pattern_matching.rs:51:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -78,5 +78,51 @@ LL | | None => true, LL | | }; | |_____^ help: try this: `None::<()>.is_none()` -error: aborting due to 10 previous errors +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching.rs:56:15 + | +LL | let foo = match None::<()> { + | _______________^ +LL | | Some(_) => false, +LL | | None => true, +LL | | }; + | |_____^ help: try this: `None::<()>.is_none()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:61:20 + | +LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; + | -------^^^^^--------------------------------------------- help: try this: `Ok::(4).is_ok()` + +error: this let-binding has unit value + --> $DIR/redundant_pattern_matching.rs:64:5 + | +LL | let _ = returns_unit(); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `returns_unit();` + | + = note: `-D clippy::let-unit-value` implied by `-D warnings` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:68:12 + | +LL | if let Ok(_) = Ok::(4) { + | _____- ^^^^^ +LL | | true +LL | | } else { +LL | | false +LL | | } + | |_____- help: try this: `Ok::(4).is_ok()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching.rs:76:12 + | +LL | if let Ok(_) = Ok::(4) { + | _____- ^^^^^ +LL | | true +LL | | } else { +LL | | false +LL | | }; + | |_____- help: try this: `Ok::(4).is_ok()` + +error: aborting due to 15 previous errors From 436d429d27ba3ce1a9a5ca3ae4074ac75ab11c2c Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Wed, 21 Aug 2019 07:43:42 +0200 Subject: [PATCH 3/3] Add two more tests, allow 2 other lints. --- tests/ui/redundant_pattern_matching.rs | 10 +++++++++ tests/ui/redundant_pattern_matching.stderr | 26 +++++++++++++--------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 74dd12edfb6..2bea7d8d961 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -1,5 +1,6 @@ #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] +#![allow(clippy::unit_arg, clippy::let_unit_value)] fn main() { if let Ok(_) = Ok::(42) {} @@ -61,8 +62,17 @@ fn main() { let _ = does_something(); let _ = returns_unit(); + + let opt = Some(false); + let x = if let Some(_) = opt { true } else { false }; + takes_bool(x); + let y = if let Some(_) = opt {}; + takes_unit(y); } +fn takes_bool(x: bool) {} +fn takes_unit(x: ()) {} + fn does_something() -> bool { if let Ok(_) = Ok::(4) { true diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index e3ba152d58a..df12b7e169a 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -79,10 +79,10 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:56:15 + --> $DIR/redundant_pattern_matching.rs:56:13 | -LL | let foo = match None::<()> { - | _______________^ +LL | let _ = match None::<()> { + | _____________^ LL | | Some(_) => false, LL | | None => true, LL | | }; @@ -94,16 +94,20 @@ error: redundant pattern matching, consider using `is_ok()` LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------------------------------- help: try this: `Ok::(4).is_ok()` -error: this let-binding has unit value - --> $DIR/redundant_pattern_matching.rs:64:5 +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:67:20 | -LL | let _ = returns_unit(); - | ^^^^^^^^^^^^^^^^^^^^^^^ help: omit the `let` binding: `returns_unit();` +LL | let x = if let Some(_) = opt { true } else { false }; + | -------^^^^^^^------------------------------ help: try this: `opt.is_some()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching.rs:69:20 | - = note: `-D clippy::let-unit-value` implied by `-D warnings` +LL | let y = if let Some(_) = opt {}; + | -------^^^^^^^--------- help: try this: `opt.is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:68:12 + --> $DIR/redundant_pattern_matching.rs:77:12 | LL | if let Ok(_) = Ok::(4) { | _____- ^^^^^ @@ -114,7 +118,7 @@ LL | | } | |_____- help: try this: `Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:76:12 + --> $DIR/redundant_pattern_matching.rs:85:12 | LL | if let Ok(_) = Ok::(4) { | _____- ^^^^^ @@ -124,5 +128,5 @@ LL | | false LL | | }; | |_____- help: try this: `Ok::(4).is_ok()` -error: aborting due to 15 previous errors +error: aborting due to 16 previous errors