diff options
| author | bors <bors@rust-lang.org> | 2021-07-01 15:02:21 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2021-07-01 15:02:21 +0000 |
| commit | 753bce30f057c8a51c1121e0d1958da4cb28059b (patch) | |
| tree | 1497b30a1547cfe2f69e6569d9edc26085617efc | |
| parent | cadb93b6de1b5ba5aca53c5c4c7987f3c09962a8 (diff) | |
| parent | fae7a09eea5644567ff7239abc3970d1e9a2d159 (diff) | |
| download | rust-753bce30f057c8a51c1121e0d1958da4cb28059b.tar.gz rust-753bce30f057c8a51c1121e0d1958da4cb28059b.zip | |
Auto merge of #7407 - m-ou-se:doc-hidden-variants, r=flip1995
Don't suggest doc(hidden) or unstable variants in wildcard lint Clippy's wildcard lint would suggest doc(hidden) and unstable variants for non_exhaustive enums, even though those aren't part of the public interface (yet) and should only be matched on using a `_`, just like potential future additions to the enum. There was already some logic to exclude a *single* doc(hidden) variant. This extends that to all hidden variants, and also hides `#[unstable]` variants. See https://github.com/rust-lang/rust/pull/85746#issuecomment-868886893 This PR includes https://github.com/rust-lang/rust-clippy/pull/7406 as the first commit. Here's the diff that this PR adds on top of that PR: https://github.com/m-ou-se/rust-clippy/compare/std-errorkind...m-ou-se:doc-hidden-variants --- *Please write a short comment explaining your change (or "none" for internal only changes)* changelog: No longer suggest unstable and doc(hidden) variants in wildcard lint. wildcard_enum_match_arm, match_wildcard_for_single_variants
| -rw-r--r-- | clippy_lints/src/matches.rs | 11 | ||||
| -rw-r--r-- | clippy_utils/src/attrs.rs | 5 | ||||
| -rw-r--r-- | tests/ui/auxiliary/non-exhaustive-enum.rs | 8 | ||||
| -rw-r--r-- | tests/ui/match_wildcard_for_single_variants.fixed | 7 | ||||
| -rw-r--r-- | tests/ui/match_wildcard_for_single_variants.rs | 7 | ||||
| -rw-r--r-- | tests/ui/wildcard_enum_match_arm.fixed | 39 | ||||
| -rw-r--r-- | tests/ui/wildcard_enum_match_arm.rs | 37 | ||||
| -rw-r--r-- | tests/ui/wildcard_enum_match_arm.stderr | 22 |
8 files changed, 84 insertions, 52 deletions
diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 386349d9f59..f1e3492c4ec 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,8 @@ 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 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; for arm in arms { @@ -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() && !is_doc_hidden(cx, x) => 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/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 new file mode 100644 index 00000000000..420232f9f8d --- /dev/null +++ b/tests/ui/auxiliary/non-exhaustive-enum.rs @@ -0,0 +1,8 @@ +// Stripped down version of the ErrorKind enum of std +#[non_exhaustive] +pub enum ErrorKind { + NotFound, + PermissionDenied, + #[doc(hidden)] + Uncategorized, +} 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,12 +115,19 @@ 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,12 +115,19 @@ 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 5ad27bb1450..3ee4ab48ac8 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 { @@ -77,29 +80,25 @@ 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 => {}, _ => {}, } + + { + #![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 adca0738bba..46886550453 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 { @@ -82,24 +85,20 @@ 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 => {}, _ => {}, } + + { + #![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 73f6a4a80c9..d63f2090353 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -1,38 +1,44 @@ 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 | _` + | ^ 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 |
