Skip to content

Commit

Permalink
Fix suggestions for redundant_pattern_matching
Browse files Browse the repository at this point in the history
Fixes the problem displayed in rust-lang#4344 (comment).

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.
  • Loading branch information
phansch committed Aug 7, 2019
1 parent ea26a95 commit 85a736c
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 23 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/redundant_pattern_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ 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),
format!("if {}.{} {{}}", snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
Expand Down Expand Up @@ -153,7 +153,7 @@ fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, o
db.span_suggestion(
span,
"try this",
format!("{}.{}", snippet(cx, op.span, "_"), good_method),
format!("if {}.{} {{}}", snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
Expand Down
41 changes: 41 additions & 0 deletions tests/ui/redundant_pattern_matching.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// run-rustfix

#![warn(clippy::all)]
#![warn(clippy::redundant_pattern_matching)]

fn main() {
if Ok::<i32, i32>(42).is_ok() {}

if Err::<i32, i32>(42).is_err() {}

if None::<()>.is_none() {}

if Some(42).is_some() {}

if Ok::<i32, i32>(42).is_ok() {}

if Err::<i32, i32>(42).is_err() {}

if None::<i32>.is_none() {}

if Some(42).is_some() {}

if let Ok(x) = Ok::<i32, i32>(42) {
println!("{}", x);
}

if Ok::<i32, i32>(42).is_ok() {};

if Ok::<i32, i32>(42).is_err() {};

if Err::<i32, i32>(42).is_err() {};

if Err::<i32, i32>(42).is_ok() {};

if Some(42).is_some() {};

if None::<()>.is_none() {};

let bla: Result<usize, ()> = Ok(4);
if bla.is_ok() {};
}
9 changes: 9 additions & 0 deletions tests/ui/redundant_pattern_matching.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// run-rustfix

#![warn(clippy::all)]
#![warn(clippy::redundant_pattern_matching)]

Expand Down Expand Up @@ -51,4 +53,11 @@ fn main() {
Some(_) => false,
None => true,
};

let bla: Result<usize, ()> = Ok(4);
if let Ok(_) = bla {
true
} else {
false
};
}
53 changes: 32 additions & 21 deletions tests/ui/redundant_pattern_matching.stderr
Original file line number Diff line number Diff line change
@@ -1,82 +1,93 @@
error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:5:12
--> $DIR/redundant_pattern_matching.rs:7:12
|
LL | if let Ok(_) = Ok::<i32, i32>(42) {}
| -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok()`
| -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok() {}`
|
= 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:9:12
|
LL | if let Err(_) = Err::<i32, i32>(42) {}
| -------^^^^^^------------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
| -------^^^^^^------------------------- help: try this: `if Err::<i32, i32>(42).is_err() {}`

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:9:12
--> $DIR/redundant_pattern_matching.rs:11:12
|
LL | if let None = None::<()> {}
| -------^^^^---------------- help: try this: `if None::<()>.is_none()`
| -------^^^^---------------- help: try this: `if None::<()>.is_none() {}`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:11:12
--> $DIR/redundant_pattern_matching.rs:13:12
|
LL | if let Some(_) = Some(42) {}
| -------^^^^^^^-------------- help: try this: `if Some(42).is_some()`
| -------^^^^^^^-------------- help: try this: `if Some(42).is_some() {}`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:25:5
--> $DIR/redundant_pattern_matching.rs:27:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => true,
LL | | Err(_) => false,
LL | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`
| |_____^ help: try this: `if Ok::<i32, i32>(42).is_ok() {}`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:30:5
--> $DIR/redundant_pattern_matching.rs:32:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => false,
LL | | Err(_) => true,
LL | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`
| |_____^ help: try this: `if Ok::<i32, i32>(42).is_err() {}`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:35:5
--> $DIR/redundant_pattern_matching.rs:37:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => false,
LL | | Err(_) => true,
LL | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_err()`
| |_____^ help: try this: `if Err::<i32, i32>(42).is_err() {}`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:40:5
--> $DIR/redundant_pattern_matching.rs:42:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => true,
LL | | Err(_) => false,
LL | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`
| |_____^ help: try this: `if Err::<i32, i32>(42).is_ok() {}`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:45:5
--> $DIR/redundant_pattern_matching.rs:47:5
|
LL | / match Some(42) {
LL | | Some(_) => true,
LL | | None => false,
LL | | };
| |_____^ help: try this: `Some(42).is_some()`
| |_____^ help: try this: `if Some(42).is_some() {}`

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:50:5
--> $DIR/redundant_pattern_matching.rs:52:5
|
LL | / match None::<()> {
LL | | Some(_) => false,
LL | | None => true,
LL | | };
| |_____^ help: try this: `None::<()>.is_none()`
| |_____^ help: try this: `if None::<()>.is_none() {}`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:58:12
|
LL | if let Ok(_) = bla {
| _____- ^^^^^
LL | | true
LL | | } else {
LL | | false
LL | | };
| |_____- help: try this: `if bla.is_ok() {}`

error: aborting due to 10 previous errors
error: aborting due to 11 previous errors

0 comments on commit 85a736c

Please sign in to comment.