From 69fe6b4c9824c1469b5540302d95d980ff54659e Mon Sep 17 00:00:00 2001 From: Aleksei Latyshev Date: Thu, 23 Apr 2020 11:40:16 +0300 Subject: [PATCH] fix redundant_pattern_matching lint - now it gives correct suggestion in case of macros - better tests - remove a couple of non-relevant tests --- .../src/redundant_pattern_matching.rs | 17 ++++-- tests/ui/redundant_pattern_matching.fixed | 58 ++++++++++++++----- tests/ui/redundant_pattern_matching.rs | 58 ++++++++++++++----- tests/ui/redundant_pattern_matching.stderr | 54 ++++++++++++++--- 4 files changed, 141 insertions(+), 46 deletions(-) diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index 334ceed64c23..7ee298e9833f 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -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 ); }, diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index c58db5493ad0..fc8cb0e747c7 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -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::(42).is_ok() {} @@ -62,12 +62,31 @@ fn main() { let _ = if Ok::(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) {} @@ -76,18 +95,25 @@ fn foo() {} fn bar() {} -fn does_something() -> bool { - if Ok::(4).is_ok() { - true - } else { - false - } +macro_rules! m { + () => { + Some(42u32) + }; } -fn returns_unit() { - if Ok::(4).is_ok() { - true - } else { - false - }; +fn issue5504() { + fn result_opt() -> Result, i32> { + Err(42) + } + + fn try_result_opt() -> Result { + 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() {} } diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 9a9b3fb5ca04..51912dade035 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -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::(42) {} @@ -83,12 +83,31 @@ fn main() { let _ = if let Ok(_) = Ok::(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) {} @@ -97,18 +116,25 @@ fn foo() {} fn bar() {} -fn does_something() -> bool { - if let Ok(_) = Ok::(4) { - true - } else { - false - } +macro_rules! m { + () => { + Some(42u32) + }; } -fn returns_unit() { - if let Ok(_) = Ok::(4) { - true - } else { - false - }; +fn issue5504() { + fn result_opt() -> Result, i32> { + Err(42) + } + + fn try_result_opt() -> Result { + 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!() {} } diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index 6285a7f5fcd1..b58deb7954ef 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -137,22 +137,58 @@ LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------- help: try this: `if Ok::(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::(4) { - | -------^^^^^-------------------- help: try this: `if Ok::(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::(4) { - | -------^^^^^-------------------- help: try this: `if Ok::(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