Skip to content

Commit

Permalink
Temporal API should throw TypeErrors for unexpected primitives
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264240

Reviewed by Yusuke Suzuki.

As of the latest Temporal spec, we should never convert non-object, non-string values to a Temporal object.
At our current stage of implementation, this applies to Duration, Instant, PlainDate, PlainTime, and PlainDateTime.

Relatedly (namely, for the case of handling explicit undefined values),
toIntegerOrInfinity is no longer used by Temporal and toIntegerWithoutRounding has been removed entirely.
Spec-side, these have been replaced by toIntegerWithTruncation and toIntegerIfIntegral;
here, we implement the former and inline the latter.

* JSTests/test262/config.yaml:
Move two tests over from expectations.yaml;
even before this patch, these are hitting an assertion due to unimplemented parts of Temporal.

* JSTests/test262/expectations.yaml:
Mark 102 test cases passing.

* JSTests/stress/temporal-duration.js:
* JSTests/stress/temporal-plaindatetime.js:
* Source/JavaScriptCore/runtime/JSCJSValue.h:
* Source/JavaScriptCore/runtime/JSCJSValueInlines.h:
(JSC::JSValue::toIntegerWithTruncation const):
(JSC::JSValue::toIntegerOrInfinity const):
(JSC::JSValue::toIntegerWithoutRounding const): Deleted.
* Source/JavaScriptCore/runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::fromDurationLike):
(JSC::TemporalDuration::toISO8601Duration):
* Source/JavaScriptCore/runtime/TemporalDurationConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalInstant.cpp:
* Source/JavaScriptCore/runtime/TemporalPlainDate.cpp:
(JSC::TemporalPlainDate::from):
* Source/JavaScriptCore/runtime/TemporalPlainDateConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainDateTime.cpp:
(JSC::TemporalPlainDateTime::from):
* Source/JavaScriptCore/runtime/TemporalPlainTime.cpp:
(JSC::TemporalPlainTime::from):

Canonical link: https://commits.webkit.org/270262@main
  • Loading branch information
rkirsling committed Nov 6, 2023
1 parent f3c296b commit 681701c
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 176 deletions.
8 changes: 4 additions & 4 deletions JSTests/stress/temporal-duration.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ shouldBe(pos.blank, false);
shouldBe(neg.blank, false);

