Skip to content

Commit 56afe56

Browse files
Manishearthsffc
Manishearth
and
sffc
authored
Handle possible Overflow values on individual calendars (#7795)
Fixes #7794. `overflow` takes three values: `Constrain`, `Reject`, and `None`. `None` is equivalent to `Constrain`, but we don't have a `ResolvedOptions` type to fix that distinction. _Almost_ all of `overflow`-consuming code is matching the Temporal spec, and the Temporal spec handles unset values correctly, so this is by and large not a problem. However, _calendar-specific_ code is not specced by Temporal, and any place where we handle overflows there needs to handle all cases. @sffc and I discussed this and using `match` statements seemed to be the most straightforward option. We could in theory introduce `ResolvedOptions` or pre-handle the None but most of the code is written to match the spec so this would actually potentially make the spec-matching less clear. ## Changelog icu_calendar: Handle possible Overflow values on individual calendars --------- Co-authored-by: Shane F. Carr <shane@unicode.org>
1 parent 5c01a77 commit 56afe56

File tree

5 files changed

+64
-52
lines changed

5 files changed

+64
-52
lines changed

‎components/calendar/src/cal/east_asian_traditional.rs‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
647647
&self,
648648
year: Self::YearInfo,
649649
month: types::Month,
650-
options: DateFromFieldsOptions,
650+
overflow: Overflow,
651651
) -> Result<u8, MonthError> {
652652
let (number @ 1..=12, leap) = (month.number(), month.is_leap()) else {
653653
return Err(MonthError::NotInCalendar);
@@ -663,9 +663,13 @@ impl<R: Rules> DateFieldsResolver for EastAsianTraditional<R> {
663663
return Ok(leap_month_sentinel);
664664
}
665665

666-
if leap && options.overflow != Some(Overflow::Constrain) {
667-
// wrong leap month and not constraining
668-
return Err(MonthError::NotInYear);
666+
if leap {
667+
// This leap month doesn't exist in the year, reject if needed
668+
match overflow {
669+
Overflow::Reject => return Err(MonthError::NotInYear),
670+
// Written as a match for exhaustiveness
671+
Overflow::Constrain => (),
672+
}
669673
}
670674

671675
// add one if there was a leap month before
@@ -1650,10 +1654,6 @@ mod test {
16501654
#[test]
16511655
fn test_month_to_ordinal() {
16521656
let cal = ChineseTraditional::new();
1653-
let reject = DateFromFieldsOptions {
1654-
overflow: Some(Overflow::Reject),
1655-
..Default::default()
1656-
};
16571657
let year = cal.year_info_from_extended(2023);
16581658
for (ordinal, month) in [
16591659
Month::new(1),
@@ -1675,7 +1675,7 @@ mod test {
16751675
{
16761676
let ordinal = ordinal as u8 + 1;
16771677
assert_eq!(
1678-
cal.ordinal_from_month(year, month, reject),
1678+
cal.ordinal_from_month(year, month, Overflow::Reject),
16791679
Ok(ordinal),
16801680
"Code to ordinal failed for year: {}, code: {ordinal}",
16811681
year.related_iso
@@ -1686,18 +1686,14 @@ mod test {
16861686
#[test]
16871687
fn check_invalid_month_to_ordinal() {
16881688
let cal = ChineseTraditional::new();
1689-
let reject = DateFromFieldsOptions {
1690-
overflow: Some(Overflow::Reject),
1691-
..Default::default()
1692-
};
16931689
for year in [4659, 4660] {
16941690
let year = cal.year_info_from_extended(year);
16951691
for (month, error) in [
16961692
(Month::leap(4), MonthError::NotInYear),
16971693
(Month::new(13), MonthError::NotInCalendar),
16981694
] {
16991695
assert_eq!(
1700-
cal.ordinal_from_month(year, month, reject),
1696+
cal.ordinal_from_month(year, month, Overflow::Reject),
17011697
Err(error),
17021698
"Invalid month code failed for year: {}, code: {month:?}",
17031699
year.related_iso,

‎components/calendar/src/cal/hebrew.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,19 +217,20 @@ impl DateFieldsResolver for Hebrew {
217217
&self,
218218
year: Self::YearInfo,
219219
month: Month,
220-
options: DateFromFieldsOptions,
220+
overflow: Overflow,
221221
) -> Result<u8, MonthError> {
222222
let is_leap_year = year.keviyah.is_leap();
223223
let ordinal_month = match (month.number(), month.is_leap()) {
224224
(n @ 1..=12, false) => n + (n >= 6 && is_leap_year) as u8,
225225
(5, true) => {
226226
if is_leap_year {
227227
6
228-
} else if matches!(options.overflow, Some(Overflow::Constrain)) {
229-
// M05L maps to M06 in a common year
230-
6
231228
} else {
232-
return Err(MonthError::NotInYear);
229+
// Requesting Adar 1 in non-leap year, handle constrain/reject behavior
230+
match overflow {
231+
Overflow::Constrain => 6,
232+
Overflow::Reject => return Err(MonthError::NotInYear),
233+
}
233234
}
234235
}
235236
_ => return Err(MonthError::NotInCalendar),

‎components/calendar/src/calendar_arithmetic.rs‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ pub(crate) trait DateFieldsResolver: Calendar {
258258
&self,
259259
year: Self::YearInfo,
260260
month: Month,
261-
_options: DateFromFieldsOptions,
261+
_overflow: Overflow,
262262
) -> Result<u8, MonthError> {
263263
match (month.number(), month.is_leap()) {
264264
(month_number, false)
@@ -340,7 +340,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
340340
let year = calendar.year_info_from_extended(extended_year);
341341

342342
let month = calendar
343-
.ordinal_from_month(year, month, Default::default())
343+
.ordinal_from_month(year, month, Overflow::Reject)
344344
.map_err(|e| match e {
345345
MonthError::NotInCalendar => DateNewError::MonthNotInCalendar,
346346
MonthError::NotInYear => DateNewError::MonthNotInYear,
@@ -367,7 +367,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
367367
}
368368
let year = calendar.year_info_from_extended(year);
369369

370-
let month = match calendar.ordinal_from_month(year, month, Default::default()) {
370+
let month = match calendar.ordinal_from_month(year, month, Overflow::Reject) {
371371
Ok(month) => month,
372372
Err(MonthError::NotInCalendar) => return Err(LunisolarDateError::MonthNotInCalendar),
373373
Err(MonthError::NotInYear) => return Err(LunisolarDateError::MonthNotInYear),
@@ -389,6 +389,9 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
389389
) -> Result<Self, DateFromFieldsError> {
390390
let missing_fields_strategy = options.missing_fields_strategy.unwrap_or_default();
391391

392+
// DateFromFieldsOptions documents the default as reject
393+
let overflow = options.overflow.unwrap_or(Overflow::Reject);
394+
392395
let day = match fields.day {
393396
Some(day) => day,
394397
None => match missing_fields_strategy {
@@ -444,7 +447,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
444447
};
445448
let ref_year = calendar.reference_year_from_month_day(m, d);
446449
if ref_year.err() == Some(EcmaReferenceYearError::UseRegularIfConstrain)
447-
&& options.overflow == Some(Overflow::Constrain)
450+
&& overflow == Overflow::Constrain
448451
{
449452
let new_valid_month = Month::new(m.number());
450453
valid_month = Some(new_valid_month);
@@ -475,7 +478,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
475478

476479
let month = match (fields.month_code, fields.month) {
477480
(_, Some(month)) => {
478-
let computed_month = calendar.ordinal_from_month(year, month, options)?;
481+
let computed_month = calendar.ordinal_from_month(year, month, overflow)?;
479482
if let Some(ordinal_month) = fields.ordinal_month {
480483
if computed_month != ordinal_month {
481484
return Err(DateFromFieldsError::InconsistentMonth);
@@ -488,7 +491,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
488491
Some(validated) => validated,
489492
None => Month::try_from_utf8(month_code)?,
490493
};
491-
let computed_month = calendar.ordinal_from_month(year, validated, options)?;
494+
let computed_month = calendar.ordinal_from_month(year, validated, overflow)?;
492495
if let Some(ordinal_month) = fields.ordinal_month {
493496
if computed_month != ordinal_month {
494497
return Err(DateFromFieldsError::InconsistentMonth);
@@ -506,7 +509,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
506509
};
507510

508511
let max_month = C::months_in_provided_year(year);
509-
let month = if matches!(options.overflow.unwrap_or_default(), Overflow::Constrain) {
512+
let month = if matches!(overflow, Overflow::Constrain) {
510513
month.clamp(1, max_month)
511514
} else if (1..=max_month).contains(&month) {
512515
month
@@ -515,7 +518,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
515518
};
516519

517520
let max_day = C::days_in_provided_month(year, month);
518-
let day = if matches!(options.overflow.unwrap_or_default(), Overflow::Constrain) {
521+
let day = if matches!(overflow, Overflow::Constrain) {
519522
day.clamp(1, max_day)
520523
} else if (1..=max_day).contains(&day) {
521524
day
@@ -770,11 +773,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
770773
return true;
771774
}
772775
// 1. Let _m0_ be MonthCodeToOrdinal(_calendar_, _y0_, ! ConstrainMonthCode(_calendar_, _y0_, _parts_.[[MonthCode]], ~constrain~)).
773-
let constrain = DateFromFieldsOptions {
774-
overflow: Some(Overflow::Constrain),
775-
..Default::default()
776-
};
777-
let m0_result = cal.ordinal_from_month(y0, base_month, constrain);
776+
let m0_result = cal.ordinal_from_month(y0, base_month, Overflow::Constrain);
778777
let m0 = match m0_result {
779778
Ok(m0) => m0,
780779
Err(_) => {
@@ -870,6 +869,9 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
870869
return Err(DateAddError::Overflow);
871870
}
872871

872+
// DateAddOptions documents the default as constrain
873+
let overflow = options.overflow.unwrap_or(Overflow::Constrain);
874+
873875
// 1. Let _parts_ be CalendarISOToDate(_calendar_, _isoDate_).
874876
// 1. Let _y0_ be _parts_.[[Year]] + _duration_.[[Years]].
875877
let extended_year = duration.add_years_to(self.year().to_extended_year());
@@ -879,11 +881,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
879881
let y0 = cal.year_info_from_extended(extended_year);
880882
// 1. Let _m0_ be MonthCodeToOrdinal(_calendar_, _y0_, ! ConstrainMonthCode(_calendar_, _y0_, _parts_.[[MonthCode]], _overflow_)).
881883
let base_month = cal.month_from_ordinal(self.year(), self.month());
882-
let m0_result = cal.ordinal_from_month(
883-
y0,
884-
base_month,
885-
DateFromFieldsOptions::from_add_options(options),
886-
);
884+
let m0_result = cal.ordinal_from_month(y0, base_month, overflow);
887885
let m0 = match m0_result {
888886
Ok(m0) => m0,
889887
Err(MonthError::NotInCalendar) => {
@@ -907,7 +905,7 @@ impl<C: DateFieldsResolver> ArithmeticDate<C> {
907905
// 1. Else,
908906
// 1. If _overflow_ is ~reject~, throw a *RangeError* exception.
909907
// Note: ICU4X default is constrain here
910-
if matches!(options.overflow, Some(Overflow::Reject)) {
908+
if overflow == Overflow::Reject {
911909
return Err(DateAddError::InvalidDay {
912910
max: end_of_month.day,
913911
});
@@ -1155,11 +1153,10 @@ impl<'a, C: DateFieldsResolver> SurpassesChecker<'a, C> {
11551153
self.cal,
11561154
);
11571155
// 5. Let m0 be MonthCodeToOrdinal(calendar, y0, ! ConstrainMonthCode(calendar, y0, parts.[[MonthCode]], constrain)).
1158-
let constrain = DateFromFieldsOptions {
1159-
overflow: Some(Overflow::Constrain),
1160-
..Default::default()
1161-
};
1162-
let m0_result = self.cal.ordinal_from_month(self.y0, base_month, constrain);
1156+
1157+
let m0_result = self
1158+
.cal
1159+
.ordinal_from_month(self.y0, base_month, Overflow::Constrain);
11631160
self.m0 = match m0_result {
11641161
Ok(m0) => m0,
11651162
Err(_) => {

‎components/calendar/src/date.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,4 +793,31 @@ mod tests {
793793
Date::try_new_japanese_with_calendar("reiwa", 7, 1, 1, Japanese::new()).unwrap();
794794
assert_eq!(date2, date2_expected);
795795
}
796+
#[test]
797+
fn date_add_options_default_is_constrain() {
798+
use crate::cal::ChineseTraditional;
799+
use crate::duration::DateDuration;
800+
use crate::types::Month;
801+
802+
// 10 Adar I 5787
803+
// 5787 is a leap year; 5788 is not
804+
let mut date = Date::try_new(5787.into(), Month::leap(5), 10, Hebrew).unwrap();
805+
date.try_add_with_options(DateDuration::for_years(1), DateAddOptions::default())
806+
.unwrap();
807+
assert_eq!(date.month().to_input(), Month::new(6));
808+
809+
// Leap Month 6, day 1, 2025.
810+
// 2025 is a leap year; 2026 is not
811+
let mut date = Date::try_new(
812+
2025.into(),
813+
Month::leap(6),
814+
1,
815+
ChineseTraditional::default(),
816+
)
817+
.unwrap();
818+
819+
date.try_add_with_options(DateDuration::for_years(1), DateAddOptions::default())
820+
.unwrap();
821+
assert_eq!(date.month().to_input(), Month::new(6));
822+
}
796823
}

‎components/calendar/src/options.rs‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,6 @@ mod unstable {
100100
pub missing_fields_strategy: Option<MissingFieldsStrategy>,
101101
}
102102

103-
impl DateFromFieldsOptions {
104-
pub(crate) fn from_add_options(options: DateAddOptions) -> Self {
105-
Self {
106-
overflow: options.overflow,
107-
missing_fields_strategy: Default::default(),
108-
}
109-
}
110-
}
111-
112103
/// Options for adding a duration to a date.
113104
///
114105
/// <div class="stab unstable">

0 commit comments

Comments
 (0)