Skip to content
Permalink
Browse files
Temporal round and total methods should accept string param
https://bugs.webkit.org/show_bug.cgi?id=240249

Reviewed by Yusuke Suzuki.

This patch implements tc39/proposal-temporal#1875,
which allows certain required string options to be passed directly instead of as part of an options object.

Namely:
- `{Duration, Instant, PlainTime}::round` now accept `smallestUnit` as a string param
- `Duration::total` now accepts `unit` as a string param

* stress/temporal-duration.js:
* stress/temporal-instant.js:
* stress/temporal-plaintime.js:
Add test cases.

* test262/expectations.yaml:
Mark 24 test cases passing.
(This number should be 26, but two still fail as the harness expects PlainDateTime and ZonedDateTime to exist.)

* runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::round const):
(JSC::TemporalDuration::total const):
* runtime/TemporalInstant.cpp:
* runtime/TemporalPlainTime.cpp:
(JSC::TemporalPlainTime::round const):

Canonical link: https://commits.webkit.org/250433@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293997 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rkirsling committed May 10, 2022
1 parent 5897b82 commit 95ded74861045448e239cda75fab5571ff1a7432
Showing 9 changed files with 135 additions and 72 deletions.
@@ -1,3 +1,19 @@
2022-05-09 Ross Kirsling <ross.kirsling@sony.com>

Temporal round and total methods should accept string param
https://bugs.webkit.org/show_bug.cgi?id=240249

Reviewed by Yusuke Suzuki.

* stress/temporal-duration.js:
* stress/temporal-instant.js:
* stress/temporal-plaintime.js:
Add test cases.

* test262/expectations.yaml:
Mark 24 test cases passing.
(This number should be 26, but two still fail as the harness expects PlainDateTime and ZonedDateTime to exist.)

2022-05-09 Ross Kirsling <ross.kirsling@sony.com>