shouldBe(Temporal.Duration.from.length, 1);
shouldThrow(() => Temporal.Duration.from(), RangeError);
shouldThrow(() => Temporal.Duration.from(), TypeError);
shouldThrow(() => Temporal.Duration.from({}), TypeError);
{
const badStrings = [
Expand Down Expand Up @@ -102,9 +102,9 @@ shouldBe(Temporal.Duration.from('PT1.03125M').toString(), 'PT1M1.875S');
const posAbsolute = new Temporal.Duration(0,0,0,1,2,3,4,5,6,7);

shouldBe(Temporal.Duration.compare.length, 2);
shouldThrow(() => Temporal.Duration.compare(), RangeError);
shouldThrow(() => Temporal.Duration.compare(), TypeError);
shouldThrow(() => Temporal.Duration.compare({}), TypeError);
shouldThrow(() => Temporal.Duration.compare(zero), RangeError);
shouldThrow(() => Temporal.Duration.compare(zero), TypeError);
shouldThrow(() => Temporal.Duration.compare(zero, {}), TypeError);
shouldThrow(() => Temporal.Duration.compare({ years: 1 }, zero), RangeError);
shouldThrow(() => Temporal.Duration.compare({ months: 1 }, zero), RangeError);
Expand Down Expand Up @@ -160,7 +160,7 @@ shouldThrow(() => Temporal.Duration.prototype.abs.call({}), TypeError);
for (const method of ['add', 'subtract']) {
shouldBe(Temporal.Duration.prototype[method].length, 1);
shouldThrow(() => Temporal.Duration.prototype[method].call({ hours: 1 }), TypeError);
shouldThrow(() => zero[method](), RangeError);
shouldThrow(() => zero[method](), TypeError);
shouldThrow(() => zero[method]({}), TypeError);
shouldThrow(() => zero[method]({ years: 1 }), RangeError);
shouldThrow(() => zero[method]({ months: 1 }), RangeError);
Expand Down
4 changes: 2 additions & 2 deletions JSTests/stress/temporal-plaindatetime.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ shouldBe(Temporal.PlainDateTime.prototype.getISOFields.length, 0);
shouldBe(JSON.stringify(pdt.getISOFields()), `{"calendar":"iso8601","isoDay":3,"isoHour":4,"isoMicrosecond":8,"isoMillisecond":7,"isoMinute":5,"isoMonth":2,"isoNanosecond":9,"isoSecond":6,"isoYear":1}`);

shouldBe(Temporal.PlainDateTime.from.length, 1);
shouldThrow(() => Temporal.PlainDateTime.from(), RangeError);
shouldThrow(() => Temporal.PlainDateTime.from(), TypeError);
shouldBe(Temporal.PlainDateTime.from('0001-02-03T04:05:06.007008009').toString(), '0001-02-03T04:05:06.007008009');
{
const dateTime = Temporal.PlainDateTime.from('2007-01-09T03:24:30+01:00[Europe/Brussels]');
Expand Down Expand Up @@ -278,7 +278,7 @@ shouldBe(pdt.with({ day: 30 }).toString(), '0001-02-28T04:05:06.007008009');
shouldThrow(() => { pdt.with({ day: 30 }, { overflow: 'reject' }); }, RangeError);

shouldBe(Temporal.PlainDateTime.prototype.withPlainDate.length, 1);
shouldThrow(() => { pdt.withPlainDate(); }, RangeError);
shouldThrow(() => { pdt.withPlainDate(); }, TypeError);
shouldBe(pdt.withPlainDate({ year: 2000, month: 10, day: 30 }).toString(), '2000-10-30T04:05:06.007008009');

shouldBe(Temporal.PlainDateTime.prototype.withPlainTime.length, 0);
Expand Down
2 changes: 2 additions & 0 deletions JSTests/test262/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ skip:
- test/built-ins/Temporal/Duration/prototype/round/calendar-possibly-required.js
- test/built-ins/Temporal/Duration/prototype/round/calendar-temporal-object.js
- test/built-ins/Temporal/Duration/prototype/round/dateuntil-field.js
- test/built-ins/Temporal/Duration/prototype/round/duration-out-of-range-added-to-relativeto.js
- test/built-ins/Temporal/Duration/prototype/round/february-leap-year.js
- test/built-ins/Temporal/Duration/prototype/round/largestunit-smallestunit-default.js
- test/built-ins/Temporal/Duration/prototype/round/nanoseconds-to-days-loop-indefinitely-1.js
Expand Down Expand Up @@ -304,6 +305,7 @@ skip:
- test/built-ins/Temporal/Duration/prototype/total/calendar-possibly-required.js
- test/built-ins/Temporal/Duration/prototype/total/calendar-temporal-object.js
- test/built-ins/Temporal/Duration/prototype/total/dateuntil-field.js
- test/built-ins/Temporal/Duration/prototype/total/duration-out-of-range-added-to-relativeto.js
- test/built-ins/Temporal/Duration/prototype/total/nanoseconds-to-days-loop-indefinitely-1.js
- test/built-ins/Temporal/Duration/prototype/total/nanoseconds-to-days-loop-indefinitely-2.js
- test/built-ins/Temporal/Duration/prototype/total/order-of-operations.js
Expand Down
159 changes: 0 additions & 159 deletions JSTests/test262/expectations.yaml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Source/JavaScriptCore/runtime/JSCJSValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ class JSValue {

// Integer conversions.
JS_EXPORT_PRIVATE double toIntegerPreserveNaN(JSGlobalObject*) const;
double toIntegerWithoutRounding(JSGlobalObject*) const;
double toIntegerWithTruncation(JSGlobalObject*) const;
double toIntegerOrInfinity(JSGlobalObject*) const;
int32_t toInt32(JSGlobalObject*) const;
uint32_t toUInt32(JSGlobalObject*) const;
Expand Down
12 changes: 7 additions & 5 deletions Source/JavaScriptCore/runtime/JSCJSValueInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,21 @@ inline size_t JSValue::toTypedArrayIndex(JSGlobalObject* globalObject, ASCIILite
RELEASE_AND_RETURN(scope, outputOffset + static_cast<size_t>(static_cast<uint32_t>(JSC::toInt32(d - inputOffset))));
}

// https://tc39.es/proposal-temporal/#sec-temporal-tointegerwithoutrounding
inline double JSValue::toIntegerWithoutRounding(JSGlobalObject* globalObject) const
// https://tc39.es/proposal-temporal/#sec-tointegerwithtruncation
inline double JSValue::toIntegerWithTruncation(JSGlobalObject* globalObject) const
{
if (isInt32())
return asInt32();
double d = toNumber(globalObject);
return std::isnan(d) ? 0.0 : d + 0.0;
return trunc(toNumber(globalObject) + 0.0);
}

// https://tc39.es/ecma262/#sec-tointegerorinfinity
inline double JSValue::toIntegerOrInfinity(JSGlobalObject* globalObject) const
{
return trunc(toIntegerWithoutRounding(globalObject));
if (isInt32())
return asInt32();
double d = toNumber(globalObject);
return trunc(std::isnan(d) ? 0.0 : d + 0.0);
}

inline bool JSValue::isUInt32() const
Expand Down
7 changes: 6 additions & 1 deletion Source/JavaScriptCore/runtime/TemporalDuration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ ISO8601::Duration TemporalDuration::fromDurationLike(JSGlobalObject* globalObjec
continue;

hasRelevantProperty = true;
result[unit] = value.toIntegerWithoutRounding(globalObject);
result[unit] = value.toNumber(globalObject) + 0.0;
RETURN_IF_EXCEPTION(scope, { });

if (!isInteger(result[unit])) {
Expand Down Expand Up @@ -121,6 +121,11 @@ ISO8601::Duration TemporalDuration::toISO8601Duration(JSGlobalObject* globalObje
duration = fromDurationLike(globalObject, asObject(itemValue));
RETURN_IF_EXCEPTION(scope, { });
} else {
if (!itemValue.isString()) {
throwTypeError(globalObject, scope, "can only convert to Duration from object or string values"_s);
return { };
}

String string = itemValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ JSC_DEFINE_HOST_FUNCTION(constructTemporalDuration, (JSGlobalObject* globalObjec
if (value.isUndefined())
continue;

result[i] = value.toIntegerWithoutRounding(globalObject);
result[i] = value.toNumber(globalObject) + 0.0;
RETURN_IF_EXCEPTION(scope, { });

if (!isInteger(result[i]))
Expand Down
5 changes: 5 additions & 0 deletions Source/JavaScriptCore/runtime/TemporalInstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ TemporalInstant* TemporalInstant::toInstant(JSGlobalObject* globalObject, JSValu
VM& vm = globalObject->vm();
auto scope = DECLARE_THROW_SCOPE(vm);

if (!itemValue.isObject() && !itemValue.isString()) {
throwTypeError(globalObject, scope, "can only convert to Instant from object or string values"_s);
return nullptr;
}

if (itemValue.inherits<TemporalInstant>())
return jsCast<TemporalInstant*>(itemValue);

Expand Down
5 changes: 5 additions & 0 deletions Source/JavaScriptCore/runtime/TemporalPlainDate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ TemporalPlainDate* TemporalPlainDate::from(JSGlobalObject* globalObject, JSValue
return TemporalPlainDate::create(vm, globalObject->plainDateStructure(), WTFMove(plainDate));
}

if (!itemValue.isString()) {
throwTypeError(globalObject, scope, "can only convert to PlainDate from object or string values"_s);
return { };
}

auto string = itemValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,23 +93,23 @@ JSC_DEFINE_HOST_FUNCTION(constructTemporalPlainDate, (JSGlobalObject* globalObje
auto argumentCount = callFrame->argumentCount();

if (argumentCount > 0) {
auto value = callFrame->uncheckedArgument(0).toIntegerOrInfinity(globalObject);
auto value = callFrame->uncheckedArgument(0).toIntegerWithTruncation(globalObject);
if (!std::isfinite(value))
return throwVMRangeError(globalObject, scope, "Temporal.PlainDate year property must be finite"_s);
duration.setYears(value);
RETURN_IF_EXCEPTION(scope, { });
}

if (argumentCount > 1) {
auto value = callFrame->uncheckedArgument(1).toIntegerOrInfinity(globalObject);
auto value = callFrame->uncheckedArgument(1).toIntegerWithTruncation(globalObject);
if (!std::isfinite(value))
return throwVMRangeError(globalObject, scope, "Temporal.PlainDate month property must be finite"_s);
duration.setMonths(value);
RETURN_IF_EXCEPTION(scope, { });
}

if (argumentCount > 2) {
auto value = callFrame->uncheckedArgument(2).toIntegerOrInfinity(globalObject);
auto value = callFrame->uncheckedArgument(2).toIntegerWithTruncation(globalObject);
if (!std::isfinite(value))
return throwVMRangeError(globalObject, scope, "Temporal.PlainDate day property must be finite"_s);
duration.setDays(value);
Expand Down
5 changes: 5 additions & 0 deletions Source/JavaScriptCore/runtime/TemporalPlainDateTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ TemporalPlainDateTime* TemporalPlainDateTime::from(JSGlobalObject* globalObject,
RELEASE_AND_RETURN(scope, TemporalPlainDateTime::tryCreateIfValid(globalObject, globalObject->plainDateTimeStructure(), WTFMove(plainDate), WTFMove(plainTime)));
}

if (!itemValue.isString()) {
throwTypeError(globalObject, scope, "can only convert to PlainDateTime from object or string values"_s);
return { };
}

auto string = itemValue.toWTFString(globalObject);
RETURN_IF_EXCEPTION(scope, { });

Expand Down
5 changes: 5 additions & 0 deletions Source/JavaScriptCore/runtime/TemporalPlainTime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,11 @@ TemporalPlainTime* TemporalPlainTime::from(JSGlobalObject* globalObject, JSValue
return TemporalPlainTime::create(vm, globalObject->plainTimeStructure(), WTFMove(plainTime));
}

if (!itemValue.isString()) {
throwTypeError(globalObject, scope, "can only convert to PlainTime from object or string values"_s);
return { };
}

// https://tc39.es/proposal-temporal/#sec-temporal-parsetemporaltimestring
// TemporalTimeString :
// CalendarTime
Expand Down

0 comments on commit 681701c

Please sign in to comment.