Skip to content

Commit

Permalink
Add reachable_patterns lint to rfc-2008-non_exhaustive
Browse files Browse the repository at this point in the history
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
  • Loading branch information
DevinR528 committed Sep 14, 2021
1 parent c3c0f80 commit 33a06b7
Show file tree
Hide file tree
Showing 10 changed files with 626 additions and 68 deletions.
54 changes: 54 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3010,6 +3010,7 @@ declare_lint_pass! {
UNSUPPORTED_CALLING_CONVENTIONS,
BREAK_WITH_LABEL_AND_LOOP,
UNUSED_ATTRIBUTES,
NON_EXHAUSTIVE_OMITTED_PATTERNS,
]
}

Expand Down Expand Up @@ -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",
}
9 changes: 5 additions & 4 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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!(),
Expand All @@ -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)
}

Expand Down
58 changes: 44 additions & 14 deletions compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 }
}

Expand Down Expand Up @@ -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
};
Expand Down Expand Up @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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`"
),
};
Expand Down
Loading

0 comments on commit 33a06b7

Please sign in to comment.