Skip to content

Commit

Permalink
Normative: Balance durations after rounding in until()/since()/toStri…
Browse files Browse the repository at this point in the history
…ng()

In cases where a number rounded up to the next unit, we would previously
return a result that didn't respect the `largestUnit` option in the same
way that Duration.prototype.round() would.

To fix this, we need to call BalanceDateDurationRelative after
RoundDuration in the until()/since() methods that deal with date units
(time units already behaved correctly).

In Duration.prototype.toString() where time units did not already behave
correctly, we need to call BalanceTimeDuration. I was not sure whether it
was intentional that we didn't call this, but as far as I can determine
from the previous discussions, `toString()` was "controls rounding, works
same as `round()`", but the status quo was that it worked differently from
`round()`.)

Closes: #2563
  • Loading branch information
ptomato committed Sep 28, 2023
1 parent 801a41f commit 0675864
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 40 deletions.
78 changes: 57 additions & 21 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -600,28 +600,64 @@ export class Duration {
}
const { precision, unit, increment } = ES.ToSecondsStringPrecisionRecord(smallestUnit, digits);

const { seconds, milliseconds, microseconds, nanoseconds } = ES.RoundDuration(
0,
0,
0,
0,
0,
0,
GetSlot(this, SECONDS),
GetSlot(this, MILLISECONDS),
GetSlot(this, MICROSECONDS),
GetSlot(this, NANOSECONDS),
increment,
unit,
roundingMode
);
let years = GetSlot(this, YEARS);
let months = GetSlot(this, MONTHS);
let weeks = GetSlot(this, WEEKS);
let days = GetSlot(this, DAYS);
let hours = GetSlot(this, HOURS);
let minutes = GetSlot(this, MINUTES);
let seconds = GetSlot(this, SECONDS);
let milliseconds = GetSlot(this, MILLISECONDS);
let microseconds = GetSlot(this, MICROSECONDS);
let nanoseconds = GetSlot(this, NANOSECONDS);

if (unit !== 'nanosecond' || increment !== 1) {
const largestUnit = ES.DefaultTemporalLargestUnit(
years,
months,
weeks,
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds
);
({ seconds, milliseconds, microseconds, nanoseconds } = ES.RoundDuration(
0,
0,
0,
0,
0,
0,
seconds,
milliseconds,
microseconds,
nanoseconds,
increment,
unit,
roundingMode
));
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceTimeDuration(
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds,
largestUnit
));
}

return ES.TemporalDurationToString(
GetSlot(this, YEARS),
GetSlot(this, MONTHS),
GetSlot(this, WEEKS),
GetSlot(this, DAYS),
GetSlot(this, HOURS),
GetSlot(this, MINUTES),
years,
months,
weeks,
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
Expand Down
55 changes: 51 additions & 4 deletions polyfill/lib/ecmascript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4349,7 +4349,14 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
}

