Skip to content

Commit

Permalink
Remove ToString from ISO string parsing
Browse files Browse the repository at this point in the history
This commit stops converting non-string primitive inputs to strings when
parsing ISO strings or subsets of ISO strings like time zone offsets.

While numbers are sometimes allowed for these fields, the result is
often brittle, yielding "data driven exceptions" that we've tried to
avoid.

Numbers can also be ambiguous, e.g. is a Number passed for
`offset` mean hours like an ISO string, minutes like
Date.p.getTimezoneOffset, of msecs like Date.p.getTime().

Objects are still coerced to strings when appropriate, e.g.
when parsing a string for the `offset` field.
  • Loading branch information
justingrant committed May 26, 2023
1 parent dac765f commit 5beac8b
Show file tree
Hide file tree
Showing 11 changed files with 35 additions and 28 deletions.
9 changes: 6 additions & 3 deletions spec/abstractops.html
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,8 @@ <h1>ToRelativeTemporalObject ( _options_ )</h1>
1. If _offsetString_ is *undefined*, then
1. Set _offsetBehaviour_ to ~wall~.
1. Else,
1. Let _string_ be ? ToString(_value_).
1. Let _result_ be ? ParseTemporalRelativeToString(_string_).
1. If _value_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalRelativeToString(_value_).
1. Let _offsetString_ be _result_.[[TimeZone]].[[OffsetString]].
1. Let _timeZoneName_ be _result_.[[TimeZone]].[[Name]].
1. If _timeZoneName_ is *undefined*, then
Expand Down Expand Up @@ -1832,6 +1832,9 @@ <h1>
1. Else if _Conversion_ is ~ToPositiveIntegerWithTruncation~, then
1. Set _value_ to ? ToPositiveIntegerWithTruncation(_value_).
1. Set _value_ to 𝔽(_value_).
1. Else if _Conversion_ is ~StringOnly~, then
1. If _value_ is an Object, set _value_ to ? ToString(_value_).
1. Else if _value_ is not a String, throw a *TypeError* exception.
1. Else,
1. Assert: _Conversion_ is ~ToString~.
1. Set _value_ to ? ToString(_value_).
Expand Down Expand Up @@ -1909,7 +1912,7 @@ <h1>
</tr>
<tr>
<td>*"offset"*</td>
<td>~ToString~</td>
<td>~StringOnly~</td>
<td>*undefined*</td>
</tr>
<tr>
Expand Down
14 changes: 8 additions & 6 deletions spec/calendar.html
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ <h1>
</dd>
</dl>
<emu-alg>
1. Assert: No non-String, non-Object value, when coerced to a String, is a built-in calendar identifier.
1. If _temporalCalendarLike_ is *undefined* and _default_ is present, then
1. Assert: IsBuiltinCalendar(_default_) is *true*.
1. Return _default_.
Expand All @@ -525,8 +526,8 @@ <h1>
1. Return _temporalCalendarLike_.[[Calendar]].
1. If ? ObjectImplementsTemporalCalendarProtocol(_temporalCalendarLike_) is *false*, throw a *TypeError* exception.
1. Return _temporalCalendarLike_.
1. Let _identifier_ be ? ToString(_temporalCalendarLike_).
1. Set _identifier_ to ? ParseTemporalCalendarString(_identifier_).
1. If _temporalCalendarLike_ is not a String, throw a *TypeError* exception.
1. Let _identifier_ be ? ParseTemporalCalendarString(_temporalCalendarLike_).
1. If IsBuiltinCalendar(_identifier_) is *false*, throw a *RangeError* exception.
1. Return the ASCII-lowercase of _identifier_.
</emu-alg>
Expand Down Expand Up @@ -1013,17 +1014,18 @@ <h1>The Temporal.Calendar Constructor</h1>
</ul>

