From afa2741e6a51f94ec408b78bb7589fa0a0534a3c Mon Sep 17 00:00:00 2001 From: disco07 Date: Fri, 5 May 2023 21:33:16 +0200 Subject: [PATCH] redundant_pattern_matching --- .../src/matches/redundant_pattern_match.rs | 91 +++++++++++++++++- .../redundant_pattern_matching_option.fixed | 19 ++++ tests/ui/redundant_pattern_matching_option.rs | 43 +++++++++ .../redundant_pattern_matching_option.stderr | 96 ++++++++++++++++--- .../redundant_pattern_matching_result.fixed | 11 +++ tests/ui/redundant_pattern_matching_result.rs | 23 +++++ .../redundant_pattern_matching_result.stderr | 62 +++++++++--- 7 files changed, 319 insertions(+), 26 deletions(-) diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index af121f317cd1..9656049feeb9 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -186,9 +186,9 @@ fn find_sugg_for_if_let<'tcx>( } pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) { + //eprintln!("{:#?}", expr); if arms.len() == 2 { let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind); - let found_good_method = match node_pair { ( PatKind::TupleStruct(ref path_left, patterns_left, _), @@ -252,6 +252,68 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op None } }, + (PatKind::TupleStruct(ref path_left, patterns, _), PatKind::Wild) if patterns.len() == 1 => { + if let PatKind::Wild = patterns[0].kind { + let ident = match path_left { + QPath::Resolved(_, path) => { + let name = path.segments[0].ident; + Some(name) + }, + _ => None, + }; + if let Some(name) = ident { + match name.as_str() { + "Ok" => find_good_method_for_matches_macro( + cx, + arms, + path_left, + Item::Lang(ResultOk), + "is_ok()", + "is_err()", + ), + "Some" => find_good_method_for_matches_macro( + cx, + arms, + path_left, + Item::Lang(OptionSome), + "is_some()", + "is_none()", + ), + _ => None, + } + } else { + None + } + } else { + None + } + }, + (PatKind::Path(ref path_left), PatKind::Wild) => { + let ident = match path_left { + QPath::Resolved(_, path) => { + let name = path.segments[0].ident; + Some(name) + }, + _ => None, + }; + + if let Some(name) = ident { + match name.as_str() { + "None" => find_good_method_for_matches_macro( + cx, + arms, + path_left, + Item::Lang(OptionNone), + "is_none()", + "is_some()", + ), + _ => None, + } + } else { + None + } + + } _ => None, }; @@ -345,3 +407,30 @@ fn find_good_method_for_match<'a>( _ => None, } } + +#[expect(clippy::too_many_arguments)] +fn find_good_method_for_matches_macro<'a>( + cx: &LateContext<'_>, + arms: &[Arm<'_>], + path_left: &QPath<'_>, + expected_item_left: Item, + should_be_left: &'a str, + should_be_right: &'a str, +) -> Option<&'a str> { + let first_pat = arms[0].pat; + + let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) { + (&arms[0].body.kind, &arms[1].body.kind) + } else { + return None; + }; + + match body_node_pair { + (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) { + (LitKind::Bool(true), LitKind::Bool(false)) => Some(should_be_left), + (LitKind::Bool(false), LitKind::Bool(true)) => Some(should_be_right), + _ => None, + }, + _ => None, + } +} diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed index d62f7d26a35c..c22a4d7456e4 100644 --- a/tests/ui/redundant_pattern_matching_option.fixed +++ b/tests/ui/redundant_pattern_matching_option.fixed @@ -46,6 +46,7 @@ fn main() { let _ = if opt.is_some() { true } else { false }; issue6067(); + issue10726(); let _ = if gen_opt().is_some() { 1 @@ -88,3 +89,21 @@ fn issue7921() { if (&None::<()>).is_none() {} if (&None::<()>).is_none() {} } + +fn issue10726() { + Some(42).is_some(); + + Some(42).is_none(); + + Some(42).is_none(); + + Some(42).is_some(); + + None::<()>.is_none(); + + None::<()>.is_none(); + + None::<()>.is_none(); + + None::<()>.is_some(); +} diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs index d64294265731..cd96e0d29a5a 100644 --- a/tests/ui/redundant_pattern_matching_option.rs +++ b/tests/ui/redundant_pattern_matching_option.rs @@ -55,6 +55,7 @@ fn main() { let _ = if let Some(_) = opt { true } else { false }; issue6067(); + issue10726(); let _ = if let Some(_) = gen_opt() { 1 @@ -103,3 +104,45 @@ fn issue7921() { if let None = *(&None::<()>) {} if let None = *&None::<()> {} } + +fn issue10726() { + match Some(42) { + Some(_) => true, + _ => false, + }; + + match Some(42) { + Some(_) => false, + _ => true, + }; + + match Some(42) { + None => true, + _ => false, + }; + + match Some(42) { + None => false, + _ => true, + }; + + match None::<()> { + Some(_) => false, + _ => true, + }; + + match None::<()> { + Some(_) => false, + _ => true, + }; + + match None::<()> { + None => true, + _ => false, + }; + + match None::<()> { + None => false, + _ => true, + }; +} diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr index 7c5a047e455c..d397297074b4 100644 --- a/tests/ui/redundant_pattern_matching_option.stderr +++ b/tests/ui/redundant_pattern_matching_option.stderr @@ -77,49 +77,49 @@ LL | let _ = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------ help: try this: `if opt.is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:59:20 + --> $DIR/redundant_pattern_matching_option.rs:60: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_option.rs:61:19 + --> $DIR/redundant_pattern_matching_option.rs:62:19 | LL | } else if let None = gen_opt() { | -------^^^^------------ help: try this: `if gen_opt().is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:67:12 + --> $DIR/redundant_pattern_matching_option.rs:68:12 | LL | if let Some(..) = gen_opt() {} | -------^^^^^^^^------------ help: try this: `if gen_opt().is_some()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:82:12 + --> $DIR/redundant_pattern_matching_option.rs:83:12 | LL | if let Some(_) = Some(42) {} | -------^^^^^^^----------- help: try this: `if Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:84:12 + --> $DIR/redundant_pattern_matching_option.rs:85:12 | LL | if let None = None::<()> {} | -------^^^^------------- help: try this: `if None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:86:15 + --> $DIR/redundant_pattern_matching_option.rs:87: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_option.rs:88:15 + --> $DIR/redundant_pattern_matching_option.rs:89:15 | LL | while let None = None::<()> {} | ----------^^^^------------- help: try this: `while None::<()>.is_none()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching_option.rs:90:5 + --> $DIR/redundant_pattern_matching_option.rs:91:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -128,7 +128,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:95:5 + --> $DIR/redundant_pattern_matching_option.rs:96:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -137,16 +137,88 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:103:12 + --> $DIR/redundant_pattern_matching_option.rs:104:12 | LL | if let None = *(&None::<()>) {} | -------^^^^----------------- help: try this: `if (&None::<()>).is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching_option.rs:104:12 + --> $DIR/redundant_pattern_matching_option.rs:105:12 | LL | if let None = *&None::<()> {} | -------^^^^--------------- help: try this: `if (&None::<()>).is_none()` -error: aborting due to 22 previous errors +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching_option.rs:109:5 + | +LL | / match Some(42) { +LL | | Some(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `Some(42).is_some()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:114:5 + | +LL | / match Some(42) { +LL | | Some(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `Some(42).is_none()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:119:5 + | +LL | / match Some(42) { +LL | | None => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `Some(42).is_none()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching_option.rs:124:5 + | +LL | / match Some(42) { +LL | | None => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `Some(42).is_some()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:129:5 + | +LL | / match None::<()> { +LL | | Some(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `None::<()>.is_none()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:134:5 + | +LL | / match None::<()> { +LL | | Some(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `None::<()>.is_none()` + +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:139:5 + | +LL | / match None::<()> { +LL | | None => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `None::<()>.is_none()` + +error: redundant pattern matching, consider using `is_some()` + --> $DIR/redundant_pattern_matching_option.rs:144:5 + | +LL | / match None::<()> { +LL | | None => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `None::<()>.is_some()` + +error: aborting due to 30 previous errors diff --git a/tests/ui/redundant_pattern_matching_result.fixed b/tests/ui/redundant_pattern_matching_result.fixed index c48d1522935c..a51e14a5b568 100644 --- a/tests/ui/redundant_pattern_matching_result.fixed +++ b/tests/ui/redundant_pattern_matching_result.fixed @@ -43,6 +43,7 @@ fn main() { issue5504(); issue6067(); issue6065(); + issue10726(); let _ = if gen_res().is_ok() { 1 @@ -107,3 +108,13 @@ const fn issue6067() { Err::(42).is_err(); } + +fn issue10726() { + Ok::(42).is_ok(); + + Ok::(42).is_err(); + + Err::(42).is_err(); + + Err::(42).is_ok(); +} diff --git a/tests/ui/redundant_pattern_matching_result.rs b/tests/ui/redundant_pattern_matching_result.rs index 26f37d169fac..709e3d526a83 100644 --- a/tests/ui/redundant_pattern_matching_result.rs +++ b/tests/ui/redundant_pattern_matching_result.rs @@ -55,6 +55,7 @@ fn main() { issue5504(); issue6067(); issue6065(); + issue10726(); let _ = if let Ok(_) = gen_res() { 1 @@ -125,3 +126,25 @@ const fn issue6067() { Err(_) => true, }; } + +fn issue10726() { + match Ok::(42) { + Ok(_) => true, + _ => false, + }; + + match Ok::(42) { + Ok(_) => false, + _ => true, + }; + + match Err::(42) { + Ok(_) => false, + _ => true, + }; + + match Err::(42) { + Ok(_) => true, + _ => false, + }; +} diff --git a/tests/ui/redundant_pattern_matching_result.stderr b/tests/ui/redundant_pattern_matching_result.stderr index d6a46babb779..0e8a983bf44a 100644 --- a/tests/ui/redundant_pattern_matching_result.stderr +++ b/tests/ui/redundant_pattern_matching_result.stderr @@ -73,67 +73,67 @@ 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_ok()` - --> $DIR/redundant_pattern_matching_result.rs:59:20 + --> $DIR/redundant_pattern_matching_result.rs:60:20 | LL | let _ = 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_result.rs:61:19 + --> $DIR/redundant_pattern_matching_result.rs:62: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_result.rs:84:19 + --> $DIR/redundant_pattern_matching_result.rs:85: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_result.rs:85:16 + --> $DIR/redundant_pattern_matching_result.rs:86: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_result.rs:91:12 + --> $DIR/redundant_pattern_matching_result.rs:92: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_result.rs:92:15 + --> $DIR/redundant_pattern_matching_result.rs:93:15 | LL | while let Some(_) = m!() {} | ----------^^^^^^^------- help: try this: `while m!().is_some()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:110:12 + --> $DIR/redundant_pattern_matching_result.rs:111:12 | LL | if let Ok(_) = Ok::(42) {} | -------^^^^^--------------------- help: try this: `if Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:112:12 + --> $DIR/redundant_pattern_matching_result.rs:113:12 | LL | if let Err(_) = Err::(42) {} | -------^^^^^^---------------------- help: try this: `if Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:114:15 + --> $DIR/redundant_pattern_matching_result.rs:115:15 | LL | while let Ok(_) = Ok::(10) {} | ----------^^^^^--------------------- help: try this: `while Ok::(10).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:116:15 + --> $DIR/redundant_pattern_matching_result.rs:117:15 | LL | while let Err(_) = Ok::(10) {} | ----------^^^^^^--------------------- help: try this: `while Ok::(10).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching_result.rs:118:5 + --> $DIR/redundant_pattern_matching_result.rs:119:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -142,7 +142,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching_result.rs:123:5 + --> $DIR/redundant_pattern_matching_result.rs:124:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -150,5 +150,41 @@ LL | | Err(_) => true, LL | | }; | |_____^ help: try this: `Err::(42).is_err()` -error: aborting due to 22 previous errors +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_result.rs:131:5 + | +LL | / match Ok::(42) { +LL | | Ok(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `Ok::(42).is_ok()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_result.rs:136:5 + | +LL | / match Ok::(42) { +LL | | Ok(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `Ok::(42).is_err()` + +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_result.rs:141:5 + | +LL | / match Err::(42) { +LL | | Ok(_) => false, +LL | | _ => true, +LL | | }; + | |_____^ help: try this: `Err::(42).is_err()` + +error: redundant pattern matching, consider using `is_ok()` + --> $DIR/redundant_pattern_matching_result.rs:146:5 + | +LL | / match Err::(42) { +LL | | Ok(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `Err::(42).is_ok()` + +error: aborting due to 26 previous errors