Add reachable_patterns lint to rfc-2008-non_exhaustive

Add linting on non_exhaustive structs and enum variants

Add ui tests for non_exhaustive reachable lint

Rename to non_exhaustive_omitted_patterns and avoid triggering on if let

Author: DevinR528

compiler/rustc_lint_defs/src/builtin.rs

@@ -3010,6 +3010,7 @@ declare_lint_pass! {
         UNSUPPORTED_CALLING_CONVENTIONS,
         BREAK_WITH_LABEL_AND_LOOP,
         UNUSED_ATTRIBUTES,
+        NON_EXHAUSTIVE_OMITTED_PATTERNS,
     ]
 }
 
@@ -3416,3 +3417,56 @@ declare_lint! {
     Warn,
     "`break` expression with label and unlabeled loop as value expression"
 }
+
+declare_lint! {
+    /// The `non_exhaustive_omitted_patterns` lint detects when a wildcard (`_` or `..`) in a
+    /// pattern for a `#[non_exhaustive]` struct or enum is reachable.
+    ///
+    /// ### Example
+    ///
+    /// ```rust,ignore (needs separate crate)
+    /// // crate A
+    /// #[non_exhaustive]
+    /// pub enum Bar {
+    ///     A,
+    ///     B, // added variant in non breaking change
+    /// }
+    ///
+    /// // in crate B
+    /// match Bar::A {
+    ///     Bar::A => {},
+    ///     #[warn(non_exhaustive_omitted_patterns)]
+    ///     _ => {},
+    /// }
+    /// ```
+    ///
+    /// This will produce:
+    ///
+    /// ```text
+    /// warning: reachable patterns not covered of non exhaustive enum
+    ///    --> $DIR/reachable-patterns.rs:70:9
+    ///    |
+    /// LL |         _ => {}
+    ///    |         ^ pattern `B` not covered
+    ///    |
+    ///  note: the lint level is defined here
+    ///   --> $DIR/reachable-patterns.rs:69:16
+    ///    |
+    /// LL |         #[warn(non_exhaustive_omitted_patterns)]
+    ///    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    ///    = help: ensure that all possible cases are being handled by adding the suggested match arms
+    ///    = note: the matched value is of type `Bar` and the `non_exhaustive_omitted_patterns` attribute was found
+    /// ```
+    ///
+    /// ### Explanation
+    ///
+    /// Structs and enums tagged with `#[non_exhaustive]` force the user to add a
+    /// (potentially redundant) wildcard when pattern-matching, to allow for future
+    /// addition of fields or variants. The `non_exhaustive_omitted_patterns` lint
+    /// detects when such a wildcard happens to actually catch some fields/variants.
+    /// In other words, when the match without the wildcard would not be exhaustive.
+    /// This lets the user be informed if new fields/variants were added.
+    pub NON_EXHAUSTIVE_OMITTED_PATTERNS,
+    Allow,
+    "detect when patterns of types marked `non_exhaustive` are missed",
+}

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

@@ -14,8 +14,9 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
 use rustc_hir::{HirId, Pat};
 use rustc_middle::thir::PatKind;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_session::lint::builtin::BINDINGS_WITH_VARIANT_NAME;
-use rustc_session::lint::builtin::{IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS};
+use rustc_session::lint::builtin::{
+    BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
+};
 use rustc_session::Session;
 use rustc_span::{DesugaringKind, ExpnKind, Span};
 use std::slice;
@@ -559,7 +560,7 @@ fn non_exhaustive_match<'p, 'tcx>(
     err.emit();
 }
 
-fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
+crate fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
     const LIMIT: usize = 3;
     match witnesses {
         [] => bug!(),
@@ -576,7 +577,7 @@ fn joined_uncovered_patterns(witnesses: &[super::Pat<'_>]) -> String {
     }
 }
 
-fn pattern_not_covered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String {
+crate fn pattern_not_covered_label(witnesses: &[super::Pat<'_>], joined_patterns: &str) -> String {
     format!("pattern{} {} not covered", rustc_errors::pluralize!(witnesses.len()), joined_patterns)
 }
 

compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs

@@ -606,8 +606,9 @@ pub(super) enum Constructor<'tcx> {
     /// for those types for which we cannot list constructors explicitly, like `f64` and `str`.
     NonExhaustive,
     /// Stands for constructors that are not seen in the matrix, as explained in the documentation
-    /// for [`SplitWildcard`].
-    Missing,
+    /// for [`SplitWildcard`]. The carried `bool` is used for the `non_exhaustive_omitted_patterns`
+    /// lint.
+    Missing { nonexhaustive_enum_missing_real_variants: bool },
     /// Wildcard pattern.
     Wildcard,
 }
@@ -617,6 +618,10 @@ impl<'tcx> Constructor<'tcx> {
         matches!(self, Wildcard)
     }
 
+    pub(super) fn is_non_exhaustive(&self) -> bool {
+        matches!(self, NonExhaustive)
+    }
+
     fn as_int_range(&self) -> Option<&IntRange> {
         match self {
             IntRange(range) => Some(range),
@@ -756,7 +761,7 @@ impl<'tcx> Constructor<'tcx> {
             // Wildcards cover anything
             (_, Wildcard) => true,
             // The missing ctors are not covered by anything in the matrix except wildcards.
-            (Missing | Wildcard, _) => false,
+            (Missing { .. } | Wildcard, _) => false,
 
             (Single, Single) => true,
             (Variant(self_id), Variant(other_id)) => self_id == other_id,
@@ -829,7 +834,7 @@ impl<'tcx> Constructor<'tcx> {
                 .any(|other| slice.is_covered_by(other)),
             // This constructor is never covered by anything else
             NonExhaustive => false,
-            Str(..) | FloatRange(..) | Opaque | Missing | Wildcard => {
+            Str(..) | FloatRange(..) | Opaque | Missing { .. } | Wildcard => {
                 span_bug!(pcx.span, "found unexpected ctor in all_ctors: {:?}", self)
             }
         }
@@ -919,8 +924,14 @@ impl<'tcx> SplitWildcard<'tcx> {
                     && !cx.tcx.features().exhaustive_patterns
                     && !pcx.is_top_level;
 
-                if is_secretly_empty || is_declared_nonexhaustive {
+                if is_secretly_empty {
                     smallvec![NonExhaustive]
+                } else if is_declared_nonexhaustive {
+                    def.variants
+                        .indices()
+                        .map(|idx| Variant(idx))
+                        .chain(Some(NonExhaustive))
+                        .collect()
                 } else if cx.tcx.features().exhaustive_patterns {
                     // If `exhaustive_patterns` is enabled, we exclude variants known to be
                     // uninhabited.
@@ -975,6 +986,7 @@ impl<'tcx> SplitWildcard<'tcx> {
             // This type is one for which we cannot list constructors, like `str` or `f64`.
             _ => smallvec![NonExhaustive],
         };
+
         SplitWildcard { matrix_ctors: Vec::new(), all_ctors }
     }
 
@@ -1039,7 +1051,17 @@ impl<'tcx> SplitWildcard<'tcx> {
             // sometimes prefer reporting the list of constructors instead of just `_`.
             let report_when_all_missing = pcx.is_top_level && !IntRange::is_integral(pcx.ty);
             let ctor = if !self.matrix_ctors.is_empty() || report_when_all_missing {
-                Missing
+                if pcx.is_non_exhaustive {
+                    Missing {
+                        nonexhaustive_enum_missing_real_variants: self
+                            .iter_missing(pcx)
+                            .filter(|c| !c.is_non_exhaustive())
+                            .next()
+                            .is_some(),
+                    }
+                } else {
+                    Missing { nonexhaustive_enum_missing_real_variants: false }
+                }
             } else {
                 Wildcard
             };
@@ -1176,7 +1198,12 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
                 }
                 _ => bug!("bad slice pattern {:?} {:?}", constructor, ty),
             },
-            Str(..) | FloatRange(..) | IntRange(..) | NonExhaustive | Opaque | Missing
+            Str(..)
+            | FloatRange(..)
+            | IntRange(..)
+            | NonExhaustive
+            | Opaque
+            | Missing { .. }
             | Wildcard => Fields::Slice(&[]),
         };
         debug!("Fields::wildcards({:?}, {:?}) = {:#?}", constructor, ty, ret);
@@ -1189,15 +1216,18 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
     /// This is roughly the inverse of `specialize_constructor`.
     ///
     /// Examples:
-    /// `ctor`: `Constructor::Single`
-    /// `ty`: `Foo(u32, u32, u32)`
-    /// `self`: `[10, 20, _]`
+    ///
+    /// ```text
+    /// ctor: `Constructor::Single`
+    /// ty: `Foo(u32, u32, u32)`
+    /// self: `[10, 20, _]`
     /// returns `Foo(10, 20, _)`
     ///
-    /// `ctor`: `Constructor::Variant(Option::Some)`
-    /// `ty`: `Option<bool>`
-    /// `self`: `[false]`
+    /// ctor: `Constructor::Variant(Option::Some)`
+    /// ty: `Option<bool>`
+    /// self: `[false]`
     /// returns `Some(false)`
+    /// ```
     pub(super) fn apply(self, pcx: PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'tcx>) -> Pat<'tcx> {
         let subpatterns_and_indices = self.patterns_and_indices();
         let mut subpatterns = subpatterns_and_indices.iter().map(|&(_, p)| p).cloned();
@@ -1265,7 +1295,7 @@ impl<'p, 'tcx> Fields<'p, 'tcx> {
             NonExhaustive => PatKind::Wild,
             Wildcard => return Pat::wildcard_from_ty(pcx.ty),
             Opaque => bug!("we should not try to apply an opaque constructor"),
-            Missing => bug!(
+            Missing { .. } => bug!(
                 "trying to apply the `Missing` constructor; this should have been done in `apply_constructors`"
             ),
         };