Temporal and Date must reject expanded year -000000
@@ -203,6 +203,7 @@ shouldThrow(() => Temporal.Duration.from('P1M').round({ largestUnit: 'seconds' }
shouldThrow(() => Temporal.Duration.from('P1W').round({ largestUnit: 'seconds' }), RangeError);
shouldThrow(() => Temporal.Duration.from('P1D').round({ largestUnit: 'months' }), RangeError);
shouldThrow(() => Temporal.Duration.from('P1D').round({ largestUnit: 'weeks' }), RangeError);
shouldBe(posAbsolute.round('day').toString(), 'P1D');
shouldBe(posAbsolute.round({ largestUnit: 'day' }).toString(), 'P1DT2H3M4.005006007S');
shouldBe(posAbsolute.round({ largestUnit: 'auto' }).toString(), 'P1DT2H3M4.005006007S');
shouldBe(posAbsolute.round({ smallestUnit: 'day' }).toString(), 'P1D');
@@ -229,6 +230,7 @@ shouldThrow(() => Temporal.Duration.from('P1M').total({ unit: 'seconds' }), Rang
shouldThrow(() => Temporal.Duration.from('P1W').total({ unit: 'seconds' }), RangeError);
shouldThrow(() => Temporal.Duration.from('P1D').total({ unit: 'months' }), RangeError);
shouldThrow(() => Temporal.Duration.from('P1D').total({ unit: 'weeks' }), RangeError);
shouldBe(posAbsolute.total('days'), 1.0854630209028588);
shouldBe(posAbsolute.total({ unit: 'days' }), 1.0854630209028588);
shouldBe(posAbsolute.total({ unit: 'hours' }), 26.051112501668612);
shouldBe(posAbsolute.total({ unit: 'minutes' }), 1563.0667501001167);
@@ -229,6 +229,10 @@ shouldThrow(() => new Temporal.Instant('abc123'), SyntaxError);
// truncates to minute
[i1, i2, i3].forEach((i) => shouldBe(i.toString({ smallestUnit: 'minute' }), '1976-11-18T15:23Z'));

// ...as opposed to rounding first
shouldBe(i3.round('minute').toString(), '1976-11-18T15:24:00Z');
shouldBe(i3.round({ smallestUnit: 'minute' }).toString(), '1976-11-18T15:24:00Z');

// other smallestUnits are aliases for fractional digits
shouldBe(i3.toString({ smallestUnit: 'second' }), i3.toString({ fractionalSecondDigits: 0 }));
shouldBe(i3.toString({ smallestUnit: 'millisecond' }), i3.toString({ fractionalSecondDigits: 3 }));
@@ -284,6 +284,7 @@ shouldBe(Temporal.PlainTime.from("20:34").calendar instanceof Temporal.Calendar,

{
let time = Temporal.PlainTime.from('19:39:09.068346205');
shouldBe(String(time.round('hour')), `20:00:00`);
shouldBe(String(time.round({ smallestUnit: 'hour' })), `20:00:00`);
shouldBe(String(time.round({ roundingIncrement: 30, smallestUnit: 'minute' })), `19:30:00`);
shouldBe(String(time.round({ roundingIncrement: 30, smallestUnit: 'minute', roundingMode: 'ceil' })), `20:00:00`);
@@ -948,18 +948,6 @@ test/built-ins/Temporal/Duration/prototype/round/relativeto-wrong-type.js:
test/built-ins/Temporal/Duration/prototype/round/relativeto-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/Duration/prototype/round/roundto-invalid-string.js:
default: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
test/built-ins/Temporal/Duration/prototype/round/smallestunit-plurals-accepted-string.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Duration/prototype/round/smallestunit-string-shorthand-string.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Duration/prototype/round/smallestunit.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Duration/prototype/round/timezone-string-leap-second.js:
default: 'RangeError: Cannot round a duration of years, months, or weeks without a relativeTo option'
strict mode: 'RangeError: Cannot round a duration of years, months, or weeks without a relativeTo option'
@@ -1038,9 +1026,6 @@ test/built-ins/Temporal/Duration/prototype/total/relativeto-propertybag-calendar
test/built-ins/Temporal/Duration/prototype/total/relativeto-propertybag-timezone-getoffsetnanosecondsfor-not-callable.js:
default: 'Test262Error: Uncallable undefined getOffsetNanosecondsFor should throw TypeError Expected a TypeError but got a RangeError'
strict mode: 'Test262Error: Uncallable undefined getOffsetNanosecondsFor should throw TypeError Expected a TypeError but got a RangeError'
test/built-ins/Temporal/Duration/prototype/total/relativeto-undefined-throw-on-calendar-units.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Duration/prototype/total/relativeto-wrong-type.js:
default: "TypeError: undefined is not an object (evaluating 'Temporal.ZonedDateTime.prototype')"
strict mode: "TypeError: undefined is not an object (evaluating 'Temporal.ZonedDateTime.prototype')"
@@ -1053,27 +1038,12 @@ test/built-ins/Temporal/Duration/prototype/total/timezone-string-leap-second.js:
test/built-ins/Temporal/Duration/prototype/total/timezone-wrong-type.js:
default: 'Test262Error: symbol is not a valid object and does not convert to a string Expected a TypeError but got a RangeError'
strict mode: 'Test262Error: symbol is not a valid object and does not convert to a string Expected a TypeError but got a RangeError'
test/built-ins/Temporal/Duration/prototype/total/unit-disallowed-units-string.js:
default: 'Test262Error: "era" should not be allowed as an argument to total Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: "era" should not be allowed as an argument to total Expected a RangeError but got a TypeError'
test/built-ins/Temporal/Duration/prototype/total/unit-plurals-accepted-string.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Duration/prototype/total/unit-string-shorthand-string.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
default: 'TypeError: Right hand side of instanceof is not an object'
strict mode: 'TypeError: Right hand side of instanceof is not an object'
test/built-ins/Temporal/Instant/prototype/round/rounding-direction.js:
default: 'Test262Error: Rounding down is towards the Big Bang, not the epoch or 1 BCE (roundingMode trunc) Expected SameValue(«-65261246399000000000», «-65261246400000000000») to be true'
strict mode: 'Test262Error: Rounding down is towards the Big Bang, not the epoch or 1 BCE (roundingMode trunc) Expected SameValue(«-65261246399000000000», «-65261246400000000000») to be true'
test/built-ins/Temporal/Instant/prototype/round/roundto-invalid-string.js:
default: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
test/built-ins/Temporal/Instant/prototype/round/smallestunit-plurals-accepted.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Instant/prototype/round/smallestunit-string-shorthand.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/Instant/prototype/since/largestunit.js:
default: 'Test262Error: does not include higher units than necessary (largest unit unspecified) nanoseconds result Expected SameValue(«40», «101») to be true'
strict mode: 'Test262Error: does not include higher units than necessary (largest unit unspecified) nanoseconds result Expected SameValue(«40», «101») to be true'
@@ -1170,12 +1140,6 @@ test/built-ins/Temporal/PlainTime/prototype/equals/leap-second.js:
test/built-ins/Temporal/PlainTime/prototype/equals/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/PlainTime/prototype/round/roundto-invalid-string.js:
default: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: "era" is not a valid value for smallest unit Expected a RangeError but got a TypeError'
test/built-ins/Temporal/PlainTime/prototype/round/smallestunit-string-shorthand.js:
default: 'TypeError: options argument is not an object or undefined'
strict mode: 'TypeError: options argument is not an object or undefined'
test/built-ins/Temporal/PlainTime/prototype/since/argument-cast.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
@@ -1,3 +1,24 @@
2022-05-09 Ross Kirsling <ross.kirsling@sony.com>

Temporal round and total methods should accept string param
https://bugs.webkit.org/show_bug.cgi?id=240249

Reviewed by Yusuke Suzuki.

This patch implements https://github.com/tc39/proposal-temporal/pull/1875,
which allows certain required string options to be passed directly instead of as part of an options object.

Namely:
- `{Duration, Instant, PlainTime}::round` now accept `smallestUnit` as a string param
- `Duration::total` now accepts `unit` as a string param

* runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::round const):
(JSC::TemporalDuration::total const):
* runtime/TemporalInstant.cpp:
* runtime/TemporalPlainTime.cpp:
(JSC::TemporalPlainTime::round const):

2022-05-09 Ross Kirsling <ross.kirsling@sony.com>

Temporal and Date must reject expanded year -000000
@@ -462,26 +462,39 @@ ISO8601::Duration TemporalDuration::round(JSGlobalObject* globalObject, JSValue
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });
JSObject* options = nullptr;
std::optional<TemporalUnit> smallest;
std::optional<TemporalUnit> largest;
TemporalUnit defaultLargestUnit = largestSubduration(m_duration);
if (optionsValue.isString()) {
auto string = optionsValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

auto smallest = temporalSmallestUnit(globalObject, options, { });
RETURN_IF_EXCEPTION(scope, { });
smallest = temporalUnitType(string);
if (!smallest) {
throwRangeError(globalObject, scope, "smallestUnit is an invalid Temporal unit"_s);
return { };
}
} else {
options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });

TemporalUnit defaultLargestUnit = largestSubduration(m_duration);
auto largest = temporalLargestUnit(globalObject, options, { }, defaultLargestUnit);
RETURN_IF_EXCEPTION(scope, { });
smallest = temporalSmallestUnit(globalObject, options, { });
RETURN_IF_EXCEPTION(scope, { });

if (!smallest && !largest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit or largestUnit option"_s);
return { };
}
largest = temporalLargestUnit(globalObject, options, { }, defaultLargestUnit);
RETURN_IF_EXCEPTION(scope, { });

if (smallest && largest && smallest.value() < largest.value()) {
throwRangeError(globalObject, scope, "smallestUnit must be smaller than largestUnit"_s);
return { };
}
if (!smallest && !largest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit or largestUnit option"_s);
return { };
}

if (smallest && largest && smallest.value() < largest.value()) {
throwRangeError(globalObject, scope, "smallestUnit must be smaller than largestUnit"_s);
return { };
}
}
TemporalUnit smallestUnit = smallest.value_or(TemporalUnit::Nanosecond);
TemporalUnit largestUnit = largest.value_or(std::min(defaultLargestUnit, smallestUnit));

@@ -512,11 +525,17 @@ double TemporalDuration::total(JSGlobalObject* globalObject, JSValue optionsValu
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, 0);
String unitString;
if (optionsValue.isString()) {
unitString = optionsValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, 0);
} else {
JSObject* options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, 0);

