Skip to content

Commit

Permalink
fix redundant_pattern_matching lint
Browse files Browse the repository at this point in the history
- now it gives correct suggestion in case of macros
- better tests
- remove a couple of non-relevant tests
  • Loading branch information
alex-700 committed Apr 25, 2020
1 parent 77c23b7 commit 69fe6b4
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 46 deletions.
17 changes: 12 additions & 5 deletions clippy_lints/src/redundant_pattern_matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,21 @@ fn find_sugg_for_if_let<'a, 'tcx>(
arms[0].pat.span,
&format!("redundant pattern matching, consider using `{}`", good_method),
|diag| {
// 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());
// while let ... = ... { ... }
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^
let expr_span = expr.span;

// while let ... = ... { ... }
// ^^^
let op_span = op.span.source_callsite();

// while let ... = ... { ... }
// ^^^^^^^^^^^^^^^^^^^
let span = expr_span.until(op_span.shrink_to_hi());
diag.span_suggestion(
span,
"try this",
format!("{} {}.{}", keyword, snippet(cx, op.span, "_"), good_method),
format!("{} {}.{}", keyword, snippet(cx, op_span, "_"), good_method),
Applicability::MachineApplicable, // snippet
);
},
Expand Down
58 changes: 42 additions & 16 deletions tests/ui/redundant_pattern_matching.fixed
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, clippy::needless_bool)]
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]

fn main() {
if Ok::<i32, i32>(42).is_ok() {}
Expand Down Expand Up @@ -62,12 +62,31 @@ fn main() {

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

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

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

issue5504();

let _ = if gen_opt().is_some() {
1
} else if gen_opt().is_none() {
2
} else if gen_res().is_ok() {
3
} else if gen_res().is_err() {
4
} else {
5
};
}

fn gen_opt() -> Option<()> {
None
}

fn gen_res() -> Result<(), ()> {
Ok(())
}

fn takes_bool(_: bool) {}
Expand All @@ -76,18 +95,25 @@ fn foo() {}

fn bar() {}

fn does_something() -> bool {
if Ok::<i32, i32>(4).is_ok() {
true
} else {
false
}
macro_rules! m {
() => {
Some(42u32)
};
}

fn returns_unit() {
if Ok::<i32, i32>(4).is_ok() {
true
} else {
false
};
fn issue5504() {
fn result_opt() -> Result<Option<i32>, i32> {
Err(42)
}

fn try_result_opt() -> Result<i32, i32> {
while r#try!(result_opt()).is_some() {}
if r#try!(result_opt()).is_some() {}
Ok(42)
}

try_result_opt();

if m!().is_some() {}
while m!().is_some() {}
}
58 changes: 42 additions & 16 deletions tests/ui/redundant_pattern_matching.rs
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, clippy::needless_bool)]
#![allow(clippy::unit_arg, unused_must_use, clippy::needless_bool, deprecated)]

fn main() {
if let Ok(_) = Ok::<i32, i32>(42) {}
Expand Down Expand Up @@ -83,12 +83,31 @@ fn main() {

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);

issue5504();

let _ = if let Some(_) = gen_opt() {
1
} else if let None = gen_opt() {
2
} else if let Ok(_) = gen_res() {
3
} else if let Err(_) = gen_res() {
4
} else {
5
};
}

fn gen_opt() -> Option<()> {
None
}

fn gen_res() -> Result<(), ()> {
Ok(())
}

fn takes_bool(_: bool) {}
Expand All @@ -97,18 +116,25 @@ fn foo() {}

fn bar() {}

fn does_something() -> bool {
if let Ok(_) = Ok::<i32, i32>(4) {
true
} else {
false
}
macro_rules! m {
() => {
Some(42u32)
};
}

fn returns_unit() {
if let Ok(_) = Ok::<i32, i32>(4) {
true
} else {
false
};
fn issue5504() {
fn result_opt() -> Result<Option<i32>, i32> {
Err(42)
}

fn try_result_opt() -> Result<i32, i32> {
while let Some(_) = r#try!(result_opt()) {}
if let Some(_) = r#try!(result_opt()) {}
Ok(42)
}

try_result_opt();

if let Some(_) = m!() {}
while let Some(_) = m!() {}
}
54 changes: 45 additions & 9 deletions tests/ui/redundant_pattern_matching.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -137,22 +137,58 @@ LL | let _ = if let Ok(_) = Ok::<usize, ()>(4) { true } else { false };
| -------^^^^^--------------------- help: try this: `if Ok::<usize, ()>(4).is_ok()`

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

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

error: redundant pattern matching, consider using `is_none()`
--> $DIR/redundant_pattern_matching.rs:94:19
|
LL | if let Ok(_) = Ok::<i32, i32>(4) {
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
LL | } else if let None = gen_opt() {
| -------^^^^------------ help: try this: `if gen_opt().is_none()`

error: redundant pattern matching, consider using `is_ok()`
--> $DIR/redundant_pattern_matching.rs:109:12
--> $DIR/redundant_pattern_matching.rs:96:19
|
LL | } else if let Ok(_) = gen_res() {
| -------^^^^^------------ help: try this: `if gen_res().is_ok()`

error: redundant pattern matching, consider using `is_err()`
--> $DIR/redundant_pattern_matching.rs:98:19
|
LL | } else if let Err(_) = gen_res() {
| -------^^^^^^------------ help: try this: `if gen_res().is_err()`

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:131:19
|
LL | while let Some(_) = r#try!(result_opt()) {}
| ----------^^^^^^^----------------------- help: try this: `while r#try!(result_opt()).is_some()`

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

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

error: redundant pattern matching, consider using `is_some()`
--> $DIR/redundant_pattern_matching.rs:139:15
|
LL | if let Ok(_) = Ok::<i32, i32>(4) {
| -------^^^^^-------------------- help: try this: `if Ok::<i32, i32>(4).is_ok()`
LL | while let Some(_) = m!() {}
| ----------^^^^^^^------- help: try this: `while m!().is_some()`

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

0 comments on commit 69fe6b4

Please sign in to comment.