Skip to content

Commit

Permalink
Auto merge of rust-lang#5483 - alex-700:fix-redundant-pattern-matchin…
Browse files Browse the repository at this point in the history 
…g, r=flip1995

fix redundant_pattern_matching lint

- now it handles `while let` case  (related to rust-lang#5462)
- better suggestions in `if let` case

changelog: Fix suggestion in `redundant_pattern_matching` and also apply this lint to the `while let` case
  • Loading branch information
@bors
bors committed Apr 17, 2020
2 parents f1fb815 + 092c459 commit 1c0e4e5
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 51 deletions.
30 changes: 21 additions & 9 deletions clippy_lints/src/redundant_pattern_matching.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
use crate::utils::{match_qpath, match_trait_method, paths, snippet, span_lint_and_then};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Arm, Expr, ExprKind, MatchSource, PatKind, QPath};
Expand Down Expand Up @@ -48,9 +49,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching {
if let ExprKind::Match(op, arms, ref match_source) = &expr.kind {
match match_source {
MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms),
MatchSource::IfLetDesugar { contains_else_clause } => {
find_sugg_for_if_let(cx, expr, op, arms, *contains_else_clause)
},
MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms, "if"),
MatchSource::WhileLetDesugar => find_sugg_for_if_let(cx, expr, op, arms, "while"),
_ => return,
}
}
Expand All @@ -62,7 +62,7 @@ fn find_sugg_for_if_let<'a, 'tcx>(
expr: &'tcx Expr<'_>,
op: &Expr<'_>,
arms: &[Arm<'_>],
has_else: bool,
keyword: &'static str,
) {
let good_method = match arms[0].pat.kind {
PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => {
Expand All @@ -86,20 +86,32 @@ fn find_sugg_for_if_let<'a, 'tcx>(
_ => return,
};

let maybe_semi = if has_else { "" } else { ";" };
// check that `while_let_on_iterator` lint does not trigger
if_chain! {
if keyword == "while";
if let ExprKind::MethodCall(method_path, _, _) = op.kind;
if method_path.ident.name == sym!(next);
if match_trait_method(cx, op, &paths::ITERATOR);
then {
return;
}
}

span_lint_and_then(
cx,
REDUNDANT_PATTERN_MATCHING,
arms[0].pat.span,
&format!("redundant pattern matching, consider using `{}`", good_method),
|diag| {
let span = expr.span.to(op.span);
// in the case of WhileLetDesugar expr.span == op.span incorrectly.
// this is a workaround to restore true value of expr.span
let expr_span = expr.span.to(arms[1].span);
let span = expr_span.until(op.span.shrink_to_hi());
diag.span_suggestion(
span,
"try this",
format!("{}.{}{}", snippet(cx, op.span, "_"), good_method, maybe_semi),
Applicability::MaybeIncorrect, // snippet
format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
);
Expand Down
51 changes: 42 additions & 9 deletions tests/ui/redundant_pattern_matching.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,37 @@

#![warn(clippy::all)]
#![warn(clippy::redundant_pattern_matching)]
#![allow(clippy::unit_arg, unused_must_use)]
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]

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

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

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

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

if Some(42).is_some() {
foo();
} else {
bar();
}

while Some(42).is_some() {}

while Some(42).is_none() {}

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

while Ok::<i32, i32>(10).is_ok() {}

while Ok::<i32, i32>(10).is_err() {}

let mut v = vec![1, 2, 3];
while v.pop().is_some() {
foo();
}

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

Expand Down Expand Up @@ -39,22 +60,34 @@ fn main() {

let _ = None::<()>.is_none();

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

let _ = does_something();
let _ = returns_unit();

let opt = Some(false);
let x = opt.is_some();
let x = if opt.is_some() { true } else { false };
takes_bool(x);
}

fn takes_bool(_: bool) {}

fn foo() {}

fn bar() {}

fn does_something() -> bool {
Ok::<i32, i32>(4).is_ok()
if Ok::<i32, i32>(4).is_ok() {
true
} else {
false
}
}

fn returns_unit() {
Ok::<i32, i32>(4).is_ok();
if Ok::<i32, i32>(4).is_ok() {
true
} else {
false
};
}
27 changes: 26 additions & 1 deletion tests/ui/redundant_pattern_matching.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#![warn(clippy::all)]
#![warn(clippy::redundant_pattern_matching)]
#![allow(clippy::unit_arg, unused_must_use)]
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool)]

fn main() {
if let Ok(_) = Ok::<i32, i32>(42) {}
Expand All @@ -13,6 +13,27 @@ fn main() {

if let Some(_) = Some(42) {}

if let Some(_) = Some(42) {
foo();
} else {
bar();
}

while let Some(_) = Some(42) {}

while let None = Some(42) {}

while let None = None::<()> {}

while let Ok(_) = Ok::<i32, i32>(10) {}

while let Err(_) = Ok::<i32, i32>(10) {}

let mut v = vec![1, 2, 3];
while let Some(_) = v.pop() {
foo();
}

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

if Err::<i32, i32>(42).is_err() {}
Expand Down Expand Up @@ -72,6 +93,10 @@ fn main() {

fn takes_bool(_: bool) {}

fn foo() {}

fn bar() {}

fn does_something() -> bool {
if let Ok(_) = Ok::<i32, i32>(4) {
true
Expand Down
96 changes: 64 additions & 32 deletions tests/ui/redundant_pattern_matching.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,72 @@ error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:8:12
|
LL | if let Ok(_) = Ok::<i32, i32>(42) {}
| -------^^^^^------------------------ help: try this: `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:10:12
|
LL | if let Err(_) = Err::<i32, i32>(42) {}
| -------^^^^^^------------------------- 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_none()`
--> $DIR/redundant_pattern_matching.rs:12:12
|
LL | if let None = None::<()> {}
| -------^^^^---------------- help: try this: `None::<()>.is_none();`
| -------^^^^------------- help: try this: `if None::<()>.is_none()`

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

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

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

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:24:15
|
LL | while let None = Some(42) {}
| ----------^^^^----------- help: try this: `while Some(42).is_none()`

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

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:28:5
--> $DIR/redundant_pattern_matching.rs:28:15
|
LL | while let Ok(_) = Ok::<i32, i32>(10) {}
| ----------^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_ok()`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:30:15
|
LL | while let Err(_) = Ok::<i32, i32>(10) {}
| ----------^^^^^^--------------------- help: try this: `while Ok::<i32, i32>(10).is_err()`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:33:15
|
LL | while let Some(_) = v.pop() {
| ----------^^^^^^^---------- help: try this: `while v.pop().is_some()`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:49:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => true,
Expand All @@ -34,7 +76,7 @@ LL | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_ok()`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:33:5
--> $DIR/redundant_pattern_matching.rs:54:5
|
LL | / match Ok::<i32, i32>(42) {
LL | | Ok(_) => false,
Expand All @@ -43,7 +85,7 @@ LL | | };
| |_____^ help: try this: `Ok::<i32, i32>(42).is_err()`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:38:5
--> $DIR/redundant_pattern_matching.rs:59:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => false,
Expand All @@ -52,7 +94,7 @@ LL | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_err()`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:43:5
--> $DIR/redundant_pattern_matching.rs:64:5
|
LL | / match Err::<i32, i32>(42) {
LL | | Ok(_) => true,
Expand All @@ -61,7 +103,7 @@ LL | | };
| |_____^ help: try this: `Err::<i32, i32>(42).is_ok()`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:48:5
--> $DIR/redundant_pattern_matching.rs:69:5
|
LL | / match Some(42) {
LL | | Some(_) => true,
Expand All @@ -70,7 +112,7 @@ LL | | };
| |_____^ help: try this: `Some(42).is_some()`

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:53:5
--> $DIR/redundant_pattern_matching.rs:74:5
|
LL | / match None::<()> {
LL | | Some(_) => false,
Expand All @@ -79,7 +121,7 @@ LL | | };
| |_____^ help: try this: `None::<()>.is_none()`

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:58:13
--> $DIR/redundant_pattern_matching.rs:79:13
|
LL | let _ = match None::<()> {
| _____________^
Expand All @@ -89,38 +131,28 @@ LL | | };
| |_____^ help: try this: `None::<()>.is_none()`

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

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:69:20
--> $DIR/redundant_pattern_matching.rs:90:20
|
LL | let x = if let Some(_) = opt { true } else { false };
| -------^^^^^^^------------------------------ help: try this: `opt.is_some()`
| -------^^^^^^^------ help: try this: `if opt.is_some()`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:76:12
--> $DIR/redundant_pattern_matching.rs:101: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()`
LL | if let Ok(_) = Ok::<i32, i32>(4) {
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:84:12
--> $DIR/redundant_pattern_matching.rs:109: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()`
LL | if let Ok(_) = Ok::<i32, i32>(4) {
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`

error: aborting due to 15 previous errors
error: aborting due to 22 previous errors

0 comments on commit 1c0e4e5

Please sign in to comment.