Skip to content

Add new unit validation code #542

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
Manishearth merged 1 commit into from
Sep 8, 2025
Merged

Add new unit validation code #542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 24 additions & 21 deletions src/builtins/core/duration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
iso::{IsoDateTime, IsoTime},
options::{
Overflow, RelativeTo, ResolvedRoundingOptions, RoundingIncrement, RoundingOptions,
ToStringRoundingOptions, Unit,
ToStringRoundingOptions, Unit, UnitGroup,
},
parsers::{FormattableDateDuration, FormattableDuration, FormattableTimeDuration, Precision},
primitive::FiniteF64,
Expand Down Expand Up @@ -1041,7 +1041,7 @@ impl Duration {
///
/// Spec: <https://tc39.es/proposal-temporal/#sec-temporal.duration.prototype.round>
///
// Spec last accessed: 2025-05-16, <https://github.com/tc39/proposal-temporal/tree/c150e7135c56afc9114032e93b53ac49f980d254>
// Spec last accessed: 2025-09-08, <https://github.com/tc39/proposal-temporal/commit/9b64df55465ef25a20bada50eca26515bb82669d>
#[inline]
pub fn round_with_provider(
&self,
Expand All @@ -1067,7 +1067,7 @@ impl Duration {
// (GetTemporalRelativeToOption reads "relativeTo",
// GetRoundingIncrementOption reads "roundingIncrement"
// and GetRoundingModeOption reads "roundingMode").
// SKIP: 9. Let largestUnit be ? GetTemporalUnitValuedOption(roundTo, "largestUnit", datetime, unset, « auto »).
// SKIP: 9. Let largestUnit be ? GetTemporalUnitValuedOption(roundTo, "largestUnit", unset).
// SKIP: 10. Let relativeToRecord be ? GetTemporalRelativeToOption(roundTo).
// SKIP: 11. Let zonedRelativeTo be relativeToRecord.[[ZonedRelativeTo]].
// SKIP: 12. Let plainRelativeTo be relativeToRecord.[[PlainRelativeTo]].
Expand All @@ -1081,50 +1081,53 @@ impl Duration {
// 15. Let smallestUnit be ? GetTemporalUnitValuedOption(roundTo, "smallestUnit", datetime, unset).
let smallest_unit = options.smallest_unit;

// 16. If smallestUnit is unset, then
// 16. Perform ?ValidateTemporalUnitValue(smallestUnit, datetime).
UnitGroup::DateTime.validate_unit(smallest_unit, None)?;

// 17. If smallestUnit is unset, then
// a. Set smallestUnitPresent to false.
// b. Set smallestUnit to nanosecond.
let smallest_unit = smallest_unit.unwrap_or(Unit::Nanosecond);

// 17. Let existingLargestUnit be DefaultTemporalLargestUnit(duration).
// 18. Let existingLargestUnit be DefaultTemporalLargestUnit(duration).
let existing_largest_unit = self.default_largest_unit();

// 18. Let defaultLargestUnit be LargerOfTwoTemporalUnits(existingLargestUnit, smallestUnit).
// 19. Let defaultLargestUnit be LargerOfTwoTemporalUnits(existingLargestUnit, smallestUnit).
let default_largest_unit = Unit::larger(existing_largest_unit, smallest_unit)?;

let largest_unit = match options.largest_unit {
// 19. If largestUnit is unset, then
// 20. If largestUnit is unset, then
// a. Set largestUnitPresent to false.
// b. Set largestUnit to defaultLargestUnit.
// 20. Else if largestUnit is auto, then
// 21. Else if largestUnit is auto, then
// a. Set largestUnit to defaultLargestUnit.
Some(Unit::Auto) | None => default_largest_unit,
Some(unit) => unit,
};

// 21. If smallestUnitPresent is false and largestUnitPresent is false, then
// 22. If smallestUnitPresent is false and largestUnitPresent is false, then
if options.largest_unit.is_none() && options.smallest_unit.is_none() {
// a. Throw a RangeError exception.
return Err(TemporalError::range()
.with_message("smallestUnit and largestUnit cannot both be None."));
}

// 22. If LargerOfTwoTemporalUnits(largestUnit, smallestUnit) is not largestUnit, throw a RangeError exception.
// 23. If LargerOfTwoTemporalUnits(largestUnit, smallestUnit) is not largestUnit, throw a RangeError exception.
if Unit::larger(largest_unit, smallest_unit)? != largest_unit {
return Err(
TemporalError::range().with_message("smallestUnit is larger than largestUnit.")
);
}

// 23. Let maximum be MaximumTemporalDurationRoundingIncrement(smallestUnit).
// 24. Let maximum be MaximumTemporalDurationRoundingIncrement(smallestUnit).
let maximum = smallest_unit.to_maximum_rounding_increment();

// 24. If maximum is not unset, perform ? ValidateTemporalRoundingIncrement(roundingIncrement, maximum, false).
// 25. If maximum is not unset, perform ? ValidateTemporalRoundingIncrement(roundingIncrement, maximum, false).
if let Some(maximum) = maximum {
rounding_increment.validate(maximum.into(), false)?;
}

// 25. If roundingIncrement > 1, and largestUnit is not smallestUnit, and TemporalUnitCategory(smallestUnit) is date, throw a RangeError exception.
// 26. If roundingIncrement > 1, and largestUnit is not smallestUnit, and TemporalUnitCategory(smallestUnit) is date, throw a RangeError exception.
if rounding_increment > RoundingIncrement::ONE
&& largest_unit != smallest_unit
&& smallest_unit.is_date_unit()
Expand All @@ -1142,7 +1145,7 @@ impl Duration {
};

match relative_to {
// 26. If zonedRelativeTo is not undefined, then
// 27. If zonedRelativeTo is not undefined, then
Some(RelativeTo::ZonedDateTime(zoned_relative_to)) => {
// a. Let internalDuration be ToInternalDurationRecord(duration).
let internal_duration = self.to_internal_duration_record();
Expand Down Expand Up @@ -1178,7 +1181,7 @@ impl Duration {
return Duration::from_internal(internal, largest_unit);
}

// 27. If plainRelativeTo is not undefined, then
// 28. If plainRelativeTo is not undefined, then
Some(RelativeTo::PlainDate(plain_relative_to)) => {
// a. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
let internal_duration =
Expand Down Expand Up @@ -1225,7 +1228,7 @@ impl Duration {
None => {}
}

// 28. If IsCalendarUnit(existingLargestUnit) is true, or IsCalendarUnit(largestUnit) is true, throw a RangeError exception.
// 29. If IsCalendarUnit(existingLargestUnit) is true, or IsCalendarUnit(largestUnit) is true, throw a RangeError exception.
if existing_largest_unit.is_calendar_unit()
|| resolved_options.largest_unit.is_calendar_unit()
{
Expand All @@ -1234,13 +1237,13 @@ impl Duration {
));
}

// 29. Assert: IsCalendarUnit(smallestUnit) is false.
// 30. Assert: IsCalendarUnit(smallestUnit) is false.
temporal_assert!(!resolved_options.smallest_unit.is_calendar_unit());

// 30. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
// 31. Let internalDuration be ToInternalDurationRecordWith24HourDays(duration).
let internal_duration = InternalDurationRecord::from_duration_with_24_hour_days(self)?;

// 31. If smallestUnit is day, then
// 32. If smallestUnit is day, then
let internal_duration = if resolved_options.smallest_unit == Unit::Day {
// a. Let fractionalDays be TotalTimeDuration(internalDuration.[[Time]], day).
// b. Let days be RoundNumberToIncrement(fractionalDays, roundingIncrement, roundingMode).
Expand All @@ -1257,7 +1260,7 @@ impl Duration {
// d. Set internalDuration to CombineDateAndTimeDuration(dateDuration, 0).
InternalDurationRecord::new(date, TimeDuration::default())?
} else {
// 32. Else,
// 33. Else,
// a. Let timeDuration be ? RoundTimeDuration(internalDuration.[[Time]], roundingIncrement, smallestUnit, roundingMode).
let time_duration = internal_duration
.normalized_time_duration()
Expand All @@ -1267,7 +1270,7 @@ impl Duration {
InternalDurationRecord::new(DateDuration::default(), time_duration)?
};

// 33. Return ? TemporalDurationFromInternal(internalDuration, largestUnit).
// 34. Return ? TemporalDurationFromInternal(internalDuration, largestUnit).
Duration::from_internal(internal_duration, resolved_options.largest_unit)
}

Expand Down
15 changes: 15 additions & 0 deletions src/builtins/core/year_month.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,4 +1129,19 @@ mod tests {
1
);
}

#[test]
/// https://g-issues.chromium.org/issues/443275104
fn test_max_rounding_increment_auto() {
let ym = PlainYearMonth::try_new_iso(1639, 11, None).unwrap();
assert!(ym
.until(
&ym,
DifferenceSettings {
smallest_unit: Some(Unit::Auto),
..Default::default()
}
)
.is_err());
}
}
27 changes: 17 additions & 10 deletions src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,16 @@ impl ResolvedRoundingOptions {
DifferenceOperation::Until => options.rounding_mode.unwrap_or(RoundingMode::Trunc),
};
// 7. Let smallestUnit be ? GetUnitValuedOption(options, "smallestUnit", unitGroup, fallbackSmallestUnit).
unit_group.validate_unit(options.smallest_unit, None)?;
let smallest_unit = options.smallest_unit.unwrap_or(fallback_smallest);
// 8. If disallowedUnits contains smallestUnit, throw a RangeError exception.
unit_group.validate_unit(options.smallest_unit, None)?;
// 9. Let defaultLargestUnit be LargerOfTwoUnits(smallestLargestDefaultUnit, smallestUnit).
let default_largest_unit = smallest_unit.max(fallback_largest);
// 10. If largestUnit is auto, set largestUnit to defaultLargestUnit.
let largest_unit = options
.largest_unit
.unwrap_unit_or(smallest_unit.max(fallback_largest));
let mut largest_unit = options.largest_unit.unwrap_unit_or(default_largest_unit);
if largest_unit == Unit::Auto {
largest_unit = default_largest_unit;
}
// 11. If LargerOfTwoUnits(largestUnit, smallestUnit) is not largestUnit, throw a RangeError exception.
if largest_unit < smallest_unit {
return Err(
Expand Down Expand Up @@ -307,22 +309,24 @@ impl UnitGroup {
Ok(unit)
}

/// Note: this always rejects Auto unless extra_unit is specified
///
/// <https://tc39.es/proposal-temporal/#sec-temporal-validatetemporalunitvaluedoption>
pub fn validate_unit(self, unit: Option<Unit>, extra_unit: Option<Unit>) -> TemporalResult<()> {
// TODO: Determine proper handling of Auto.
match self {
_ if unit == extra_unit => Ok(()),
UnitGroup::Date => match unit {
Some(unit) if !unit.is_time_unit() => Ok(()),
Some(unit) if unit.is_date_unit() => Ok(()),
None => Ok(()),
_ if unit == extra_unit => Ok(()),
_ => Err(TemporalError::range().with_enum(ErrorMessage::UnitNotDate)),
},
UnitGroup::Time => match unit {
Some(unit) if unit.is_time_unit() => Ok(()),
None => Ok(()),
_ if unit == extra_unit => Ok(()),
_ => Err(TemporalError::range().with_enum(ErrorMessage::UnitNotTime)),
},
UnitGroup::DateTime => Ok(()),
UnitGroup::DateTime if unit != Some(Unit::Auto) => Ok(()),
_ => Err(TemporalError::range().with_enum(ErrorMessage::UnitNoAutoDuringComparison)),
}
}
}
Expand Down Expand Up @@ -399,7 +403,10 @@ impl Unit {
Hour => 24,
Minute | Second => 60,
Millisecond | Microsecond | Nanosecond => 1000,
Auto => unreachable!(),
Auto => {
debug_assert!(false, "Auto units should be resolved by this point");
return None;
}
};

Some(max)
Expand Down
Loading