From 3e5956339482e06dfcb8690dee9211b77aa6e218 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 26 Jun 2021 14:50:42 +0200 Subject: [PATCH 1/4] Don't use exact definition of std's ErrorKind in test. Every time we add something to this enum in std, this test breaks. --- tests/ui/auxiliary/non-exhaustive-enum.rs | 23 +++++++++++++++++++++++ tests/ui/wildcard_enum_match_arm.fixed | 5 ++++- tests/ui/wildcard_enum_match_arm.rs | 5 ++++- tests/ui/wildcard_enum_match_arm.stderr | 12 ++++++------ 4 files changed, 37 insertions(+), 8 deletions(-) create mode 100644 tests/ui/auxiliary/non-exhaustive-enum.rs diff --git a/tests/ui/auxiliary/non-exhaustive-enum.rs b/tests/ui/auxiliary/non-exhaustive-enum.rs new file mode 100644 index 00000000000..67d4d255701 --- /dev/null +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -0,0 +1,23 @@ +#[non_exhaustive] +pub enum ErrorKind { + NotFound, + PermissionDenied, + ConnectionRefused, + ConnectionReset, + ConnectionAborted, + NotConnected, + AddrInUse, + AddrNotAvailable, + BrokenPipe, + AlreadyExists, + WouldBlock, + InvalidInput, + InvalidData, + TimedOut, + WriteZero, + Interrupted, + Other, + UnexpectedEof, + Unsupported, + OutOfMemory, +} diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 5ad27bb1450..2b8f260397c 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -1,4 +1,5 @@ // run-rustfix +// aux-build:non-exhaustive-enum.rs #![deny(clippy::wildcard_enum_match_arm)] #![allow( @@ -11,7 +12,9 @@ clippy::diverging_sub_expression )] -use std::io::ErrorKind; +extern crate non_exhaustive_enum; + +use non_exhaustive_enum::ErrorKind; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index adca0738bba..1f7bcffd5f5 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -1,4 +1,5 @@ // run-rustfix +// aux-build:non-exhaustive-enum.rs #![deny(clippy::wildcard_enum_match_arm)] #![allow( @@ -11,7 +12,9 @@ clippy::diverging_sub_expression )] -use std::io::ErrorKind; +extern crate non_exhaustive_enum; + +use non_exhaustive_enum::ErrorKind; #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 73f6a4a80c9..80ba5ee4f04 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -1,35 +1,35 @@ error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:39:9 + --> $DIR/wildcard_enum_match_arm.rs:42:9 | LL | _ => eprintln!("Not red"), | ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` | note: the lint level is defined here - --> $DIR/wildcard_enum_match_arm.rs:3:9 + --> $DIR/wildcard_enum_match_arm.rs:4:9 | LL | #![deny(clippy::wildcard_enum_match_arm)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:43:9 + --> $DIR/wildcard_enum_match_arm.rs:46:9 | LL | _not_red => eprintln!("Not red"), | ^^^^^^^^ help: try this: `_not_red @ Color::Green | _not_red @ Color::Blue | _not_red @ Color::Rgb(..) | _not_red @ Color::Cyan` error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:47:9 + --> $DIR/wildcard_enum_match_arm.rs:50:9 | LL | not_red => format!("{:?}", not_red), | ^^^^^^^ help: try this: `not_red @ Color::Green | not_red @ Color::Blue | not_red @ Color::Rgb(..) | not_red @ Color::Cyan` error: wildcard match will also match any future added variants - --> $DIR/wildcard_enum_match_arm.rs:63:9 + --> $DIR/wildcard_enum_match_arm.rs:66:9 | LL | _ => "No red", | ^ help: try this: `Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` error: wildcard matches known variants and will also match future added variants - --> $DIR/wildcard_enum_match_arm.rs:80:9 + --> $DIR/wildcard_enum_match_arm.rs:83:9 | LL | _ => {}, | ^ help: try this: `ErrorKind::PermissionDenied | ErrorKind::ConnectionRefused | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted | ErrorKind::NotConnected | ErrorKind::AddrInUse | ErrorKind::AddrNotAvailable | ErrorKind::BrokenPipe | ErrorKind::AlreadyExists | ErrorKind::WouldBlock | ErrorKind::InvalidInput | ErrorKind::InvalidData | ErrorKind::TimedOut | ErrorKind::WriteZero | ErrorKind::Interrupted | ErrorKind::Other | ErrorKind::UnexpectedEof | ErrorKind::Unsupported | ErrorKind::OutOfMemory | _` From 38569c03eb0d0917698d83aea5fbbc35acf7305c Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Sat, 26 Jun 2021 15:22:15 +0200 Subject: [PATCH 2/4] Don't suggest unstable and doc(hidden) variants. --- clippy_lints/src/matches.rs | 8 ++++---- clippy_utils/src/attrs.rs | 5 +++++ tests/ui/auxiliary/non-exhaustive-enum.rs | 2 ++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index cd3e3b97928..da5ac96e3db 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -992,9 +992,9 @@ impl CommonPrefixSearcher<'a> { } } -fn is_doc_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool { +fn is_hidden(cx: &LateContext<'_>, variant_def: &VariantDef) -> bool { let attrs = cx.tcx.get_attrs(variant_def.def_id); - clippy_utils::attrs::is_doc_hidden(attrs) + clippy_utils::attrs::is_doc_hidden(attrs) || clippy_utils::attrs::is_unstable(attrs) } #[allow(clippy::too_many_lines)] @@ -1033,7 +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 mut missing_variants: Vec<_> = adt_def.variants.iter().collect(); + let mut missing_variants: Vec<_> = adt_def.variants.iter().filter(|x| !is_hidden(cx, x)).collect(); let mut path_prefix = CommonPrefixSearcher::None; for arm in arms { @@ -1118,7 +1118,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() && !is_doc_hidden(cx, x) => span_lint_and_sugg( + [x] if !adt_def.is_variant_list_non_exhaustive() => span_lint_and_sugg( cx, MATCH_WILDCARD_FOR_SINGLE_VARIANTS, wildcard_span, diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 0318c483959..c19b558cd8c 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -157,3 +157,8 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool { .filter_map(ast::Attribute::meta_item_list) .any(|l| attr::list_contains_name(&l, sym::hidden)) } + +/// Return true if the attributes contain `#[unstable]` +pub fn is_unstable(attrs: &[ast::Attribute]) -> bool { + attrs.iter().any(|attr| attr.has_name(sym::unstable)) +} diff --git a/tests/ui/auxiliary/non-exhaustive-enum.rs b/tests/ui/auxiliary/non-exhaustive-enum.rs index 67d4d255701..18560bc5e1e 100644 --- a/tests/ui/auxiliary/non-exhaustive-enum.rs +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -20,4 +20,6 @@ pub enum ErrorKind { UnexpectedEof, Unsupported, OutOfMemory, + #[doc(hidden)] + Uncategorized, } From 0ffba7a68442767cb9ddcc1cc4224d17800f7f63 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Jul 2021 11:47:56 +0200 Subject: [PATCH 3/4] Simplify wildcard_enum_match_arm test --- tests/ui/auxiliary/non-exhaustive-enum.rs | 19 +------------------ tests/ui/wildcard_enum_match_arm.fixed | 20 +------------------- tests/ui/wildcard_enum_match_arm.rs | 18 ------------------ tests/ui/wildcard_enum_match_arm.stderr | 2 +- 4 files changed, 3 insertions(+), 56 deletions(-) diff --git a/tests/ui/auxiliary/non-exhaustive-enum.rs b/tests/ui/auxiliary/non-exhaustive-enum.rs index 18560bc5e1e..420232f9f8d 100644 --- a/tests/ui/auxiliary/non-exhaustive-enum.rs +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -1,25 +1,8 @@ +// Stripped down version of the ErrorKind enum of std #[non_exhaustive] pub enum ErrorKind { NotFound, PermissionDenied, - ConnectionRefused, - ConnectionReset, - ConnectionAborted, - NotConnected, - AddrInUse, - AddrNotAvailable, - BrokenPipe, - AlreadyExists, - WouldBlock, - InvalidInput, - InvalidData, - TimedOut, - WriteZero, - Interrupted, - Other, - UnexpectedEof, - Unsupported, - OutOfMemory, #[doc(hidden)] Uncategorized, } diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 2b8f260397c..64aba68635a 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -80,29 +80,11 @@ fn main() { let error_kind = ErrorKind::NotFound; match error_kind { ErrorKind::NotFound => {}, - ErrorKind::PermissionDenied | ErrorKind::ConnectionRefused | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted | ErrorKind::NotConnected | ErrorKind::AddrInUse | ErrorKind::AddrNotAvailable | ErrorKind::BrokenPipe | ErrorKind::AlreadyExists | ErrorKind::WouldBlock | ErrorKind::InvalidInput | ErrorKind::InvalidData | ErrorKind::TimedOut | ErrorKind::WriteZero | ErrorKind::Interrupted | ErrorKind::Other | ErrorKind::UnexpectedEof | ErrorKind::Unsupported | ErrorKind::OutOfMemory | _ => {}, + ErrorKind::PermissionDenied | _ => {}, } match error_kind { ErrorKind::NotFound => {}, ErrorKind::PermissionDenied => {}, - ErrorKind::ConnectionRefused => {}, - ErrorKind::ConnectionReset => {}, - ErrorKind::ConnectionAborted => {}, - ErrorKind::NotConnected => {}, - ErrorKind::AddrInUse => {}, - ErrorKind::AddrNotAvailable => {}, - ErrorKind::BrokenPipe => {}, - ErrorKind::AlreadyExists => {}, - ErrorKind::WouldBlock => {}, - ErrorKind::InvalidInput => {}, - ErrorKind::InvalidData => {}, - ErrorKind::TimedOut => {}, - ErrorKind::WriteZero => {}, - ErrorKind::Interrupted => {}, - ErrorKind::Other => {}, - ErrorKind::UnexpectedEof => {}, - ErrorKind::Unsupported => {}, - ErrorKind::OutOfMemory => {}, _ => {}, } } diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index 1f7bcffd5f5..5cafdf59bf5 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -85,24 +85,6 @@ fn main() { match error_kind { ErrorKind::NotFound => {}, ErrorKind::PermissionDenied => {}, - ErrorKind::ConnectionRefused => {}, - ErrorKind::ConnectionReset => {}, - ErrorKind::ConnectionAborted => {}, - ErrorKind::NotConnected => {}, - ErrorKind::AddrInUse => {}, - ErrorKind::AddrNotAvailable => {}, - ErrorKind::BrokenPipe => {}, - ErrorKind::AlreadyExists => {}, - ErrorKind::WouldBlock => {}, - ErrorKind::InvalidInput => {}, - ErrorKind::InvalidData => {}, - ErrorKind::TimedOut => {}, - ErrorKind::WriteZero => {}, - ErrorKind::Interrupted => {}, - ErrorKind::Other => {}, - ErrorKind::UnexpectedEof => {}, - ErrorKind::Unsupported => {}, - ErrorKind::OutOfMemory => {}, _ => {}, } } diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 80ba5ee4f04..807947582b0 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -32,7 +32,7 @@ error: wildcard matches known variants and will also match future added variants --> $DIR/wildcard_enum_match_arm.rs:83:9 | LL | _ => {}, - | ^ help: try this: `ErrorKind::PermissionDenied | ErrorKind::ConnectionRefused | ErrorKind::ConnectionReset | ErrorKind::ConnectionAborted | ErrorKind::NotConnected | ErrorKind::AddrInUse | ErrorKind::AddrNotAvailable | ErrorKind::BrokenPipe | ErrorKind::AlreadyExists | ErrorKind::WouldBlock | ErrorKind::InvalidInput | ErrorKind::InvalidData | ErrorKind::TimedOut | ErrorKind::WriteZero | ErrorKind::Interrupted | ErrorKind::Other | ErrorKind::UnexpectedEof | ErrorKind::Unsupported | ErrorKind::OutOfMemory | _` + | ^ help: try this: `ErrorKind::PermissionDenied | _` error: aborting due to 5 previous errors From fae7a09eea5644567ff7239abc3970d1e9a2d159 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 1 Jul 2021 12:35:16 +0200 Subject: [PATCH 4/4] 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 da5ac96e3db..4c90b95a42b 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 31b743f7a18..e675c183ea7 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 d19e6ab7eb2..38c3ffc00c7 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 64aba68635a..3ee4ab48ac8 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 5cafdf59bf5..46886550453 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 807947582b0..d63f2090353 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