Auto merge of #7407 - m-ou-se:doc-hidden-variants, r=flip1995

bors · bors · commit 753bce30f057 · 2021-07-01T15:02:21.000Z
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 #85746 (comment) This PR includes rust-lang/rust-clippy#7406 as the first commit. Here's the diff that this PR adds on top of that PR: m-ou-se/fork@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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
 

Original file line number	Diff line number	Diff line change
`@@ -157,3 +157,8 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool {`
`157`	`157`	`.filter_map(ast::Attribute::meta_item_list)`
`158`	`158`	`.any(\|l\| attr::list_contains_name(&l, sym::hidden))`
`159`	`159`	`}`
	`160`	`+`
	`161`	+/// Return true if the attributes contain `#[unstable]`
	`162`	`+pub fn is_unstable(attrs: &[ast::Attribute]) -> bool {`
	`163`	`+ attrs.iter().any(\|attr\| attr.has_name(sym::unstable))`
	`164`	`+}`