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 BalanceDurationRelative 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 BalanceDuration. I was not sure whether it was
intentional that we didn't call this, but as far as I can determine from
in `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 May 13, 2023
1 parent c68dccc commit 282dd60
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 30 deletions.
65 changes: 50 additions & 15 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -541,22 +541,57 @@ export class Duration {
}
const { precision, unit, increment } = ES.ToSecondsStringPrecisionRecord(smallestUnit, digits);

let { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.RoundDuration(
GetSlot(this, YEARS),
GetSlot(this, MONTHS),
GetSlot(this, WEEKS),
GetSlot(this, DAYS),
GetSlot(this, HOURS),
GetSlot(this, MINUTES),
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
);
({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.RoundDuration(
years,
months,
weeks,
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds,
increment,
unit,
roundingMode
));
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceDuration(
days,
hours,
minutes,
seconds,
milliseconds,
microseconds,
nanoseconds,
largestUnit
));
}
return ES.TemporalDurationToString(
years,
months,
Expand Down
58 changes: 53 additions & 5 deletions polyfill/lib/ecmascript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4098,7 +4098,14 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
}

const calendarRec = new MethodRecord(calendar);
if (settings.smallestUnit === 'year' || settings.smallestUnit === 'month' || settings.smallestUnit === 'week') {
const roundingRequested = settings.smallestUnit !== 'day' || settings.roundingIncrement !== 1;
if (
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week' ||
(roundingRequested &&
(settings.largestUnit === 'year' || settings.largestUnit === 'month' || settings.largestUnit === 'week'))
) {
calendarRec.lookup('dateAdd');
}
if (
Expand All @@ -4116,7 +4123,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 (roundingRequested) {
({ years, months, weeks, days } = RoundDuration(
years,
months,
Expand All @@ -4134,6 +4141,15 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
plainDate,
calendarRec
));
({ years, months, weeks, days } = BalanceDurationRelative(
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 @@ -4167,7 +4183,15 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
}

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

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

return new Duration(
Expand Down Expand Up @@ -4373,6 +4406,7 @@ export function DifferenceTemporalPlainYearMonth(operation, yearMonth, other, op
thisDate,
calendarRec
));
({ years, months } = BalanceDurationRelative(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 @@ -4432,7 +4466,11 @@ export function DifferenceTemporalZonedDateTime(operation, zonedDateTime, other,

if (settings.smallestUnit !== 'nanosecond' || settings.roundingIncrement !== 1) {
const plainRelativeToWillBeUsed =
settings.smallestUnit === 'year' || settings.smallestUnit === 'month' || settings.smallestUnit === 'week';
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week' ||
((years !== 0 || months !== 0 || weeks !== 0 || days !== 0) &&
(settings.largestUnit === 'year' || settings.largestUnit === 'month' || settings.largestUnit === 'week'));
let plainRelativeTo;
if (plainRelativeToWillBeUsed) {
const dt = GetPlainDateTimeFor(timeZoneRec, GetSlot(zonedDateTime, INSTANT), GetSlot(zonedDateTime, CALENDAR));
Expand Down Expand Up @@ -4476,6 +4514,16 @@ export function DifferenceTemporalZonedDateTime(operation, zonedDateTime, other,
calendarRec,
timeZoneRec
));
// BalanceDuration already performed in AdjustRoundedDurationDays
({ years, months, weeks, days } = BalanceDurationRelative(
years,
months,
weeks,
days,
settings.largestUnit,
plainRelativeTo,
calendarRec
));
}
}

Expand Down
8 changes: 7 additions & 1 deletion spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,13 @@ <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 _result_ be (? RoundDuration(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _precision_.[[Increment]], _precision_.[[Unit]], _roundingMode_)).[[DurationRecord]].
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 _roundResult_ be (? RoundDuration(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _precision_.[[Increment]], _precision_.[[Unit]], _roundingMode_)).[[DurationRecord]].
1. Let _balanceResult_ be <emu-meta suppress-effects="user-code">? BalanceDuration(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _largestUnit_)</emu-meta>.
1. Let _result_ be ! CreateDurationRecord(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[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
13 changes: 10 additions & 3 deletions spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -1052,13 +1052,20 @@ <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 ! CreateCalendarRecord(_temporalDate_.[[Calendar]], « »).
1. If _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Let _roundingRequested_ be *false*.
1. If _settings_.[[SmallestUnit]] is not *"day"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. Set _roundingRequested to *true*.
1. Let _roundingRequiresDateAddLookup_ be *false*.
1. If _roundingRequested_ is *true* and _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Set _roundingRequiresDateAddLookup_ to *true*.
1. If _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, or *"week"*, or _roundingRequiresDateAddLookup_ is *true*, then
1. Set _calendarRec_.[[DateAdd]] to ? GetMethod(_temporalDate_.[[Calendar]], *"dateAdd"*).
1. If _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_temporalDate_.[[Calendar]], *"dateUntil"*).
1. Let _result_ be ? DifferenceDate(_calendarRec_, _temporalDate_, _other_, _resolvedOptions_).
1. If _settings_.[[SmallestUnit]] is not *"day"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. Set _result_ to (? RoundDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _temporalDate_, _calendarRec_)).[[DurationRecord]].
1. If _roundingRequested_ is *true*, then
1. Let _roundResult_ be (? RoundDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _temporalDate_, _calendarRec_)).[[DurationRecord]].
1. Set _result_ to ? BalanceDurationRelative(_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
13 changes: 10 additions & 3 deletions spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -1189,20 +1189,27 @@ <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 ! CreateCalendarRecord(_dateTime_.[[Calendar]], « »).
1. If _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Let _roundingRequested_ be *false*.
1. If _settings_.[[SmallestUnit]] is not *"day"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. Set _roundingRequested to *true*.
1. Let _roundingRequiresDateAddLookup_ be *false*.
1. If _roundingRequested_ is *true* and _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Set _roundingRequiresDateAddLookup_ to *true*.
1. If _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, or *"week"*, or _roundingRequiresDateAddLookup_ is *true*, then
1. Set _calendarRec_.[[DateAdd]] to ? GetMethod(_dateTime_.[[Calendar]], *"dateAdd"*).
1. Let _largestUnitRequiresDateUntilLookup_ be *false*.
1. If _datePartsIdentical_ is *false* and _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Set _largestUnitRequiresDateUntilLookup_ to *true*.
1. If _largestUnitRequiresDateUntilLookup_ is *true*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_dateTime_.[[Calendar]], *"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]], _dateTime_.[[Calendar]], _settings_.[[LargestUnit]], _resolvedOptions_, _calendarRec_).
1. If _settings_.[[SmallestUnit]] is *"nanosecond"* and _settings_.[[RoundingIncrement]] is 1, then
1. If _roundingRequested_ is *false*, 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 _roundResult_ 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_)).[[DurationRecord]].
1. Let _result_ be ? <emu-meta suppress-effects="user-code">BalanceDuration(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _settings_.[[LargestUnit]])</emu-meta>.
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 ? BalanceDurationRelative(_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
1 change: 1 addition & 0 deletions spec/plainyearmonth.html
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ <h1>
1. Let _result_ be ? CalendarDateUntil(_calendarRec_.[[Receiver]], _thisDate_, _otherDate_, _resolvedOptions_, _calendarRec_.[[DateUntil]]).
1. If _settings_.[[SmallestUnit]] is not *"month"* or _settings_.[[RoundingIncrement]] &ne; 1, then
1. Set _result_ to (? RoundDuration(_result_.[[Years]], _result_.[[Months]], 0, 0, 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _thisDate_, _calendarRec_)).[[DurationRecord]].
1. Set _result_ to ? BalanceDurationRelative(_result_.[[Years]], _result_.[[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 282dd60

Please sign in to comment.