Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some suggestions for redundant_pattern_matching #4352

Merged
merged 3 commits into from Aug 21, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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),
This conversation was marked as resolved by phansch

This comment has been minimized.

Copy link
@phansch

phansch Aug 8, 2019

Author Member

Before the force-push this was format!("if {}.{} {{}}", .... It's now format!("{}.{}", ...

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
);
},
);
@@ -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::<i32, i32>(42) {}
@@ -51,4 +52,39 @@ fn main() {
Some(_) => false,
None => true,
};

let _ = match None::<()> {
Some(_) => false,
None => true,
};

let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };

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::<i32, i32>(4) {
true
} else {
false
}
}

fn returns_unit() {
if let Ok(_) = Ok::<i32, i32>(4) {
true
} else {
false
};
}
@@ -1,31 +1,31 @@
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::<i32, i32>(42) {}
| -------^^^^^------------------------ help: try this: `if Ok::<i32, i32>(42).is_ok()`
| -------^^^^^------------------------ help: try this: `Ok::<i32, i32>(42).is_ok()`
This conversation was marked as resolved by flip1995

This comment has been minimized.

Copy link
@flip1995

flip1995 Aug 9, 2019

Member

Not for this PR, but applying this suggestion results in a syntax error, since the ; is missing. Not sure how to solve this, since on a let _ = if ...; the ; is already there, on a returning if the ; shouldn't be added, and on a stray if it has to be added.

|
= 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::<i32, i32>(42) {}
| -------^^^^^^------------------------- help: try this: `if Err::<i32, i32>(42).is_err()`
| -------^^^^^^------------------------- help: try this: `Err::<i32, i32>(42).is_err()`
This conversation was marked as resolved by flip1995

This comment has been minimized.

Copy link
@flip1995

flip1995 Aug 9, 2019

Member

Well now this is semantically wrong, since the if let returns the unit type () and the suggestion a bool. Is there an easy way to differentiate between those two cases and in one case suggest if _.is_none() {} and in the other _.is_none()? Otherwise we can open an "Steps-to-fix-the-redundant_pattern_matching-suggestion" issue.

This comment has been minimized.

Copy link
@phansch

phansch Aug 21, 2019

Author Member

I'd much rather fix the problems in a separate PR. This is now already marked as MaybeIncorrect so we can focus on fixing other low-hanging fruit before cargo fix --clippy becomes available on stable.

I'll create an issue for the improvements add a comment to #4344 👍

This comment has been minimized.

Copy link
@flip1995

flip1995 Aug 21, 2019

Member

Ok, I added the comment, for both fixes, that need to be made for this to be MachineApplicable

This comment has been minimized.

Copy link
@phansch

phansch Aug 21, 2019

Author Member

Thanks! Removing it from my todo list then 😅


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: `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
--> $DIR/redundant_pattern_matching.rs:12: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
--> $DIR/redundant_pattern_matching.rs:26:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => true,
@@ -34,7 +34,7 @@ LL | | };
| |_____^ help: try this: `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:31:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => false,
@@ -43,7 +43,7 @@ LL | | };
| |_____^ help: try this: `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:36:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => false,
@@ -52,7 +52,7 @@ LL | | };
| |_____^ help: try this: `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:41:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => true,
@@ -61,7 +61,7 @@ LL | | };
| |_____^ help: try this: `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:46:5
|
LL | / match Some(42) {
LL | | Some(_) => true,
@@ -70,13 +70,63 @@ 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,
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:13
|
LL | let _ = 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::<usize, ()>(4) { true } else { false };
| -------^^^^^--------------------------------------------- help: try this: `Ok::<usize, ()>(4).is_ok()`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:67:20
|
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
|
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:77:12
|
LL | if let Ok(_) = Ok::<i32, i32>(4) {
| _____- ^^^^^
LL | | true
LL | | } else {
LL | | false
LL | | }
| |_____- help: try this: `Ok::<i32, i32>(4).is_ok()`

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

error: aborting due to 16 previous errors

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.