Skip to content

Commit

Permalink
Audit sealed traits, take 2 (#6015)
Browse files Browse the repository at this point in the history
Fixes #4338

This:

 - Adds a doc header to all sealed traits
 - Adds a (different) doc header to all UnstableSealed traits
 - Ensures all sealed traits have hidden methods
- Seals the segmenter type traits since they're implementing a "compile
time enum" pattern
- Audits the rest of the traits and adds comments explaining that they
are explicitly implementable (or de-facto sealed by dint of having a
blanket impl)
 - Makes CldrCalendar Sealed instead of UnstableSealed
  • Loading branch information
Manishearth authored Jan 20, 2025
1 parent 6533329 commit 9fc2b42
Show file tree
Hide file tree
Showing 18 changed files with 169 additions and 6 deletions.
3 changes: 3 additions & 0 deletions components/casemap/src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ use icu_collections::codepointinvlist::CodePointInversionListBuilder;
/// will be some kind of set over codepoints and strings, or something that
/// can be built into one.
///
/// An implementation is provided for [`CodePointInversionListBuilder`], but users are encouraged
/// to implement this trait on their own collections as needed.
///
/// [`CaseMapCloser::add_string_case_closure_to()`]: crate::CaseMapCloser::add_string_case_closure_to
/// [`CaseMapCloser::add_case_closure_to()`]: crate::CaseMapCloser::add_case_closure_to
pub trait ClosureSink {
Expand Down
2 changes: 2 additions & 0 deletions components/collections/src/codepointtrie/cptrie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ pub enum TrieType {

/// A trait representing the values stored in the data array of a [`CodePointTrie`].
/// This trait is used as a type parameter in constructing a `CodePointTrie`.
///
/// This trait can be implemented on anything that can be represented as a u32s worth of data.
pub trait TrieValue: Copy + Eq + PartialEq + zerovec::ule::AsULE + 'static {
/// Last-resort fallback value to return if we cannot read data from the trie.
///
Expand Down
35 changes: 32 additions & 3 deletions components/datetime/src/scaffold/calendar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@ use icu_provider::marker::NeverMarker;
use icu_provider::prelude::*;
use icu_timezone::{DateTime, Time, TimeZoneInfo, TimeZoneModel, UtcOffset, ZonedDateTime};

mod private {
pub trait Sealed {}
}

/// A calendar that can be found in CLDR.
///
/// New implementors of this trait will likely also wish to modify `get_era_code_map()`
/// in the CLDR transformer to support any new era maps.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland.
/// 🚫 This trait is sealed; it cannot be implemented by user code. If an API requests an item that implements this
/// trait, please consider using a type from the implementors listed below.
/// </div>
pub trait CldrCalendar: UnstableSealed {
pub trait CldrCalendar: private::Sealed {
/// The data marker for loading year symbols for this calendar.
type YearNamesV1Marker: DataMarker<DataStruct = YearNamesV1<'static>>;

Expand All @@ -40,60 +44,70 @@ pub trait CldrCalendar: UnstableSealed {
type SkeletaV1Marker: DataMarker<DataStruct = PackedPatternsV1<'static>>;
}

impl private::Sealed for () {}
impl CldrCalendar for () {
type YearNamesV1Marker = NeverMarker<YearNamesV1<'static>>;
type MonthNamesV1Marker = NeverMarker<MonthNamesV1<'static>>;
type SkeletaV1Marker = NeverMarker<PackedPatternsV1<'static>>;
}

impl private::Sealed for Buddhist {}
impl CldrCalendar for Buddhist {
type YearNamesV1Marker = BuddhistYearNamesV1Marker;
type MonthNamesV1Marker = BuddhistMonthNamesV1Marker;
type SkeletaV1Marker = BuddhistDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Chinese {}
impl CldrCalendar for Chinese {
type YearNamesV1Marker = ChineseYearNamesV1Marker;
type MonthNamesV1Marker = ChineseMonthNamesV1Marker;
type SkeletaV1Marker = ChineseDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Coptic {}
impl CldrCalendar for Coptic {
type YearNamesV1Marker = CopticYearNamesV1Marker;
type MonthNamesV1Marker = CopticMonthNamesV1Marker;
type SkeletaV1Marker = CopticDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Dangi {}
impl CldrCalendar for Dangi {
type YearNamesV1Marker = DangiYearNamesV1Marker;
type MonthNamesV1Marker = DangiMonthNamesV1Marker;
type SkeletaV1Marker = DangiDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Ethiopian {}
impl CldrCalendar for Ethiopian {
type YearNamesV1Marker = EthiopianYearNamesV1Marker;
type MonthNamesV1Marker = EthiopianMonthNamesV1Marker;
type SkeletaV1Marker = EthiopianDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Gregorian {}
impl CldrCalendar for Gregorian {
type YearNamesV1Marker = GregorianYearNamesV1Marker;
type MonthNamesV1Marker = GregorianMonthNamesV1Marker;
type SkeletaV1Marker = GregorianDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Hebrew {}
impl CldrCalendar for Hebrew {
type YearNamesV1Marker = HebrewYearNamesV1Marker;
type MonthNamesV1Marker = HebrewMonthNamesV1Marker;
type SkeletaV1Marker = HebrewDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Indian {}
impl CldrCalendar for Indian {
type YearNamesV1Marker = IndianYearNamesV1Marker;
type MonthNamesV1Marker = IndianMonthNamesV1Marker;
type SkeletaV1Marker = IndianDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for IslamicCivil {}
impl CldrCalendar for IslamicCivil {
// this value is not actually a valid identifier for this calendar,
// however since we are overriding is_identifier_allowed_for_calendar we are using
Expand All @@ -103,42 +117,49 @@ impl CldrCalendar for IslamicCivil {
type SkeletaV1Marker = IslamicDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for IslamicObservational {}
impl CldrCalendar for IslamicObservational {
type YearNamesV1Marker = IslamicYearNamesV1Marker;
type MonthNamesV1Marker = IslamicMonthNamesV1Marker;
type SkeletaV1Marker = IslamicDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for IslamicTabular {}
impl CldrCalendar for IslamicTabular {
type YearNamesV1Marker = IslamicYearNamesV1Marker;
type MonthNamesV1Marker = IslamicMonthNamesV1Marker;
type SkeletaV1Marker = IslamicDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for IslamicUmmAlQura {}
impl CldrCalendar for IslamicUmmAlQura {
type YearNamesV1Marker = IslamicYearNamesV1Marker;
type MonthNamesV1Marker = IslamicMonthNamesV1Marker;
type SkeletaV1Marker = IslamicDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Japanese {}
impl CldrCalendar for Japanese {
type YearNamesV1Marker = JapaneseYearNamesV1Marker;
type MonthNamesV1Marker = JapaneseMonthNamesV1Marker;
type SkeletaV1Marker = JapaneseDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for JapaneseExtended {}
impl CldrCalendar for JapaneseExtended {
type YearNamesV1Marker = JapaneseExtendedYearNamesV1Marker;
type MonthNamesV1Marker = JapaneseExtendedMonthNamesV1Marker;
type SkeletaV1Marker = JapaneseExtendedDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Persian {}
impl CldrCalendar for Persian {
type YearNamesV1Marker = PersianYearNamesV1Marker;
type MonthNamesV1Marker = PersianMonthNamesV1Marker;
type SkeletaV1Marker = PersianDateNeoSkeletonPatternsV1Marker;
}

impl private::Sealed for Roc {}
impl CldrCalendar for Roc {
type YearNamesV1Marker = RocYearNamesV1Marker;
type MonthNamesV1Marker = RocMonthNamesV1Marker;
Expand Down Expand Up @@ -169,6 +190,11 @@ impl UnstableSealed for Roc {}
/// [`DynamicDataMarker`]. For example, this trait can be implemented for [`YearNamesV1Marker`].
///
/// This trait serves as a building block for a cross-calendar [`BoundDataProvider`].
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait CalMarkers<M>: UnstableSealed
where
M: DynamicDataMarker,
Expand Down Expand Up @@ -382,6 +408,7 @@ impl_load_any_calendar!([
]);

/// A type that can be converted into a specific calendar system.
// This trait is implementable
pub trait ConvertCalendar {
/// The converted type. This can be the same as the receiver type.
type Converted<'a>: Sized;
Expand Down Expand Up @@ -439,6 +466,7 @@ impl<O: TimeZoneModel> ConvertCalendar for TimeZoneInfo<O> {
}

/// An input that may be associated with a specific runtime calendar.
// This trait is implementable
pub trait InSameCalendar {
/// Checks whether this type is compatible with the given calendar.
///
Expand Down Expand Up @@ -512,6 +540,7 @@ impl<O: TimeZoneModel> InSameCalendar for TimeZoneInfo<O> {
}

/// An input associated with a fixed, static calendar.
// This trait is implementable
pub trait InFixedCalendar<C> {}

impl<C: CldrCalendar, A: AsCalendar<Calendar = C>> InFixedCalendar<C> for Date<A> {}
Expand Down
35 changes: 35 additions & 0 deletions components/datetime/src/scaffold/fieldset_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ use icu_timezone::{
/// (input types only).
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait DateInputMarkers: UnstableSealed {
/// Marker for resolving the year input field.
type YearInput: IntoOption<YearInfo>;
Expand All @@ -45,6 +50,11 @@ pub trait DateInputMarkers: UnstableSealed {
/// (data markers only).
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait TypedDateDataMarkers<C>: UnstableSealed {
/// Marker for loading date skeleton patterns.
type DateSkeletonPatternsV1Marker: DataMarker<DataStruct = PackedPatternsV1<'static>>;
Expand All @@ -60,6 +70,11 @@ pub trait TypedDateDataMarkers<C>: UnstableSealed {
/// (data markers only).
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait DateDataMarkers: UnstableSealed {
/// Cross-calendar data markers for date skeleta.
type Skel: CalMarkers<ErasedPackedPatterns>;
Expand All @@ -75,6 +90,11 @@ pub trait DateDataMarkers: UnstableSealed {
/// (input types and data markers).
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait TimeMarkers: UnstableSealed {
/// Marker for resolving the day-of-month input field.
type HourInput: IntoOption<IsoHour>;
Expand All @@ -94,6 +114,11 @@ pub trait TimeMarkers: UnstableSealed {
/// (input types and data markers).
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait ZoneMarkers: UnstableSealed {
/// Marker for resolving the time zone id input field.
type TimeZoneIdInput: IntoOption<TimeZoneBcp47Id>;
Expand Down Expand Up @@ -123,6 +148,11 @@ pub trait ZoneMarkers: UnstableSealed {
/// required for datetime formatting.
///
/// This is a sealed trait implemented on field set markers.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait DateTimeMarkers: UnstableSealed + DateTimeNamesMarker {
/// Associated types for date formatting.
///
Expand Down Expand Up @@ -156,6 +186,7 @@ pub trait DateTimeMarkers: UnstableSealed + DateTimeNamesMarker {
/// - [`TimeZoneInfo`](icu_timezone::TimeZoneInfo)
///
/// [`fieldsets::YMD`]: crate::fieldsets::YMD
// This trait is implicitly sealed due to sealed supertraits
pub trait AllInputMarkers<R: DateTimeMarkers>:
GetField<<R::D as DateInputMarkers>::YearInput>
+ GetField<<R::D as DateInputMarkers>::MonthInput>
Expand Down Expand Up @@ -204,6 +235,7 @@ where
///
/// This trait is implemented on all providers that support datetime formatting,
/// including [`crate::provider::Baked`].
// This trait is implicitly sealed due to sealed supertraits
pub trait AllFixedCalendarFormattingDataMarkers<C: CldrCalendar, FSet: DateTimeMarkers>:
DataProvider<<FSet::D as TypedDateDataMarkers<C>>::YearNamesV1Marker>
+ DataProvider<<FSet::D as TypedDateDataMarkers<C>>::MonthNamesV1Marker>
Expand Down Expand Up @@ -256,6 +288,7 @@ where
///
/// This trait is implemented on all providers that support datetime formatting,
/// including [`crate::provider::Baked`].
// This trait is implicitly sealed due to sealed supertraits
pub trait AllAnyCalendarFormattingDataMarkers<FSet: DateTimeMarkers>:
DataProvider<<<FSet::D as DateDataMarkers>::Year as CalMarkers<YearNamesV1Marker>>::Buddhist>
+ DataProvider<<<FSet::D as DateDataMarkers>::Year as CalMarkers<YearNamesV1Marker>>::Chinese>
Expand Down Expand Up @@ -400,6 +433,7 @@ where

/// Trait to consolidate data provider markers external to this crate
/// for datetime formatting with a fixed calendar.
// This trait is implicitly sealed due to sealed supertraits
pub trait AllFixedCalendarExternalDataMarkers:
DataProvider<DecimalSymbolsV2Marker> + DataProvider<DecimalDigitsV1Marker>
{
Expand All @@ -412,6 +446,7 @@ impl<T> AllFixedCalendarExternalDataMarkers for T where

/// Trait to consolidate data provider markers external to this crate
/// for datetime formatting with any calendar.
// This trait is implicitly sealed due to sealed supertraits
pub trait AllAnyCalendarExternalDataMarkers:
DataProvider<ChineseCacheV1Marker>
+ DataProvider<DangiCacheV1Marker>
Expand Down
5 changes: 5 additions & 0 deletions components/datetime/src/scaffold/get_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ use super::UnstableSealed;
/// A type that can return a certain field `T`.
///
/// This is used as a bound on various datetime functions.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait GetField<T>: UnstableSealed {
/// Returns the value of this trait's field `T`.
fn get_field(&self) -> T;
Expand Down
2 changes: 1 addition & 1 deletion components/datetime/src/scaffold/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ pub(crate) use names_storage::OptionalNames;
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland.
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
pub trait UnstableSealed {}
16 changes: 16 additions & 0 deletions components/datetime/src/scaffold/names_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ use super::UnstableSealed;
/// This trait allows for types that contain data for some but not all types of datetime names,
/// allowing for reduced stack size. For example, a type could contain year and month names but
/// not weekday, day period, or time zone names.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
#[allow(missing_docs)]
pub trait DateTimeNamesMarker: UnstableSealed {
type YearNames: NamesContainer<YearNamesV1Marker, YearNameLength>;
Expand All @@ -35,6 +40,11 @@ pub trait DateTimeNamesMarker: UnstableSealed {
}

/// Trait that associates a container for a payload parameterized by the given variables.
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
#[allow(missing_docs)]
pub trait NamesContainer<M: DynamicDataMarker, Variables>: UnstableSealed
where
Expand Down Expand Up @@ -100,6 +110,11 @@ impl MaybePayloadError {
/// a value depending on the type parameter `Variables`.
///
/// Helper trait for [`DateTimeNamesMarker`].
///
/// <div class="stab unstable">
/// 🚧 This trait is considered unstable; it may change at any time, in breaking or non-breaking ways,
/// including in SemVer minor releases. Do not implement this trait in userland unless you are prepared for things to occasionally break.
/// </div>
#[allow(missing_docs)]
pub trait MaybePayload<M: DynamicDataMarker, Variables>: UnstableSealed {
fn new_empty() -> Self;
Expand Down Expand Up @@ -334,6 +349,7 @@ where
/// is_trait_implemented::<TimeFieldSet, CompositeFieldSet>();
/// ```
#[allow(missing_docs)]
// This trait is implicitly sealed due to sealed supertraits
pub trait DateTimeNamesFrom<M: DateTimeNamesMarker>: DateTimeNamesMarker {
fn map_year_names(
other: <M::YearNames as NamesContainer<YearNamesV1Marker, YearNameLength>>::Container,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,11 @@ pub(super) struct Reverse;
/// The direction a match can be applied. Used in [`Utf8Matcher`].
///
/// See [`Forward`] and [`Reverse`] for implementors.
///
/// <div class="stab unstable">
/// 🚫 This trait is sealed; it cannot be implemented by user code. If an API requests an item that implements this
/// trait, please consider using a type from the implementors listed below.
/// </div>
pub(super) trait MatchDirection: sealed::Sealed {}
impl MatchDirection for Forward {}
impl MatchDirection for Reverse {}
Expand Down
Loading

0 comments on commit 9fc2b42

Please sign in to comment.