From 33a06b73d90bde1d2fc4902672f4cbeb233b83e2 Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Fri, 10 Sep 2021 16:45:04 -0400 Subject: [PATCH] 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 --- compiler/rustc_lint_defs/src/builtin.rs | 54 ++++++ .../src/thir/pattern/check_match.rs | 9 +- .../src/thir/pattern/deconstruct_pat.rs | 58 +++++-- .../src/thir/pattern/usefulness.rs | 154 +++++++++++++---- compiler/rustc_typeck/src/check/pat.rs | 74 ++++++-- .../auxiliary/enums.rs | 23 ++- .../auxiliary/structs.rs | 10 +- .../reachable-patterns.rs | 160 ++++++++++++++++++ .../reachable-patterns.stderr | 146 ++++++++++++++++ .../ui/rfc-2008-non-exhaustive/struct.stderr | 6 +- 10 files changed, 626 insertions(+), 68 deletions(-) create mode 100644 src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.rs create mode 100644 src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.stderr diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 8fb678e2d20..a00561e5213 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -3010,6 +3010,7 @@ UNSUPPORTED_CALLING_CONVENTIONS, BREAK_WITH_LABEL_AND_LOOP, UNUSED_ATTRIBUTES, + NON_EXHAUSTIVE_OMITTED_PATTERNS, ] } @@ -3416,3 +3417,56 @@ 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", +} diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index b34c1e07be7..4c51b9207bb 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -14,8 +14,9 @@ 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) } diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index ace13ea4462..cee2a4db0a8 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/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 @@ pub(super) fn is_wildcard(&self) -> bool { 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 @@ pub(super) fn is_covered_by<'p>(&self, pcx: PatCtxt<'_, 'p, 'tcx>, other: &Self) // 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 @@ fn is_covered_by_any<'p>( .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 @@ pub(super) fn new<'p>(pcx: PatCtxt<'_, 'p, 'tcx>) -> Self { && !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 @@ pub(super) fn new<'p>(pcx: PatCtxt<'_, 'p, 'tcx>) -> Self { // 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 @@ pub(super) fn iter_missing<'a, 'p>( // 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 @@ pub(super) fn wildcards(pcx: PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'t } _ => 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 @@ pub(super) fn wildcards(pcx: PatCtxt<'_, 'p, 'tcx>, constructor: &Constructor<'t /// 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` - /// `self`: `[false]` + /// ctor: `Constructor::Variant(Option::Some)` + /// ty: `Option` + /// 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 @@ pub(super) fn apply(self, pcx: PatCtxt<'_, 'p, 'tcx>, ctor: &Constructor<'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`" ), }; diff --git a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs index 344006e9fb4..f4255713e2a 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs @@ -280,20 +280,23 @@ //! The details are not necessary to understand this file, so we explain them in //! [`super::deconstruct_pat`]. Splitting is done by the [`Constructor::split`] function. +use self::ArmType::*; use self::Usefulness::*; -use self::WitnessPreference::*; +use super::check_match::{joined_uncovered_patterns, pattern_not_covered_label}; use super::deconstruct_pat::{Constructor, Fields, SplitWildcard}; use super::{PatternFoldable, PatternFolder}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashMap; +use hir::def_id::DefId; +use hir::HirId; use rustc_arena::TypedArena; -use rustc_hir::def_id::DefId; -use rustc_hir::HirId; +use rustc_hir as hir; use rustc_middle::thir::{Pat, PatKind}; use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_span::Span; use smallvec::{smallvec, SmallVec}; @@ -343,6 +346,8 @@ pub(super) struct PatCtxt<'a, 'p, 'tcx> { /// Whether the current pattern is the whole pattern as found in a match arm, or if it's a /// subpattern. pub(super) is_top_level: bool, + /// Wether the current pattern is from a `non_exhaustive` enum. + pub(super) is_non_exhaustive: bool, } impl<'a, 'p, 'tcx> fmt::Debug for PatCtxt<'a, 'p, 'tcx> { @@ -862,7 +867,7 @@ fn unsplit_or_pat(mut self, alt_id: usize, alt_count: usize, pat: &'p Pat<'tcx>) /// of potential unreachable sub-patterns (in the presence of or-patterns). When checking /// exhaustiveness of a whole match, we use the `WithWitnesses` variant, which carries a list of /// witnesses of non-exhaustiveness when there are any. -/// Which variant to use is dictated by `WitnessPreference`. +/// Which variant to use is dictated by `ArmType`. #[derive(Clone, Debug)] enum Usefulness<'p, 'tcx> { /// Carries a set of subpatterns that have been found to be reachable. If empty, this indicates @@ -877,16 +882,24 @@ enum Usefulness<'p, 'tcx> { } impl<'p, 'tcx> Usefulness<'p, 'tcx> { - fn new_useful(preference: WitnessPreference) -> Self { + fn new_useful(preference: ArmType) -> Self { match preference { - ConstructWitness => WithWitnesses(vec![Witness(vec![])]), - LeaveOutWitness => NoWitnesses(SubPatSet::full()), + FakeExtraWildcard => WithWitnesses(vec![Witness(vec![])]), + RealArm => NoWitnesses(SubPatSet::full()), } } - fn new_not_useful(preference: WitnessPreference) -> Self { + + fn new_not_useful(preference: ArmType) -> Self { match preference { - ConstructWitness => WithWitnesses(vec![]), - LeaveOutWitness => NoWitnesses(SubPatSet::empty()), + FakeExtraWildcard => WithWitnesses(vec![]), + RealArm => NoWitnesses(SubPatSet::empty()), + } + } + + fn is_useful(&self) -> bool { + match self { + Usefulness::NoWitnesses(set) => !set.is_empty(), + Usefulness::WithWitnesses(witnesses) => !witnesses.is_empty(), } } @@ -903,7 +916,7 @@ fn extend(&mut self, other: Self) { /// When trying several branches and each returns a `Usefulness`, we need to combine the /// results together. - fn merge(pref: WitnessPreference, usefulnesses: impl Iterator) -> Self { + fn merge(pref: ArmType, usefulnesses: impl Iterator) -> Self { let mut ret = Self::new_not_useful(pref); for u in usefulnesses { ret.extend(u); @@ -926,7 +939,7 @@ fn unsplit_or_pat(self, alt_id: usize, alt_count: usize, pat: &'p Pat<'tcx>) -> } } - /// After calculating usefulness after a specialization, call this to recontruct a usefulness + /// After calculating usefulness after a specialization, call this to reconstruct a usefulness /// that makes sense for the matrix pre-specialization. This new usefulness can then be merged /// with the results of specializing with the other constructors. fn apply_constructor( @@ -939,19 +952,31 @@ fn apply_constructor( match self { WithWitnesses(witnesses) if witnesses.is_empty() => WithWitnesses(witnesses), WithWitnesses(witnesses) => { - let new_witnesses = if matches!(ctor, Constructor::Missing) { - let mut split_wildcard = SplitWildcard::new(pcx); - split_wildcard.split(pcx, matrix.head_ctors(pcx.cx)); - // Construct for each missing constructor a "wild" version of this - // constructor, that matches everything that can be built with - // it. For example, if `ctor` is a `Constructor::Variant` for - // `Option::Some`, we get the pattern `Some(_)`. - let new_patterns: Vec<_> = split_wildcard - .iter_missing(pcx) - .map(|missing_ctor| { - Fields::wildcards(pcx, missing_ctor).apply(pcx, missing_ctor) - }) - .collect(); + let new_witnesses = if let Constructor::Missing { .. } = ctor { + // We got the special `Missing` constructor, so each of the missing constructors + // gives a new pattern that is not caught by the match. We list those patterns. + let new_patterns = if pcx.is_non_exhaustive { + // Here we don't want the user to try to list all variants, we want them to add + // a wildcard, so we only suggest that. + vec![ + Fields::wildcards(pcx, &Constructor::NonExhaustive) + .apply(pcx, &Constructor::NonExhaustive), + ] + } else { + let mut split_wildcard = SplitWildcard::new(pcx); + split_wildcard.split(pcx, matrix.head_ctors(pcx.cx)); + // Construct for each missing constructor a "wild" version of this + // constructor, that matches everything that can be built with + // it. For example, if `ctor` is a `Constructor::Variant` for + // `Option::Some`, we get the pattern `Some(_)`. + split_wildcard + .iter_missing(pcx) + .map(|missing_ctor| { + Fields::wildcards(pcx, missing_ctor).apply(pcx, missing_ctor) + }) + .collect() + }; + witnesses .into_iter() .flat_map(|witness| { @@ -976,9 +1001,9 @@ fn apply_constructor( } #[derive(Copy, Clone, Debug)] -enum WitnessPreference { - ConstructWitness, - LeaveOutWitness, +enum ArmType { + FakeExtraWildcard, + RealArm, } /// A witness of non-exhaustiveness for error reporting, represented @@ -1056,6 +1081,32 @@ fn apply_constructor<'p>( } } +/// Report that a match of a `non_exhaustive` enum marked with `non_exhaustive_omitted_patterns` +/// is not exhaustive enough. +/// +/// NB: The partner lint for structs lives in `compiler/rustc_typeck/src/check/pat.rs`. +fn lint_non_exhaustive_omitted_patterns<'p, 'tcx>( + cx: &MatchCheckCtxt<'p, 'tcx>, + scrut_ty: Ty<'tcx>, + sp: Span, + hir_id: HirId, + witnesses: Vec>, +) { + let joined_patterns = joined_uncovered_patterns(&witnesses); + cx.tcx.struct_span_lint_hir(NON_EXHAUSTIVE_OMITTED_PATTERNS, hir_id, sp, |build| { + let mut lint = build.build("some variants are not matched explicitly"); + lint.span_label(sp, pattern_not_covered_label(&witnesses, &joined_patterns)); + lint.help( + "ensure that all variants are matched explicitly by adding the suggested match arms", + ); + lint.note(&format!( + "the matched value is of type `{}` and the `non_exhaustive_omitted_patterns` attribute was found", + scrut_ty, + )); + lint.emit(); + }); +} + /// Algorithm from . /// The algorithm from the paper has been modified to correctly handle empty /// types. The changes are: @@ -1086,7 +1137,7 @@ fn is_useful<'p, 'tcx>( cx: &MatchCheckCtxt<'p, 'tcx>, matrix: &Matrix<'p, 'tcx>, v: &PatStack<'p, 'tcx>, - witness_preference: WitnessPreference, + witness_preference: ArmType, hir_id: HirId, is_under_guard: bool, is_top_level: bool, @@ -1113,7 +1164,8 @@ fn is_useful<'p, 'tcx>( // FIXME(Nadrieril): Hack to work around type normalization issues (see #72476). let ty = matrix.heads().next().map_or(v.head().ty, |r| r.ty); - let pcx = PatCtxt { cx, ty, span: v.head().span, is_top_level }; + let is_non_exhaustive = cx.is_foreign_non_exhaustive_enum(ty); + let pcx = PatCtxt { cx, ty, span: v.head().span, is_top_level, is_non_exhaustive }; // If the first pattern is an or-pattern, expand it. let ret = if is_or_pat(v.head()) { @@ -1148,6 +1200,7 @@ fn is_useful<'p, 'tcx>( } // We split the head constructor of `v`. let split_ctors = v_ctor.split(pcx, matrix.head_ctors(cx)); + let is_non_exhaustive_and_wild = is_non_exhaustive && v_ctor.is_wildcard(); // For each constructor, we compute whether there's a value that starts with it that would // witness the usefulness of `v`. let start_matrix = &matrix; @@ -1160,10 +1213,46 @@ fn is_useful<'p, 'tcx>( let v = v.pop_head_constructor(&ctor_wild_subpatterns); let usefulness = is_useful(cx, &spec_matrix, &v, witness_preference, hir_id, is_under_guard, false); + + // When all the conditions are met we have a match with a `non_exhaustive` enum + // that has the potential to trigger the `non_exhaustive_omitted_patterns` lint. + // To understand the workings checkout `Constructor::split` and `SplitWildcard::new/into_ctors` + if is_non_exhaustive_and_wild + // We check that the match has a wildcard pattern and that that wildcard is useful, + // meaning there are variants that are covered by the wildcard. Without the check + // for `witness_preference` the lint would trigger on `if let NonExhaustiveEnum::A = foo {}` + && usefulness.is_useful() && matches!(witness_preference, RealArm) + && matches!( + &ctor, + Constructor::Missing { nonexhaustive_enum_missing_real_variants: true } + ) + { + let patterns = { + let mut split_wildcard = SplitWildcard::new(pcx); + split_wildcard.split(pcx, matrix.head_ctors(pcx.cx)); + // Construct for each missing constructor a "wild" version of this + // constructor, that matches everything that can be built with + // it. For example, if `ctor` is a `Constructor::Variant` for + // `Option::Some`, we get the pattern `Some(_)`. + split_wildcard + .iter_missing(pcx) + // Filter out the `Constructor::NonExhaustive` variant it's meaningless + // to our lint + .filter(|c| !c.is_non_exhaustive()) + .map(|missing_ctor| { + Fields::wildcards(pcx, missing_ctor).apply(pcx, missing_ctor) + }) + .collect::>() + }; + + lint_non_exhaustive_omitted_patterns(pcx.cx, pcx.ty, pcx.span, hir_id, patterns); + } + usefulness.apply_constructor(pcx, start_matrix, &ctor, &ctor_wild_subpatterns) }); Usefulness::merge(witness_preference, usefulnesses) }; + debug!(?ret); ret } @@ -1214,8 +1303,7 @@ fn is_useful<'p, 'tcx>( .copied() .map(|arm| { let v = PatStack::from_pattern(arm.pat); - let usefulness = - is_useful(cx, &matrix, &v, LeaveOutWitness, arm.hir_id, arm.has_guard, true); + let usefulness = is_useful(cx, &matrix, &v, RealArm, arm.hir_id, arm.has_guard, true); if !arm.has_guard { matrix.push(v); } @@ -1232,7 +1320,7 @@ fn is_useful<'p, 'tcx>( let wild_pattern = cx.pattern_arena.alloc(Pat::wildcard_from_ty(scrut_ty)); let v = PatStack::from_pattern(wild_pattern); - let usefulness = is_useful(cx, &matrix, &v, ConstructWitness, scrut_hir_id, false, true); + let usefulness = is_useful(cx, &matrix, &v, FakeExtraWildcard, scrut_hir_id, false, true); let non_exhaustiveness_witnesses = match usefulness { WithWitnesses(pats) => pats.into_iter().map(|w| w.single_pattern()).collect(), NoWitnesses(_) => bug!(), diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index 140a9d1126d..a056ab6aef2 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -11,6 +11,7 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_middle::ty::subst::GenericArg; use rustc_middle::ty::{self, Adt, BindingMode, Ty, TypeFoldable}; +use rustc_session::lint::builtin::NON_EXHAUSTIVE_OMITTED_PATTERNS; use rustc_span::hygiene::DesugaringKind; use rustc_span::lev_distance::find_best_match_for_name; use rustc_span::source_map::{Span, Spanned}; @@ -1261,7 +1262,8 @@ fn check_struct_pat_fields( }; // Require `..` if struct has non_exhaustive attribute. - if variant.is_field_list_non_exhaustive() && !adt.did.is_local() && !etc { + let non_exhaustive = variant.is_field_list_non_exhaustive() && !adt.did.is_local(); + if non_exhaustive && !etc { self.error_foreign_non_exhaustive_spat(pat, adt.variant_descr(), fields.is_empty()); } @@ -1276,7 +1278,7 @@ fn check_struct_pat_fields( if etc { tcx.sess.struct_span_err(pat.span, "`..` cannot be used in union patterns").emit(); } - } else if !etc && !unmentioned_fields.is_empty() { + } else if !unmentioned_fields.is_empty() { let accessible_unmentioned_fields: Vec<_> = unmentioned_fields .iter() .copied() @@ -1284,16 +1286,19 @@ fn check_struct_pat_fields( field.vis.is_accessible_from(tcx.parent_module(pat.hir_id).to_def_id(), tcx) }) .collect(); - - if accessible_unmentioned_fields.is_empty() { - unmentioned_err = Some(self.error_no_accessible_fields(pat, &fields)); - } else { - unmentioned_err = Some(self.error_unmentioned_fields( - pat, - &accessible_unmentioned_fields, - accessible_unmentioned_fields.len() != unmentioned_fields.len(), - &fields, - )); + if non_exhaustive { + self.non_exhaustive_reachable_pattern(pat, &accessible_unmentioned_fields, adt_ty) + } else if !etc { + if accessible_unmentioned_fields.is_empty() { + unmentioned_err = Some(self.error_no_accessible_fields(pat, &fields)); + } else { + unmentioned_err = Some(self.error_unmentioned_fields( + pat, + &accessible_unmentioned_fields, + accessible_unmentioned_fields.len() != unmentioned_fields.len(), + &fields, + )); + } } } match (inexistent_fields_err, unmentioned_err) { @@ -1604,6 +1609,51 @@ fn error_no_accessible_fields( err } + /// Report that a pattern for a `#[non_exhaustive]` struct marked with `non_exhaustive_omitted_patterns` + /// is not exhaustive enough. + /// + /// Nb: the partner lint for enums lives in `compiler/rustc_mir_build/src/thir/pattern/usefulness.rs`. + fn non_exhaustive_reachable_pattern( + &self, + pat: &Pat<'_>, + unmentioned_fields: &[(&ty::FieldDef, Ident)], + ty: Ty<'tcx>, + ) { + fn joined_uncovered_patterns(witnesses: &[&Ident]) -> String { + const LIMIT: usize = 3; + match witnesses { + [] => bug!(), + [witness] => format!("`{}`", witness), + [head @ .., tail] if head.len() < LIMIT => { + let head: Vec<_> = head.iter().map(<_>::to_string).collect(); + format!("`{}` and `{}`", head.join("`, `"), tail) + } + _ => { + let (head, tail) = witnesses.split_at(LIMIT); + let head: Vec<_> = head.iter().map(<_>::to_string).collect(); + format!("`{}` and {} more", head.join("`, `"), tail.len()) + } + } + } + let joined_patterns = joined_uncovered_patterns( + &unmentioned_fields.iter().map(|(_, i)| i).collect::>(), + ); + + self.tcx.struct_span_lint_hir(NON_EXHAUSTIVE_OMITTED_PATTERNS, pat.hir_id, pat.span, |build| { + let mut lint = build.build("some fields are not explicitly listed"); + lint.span_label(pat.span, format!("field{} {} not listed", rustc_errors::pluralize!(unmentioned_fields.len()), joined_patterns)); + + lint.help( + "ensure that all fields are mentioned explicitly by adding the suggested fields", + ); + lint.note(&format!( + "the pattern is of type `{}` and the `non_exhaustive_omitted_patterns` attribute was found", + ty, + )); + lint.emit(); + }); + } + /// Returns a diagnostic reporting a struct pattern which does not mention some fields. /// /// ```text diff --git a/src/test/ui/rfc-2008-non-exhaustive/auxiliary/enums.rs b/src/test/ui/rfc-2008-non-exhaustive/auxiliary/enums.rs index 8516bafef9b..0098f087d10 100644 --- a/src/test/ui/rfc-2008-non-exhaustive/auxiliary/enums.rs +++ b/src/test/ui/rfc-2008-non-exhaustive/auxiliary/enums.rs @@ -4,8 +4,29 @@ pub enum NonExhaustiveEnum { Unit, Tuple(u32), - Struct { field: u32 } + Struct { field: u32 }, +} + +#[non_exhaustive] +pub enum NestedNonExhaustive { + A(NonExhaustiveEnum), + B, + C, } #[non_exhaustive] pub enum EmptyNonExhaustiveEnum {} + +pub enum VariantNonExhaustive { + #[non_exhaustive] + Bar { + x: u32, + y: u64, + }, + Baz(u32, u16), +} + +#[non_exhaustive] +pub enum NonExhaustiveSingleVariant { + A(bool), +} diff --git a/src/test/ui/rfc-2008-non-exhaustive/auxiliary/structs.rs b/src/test/ui/rfc-2008-non-exhaustive/auxiliary/structs.rs index 6bfe7bf923d..5b2181d2d83 100644 --- a/src/test/ui/rfc-2008-non-exhaustive/auxiliary/structs.rs +++ b/src/test/ui/rfc-2008-non-exhaustive/auxiliary/structs.rs @@ -1,3 +1,4 @@ +#[derive(Default)] #[non_exhaustive] pub struct NormalStruct { pub first_field: u16, @@ -15,7 +16,7 @@ pub struct NormalStruct { pub struct FunctionalRecord { pub first_field: u16, pub second_field: u16, - pub third_field: bool + pub third_field: bool, } impl Default for FunctionalRecord { @@ -23,3 +24,10 @@ fn default() -> FunctionalRecord { FunctionalRecord { first_field: 640, second_field: 480, third_field: false } } } + +#[derive(Default)] +#[non_exhaustive] +pub struct NestedStruct { + pub foo: u16, + pub bar: NormalStruct, +} diff --git a/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.rs b/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.rs new file mode 100644 index 00000000000..115fd300fa5 --- /dev/null +++ b/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.rs @@ -0,0 +1,160 @@ +// Test that the `non_exhaustive_omitted_patterns` lint is triggered correctly. + +// aux-build:enums.rs +extern crate enums; + +// aux-build:structs.rs +extern crate structs; + +use enums::{ + EmptyNonExhaustiveEnum, NestedNonExhaustive, NonExhaustiveEnum, NonExhaustiveSingleVariant, + VariantNonExhaustive, +}; +use structs::{FunctionalRecord, NestedStruct, NormalStruct}; + +#[non_exhaustive] +#[derive(Default)] +pub struct Foo { + a: u8, + b: usize, + c: String, +} + +#[non_exhaustive] +pub enum Bar { + A, + B, + C, +} + +fn main() { + let enumeration = Bar::A; + + // Ok: this is a crate local non_exhaustive enum + match enumeration { + Bar::A => {} + Bar::B => {} + #[deny(non_exhaustive_omitted_patterns)] + _ => {} + } + + let non_enum = NonExhaustiveEnum::Unit; + + // Ok: without the attribute + match non_enum { + NonExhaustiveEnum::Unit => {} + NonExhaustiveEnum::Tuple(_) => {} + _ => {} + } + + match non_enum { + NonExhaustiveEnum::Unit => {} + NonExhaustiveEnum::Tuple(_) => {} + #[deny(non_exhaustive_omitted_patterns)] + _ => {} + } + //~^^ some variants are not matched explicitly + + match non_enum { + NonExhaustiveEnum::Unit | NonExhaustiveEnum::Struct { .. } => {} + #[deny(non_exhaustive_omitted_patterns)] + _ => {} + } + //~^^ some variants are not matched explicitly + + let x = 5; + match non_enum { + NonExhaustiveEnum::Unit if x > 10 => {} + NonExhaustiveEnum::Tuple(_) => {} + NonExhaustiveEnum::Struct { .. } => {} + #[deny(non_exhaustive_omitted_patterns)] + _ => {} + } + //~^^ some variants are not matched explicitly + + // Ok: all covered and not `unreachable-patterns` + #[deny(unreachable_patterns)] + match non_enum { + NonExhaustiveEnum::Unit => {} + NonExhaustiveEnum::Tuple(_) => {} + NonExhaustiveEnum::Struct { .. } => {} + #[deny(non_exhaustive_omitted_patterns)] + _ => {} + } + + #[deny(non_exhaustive_omitted_patterns)] + match NestedNonExhaustive::B { + NestedNonExhaustive::A(NonExhaustiveEnum::Unit) => {} + NestedNonExhaustive::A(_) => {} + NestedNonExhaustive::B => {} + _ => {} + } + //~^^ some variants are not matched explicitly + //~^^^^^ some variants are not matched explicitly + + // The io::ErrorKind has many `unstable` fields how do they interact with this + // lint + #[deny(non_exhaustive_omitted_patterns)] + match std::io::ErrorKind::Other { + std::io::ErrorKind::NotFound => {} + std::io::ErrorKind::PermissionDenied => {} + std::io::ErrorKind::ConnectionRefused => {} + std::io::ErrorKind::ConnectionReset => {} + std::io::ErrorKind::ConnectionAborted => {} + std::io::ErrorKind::NotConnected => {} + std::io::ErrorKind::AddrInUse => {} + std::io::ErrorKind::AddrNotAvailable => {} + std::io::ErrorKind::BrokenPipe => {} + std::io::ErrorKind::AlreadyExists => {} + std::io::ErrorKind::WouldBlock => {} + std::io::ErrorKind::InvalidInput => {} + std::io::ErrorKind::InvalidData => {} + std::io::ErrorKind::TimedOut => {} + std::io::ErrorKind::WriteZero => {} + std::io::ErrorKind::Interrupted => {} + std::io::ErrorKind::Other => {} + std::io::ErrorKind::UnexpectedEof => {} + std::io::ErrorKind::Unsupported => {} + std::io::ErrorKind::OutOfMemory => {} + // All stable variants are above and unstable in `_` + _ => {} + } + //~^^ some variants are not matched explicitly + + #[warn(non_exhaustive_omitted_patterns)] + match VariantNonExhaustive::Baz(1, 2) { + VariantNonExhaustive::Baz(_, _) => {} + VariantNonExhaustive::Bar { x, .. } => {} + } + //~^^ some fields are not explicitly listed + + #[warn(non_exhaustive_omitted_patterns)] + let FunctionalRecord { first_field, second_field, .. } = FunctionalRecord::default(); + //~^ some fields are not explicitly listed + + // Ok: this is local + #[warn(non_exhaustive_omitted_patterns)] + let Foo { a, b, .. } = Foo::default(); + + #[warn(non_exhaustive_omitted_patterns)] + let NestedStruct { bar: NormalStruct { first_field, .. }, .. } = NestedStruct::default(); + //~^ some fields are not explicitly listed + //~^^ some fields are not explicitly listed + + // Ok: because this only has 1 variant + #[deny(non_exhaustive_omitted_patterns)] + match NonExhaustiveSingleVariant::A(true) { + NonExhaustiveSingleVariant::A(true) => {} + _ => {} + } + + #[deny(non_exhaustive_omitted_patterns)] + match NonExhaustiveSingleVariant::A(true) { + _ => {} + } + //~^^ some variants are not matched explicitly + + // Ok: we don't lint on `if let` expressions + #[deny(non_exhaustive_omitted_patterns)] + if let NonExhaustiveEnum::Tuple(_) = non_enum {} +} diff --git a/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.stderr b/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.stderr new file mode 100644 index 00000000000..aebe2acb6ad --- /dev/null +++ b/src/test/ui/rfc-2008-non-exhaustive/reachable-patterns.stderr @@ -0,0 +1,146 @@ +warning: some fields are not explicitly listed + --> $DIR/reachable-patterns.rs:127:9 + | +LL | VariantNonExhaustive::Bar { x, .. } => {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ field `y` not listed + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:124:12 + | +LL | #[warn(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all fields are mentioned explicitly by adding the suggested fields + = note: the pattern is of type `VariantNonExhaustive` and the `non_exhaustive_omitted_patterns` attribute was found + +warning: some fields are not explicitly listed + --> $DIR/reachable-patterns.rs:132:9 + | +LL | let FunctionalRecord { first_field, second_field, .. } = FunctionalRecord::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ field `third_field` not listed + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:131:12 + | +LL | #[warn(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all fields are mentioned explicitly by adding the suggested fields + = note: the pattern is of type `FunctionalRecord` and the `non_exhaustive_omitted_patterns` attribute was found + +warning: some fields are not explicitly listed + --> $DIR/reachable-patterns.rs:140:29 + | +LL | let NestedStruct { bar: NormalStruct { first_field, .. }, .. } = NestedStruct::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ field `second_field` not listed + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:139:12 + | +LL | #[warn(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all fields are mentioned explicitly by adding the suggested fields + = note: the pattern is of type `NormalStruct` and the `non_exhaustive_omitted_patterns` attribute was found + +warning: some fields are not explicitly listed + --> $DIR/reachable-patterns.rs:140:9 + | +LL | let NestedStruct { bar: NormalStruct { first_field, .. }, .. } = NestedStruct::default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ field `foo` not listed + | + = help: ensure that all fields are mentioned explicitly by adding the suggested fields + = note: the pattern is of type `NestedStruct` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:54:9 + | +LL | _ => {} + | ^ pattern `Struct { .. }` not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:53:16 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:61:9 + | +LL | _ => {} + | ^ pattern `Tuple(_)` not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:60:16 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:71:9 + | +LL | _ => {} + | ^ pattern `Unit` not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:70:16 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:88:32 + | +LL | NestedNonExhaustive::A(_) => {} + | ^ patterns `Tuple(_)` and `Struct { .. }` not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:85:12 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NonExhaustiveEnum` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:90:9 + | +LL | _ => {} + | ^ pattern `C` not covered + | + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NestedNonExhaustive` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:120:9 + | +LL | _ => {} + | ^ patterns `HostUnreachable`, `NetworkUnreachable`, `NetworkDown` and 18 more not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:97:12 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `ErrorKind` and the `non_exhaustive_omitted_patterns` attribute was found + +error: some variants are not matched explicitly + --> $DIR/reachable-patterns.rs:153:9 + | +LL | _ => {} + | ^ pattern `A(_)` not covered + | +note: the lint level is defined here + --> $DIR/reachable-patterns.rs:151:12 + | +LL | #[deny(non_exhaustive_omitted_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: ensure that all variants are matched explicitly by adding the suggested match arms + = note: the matched value is of type `NonExhaustiveSingleVariant` and the `non_exhaustive_omitted_patterns` attribute was found + +error: aborting due to 7 previous errors; 4 warnings emitted + diff --git a/src/test/ui/rfc-2008-non-exhaustive/struct.stderr b/src/test/ui/rfc-2008-non-exhaustive/struct.stderr index b0c319f2c7f..272b2ef6ee1 100644 --- a/src/test/ui/rfc-2008-non-exhaustive/struct.stderr +++ b/src/test/ui/rfc-2008-non-exhaustive/struct.stderr @@ -16,13 +16,13 @@ error[E0603]: tuple struct constructor `TupleStruct` is private LL | let ts_explicit = structs::TupleStruct(640, 480); | ^^^^^^^^^^^ private tuple struct constructor | - ::: $DIR/auxiliary/structs.rs:11:24 + ::: $DIR/auxiliary/structs.rs:12:24 | LL | pub struct TupleStruct(pub u16, pub u16); | ---------------- a constructor is private if any of the fields is private | note: the tuple struct constructor `TupleStruct` is defined here - --> $DIR/auxiliary/structs.rs:11:1 + --> $DIR/auxiliary/structs.rs:12:1 | LL | pub struct TupleStruct(pub u16, pub u16); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -34,7 +34,7 @@ LL | let us_explicit = structs::UnitStruct; | ^^^^^^^^^^ private unit struct | note: the unit struct `UnitStruct` is defined here - --> $DIR/auxiliary/structs.rs:8:1 + --> $DIR/auxiliary/structs.rs:9:1 | LL | pub struct UnitStruct; | ^^^^^^^^^^^^^^^^^^^^^^