const calendarRec = new CalendarMethodRecord(calendar);
if (settings.smallestUnit === 'year' || settings.smallestUnit === 'month' || settings.smallestUnit === 'week') {
const roundingIsNoop = settings.smallestUnit === 'day' && settings.roundingIncrement === 1;
if (
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week' ||
(!roundingIsNoop &&
(settings.largestUnit === 'year' || settings.largestUnit === 'month' || settings.largestUnit === 'week'))
) {
calendarRec.lookup('dateAdd');
}
if (
Expand All @@ -4367,7 +4374,7 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
let weeks = GetSlot(untilResult, WEEKS);
let days = GetSlot(untilResult, DAYS);

if (settings.smallestUnit !== 'day' || settings.roundingIncrement !== 1) {
if (!roundingIsNoop) {
({ years, months, weeks, days } = RoundDuration(
years,
months,
Expand All @@ -4385,6 +4392,15 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
plainDate,
calendarRec
));
({ years, months, weeks, days } = BalanceDateDurationRelative(
years,
months,
weeks,
days,
settings.largestUnit,
plainDate,
calendarRec
));
}

return new Duration(sign * years, sign * months, sign * weeks, sign * days, 0, 0, 0, 0, 0, 0);
Expand Down Expand Up @@ -4418,7 +4434,15 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
}

const calendarRec = new CalendarMethodRecord(calendar);
if (settings.smallestUnit === 'year' || settings.smallestUnit === 'month' || settings.smallestUnit === 'week') {
const roundingIsNoop = settings.smallestUnit === 'nanosecond' && settings.roundingIncrement === 1;
if (
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week' ||
(!datePartsIdentical &&
!roundingIsNoop &&
(settings.largestUnit === 'year' || settings.largestUnit === 'month' || settings.largestUnit === 'week'))
) {
calendarRec.lookup('dateAdd');
}
if (
Expand Down Expand Up @@ -4454,7 +4478,7 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
resolvedOptions
);

if (settings.smallestUnit !== 'nanosecond' || settings.roundingIncrement !== 1) {
if (!roundingIsNoop) {
const relativeTo = TemporalDateTimeToDate(plainDateTime);
({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = RoundDuration(
years,
Expand Down Expand Up @@ -4483,6 +4507,15 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
nanoseconds,
settings.largestUnit
));
({ years, months, weeks, days } = BalanceDateDurationRelative(
years,
months,
weeks,
days,
settings.largestUnit,
relativeTo,
calendarRec
));
}

return new Duration(
Expand Down Expand Up @@ -4618,6 +4651,7 @@ export function DifferenceTemporalPlainYearMonth(operation, yearMonth, other, op
thisDate,
calendarRec
));
({ years, months } = BalanceDateDurationRelative(years, months, 0, 0, settings.largestUnit, thisDate, calendarRec));
}

return new Duration(sign * years, sign * months, 0, 0, 0, 0, 0, 0, 0, 0);
Expand Down Expand Up @@ -4681,6 +4715,9 @@ export function DifferenceTemporalZonedDateTime(operation, zonedDateTime, other,
const roundingIsNoop = settings.smallestUnit === 'nanosecond' && settings.roundingIncrement === 1;
const plainDateTimeOrRelativeToWillBeUsed =
!roundingIsNoop ||
settings.largestUnit === 'year' ||
settings.largestUnit === 'month' ||
settings.largestUnit === 'week' ||
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week';
Expand Down Expand Up @@ -4745,6 +4782,16 @@ export function DifferenceTemporalZonedDateTime(operation, zonedDateTime, other,
timeZoneRec,
precalculatedPlainDateTime
));
// BalanceTimeDuration already performed in AdjustRoundedDurationDays
({ years, months, weeks, days } = BalanceDateDurationRelative(
years,
months,
weeks,
days,
settings.largestUnit,
plainRelativeTo,
calendarRec
));
}
}

Expand Down
12 changes: 9 additions & 3 deletions spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -570,9 +570,15 @@ <h1>Temporal.Duration.prototype.toString ( [ _options_ ] )</h1>
1. Let _smallestUnit_ be ? GetTemporalUnit(_options_, *"smallestUnit"*, ~time~, *undefined*).
1. If _smallestUnit_ is *"hour"* or *"minute"*, throw a *RangeError* exception.
1. Let _precision_ be ToSecondsStringPrecisionRecord(_smallestUnit_, _digits_).
1. Let _roundRecord_ be ? RoundDuration(0, 0, 0, 0, 0, 0, _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _precision_.[[Increment]], _precision_.[[Unit]], _roundingMode_).
1. Let _result_ be _roundRecord_.[[DurationRecord]].
1. Return ! TemporalDurationToString(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _result_.[[Seconds]], _result_.[[Milliseconds]], _result_.[[Microseconds]], _result_.[[Nanoseconds]], _precision_.[[Precision]]).
1. If _precision_.[[Unit]] is not *"nanosecond"* or _precision_.[[Increment]] &ne; 1, then
1. Let _largestUnit_ be DefaultTemporalLargestUnit(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]]).
1. Let _roundRecord_ be ? RoundDuration(0, 0, 0, 0, 0, 0, _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _precision_.[[Increment]], _precision_.[[Unit]], _roundingMode_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Let _balanceResult_ be ? BalanceTimeDuration(_duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _largestUnit_).
1. Let _result_ be ! CreateDurationRecord(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _balanceResult_.[[Days]], _balanceResult_.[[Hours]], _balanceResult_.[[Minutes]], _balanceResult_.[[Seconds]], _balanceResult_.[[Milliseconds]], _balanceResult_.[[Microseconds]], _balanceResult_.[[Nanoseconds]]).
1. Else,
1. Let _result_ be _duration_.
1. Return ! TemporalDurationToString(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], _result_.[[Hours]], _result_.[[Minutes]], _result_.[[Seconds]], _result_.[[Milliseconds]], _result_.[[Microseconds]], _result_.[[Nanoseconds]], _precision_.[[Precision]]).
</emu-alg>
</emu-clause>

