From fae7a09eea5644567ff7239abc3970d1e9a2d159 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Jul 2021 12:35:16 +0200 Subject: [PATCH] match_wildcard_for_single_variants: don't produce bad suggestion This fixes a bug where match_wildcard_for_single_variants produced a bad suggestion where besides the missing variant, one or more hidden variants were left. This also adds tests to the ui-tests match_wildcard_for_single_variants and wildcard_enum_match_arm to make sure that the correct suggestion is produced. --- clippy_lints/src/matches.rs | 5 +++-- tests/ui/match_wildcard_for_single_variants.fixed | 7 +++++++ tests/ui/match_wildcard_for_single_variants.rs | 7 +++++++ tests/ui/wildcard_enum_match_arm.fixed | 14 ++++++++++++++ tests/ui/wildcard_enum_match_arm.rs | 14 ++++++++++++++ tests/ui/wildcard_enum_match_arm.stderr | 8 +++++++- 6 files changed, 52 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index da5ac96e3dbd..4c90b95a42b6 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -1033,6 +1033,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) // Accumulate the variants which should be put in place of the wildcard because they're not // already covered. + let has_hidden = adt_def.variants.iter().any(|x| is_hidden(cx, x)); let mut missing_variants: Vec<_> = adt_def.variants.iter().filter(|x| !is_hidden(cx, x)).collect(); let mut path_prefix = CommonPrefixSearcher::None; @@ -1118,7 +1119,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) match missing_variants.as_slice() { [] => (), - [x] if !adt_def.is_variant_list_non_exhaustive() => span_lint_and_sugg( + [x] if !adt_def.is_variant_list_non_exhaustive() && !has_hidden => span_lint_and_sugg( cx, MATCH_WILDCARD_FOR_SINGLE_VARIANTS, wildcard_span, @@ -1129,7 +1130,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) ), variants => { let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect(); - let message = if adt_def.is_variant_list_non_exhaustive() { + let message = if adt_def.is_variant_list_non_exhaustive() || has_hidden { suggestions.push("_".into()); "wildcard matches known variants and will also match future added variants" } else { diff --git a/tests/ui/match_wildcard_for_single_variants.fixed b/tests/ui/match_wildcard_for_single_variants.fixed index 31b743f7a18d..e675c183ea71 100644 --- a/tests/ui/match_wildcard_for_single_variants.fixed +++ b/tests/ui/match_wildcard_for_single_variants.fixed @@ -115,9 +115,16 @@ fn main() { pub enum Enum { A, B, + C, #[doc(hidden)] __Private, } + match Enum::A { + Enum::A => (), + Enum::B => (), + Enum::C => (), + _ => (), + } match Enum::A { Enum::A => (), Enum::B => (), diff --git a/tests/ui/match_wildcard_for_single_variants.rs b/tests/ui/match_wildcard_for_single_variants.rs index d19e6ab7eb2e..38c3ffc00c71 100644 --- a/tests/ui/match_wildcard_for_single_variants.rs +++ b/tests/ui/match_wildcard_for_single_variants.rs @@ -115,9 +115,16 @@ fn main() { pub enum Enum { A, B, + C, #[doc(hidden)] __Private, } + match Enum::A { + Enum::A => (), + Enum::B => (), + Enum::C => (), + _ => (), + } match Enum::A { Enum::A => (), Enum::B => (), diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 64aba68635a5..3ee4ab48ac84 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -87,4 +87,18 @@ fn main() { ErrorKind::PermissionDenied => {}, _ => {}, } + + { + #![allow(clippy::manual_non_exhaustive)] + pub enum Enum { + A, + B, + #[doc(hidden)] + __Private, + } + match Enum::A { + Enum::A => (), + Enum::B | _ => (), + } + } } diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index 5cafdf59bf55..468865504533 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -87,4 +87,18 @@ fn main() { ErrorKind::PermissionDenied => {}, _ => {}, } + + { + #![allow(clippy::manual_non_exhaustive)] + pub enum Enum { + A, + B, + #[doc(hidden)] + __Private, + } + match Enum::A { + Enum::A => (), + _ => (), + } + } } diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 807947582b0e..d63f20903531 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -34,5 +34,11 @@ error: wildcard matches known variants and will also match future added variants LL | _ => {}, | ^ help: try this: `ErrorKind::PermissionDenied | _` -error: aborting due to 5 previous errors +error: wildcard matches known variants and will also match future added variants + --> $DIR/wildcard_enum_match_arm.rs:101:13 + | +LL | _ => (), + | ^ help: try this: `Enum::B | _` + +error: aborting due to 6 previous errors