Skip to content

Implement DateFromFieldsError, a narrow error type for Date::try_from_fields #7100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Oct 15, 2025
Merged

Implement DateFromFieldsError, a narrow error type for Date::try_from_fields #7100

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7e6a1f6
Initial migration to DateFromFieldsError
sffc Oct 14, 2025
219caa9
Fix up docs tests
sffc Oct 14, 2025
1c904dd
ci
sffc Oct 14, 2025
37267ab
passing tests
sffc Oct 15, 2025
9af0e42
Error mapping commentary
sffc Oct 15, 2025
ab70a67
try_parse
sffc Oct 15, 2025
753bbdf
clippy, fmt
sffc Oct 15, 2025
c4b2e3c
use enums for all errors
sffc Oct 15, 2025
50b78f7
Remove MonthCode field from DateFromFieldsError
sffc Oct 15, 2025
1de767e
Merge branch 'main' into datefromfieldserror
sffc Oct 15, 2025
ffc284c
diplomat-coverage
sffc Oct 15, 2025
e78a06d
Fix errors in FFI and in new coverage test
sffc Oct 15, 2025
b5b027f
Fix rust link
sffc Oct 15, 2025
0624665
Add re-exports to diplomat-coverage allowlist
sffc Oct 15, 2025
f2de676
diplomat-gen
sffc Oct 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions components/calendar/src/any_calendar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use crate::cal::iso::IsoDateInner;
use crate::cal::*;
use crate::error::DateError;
use crate::error::{DateError, DateFromFieldsError};
use crate::options::DateFromFieldsOptions;
use crate::options::{DateAddOptions, DateDifferenceOptions};
use crate::types::{DateFields, YearInfo};
Expand Down Expand Up @@ -279,7 +279,7 @@ impl Calendar for AnyCalendar {
&self,
fields: DateFields,
options: DateFromFieldsOptions,
) -> Result<Self::DateInner, DateError> {
) -> Result<Self::DateInner, DateFromFieldsError> {
Ok(match_cal!(match self: (c) => c.from_fields(fields, options)?))
}