String unitString = intlStringOption(globalObject, options, vm.propertyNames->unit, { }, { }, { });
RETURN_IF_EXCEPTION(scope, 0);
unitString = intlStringOption(globalObject, options, vm.propertyNames->unit, { }, { }, { });
RETURN_IF_EXCEPTION(scope, 0);
}

auto unitType = temporalUnitType(unitString);
if (!unitType) {
@@ -362,17 +362,35 @@ ISO8601::ExactTime TemporalInstant::round(JSGlobalObject* globalObject, JSValue
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });
JSObject* options = nullptr;
std::optional<TemporalUnit> smallest;
if (optionsValue.isString()) {
auto string = optionsValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

auto smallest = temporalSmallestUnit(globalObject, options, { TemporalUnit::Year, TemporalUnit::Month, TemporalUnit::Week, TemporalUnit::Day });
RETURN_IF_EXCEPTION(scope, { });
smallest = temporalUnitType(string);
if (!smallest) {
throwRangeError(globalObject, scope, "smallestUnit is an invalid Temporal unit"_s);
return { };
}

if (!smallest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit option"_s);
return { };
if (smallest.value() <= TemporalUnit::Day) {
throwRangeError(globalObject, scope, "smallestUnit is a disallowed unit"_s);
return { };
}
} else {
options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });

smallest = temporalSmallestUnit(globalObject, options, { TemporalUnit::Year, TemporalUnit::Month, TemporalUnit::Week, TemporalUnit::Day });
RETURN_IF_EXCEPTION(scope, { });

if (!smallest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit option"_s);
return { };
}
}
TemporalUnit smallestUnit = smallest.value_or(TemporalUnit::Nanosecond);
TemporalUnit smallestUnit = smallest.value();

RoundingMode roundingMode = temporalRoundingMode(globalObject, options, RoundingMode::HalfExpand);
RETURN_IF_EXCEPTION(scope, { });
@@ -228,14 +228,32 @@ ISO8601::PlainTime TemporalPlainTime::round(JSGlobalObject* globalObject, JSValu
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

JSObject* options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });
JSObject* options = nullptr;
std::optional<TemporalUnit> smallest;
if (optionsValue.isString()) {
auto string = optionsValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

auto smallest = temporalSmallestUnit(globalObject, options, { TemporalUnit::Year, TemporalUnit::Month, TemporalUnit::Week, TemporalUnit::Day });
RETURN_IF_EXCEPTION(scope, { });
if (!smallest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit option"_s);
return { };
smallest = temporalUnitType(string);
if (!smallest) {
throwRangeError(globalObject, scope, "smallestUnit is an invalid Temporal unit"_s);
return { };
}

if (smallest.value() <= TemporalUnit::Day) {
throwRangeError(globalObject, scope, "smallestUnit is a disallowed unit"_s);
return { };
}
} else {
options = intlGetOptionsObject(globalObject, optionsValue);
RETURN_IF_EXCEPTION(scope, { });

smallest = temporalSmallestUnit(globalObject, options, { TemporalUnit::Year, TemporalUnit::Month, TemporalUnit::Week, TemporalUnit::Day });
RETURN_IF_EXCEPTION(scope, { });
if (!smallest) {
throwRangeError(globalObject, scope, "Cannot round without a smallestUnit option"_s);
return { };
}
}
TemporalUnit smallestUnit = smallest.value();

0 comments on commit 95ded74

Please sign in to comment.