From 342ce3da05eea9f119ecf367052d681a386b1761 Mon Sep 17 00:00:00 2001 From: disco07 Date: Tue, 9 May 2023 20:50:47 +0200 Subject: [PATCH] fix reviewer comments --- .../src/matches/match_like_matches.rs | 7 +- .../src/matches/redundant_pattern_match.rs | 6 +- .../redundant_pattern_matching_option.fixed | 17 ++--- tests/ui/redundant_pattern_matching_option.rs | 25 ++----- .../redundant_pattern_matching_option.stderr | 66 +++++-------------- .../redundant_pattern_matching_result.fixed | 7 +- tests/ui/redundant_pattern_matching_result.rs | 19 ++++-- .../redundant_pattern_matching_result.stderr | 24 +++---- 8 files changed, 63 insertions(+), 108 deletions(-) diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs index 438d14375..0064619ef 100644 --- a/clippy_lints/src/matches/match_like_matches.rs +++ b/clippy_lints/src/matches/match_like_matches.rs @@ -183,12 +183,9 @@ fn find_bool_lit(ex: &ExprKind<'_>) -> Option { fn is_some(path_kind: PatKind<'_>) -> bool { match path_kind { - PatKind::TupleStruct(QPath::Resolved(_, path), patterns, _) if is_wild(&patterns[0]) => { + PatKind::TupleStruct(QPath::Resolved(_, path), [first, ..], _) if is_wild(first) => { let name = path.segments[0].ident; - if name.name == rustc_span::sym::Some { - return true; - } - false + name.name == rustc_span::sym::Some }, _ => false, } diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 284789878..7a06443f5 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -188,9 +188,8 @@ fn find_sugg_for_if_let<'tcx>( pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op: &Expr<'_>, arms: &[Arm<'_>]) { if arms.len() == 2 { let node_pair = (&arms[0].pat.kind, &arms[1].pat.kind); - let found_good_method = found_good_method(cx, arms, node_pair); - if let Some(good_method) = found_good_method { + if let Some(good_method) = found_good_method(cx, arms, node_pair) { let span = expr.span.to(op.span); let result_expr = match &op.kind { ExprKind::AddrOf(_, _, borrowed) => borrowed, @@ -310,6 +309,9 @@ fn get_good_method<'a>(cx: &LateContext<'_>, arms: &[Arm<'_>], path_left: &QPath "Ok" => { find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultOk), "is_ok()", "is_err()") }, + "Err" => { + find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultErr), "is_err()", "is_ok()") + }, "Some" => find_good_method_for_matches_macro( cx, arms, diff --git a/tests/ui/redundant_pattern_matching_option.fixed b/tests/ui/redundant_pattern_matching_option.fixed index c22a4d745..2f907f7ae 100644 --- a/tests/ui/redundant_pattern_matching_option.fixed +++ b/tests/ui/redundant_pattern_matching_option.fixed @@ -95,15 +95,12 @@ fn issue10726() { Some(42).is_none(); - Some(42).is_none(); - - Some(42).is_some(); - - None::<()>.is_none(); - - None::<()>.is_none(); - - None::<()>.is_none(); - None::<()>.is_some(); + + None::<()>.is_none(); + + match Some(42) { + Some(21) => true, + _ => false, + }; } diff --git a/tests/ui/redundant_pattern_matching_option.rs b/tests/ui/redundant_pattern_matching_option.rs index cd96e0d29..5e80a2b38 100644 --- a/tests/ui/redundant_pattern_matching_option.rs +++ b/tests/ui/redundant_pattern_matching_option.rs @@ -111,29 +111,14 @@ fn issue10726() { _ => 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, + Some(_) => true, + _ => false, }; match None::<()> { @@ -141,8 +126,8 @@ fn issue10726() { _ => false, }; - match None::<()> { - None => false, - _ => true, + match Some(42) { + Some(21) => true, + _ => false, }; } diff --git a/tests/ui/redundant_pattern_matching_option.stderr b/tests/ui/redundant_pattern_matching_option.stderr index d39729707..97de2f1c8 100644 --- a/tests/ui/redundant_pattern_matching_option.stderr +++ b/tests/ui/redundant_pattern_matching_option.stderr @@ -161,64 +161,28 @@ 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 | | None => true, +LL | | _ => false, LL | | }; | |_____^ help: try this: `Some(42).is_none()` -error: redundant pattern matching, consider using `is_none()` +error: redundant pattern matching, consider using `is_some()` --> $DIR/redundant_pattern_matching_option.rs:119:5 | -LL | / match Some(42) { -LL | | None => true, +LL | / match None::<()> { +LL | | Some(_) => 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 +error: redundant pattern matching, consider using `is_none()` + --> $DIR/redundant_pattern_matching_option.rs:124:5 + | +LL | / match None::<()> { +LL | | None => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `None::<()>.is_none()` + +error: aborting due to 26 previous errors diff --git a/tests/ui/redundant_pattern_matching_result.fixed b/tests/ui/redundant_pattern_matching_result.fixed index a51e14a5b..a242c38e8 100644 --- a/tests/ui/redundant_pattern_matching_result.fixed +++ b/tests/ui/redundant_pattern_matching_result.fixed @@ -114,7 +114,12 @@ fn issue10726() { Ok::(42).is_err(); + Err::(42).is_ok(); + Err::(42).is_err(); - Err::(42).is_ok(); + match Ok::(42) { + Ok(21) => true, + _ => false, + }; } diff --git a/tests/ui/redundant_pattern_matching_result.rs b/tests/ui/redundant_pattern_matching_result.rs index 709e3d526..a4b0673ae 100644 --- a/tests/ui/redundant_pattern_matching_result.rs +++ b/tests/ui/redundant_pattern_matching_result.rs @@ -134,17 +134,22 @@ fn issue10726() { }; match Ok::(42) { - Ok(_) => false, - _ => true, - }; - - match Err::(42) { - Ok(_) => false, - _ => true, + Err(_) => true, + _ => false, }; match Err::(42) { Ok(_) => true, _ => false, }; + + match Err::(42) { + Err(_) => true, + _ => false, + }; + + match Ok::(42) { + Ok(21) => true, + _ => false, + }; } diff --git a/tests/ui/redundant_pattern_matching_result.stderr b/tests/ui/redundant_pattern_matching_result.stderr index 0e8a983bf..151a74402 100644 --- a/tests/ui/redundant_pattern_matching_result.stderr +++ b/tests/ui/redundant_pattern_matching_result.stderr @@ -163,22 +163,13 @@ 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 | | Err(_) => true, +LL | | _ => false, 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 + --> $DIR/redundant_pattern_matching_result.rs:141:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -186,5 +177,14 @@ LL | | _ => false, LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` +error: redundant pattern matching, consider using `is_err()` + --> $DIR/redundant_pattern_matching_result.rs:146:5 + | +LL | / match Err::(42) { +LL | | Err(_) => true, +LL | | _ => false, +LL | | }; + | |_____^ help: try this: `Err::(42).is_err()` + error: aborting due to 26 previous errors