Expand Down
12 changes: 8 additions & 4 deletions spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -1056,14 +1056,18 @@ <h1>
1. If _temporalDate_.[[ISOYear]] = _other_.[[ISOYear]], and _temporalDate_.[[ISOMonth]] = _other_.[[ISOMonth]], and _temporalDate_.[[ISODay]] = _other_.[[ISODay]], then
1. Return ! CreateTemporalDuration(0, 0, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _calendarRec_ be ! CreateCalendarMethodsRecord(_temporalDate_.[[Calendar]], « »).
1. If _settings_.[[SmallestUnit]] is *"year"*, or _settings_.[[SmallestUnit]] is *"month"*, or _settings_.[[SmallestUnit]] is *"week"*, then
1. If _settings_.[[SmallestUnit]] is *"day"* and _settings_.[[RoundingIncrement]] = 1, let _roundingGranularityIsNoop_ be *true*; else let _roundingGranularityIsNoop_ be *false*.
1. If _settings_.[[LargestUnit]] is *"year"*, or _settings_.[[LargestUnit]] is *"month"*, or _settings_.[[LargestUnit]] is *"week"*, let _largestUnitIsCalendarUnit_ be *true*; else let _largestUnitIsCalendarUnit_ be *false*.
1. If _roundingGranularityIsNoop_ is *false* and _largestUnitIsCalendarUnit_ is *true*, let _roundingRequiresDateAddLookup_ be *true*; else let _roundingRequiresDateAddLookup_ be *false*.
1. If _settings_.[[SmallestUnit]] is *"year"*, or _settings_.[[SmallestUnit]] is *"month"*, or _settings_.[[SmallestUnit]] is *"week"*, or _roundingRequiresDateAddLookup_ is *true*, then
1. Perform ? CalendarMethodsRecordLookup(_calendarRec_, ~dateAdd~).
1. If _settings_.[[LargestUnit]] is *"year"*, or _settings_.[[LargestUnit]] is *"month"*, or _settings_.[[LargestUnit]] is *"week"*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. If _largestUnitIsCalendarUnit_ is *true*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. Perform ? CalendarMethodsRecordLookup(_calendarRec_, ~dateUntil~).
1. Let _result_ be ? DifferenceDate(_calendarRec_, _temporalDate_, _other_, _resolvedOptions_).
1. If _settings_.[[SmallestUnit]] is not *"day"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. If _roundingGranularityIsNoop_ is *false*, then
1. Let _roundRecord_ be ? RoundDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _temporalDate_, _calendarRec_).
1. Set _result_ to _roundRecord_.[[DurationRecord]].
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _roundResult_.[[Days]], _settings_.[[LargestUnit]], _temporalDate_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ &times; _result_.[[Years]], _sign_ &times; _result_.[[Months]], _sign_ &times; _result_.[[Weeks]], _sign_ &times; _result_.[[Days]], 0, 0, 0, 0, 0, 0).
</emu-alg>
</emu-clause>
Expand Down
11 changes: 7 additions & 4 deletions spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -1304,20 +1304,23 @@ <h1>
1. If _datePartsIdentical_ is *true*, and _dateTime_.[[ISOHour]] = _other_.[[ISOHour]], and _dateTime_.[[ISOMinute]] = _other_.[[ISOMinute]], and _dateTime_.[[ISOSecond]] = _other_.[[ISOSecond]], and _dateTime_.[[ISOMillisecond]] = _other_.[[ISOMillisecond]], and _dateTime_.[[ISOMicrosecond]] = _other_.[[ISOMicrosecond]], and _dateTime_.[[ISONanosecond]] = _other_.[[ISONanosecond]], then
1. Return ! CreateTemporalDuration(0, 0, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _calendarRec_ be ! CreateCalendarMethodsRecord(_dateTime_.[[Calendar]], « »).
1. If _settings_.[[SmallestUnit]] is *"year"*, or _settings_.[[SmallestUnit]] is *"month"*, or _settings_.[[SmallestUnit]] is *"week"*, then
1. Perform ? CalendarMethodsRecordLookup(_calendarRec_, ~dateAdd~).
1. If _settings_.[[SmallestUnit]] is *"day"* and _settings_.[[RoundingIncrement]] = 1, let _roundingGranularityIsNoop_ be *true*; else let _roundingGranularityIsNoop_ be *false*.
1. If _settings_.[[LargestUnit]] is *"year"*, or _settings_.[[LargestUnit]] is *"month"*, or _settings_.[[LargestUnit]] is *"week"*, let _largestUnitIsCalendarUnit_ be *true*; else let _largestUnitIsCalendarUnit_ be *false*.
1. If _roundingGranularityIsNoop_ is *false* and _largestUnitIsCalendarUnit_ is *true*, let _roundingRequiresDateAddLookup_ be *true*; else let _roundingRequiresDateAddLookup_ be *false*.
1. If _settings_.[[SmallestUnit]] is *"year"*, or _settings_.[[SmallestUnit]] is *"month"*, or _settings_.[[SmallestUnit]] is *"week"*, or _roundingRequiresDateAddLookup_ is *true*, then
1. Perform ? CalendarMethodsRecordLookup(_calendarRec_, ~dateAdd~).
1. If _datePartsIdentical_ is *false* and _largestUnitIsCalendarUnit_ is *true*, let _largestUnitRequiresDateUntilLookup_ be *true*; else let _largestUnitRequiresDateUntilLookup_ be *false*.
1. If _largestUnitRequiresDateUntilLookup_ is *true*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. Perform ? CalendarMethodsRecordLookup(_calendarRec_, ~dateUntil~).
1. Let _diff_ be ? DifferenceISODateTime(_dateTime_.[[ISOYear]], _dateTime_.[[ISOMonth]], _dateTime_.[[ISODay]], _dateTime_.[[ISOHour]], _dateTime_.[[ISOMinute]], _dateTime_.[[ISOSecond]], _dateTime_.[[ISOMillisecond]], _dateTime_.[[ISOMicrosecond]], _dateTime_.[[ISONanosecond]], _other_.[[ISOYear]], _other_.[[ISOMonth]], _other_.[[ISODay]], _other_.[[ISOHour]], _other_.[[ISOMinute]], _other_.[[ISOSecond]], _other_.[[ISOMillisecond]], _other_.[[ISOMicrosecond]], _other_.[[ISONanosecond]], _calendarRec_, _settings_.[[LargestUnit]], _resolvedOptions_).
1. If _settings_.[[SmallestUnit]] is *"nanosecond"* and _settings_.[[RoundingIncrement]] is 1, then
1. If _roundingGranularityIsNoop_ is *true*, then
1. Return ! CreateTemporalDuration(_sign_ &times; _diff_.[[Years]], _sign_ &times; _diff_.[[Months]], _sign_ &times; _diff_.[[Weeks]], _sign_ &times; _diff_.[[Days]], _sign_ &times; _diff_.[[Hours]], _sign_ &times; _diff_.[[Minutes]], _sign_ &times; _diff_.[[Seconds]], _sign_ &times; _diff_.[[Milliseconds]], _sign_ &times; _diff_.[[Microseconds]], _sign_ &times; _diff_.[[Nanoseconds]]).
1. Let _relativeTo_ be ! CreateTemporalDate(_dateTime_.[[ISOYear]], _dateTime_.[[ISOMonth]], _dateTime_.[[ISODay]], _dateTime_.[[Calendar]]).
1. Let _roundRecord_ be ? RoundDuration(_diff_.[[Years]], _diff_.[[Months]], _diff_.[[Weeks]], _diff_.[[Days]], _diff_.[[Hours]], _diff_.[[Minutes]], _diff_.[[Seconds]], _diff_.[[Milliseconds]], _diff_.[[Microseconds]], _diff_.[[Nanoseconds]], _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _relativeTo_, _calendarRec_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Let _result_ be ? BalanceTimeDuration(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _settings_.[[LargestUnit]]).
1. Return ! CreateTemporalDuration(_sign_ &times; _roundResult_.[[Years]], _sign_ &times; _roundResult_.[[Months]], _sign_ &times; _roundResult_.[[Weeks]], _sign_ &times; _result_.[[Days]], _sign_ &times; _result_.[[Hours]], _sign_ &times; _result_.[[Minutes]], _sign_ &times; _result_.[[Seconds]], _sign_ &times; _result_.[[Milliseconds]], _sign_ &times; _result_.[[Microseconds]], _sign_ &times; _result_.[[Nanoseconds]]).
1. Let _balanceResult_ be ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _result_.[[Days]], _settings_.[[LargestUnit]], _relativeTo_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ &times; _balanceResult_.[[Years]], _sign_ &times; _balanceResult_.[[Months]], _sign_ &times; _balanceResult_.[[Weeks]], _sign_ &times; _balanceResult_.[[Days]], _sign_ &times; _result_.[[Hours]], _sign_ &times; _result_.[[Minutes]], _sign_ &times; _result_.[[Seconds]], _sign_ &times; _result_.[[Milliseconds]], _sign_ &times; _result_.[[Microseconds]], _sign_ &times; _result_.[[Nanoseconds]]).
</emu-alg>
</emu-clause>
<emu-clause id="sec-temporal-adddurationtoorsubtractdurationfromplaindatetime" type="abstract operation">
Expand Down
3 changes: 2 additions & 1 deletion spec/plainyearmonth.html
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,8 @@ <h1>
1. Let _result_ be ? CalendarDateUntil(_calendarRec_, _thisDate_, _otherDate_, _resolvedOptions_).
1. If _settings_.[[SmallestUnit]] is not *"month"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. Let _roundRecord_ be ? RoundDuration(_result_.[[Years]], _result_.[[Months]], 0, 0, 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _thisDate_, _calendarRec_).
1. Set _result_ to _roundRecord_.[[DurationRecord]].
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], 0, 0, _settings_.[[LargestUnit]], _thisDate_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ &times; _result_.[[Years]], _sign_ &times; _result_.[[Months]], 0, 0, 0, 0, 0, 0, 0, 0).
</emu-alg>
</emu-clause>
Expand Down
Loading

0 comments on commit 0675864

Please sign in to comment.