<emu-clause id="sec-temporal.calendar">
<h1>Temporal.Calendar ( _id_ )</h1>
<h1>Temporal.Calendar ( _identifier_ )</h1>
<p>
The `Temporal.Calendar` function performs the following steps when called:
</p>
<emu-alg>
1. If NewTarget is *undefined*, then
1. Throw a *TypeError* exception.
1. Set _id_ to ? ToString(_id_).
1. If IsBuiltinCalendar(_id_) is *false*, then
1. If _identifier_ is an Object, set _identifier_ to ? ToString(_identifier_).
1. Else if _identifier_ is not a String, throw a *TypeError* exception.
1. If IsBuiltinCalendar(_identifier_) is *false*, then
1. Throw a *RangeError* exception.
1. Return ? CreateTemporalCalendar(_id_, NewTarget).
1. Return ? CreateTemporalCalendar(_identifier_, NewTarget).
</emu-alg>
</emu-clause>
</emu-clause>
Expand Down
4 changes: 2 additions & 2 deletions spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -928,8 +928,8 @@ <h1>
</dl>
<emu-alg>
1. If Type(_temporalDurationLike_) is not Object, then
1. Let _string_ be ? ToString(_temporalDurationLike_).
1. Return ? ParseTemporalDurationString(_string_).
1. If _temporalDurationLike_ is not a String, throw a *TypeError* exception.
1. Return ? ParseTemporalDurationString(_temporalDurationLike_).
1. If _temporalDurationLike_ has an [[InitializedTemporalDuration]] internal slot, then
1. Return ! CreateDurationRecord(_temporalDurationLike_.[[Years]], _temporalDurationLike_.[[Months]], _temporalDurationLike_.[[Weeks]], _temporalDurationLike_.[[Days]], _temporalDurationLike_.[[Hours]], _temporalDurationLike_.[[Minutes]], _temporalDurationLike_.[[Seconds]], _temporalDurationLike_.[[Milliseconds]], _temporalDurationLike_.[[Microseconds]], _temporalDurationLike_.[[Nanoseconds]]).
1. Let _result_ be a new Duration Record with each field set to 0.
Expand Down
4 changes: 2 additions & 2 deletions spec/instant.html
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,8 @@ <h1>ToTemporalInstant ( _item_ )</h1>
1. Return _item_.
1. If _item_ has an [[InitializedTemporalZonedDateTime]] internal slot, then
1. Return ! CreateTemporalInstant(_item_.[[Nanoseconds]]).
1. Let _string_ be ? ToString(_item_).
1. Let _epochNanoseconds_ be ? ParseTemporalInstant(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _epochNanoseconds_ be ? ParseTemporalInstant(_item_).
1. Return ! CreateTemporalInstant(ℤ(_epochNanoseconds_)).
</emu-alg>
</emu-clause>
Expand Down
4 changes: 2 additions & 2 deletions spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,8 @@ <h1>ToTemporalDate ( _item_ [ , _options_ ] )</h1>
1. Let _fields_ be ? PrepareTemporalFields(_item_, _fieldNames_, «»).
1. Return ? CalendarDateFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalDateString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalDateString(_item_).
1. Assert: IsValidISODate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]]) is *true*.
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
Expand Down
4 changes: 2 additions & 2 deletions spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,8 @@ <h1>ToTemporalDateTime ( _item_ [ , _options_ ] )</h1>
1. Let _result_ be ? InterpretTemporalDateTimeFields(_calendar_, _fields_, _options_).
1. Else,
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalDateTimeString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalDateTimeString(_item_).
1. Assert: IsValidISODate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]]) is *true*.
1. Assert: IsValidTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]) is *true*.
1. Let _calendar_ be _result_.[[Calendar]].
Expand Down
4 changes: 2 additions & 2 deletions spec/plainmonthday.html
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ <h1>ToTemporalMonthDay ( _item_ [ , _options_ ] )</h1>
1. Perform ! CreateDataPropertyOrThrow(_fields_, *"year"*, 𝔽(_referenceISOYear_)).
1. Return ? CalendarMonthDayFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalMonthDayString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalMonthDayString(_item_).
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
1. If IsBuiltinCalendar(_calendar_) is *false*, throw a *RangeError* exception.
Expand Down
4 changes: 2 additions & 2 deletions spec/plaintime.html
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ <h1>ToTemporalTime ( _item_ [ , _overflow_ ] )</h1>
1. Let _result_ be ? ToTemporalTimeRecord(_item_).
1. Set _result_ to ? RegulateTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]], _overflow_).
1. Else,
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalTimeString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalTimeString(_item_).
1. Assert: IsValidTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]) is *true*.
1. Return ! CreateTemporalTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]).
</emu-alg>
Expand Down
4 changes: 2 additions & 2 deletions spec/plainyearmonth.html
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,8 @@ <h1>ToTemporalYearMonth ( _item_ [ , _options_ ] )</h1>
1. Let _fields_ be ? PrepareTemporalFields(_item_, _fieldNames_, «»).
1. Return ? CalendarYearMonthFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalYearMonthString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalYearMonthString(_item_).
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
1. If IsBuiltinCalendar(_calendar_) is *false*, throw a *RangeError* exception.
Expand Down
8 changes: 5 additions & 3 deletions spec/timezone.html
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ <h1>Temporal.TimeZone ( _identifier_ )</h1>
<emu-alg>
1. If NewTarget is *undefined*, then
1. Throw a *TypeError* exception.
1. Set _identifier_ to ? ToString(_identifier_).
1. If _identifier_ is an Object, set _identifier_ to ? ToString(_identifier_).
1. Else if _identifier_ is not a String, throw a *TypeError* exception.
1. If IsTimeZoneOffsetString(_identifier_) is *false*, then
1. If IsAvailableTimeZoneName(_identifier_) is *false*, then
1. Throw a *RangeError* exception.
Expand Down Expand Up @@ -612,13 +613,14 @@ <h1>
<dd>It attempts to derive a value from _temporalTimeZoneLike_ that is suitable for storing in a Temporal.ZonedDateTime's [[TimeZone]] internal slot, and returns that value if found or throws an exception if not.</dd>
</dl>
<emu-alg>
1. Assert: No non-String, non-Object value, when coerced to a String, is an available time zone identifier.
1. If Type(_temporalTimeZoneLike_) is Object, then
1. If _temporalTimeZoneLike_ has an [[InitializedTemporalZonedDateTime]] internal slot, then
1. Return _temporalTimeZoneLike_.[[TimeZone]].
1. If ? ObjectImplementsTemporalTimeZoneProtocol(_temporalTimeZoneLike_) is *false*, throw a *TypeError* exception.
1. Return _temporalTimeZoneLike_.
1. Let _identifier_ be ? ToString(_temporalTimeZoneLike_).
1. Let _parseResult_ be ? ParseTemporalTimeZoneString(_identifier_).
1. If _temporalTimeZoneLike_ is not a String, throw a *TypeError* exception.
1. Let _parseResult_ be ? ParseTemporalTimeZoneString(_temporalTimeZoneLike_).
1. If _parseResult_.[[Name]] is not *undefined*, then
1. Let _name_ be _parseResult_.[[Name]].
1. If IsTimeZoneOffsetString(_name_) is *true*, return CanonicalizeTimeZoneOffsetString(_name_).
Expand Down
4 changes: 2 additions & 2 deletions spec/zoneddatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -1169,8 +1169,8 @@ <h1>
1. Let _offsetOption_ be ? ToTemporalOffset(_options_, *"reject"*).
1. Let _result_ be ? InterpretTemporalDateTimeFields(_calendar_, _fields_, _options_).
1. Else,
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalZonedDateTimeString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalZonedDateTimeString(_item_).
1. Let _timeZoneName_ be _result_.[[TimeZone]].[[Name]].
1. Assert: _timeZoneName_ is not *undefined*.
1. Let _timeZone_ be ? ToTemporalTimeZoneSlotValue(_timeZoneName_).
Expand Down

0 comments on commit 5beac8b

Please sign in to comment.