Expand Down
20 changes: 12 additions & 8 deletions components/calendar/src/cal/abstract_gregorian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::cal::iso::{IsoDateInner, IsoEra};
use crate::calendar_arithmetic::{
ArithmeticDate, ArithmeticDateBuilder, CalendarArithmetic, DateFieldsResolver,
};
use crate::error::DateError;
use crate::error::{DateError, DateFromFieldsError, EcmaReferenceYearError, UnknownEraError};
use crate::options::DateFromFieldsOptions;
use crate::options::{DateAddOptions, DateDifferenceOptions};
use crate::preferences::CalendarAlgorithm;
Expand All @@ -22,7 +22,7 @@ pub(crate) trait GregorianYears: Clone + core::fmt::Debug {
// Positive if after 0 CE
const EXTENDED_YEAR_OFFSET: i32 = 0;

fn extended_from_era_year(&self, era: Option<&str>, year: i32) -> Result<i32, DateError>;
fn extended_from_era_year(&self, era: Option<&str>, year: i32) -> Result<i32, UnknownEraError>;

fn era_year_from_extended(&self, extended_year: i32, month: u8, day: u8) -> EraYear;

Expand Down Expand Up @@ -88,7 +88,11 @@ impl<Y: GregorianYears> DateFieldsResolver for AbstractGregorian<Y> {
type YearInfo = i32;

#[inline]
fn year_info_from_era(&self, era: &str, era_year: i32) -> Result<Self::YearInfo, DateError> {
fn year_info_from_era(
&self,
era: &str,
era_year: i32,
) -> Result<Self::YearInfo, UnknownEraError> {
Ok(self.0.extended_from_era_year(Some(era), era_year)? + Y::EXTENDED_YEAR_OFFSET)
}

Expand All @@ -102,7 +106,7 @@ impl<Y: GregorianYears> DateFieldsResolver for AbstractGregorian<Y> {
&self,
_month_code: types::MonthCode,
_day: u8,
) -> Result<Self::YearInfo, DateError> {
) -> Result<Self::YearInfo, EcmaReferenceYearError> {
Ok(REFERENCE_YEAR)
}
}
Expand All @@ -118,10 +122,10 @@ impl<Y: GregorianYears> Calendar for AbstractGregorian<Y> {
&self,
fields: types::DateFields,
options: DateFromFieldsOptions,
) -> Result<Self::DateInner, DateError> {
) -> Result<Self::DateInner, DateFromFieldsError> {
let builder = ArithmeticDateBuilder::try_from_fields(fields, self, options)?;
ArithmeticDate::try_from_builder(builder, options)
.map_err(|e| e.maybe_with_month_code(fields.month_code))
let arithmetic_date = ArithmeticDate::try_from_builder(builder, options)?;
Ok(arithmetic_date)
}

fn from_rata_die(&self, date: RataDie) -> Self::DateInner {
Expand Down Expand Up @@ -222,7 +226,7 @@ macro_rules! impl_with_abstract_gregorian {
&self,
fields: crate::types::DateFields,
options: crate::options::DateFromFieldsOptions,
) -> Result<Self::DateInner, DateError> {
) -> Result<Self::DateInner, crate::error::DateFromFieldsError> {
let $self_ident = self;
crate::cal::abstract_gregorian::AbstractGregorian($eras_expr)
.from_fields(fields, options)
Expand Down
5 changes: 3 additions & 2 deletions components/calendar/src/cal/buddhist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// called LICENSE at the top level of the ICU4X source tree
// (online at: https://github.com/unicode-org/icu4x/blob/main/LICENSE ).

use crate::error::UnknownEraError;
use crate::preferences::CalendarAlgorithm;
use crate::{
cal::abstract_gregorian::{impl_with_abstract_gregorian, GregorianYears},
Expand Down Expand Up @@ -47,10 +48,10 @@ pub(crate) struct BuddhistEra;
impl GregorianYears for BuddhistEra {
const EXTENDED_YEAR_OFFSET: i32 = -543;

fn extended_from_era_year(&self, era: Option<&str>, year: i32) -> Result<i32, DateError> {
fn extended_from_era_year(&self, era: Option<&str>, year: i32) -> Result<i32, UnknownEraError> {
match era {
Some("be") | None => Ok(year),
_ => Err(DateError::UnknownEra),
_ => Err(UnknownEraError),
}
}

Expand Down
75 changes: 48 additions & 27 deletions components/calendar/src/cal/chinese.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::calendar_arithmetic::DateFieldsResolver;
use crate::calendar_arithmetic::{
ArithmeticDate, ArithmeticDateBuilder, CalendarArithmetic, ToExtendedYear,
};
use crate::error::DateError;
use crate::error::{
DateError, DateFromFieldsError, EcmaReferenceYearError, MonthCodeError, UnknownEraError,
};
use crate::options::{DateAddOptions, DateDifferenceOptions};
use crate::options::{DateFromFieldsOptions, Overflow};
use crate::provider::chinese_based::PackedChineseBasedYearInfo;
Expand Down Expand Up @@ -116,8 +118,8 @@ pub trait Rules: Clone + core::fmt::Debug + crate::cal::scaffold::UnstableSealed
&self,
_month_code: types::MonthCode,
_day: u8,
) -> Result<i32, DateError> {
Err(DateError::NotEnoughFields)
) -> Result<i32, EcmaReferenceYearError> {
Err(EcmaReferenceYearError::NotEnoughFields)
}

/// The debug name for the calendar defined by these [`Rules`].
Expand Down Expand Up @@ -195,10 +197,12 @@ impl Rules for China {
}
}

fn ecma_reference_year(&self, month_code: types::MonthCode, day: u8) -> Result<i32, DateError> {
let Some((number, is_leap)) = month_code.parsed() else {
return Err(DateError::UnknownMonthCode(month_code));
};
fn ecma_reference_year(
&self,
month_code: types::MonthCode,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: if we allow invalid month codes to be constructed, this method should take ordinal: u8, is_leap: bool to remove the parsing error case (or introduce a new ValidMonthCode type). once we move to &str inputs we basically don't need MonthCode anymore.

this also applies to all other (internal) methods that use MonthCode. validate early so not every method needs a InvalidMonthCode error variant

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cautiously like this idea, though I think it can be a non-2.1 blocking followup as a part of graduating these traits.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it can only be non-blocking if we don't sprinkle MonthCode in other parts of the API, like error variants, and the input itself

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you both are okay with adding icu_calendar::types::ValidMonthCode, I will proceed with that.

Copy link
Member

@Manishearth Manishearth Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of that. This issue is a mandatory 2.1 blocker so we can do things piecemeal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Looked at the PR, not in favor of the PR, stated why there)

I think here we can have MonthCode, or a tuple, or an unstable lightweight ValidMonthCode type. The latter two are more optimized, and we can change our mind when graduating the trait. We should note this thing as a decision-to-be-made on the graduation issue and move on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a decision about unstable code and does not block this PR, nor should its fix involve public stable API changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make ValidMonthCode pub(crate)/unstable in the other PR for now, and remove it from the error variant. I think in both cases where we convert to DateError, we have access to the input month code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that works!

day: u8,
) -> Result<i32, EcmaReferenceYearError> {
let (number, is_leap) = month_code.try_parse()?;
// Computed by `generate_reference_years`
Ok(match (number, is_leap, day > 29) {
(1, false, false) => 1972,
Expand Down Expand Up @@ -252,7 +256,11 @@ impl Rules for China {
(12, false, true) => 1971,
(12, true, false) => 1878,
(12, true, true) => 1783,
_ => return Err(DateError::UnknownMonthCode(month_code)),
_ => {
return Err(EcmaReferenceYearError::UnknownMonthCodeForCalendar(
month_code,
))
}
})
}

Expand Down Expand Up @@ -374,10 +382,12 @@ impl Rules for Korea {
}
}

fn ecma_reference_year(&self, month_code: types::MonthCode, day: u8) -> Result<i32, DateError> {
let Some((number, is_leap)) = month_code.parsed() else {
return Err(DateError::UnknownMonthCode(month_code));
};
fn ecma_reference_year(
&self,
month_code: types::MonthCode,
day: u8,
) -> Result<i32, EcmaReferenceYearError> {
let (number, is_leap) = month_code.try_parse()?;
// Computed by `generate_reference_years`
Ok(match (number, is_leap, day > 29) {
(1, false, false) => 1972,
Expand Down Expand Up @@ -431,7 +441,11 @@ impl Rules for Korea {
(12, false, true) => 1971,
(12, true, false) => 1878,
(12, true, true) => 1783,
_ => return Err(DateError::UnknownMonthCode(month_code)),
_ => {
return Err(EcmaReferenceYearError::UnknownMonthCodeForCalendar(
month_code,
))
}
})
}

Expand Down Expand Up @@ -548,9 +562,13 @@ impl<R: Rules> DateFieldsResolver for LunarChinese<R> {
type YearInfo = LunarChineseYearData;

#[inline]
fn year_info_from_era(&self, _era: &str, _era_year: i32) -> Result<Self::YearInfo, DateError> {
fn year_info_from_era(
&self,
_era: &str,
_era_year: i32,
) -> Result<Self::YearInfo, UnknownEraError> {
// This calendar has no era codes
Err(DateError::UnknownEra)
Err(UnknownEraError)
}

#[inline]
Expand All @@ -563,7 +581,7 @@ impl<R: Rules> DateFieldsResolver for LunarChinese<R> {
&self,
month_code: types::MonthCode,
day: u8,
) -> Result<Self::YearInfo, DateError> {
) -> Result<Self::YearInfo, EcmaReferenceYearError> {
Ok(self
.0
.year_data(self.0.ecma_reference_year(month_code, day)?))
Expand All @@ -574,15 +592,19 @@ impl<R: Rules> DateFieldsResolver for LunarChinese<R> {
year: &Self::YearInfo,
month_code: types::MonthCode,
options: DateFromFieldsOptions,
) -> Result<u8, DateError> {
) -> Result<u8, MonthCodeError> {
match year.parse_month_code(month_code) {
ComputedOrdinalMonth::Exact(val) => Ok(val),
ComputedOrdinalMonth::LeapToNormal(val)
if options.overflow == Some(Overflow::Constrain) =>
{
Ok(val)
ComputedOrdinalMonth::LeapToNormal(val) => {
if options.overflow == Some(Overflow::Constrain) {
Ok(val)
} else {
Err(MonthCodeError::UnknownMonthCodeForYear(month_code))
}
}
ComputedOrdinalMonth::NotFound => {
Err(MonthCodeError::UnknownMonthCodeForCalendar(month_code))
}
_ => Err(DateError::UnknownMonthCode(month_code)),
}
}

Expand All @@ -605,11 +627,10 @@ impl<R: Rules> Calendar for LunarChinese<R> {
&self,
fields: types::DateFields,
options: DateFromFieldsOptions,
) -> Result<Self::DateInner, DateError> {
) -> Result<Self::DateInner, DateFromFieldsError> {
let builder = ArithmeticDateBuilder::try_from_fields(fields, self, options)?;
Ok(ChineseDateInner(ArithmeticDate::try_from_builder(
builder, options,
)?))
let arithmetic_date = ArithmeticDate::try_from_builder(builder, options)?;
Ok(ChineseDateInner(arithmetic_date))
}

fn from_rata_die(&self, rd: RataDie) -> Self::DateInner {
Expand Down Expand Up @@ -1703,7 +1724,7 @@ mod test {
let cal = LunarChinese::new_china();
assert!(matches!(
Date::try_from_fields(fields, options, cal).unwrap_err(),
DateError::Range { .. }
DateFromFieldsError::Range { .. }
));
}

Expand Down
35 changes: 19 additions & 16 deletions components/calendar/src/cal/coptic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
use crate::cal::iso::{Iso, IsoDateInner};
use crate::calendar_arithmetic::{ArithmeticDate, CalendarArithmetic};
use crate::calendar_arithmetic::{ArithmeticDateBuilder, DateFieldsResolver};
use crate::error::DateError;
use crate::error::{
DateError, DateFromFieldsError, EcmaReferenceYearError, MonthCodeError, UnknownEraError,
};
use crate::options::DateFromFieldsOptions;
use crate::options::{DateAddOptions, DateDifferenceOptions};
use crate::{types, Calendar, Date, RangeError};
Expand Down Expand Up @@ -85,10 +87,14 @@ impl DateFieldsResolver for Coptic {
type YearInfo = i32;

#[inline]
fn year_info_from_era(&self, era: &str, era_year: i32) -> Result<Self::YearInfo, DateError> {
fn year_info_from_era(
&self,
era: &str,
era_year: i32,
) -> Result<Self::YearInfo, UnknownEraError> {
match era {
"am" => Ok(era_year),
_ => Err(DateError::UnknownEra),
_ => Err(UnknownEraError),
}
}

Expand All @@ -102,7 +108,7 @@ impl DateFieldsResolver for Coptic {
&self,
month_code: types::MonthCode,
day: u8,
) -> Result<Self::YearInfo, DateError> {
) -> Result<Self::YearInfo, EcmaReferenceYearError> {
Coptic::reference_year_from_month_day(month_code, day)
}

Expand All @@ -112,10 +118,10 @@ impl DateFieldsResolver for Coptic {
_year: &Self::YearInfo,
month_code: types::MonthCode,
_options: DateFromFieldsOptions,
) -> Result<u8, DateError> {
match month_code.parsed() {
Some((month_number @ 1..=13, false)) => Ok(month_number),
_ => Err(DateError::UnknownMonthCode(month_code)),
) -> Result<u8, MonthCodeError> {
match month_code.try_parse()? {
(month_number @ 1..=13, false) => Ok(month_number),
_ => Err(MonthCodeError::UnknownMonthCodeForCalendar(month_code)),
}
}
}
Expand All @@ -124,10 +130,8 @@ impl Coptic {
pub(crate) fn reference_year_from_month_day(
month_code: types::MonthCode,
day: u8,
) -> Result<i32, DateError> {
let (ordinal_month, _is_leap) = month_code
.parsed()
.ok_or(DateError::UnknownMonthCode(month_code))?;
) -> Result<i32, EcmaReferenceYearError> {
let (ordinal_month, _is_leap) = month_code.try_parse()?;
// December 31, 1972 occurs on 4th month, 22nd day, 1689 AM
let anno_martyrum_year = if ordinal_month < 4 || (ordinal_month == 4 && day <= 22) {
1689
Expand All @@ -153,11 +157,10 @@ impl Calendar for Coptic {
&self,
fields: types::DateFields,
options: DateFromFieldsOptions,
) -> Result<Self::DateInner, DateError> {
) -> Result<Self::DateInner, DateFromFieldsError> {
let builder = ArithmeticDateBuilder::try_from_fields(fields, self, options)?;
ArithmeticDate::try_from_builder(builder, options)
.map(CopticDateInner)
.map_err(|e| e.maybe_with_month_code(fields.month_code))
let arithmetic_date = ArithmeticDate::try_from_builder(builder, options)?;
Ok(CopticDateInner(arithmetic_date))
}

fn from_rata_die(&self, rd: RataDie) -> Self::DateInner {
Expand Down
Loading
Loading