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
This commit is contained in:
bors 2021-07-01 15:02:21 +00:00
commit 753bce30f0
8 changed files with 84 additions and 52 deletions

View file

@ -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); 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)] #[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 // Accumulate the variants which should be put in place of the wildcard because they're not
// already covered. // 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; let mut path_prefix = CommonPrefixSearcher::None;
for arm in arms { for arm in arms {
@ -1118,7 +1119,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
match missing_variants.as_slice() { 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, cx,
MATCH_WILDCARD_FOR_SINGLE_VARIANTS, MATCH_WILDCARD_FOR_SINGLE_VARIANTS,
wildcard_span, wildcard_span,
@ -1129,7 +1130,7 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>])
), ),
variants => { variants => {
let mut suggestions: Vec<_> = variants.iter().copied().map(format_suggestion).collect(); 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()); suggestions.push("_".into());
"wildcard matches known variants and will also match future added variants" "wildcard matches known variants and will also match future added variants"
} else { } else {

View file

@ -157,3 +157,8 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {
.filter_map(ast::Attribute::meta_item_list) .filter_map(ast::Attribute::meta_item_list)
.any(|l| attr::list_contains_name(&l, sym::hidden)) .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))
}

View file

@ -0,0 +1,8 @@
// Stripped down version of the ErrorKind enum of std
#[non_exhaustive]
pub enum ErrorKind {
NotFound,
PermissionDenied,
#[doc(hidden)]
Uncategorized,
}

View file

@ -115,9 +115,16 @@ fn main() {
pub enum Enum { pub enum Enum {
A, A,
B, B,
C,
#[doc(hidden)] #[doc(hidden)]
__Private, __Private,
} }
match Enum::A {
Enum::A => (),
Enum::B => (),
Enum::C => (),
_ => (),
}
match Enum::A { match Enum::A {
Enum::A => (), Enum::A => (),
Enum::B => (), Enum::B => (),

View file

@ -115,9 +115,16 @@ fn main() {
pub enum Enum { pub enum Enum {
A, A,
B, B,
C,
#[doc(hidden)] #[doc(hidden)]
__Private, __Private,
} }
match Enum::A {
Enum::A => (),
Enum::B => (),
Enum::C => (),
_ => (),
}
match Enum::A { match Enum::A {
Enum::A => (), Enum::A => (),
Enum::B => (), Enum::B => (),

View file

@ -1,4 +1,5 @@
// run-rustfix // run-rustfix
// aux-build:non-exhaustive-enum.rs
#![deny(clippy::wildcard_enum_match_arm)] #![deny(clippy::wildcard_enum_match_arm)]
#![allow( #![allow(
@ -11,7 +12,9 @@
clippy::diverging_sub_expression clippy::diverging_sub_expression
)] )]
use std::io::ErrorKind; extern crate non_exhaustive_enum;
use non_exhaustive_enum::ErrorKind;
#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum Color { enum Color {
@ -77,29 +80,25 @@ fn main() {
let error_kind = ErrorKind::NotFound; let error_kind = ErrorKind::NotFound;
match error_kind { match error_kind {
ErrorKind::NotFound => {}, 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 { match error_kind {
ErrorKind::NotFound => {}, ErrorKind::NotFound => {},
ErrorKind::PermissionDenied => {}, 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 | _ => (),
}
}
} }

View file

@ -1,4 +1,5 @@
// run-rustfix // run-rustfix
// aux-build:non-exhaustive-enum.rs
#![deny(clippy::wildcard_enum_match_arm)] #![deny(clippy::wildcard_enum_match_arm)]
#![allow( #![allow(
@ -11,7 +12,9 @@
clippy::diverging_sub_expression clippy::diverging_sub_expression
)] )]
use std::io::ErrorKind; extern crate non_exhaustive_enum;
use non_exhaustive_enum::ErrorKind;
#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum Color { enum Color {
@ -82,24 +85,20 @@ fn main() {
match error_kind { match error_kind {
ErrorKind::NotFound => {}, ErrorKind::NotFound => {},
ErrorKind::PermissionDenied => {}, 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 => (),
_ => (),
}
}
} }

View file

@ -1,38 +1,44 @@
error: wildcard match will also match any future added variants 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"), LL | _ => eprintln!("Not red"),
| ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` | ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan`
| |
note: the lint level is defined here 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)] LL | #![deny(clippy::wildcard_enum_match_arm)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: wildcard match will also match any future added variants 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"), 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` | ^^^^^^^^ 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 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), 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` | ^^^^^^^ 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 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", LL | _ => "No red",
| ^ help: try this: `Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` | ^ 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 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 | _ => {}, 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