Don't coerce non-String inputs to strings

This commit stops converting non-string inputs to strings when
parsing ISO strings or property bag fields.

When numbers are coerced to strings, the result is sometimes a valid
ISO string subset but it often doesn't behave as expected. 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, or msecs like Date.p.getTime().

This commit also removes coercing objects to strings in the
same contexts (like TimeZone and Calendar constructors) because
it's unclear that there are use cases for this coercion.

Author: justingrant

polyfill/lib/calendar.mjs

@@ -41,20 +41,14 @@ const impl = {};
 
 export class Calendar {
   constructor(id) {
-    // Note: if the argument is not passed, IsBuiltinCalendar("undefined") will fail. This check
-    //       exists only to improve the error message.
-    if (arguments.length < 1) {
-      throw new RangeError('missing argument: id is required');
-    }
-
-    id = ES.ToString(id);
-    if (!ES.IsBuiltinCalendar(id)) throw new RangeError(`invalid calendar identifier ${id}`);
+    const stringId = ES.RequireString(id);
+    if (!ES.IsBuiltinCalendar(stringId)) throw new RangeError(`invalid calendar identifier ${stringId}`);
     CreateSlots(this);
-    SetSlot(this, CALENDAR_ID, ES.ASCIILowercase(id));
+    SetSlot(this, CALENDAR_ID, ES.ASCIILowercase(stringId));
 
     if (typeof __debug__ !== 'undefined' && __debug__) {
       Object.defineProperty(this, '_repr_', {
-        value: `${this[Symbol.toStringTag]} <${id}>`,
+        value: `${this[Symbol.toStringTag]} <${stringId}>`,
         writable: false,
         enumerable: false,
         configurable: false

polyfill/lib/ecmascript.mjs

@@ -18,6 +18,7 @@ const ObjectCreate = Object.create;
 const ObjectDefineProperty = Object.defineProperty;
 const ObjectEntries = Object.entries;
 const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
+const StringCtor = String;
 const StringFromCharCode = String.fromCharCode;
 const StringPrototypeCharCodeAt = String.prototype.charCodeAt;
 const StringPrototypeMatchAll = String.prototype.matchAll;
@@ -146,10 +147,27 @@ export function ToIntegerIfIntegral(value) {
   return number;
 }
 
+// This convenience function isn't in the spec, but is useful in the polyfill
+// for DRY and better error messages.
+export function RequireString(value) {
+  if (Type(value) !== 'String') {
+    // Use String() to ensure that Symbols won't throw
+    throw new TypeError(`expected a string, not ${StringCtor(value)}`);
+  }
+  return value;
+}
+
+// This function is an enum in the spec, but it's helpful to make it a
+// function in the polyfill.
+function ToPrimitiveAndRequireString(value) {
+  value = ToPrimitive(value, StringCtor);
+  return RequireString(value);
+}
+
 const BUILTIN_CASTS = new Map([
   ['year', ToIntegerWithTruncation],
   ['month', ToPositiveIntegerWithTruncation],
-  ['monthCode', ToString],
+  ['monthCode', ToPrimitiveAndRequireString],
   ['day', ToPositiveIntegerWithTruncation],
   ['hour', ToIntegerWithTruncation],
   ['minute', ToIntegerWithTruncation],
@@ -167,9 +185,9 @@ const BUILTIN_CASTS = new Map([
   ['milliseconds', ToIntegerIfIntegral],
   ['microseconds', ToIntegerIfIntegral],
   ['nanoseconds', ToIntegerIfIntegral],
-  ['era', ToString],
+  ['era', ToPrimitiveAndRequireString],
   ['eraYear', ToIntegerOrInfinity],
-  ['offset', ToString]
+  ['offset', ToPrimitiveAndRequireString]
 ]);
 
 const BUILTIN_DEFAULTS = new Map([
@@ -698,7 +716,7 @@ export function RegulateISOYearMonth(year, month, overflow) {
 
 export function ToTemporalDurationRecord(item) {
   if (Type(item) !== 'Object') {
-    return ParseTemporalDurationString(ToString(item));
+    return ParseTemporalDurationString(RequireString(item));
   }
   if (IsTemporalDuration(item)) {
     return {
@@ -980,7 +998,7 @@ export function ToRelativeTemporalObject(options) {
   } else {
     let tzName, z;
     ({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, calendar, tzName, offset, z } =
-      ParseISODateTime(ToString(relativeTo)));
+      ParseISODateTime(RequireString(relativeTo)));
     if (tzName) {
       timeZone = ToTemporalTimeZoneSlotValue(tzName);
       if (z) {
@@ -1128,7 +1146,7 @@ export function ToTemporalDate(item, options) {
     return CalendarDateFromFields(calendar, fields, options);
   }
   ToTemporalOverflow(options); // validate and ignore
-  let { year, month, day, calendar, z } = ParseTemporalDateString(ToString(item));
+  let { year, month, day, calendar, z } = ParseTemporalDateString(RequireString(item));
   if (z) throw new RangeError('Z designator not supported for PlainDate');
   if (!calendar) calendar = 'iso8601';
   if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
@@ -1202,7 +1220,7 @@ export function ToTemporalDateTime(item, options) {
     ToTemporalOverflow(options); // validate and ignore
     let z;
     ({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, calendar, z } =
-      ParseTemporalDateTimeString(ToString(item)));
+      ParseTemporalDateTimeString(RequireString(item)));
     if (z) throw new RangeError('Z designator not supported for PlainDateTime');
     RejectDateTime(year, month, day, hour, minute, second, millisecond, microsecond, nanosecond);
     if (!calendar) calendar = 'iso8601';
@@ -1237,7 +1255,8 @@ export function ToTemporalInstant(item) {
     const TemporalInstant = GetIntrinsic('%Temporal.Instant%');
     return new TemporalInstant(GetSlot(item, EPOCHNANOSECONDS));
   }
-  const ns = ParseTemporalInstant(ToString(item));
+  item = ToPrimitive(item, StringCtor);
+  const ns = ParseTemporalInstant(RequireString(item));
   const TemporalInstant = GetIntrinsic('%Temporal.Instant%');
   return new TemporalInstant(ns);
 }
@@ -1267,7 +1286,7 @@ export function ToTemporalMonthDay(item, options) {
   }
 
   ToTemporalOverflow(options); // validate and ignore
-  let { month, day, referenceISOYear, calendar } = ParseTemporalMonthDayString(ToString(item));
+  let { month, day, referenceISOYear, calendar } = ParseTemporalMonthDayString(RequireString(item));
   if (calendar === undefined) calendar = 'iso8601';
   if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
   calendar = ASCIILowercase(calendar);
@@ -1309,7 +1328,7 @@ export function ToTemporalTime(item, overflow = 'constrain') {
       overflow
     ));
   } else {
-    ({ hour, minute, second, millisecond, microsecond, nanosecond } = ParseTemporalTimeString(ToString(item)));
+    ({ hour, minute, second, millisecond, microsecond, nanosecond } = ParseTemporalTimeString(RequireString(item)));
     RejectTime(hour, minute, second, millisecond, microsecond, nanosecond);
   }
   const TemporalPlainTime = GetIntrinsic('%Temporal.PlainTime%');
@@ -1326,7 +1345,7 @@ export function ToTemporalYearMonth(item, options) {
   }
 
   ToTemporalOverflow(options); // validate and ignore
-  let { year, month, referenceISODay, calendar } = ParseTemporalYearMonthString(ToString(item));
+  let { year, month, referenceISODay, calendar } = ParseTemporalYearMonthString(RequireString(item));
   if (calendar === undefined) calendar = 'iso8601';
   if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
   calendar = ASCIILowercase(calendar);
@@ -1447,7 +1466,7 @@ export function ToTemporalZonedDateTime(item, options) {
   } else {
     let tzName, z;
     ({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, tzName, offset, z, calendar } =
-      ParseTemporalZonedDateTimeString(ToString(item)));
+      ParseTemporalZonedDateTimeString(RequireString(item)));
     timeZone = ToTemporalTimeZoneSlotValue(tzName);
     if (z) {
       offsetBehaviour = 'exact';
@@ -1973,7 +1992,7 @@ export function ToTemporalCalendarSlotValue(calendarLike) {
     }
     return calendarLike;
   }
-  const identifier = ToString(calendarLike);
+  const identifier = RequireString(calendarLike);
   if (IsBuiltinCalendar(identifier)) return ASCIILowercase(identifier);
   let calendar;
   try {
@@ -2092,7 +2111,7 @@ export function ToTemporalTimeZoneSlotValue(temporalTimeZoneLike) {
     }
     return temporalTimeZoneLike;
   }
-  const identifier = ToString(temporalTimeZoneLike);
+  const identifier = RequireString(temporalTimeZoneLike);
   const { tzName, offset, z } = ParseTemporalTimeZoneString(identifier);
   if (tzName) {
     // tzName is any valid identifier string in brackets, and could be an offset identifier

polyfill/lib/timezone.mjs

@@ -21,12 +21,7 @@ import {
 
 export class TimeZone {
   constructor(identifier) {
-    // Note: if the argument is not passed, GetCanonicalTimeZoneIdentifier(undefined) will throw.
-    //       This check exists only to improve the error message.
-    if (arguments.length < 1) {
-      throw new RangeError('missing argument: identifier is required');
-    }
-    let stringIdentifier = ES.ToString(identifier);
+    let stringIdentifier = ES.RequireString(identifier);
     const parseResult = ES.ParseTimeZoneIdentifier(identifier);
     if (parseResult.offsetNanoseconds !== undefined) {
       stringIdentifier = ES.FormatOffsetTimeZoneIdentifier(parseResult.offsetNanoseconds);

polyfill/test262

@@ -567,16 +567,21 @@ <h1>
     </emu-table>
   </emu-clause>
 
-  <emu-clause id="sec-temporal-torelativetemporalobject" aoid="ToRelativeTemporalObject">
-    <h1>ToRelativeTemporalObject ( _options_ )</h1>
-    <p>
-      The abstract operation ToRelativeTemporalObject examines the value of the `relativeTo` property of its _options_ argument.
-      If this is not present, it returns *undefined*.
-      Otherwise, it attempts to return a Temporal.ZonedDateTime instance or Temporal.PlainDate instance, in order of preference, by converting the value.
-      If neither of those are possible, the operation throws a *RangeError*.
-    </p>
+  <emu-clause id="sec-temporal-torelativetemporalobject" type="abstract operation">
+    <h1>ToRelativeTemporalObject (
+      _options_: an Object
+    ): either a normal completion containing either a Temporal.ZonedDateTime object or a Temporal.PlainDate object, or a throw completion</h1>
+    <dl class="header">
+      <dt>description</dt>
+      <dd>
+        It examines the value of the `relativeTo` property of its _options_ argument.
+        If the value is *undefined*, it returns *undefined*.
+        If the value is not a String or an Object, it throws a *TypeError*.
+        Otherwise, it attempts to return a Temporal.ZonedDateTime instance or Temporal.PlainDate instance, in order of preference, by converting the value.
+        If neither of those are possible, it throws a *RangeError*.
+      </dd>
+    </dl>
     <emu-alg>
-      1. Assert: Type(_options_) is Object.
       1. Let _value_ be ? Get(_options_, *"relativeTo"*).
       1. If _value_ is *undefined*, return *undefined*.
       1. Let _offsetBehaviour_ be ~option~.
@@ -601,8 +606,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
@@ -1825,8 +1830,10 @@ <h1>
               1. Set _value_ to ? ToPositiveIntegerWithTruncation(_value_).
               1. Set _value_ to 𝔽(_value_).
             1. Else,
-              1. Assert: _Conversion_ is ~ToString~.
-              1. Set _value_ to ? ToString(_value_).
+              1. Assert: _Conversion_ is ~ToPrimitiveAndRequireString~.
+              1. NOTE: Non-primitive values are supported here for consistency with other fields, but such values must coerce to Strings.
+              1. Set _value_ to ? ToPrimitive(_value_, ~string~).
+              1. If _value_ is not a String, throw a *TypeError* exception.
           1. Perform ! CreateDataPropertyOrThrow(_result_, _property_, _value_).
         1. Else if _requiredFields_ is a List, then
           1. If _requiredFields_ contains _property_, then
@@ -1861,7 +1868,7 @@ <h1>
           </tr>
           <tr>
             <td>*"monthCode"*</td>
-            <td>~ToString~</td>
+            <td>~ToPrimitiveAndRequireString~</td>
             <td>*undefined*</td>
           </tr>
           <tr>
@@ -1901,12 +1908,12 @@ <h1>
           </tr>
           <tr>
             <td>*"offset"*</td>
-            <td>~ToString~</td>
+            <td>~ToPrimitiveAndRequireString~</td>
             <td>*undefined*</td>
           </tr>
           <tr>
             <td>*"era"*</td>
-            <td>~ToString~</td>
+            <td>~ToPrimitiveAndRequireString~</td>
             <td>*undefined*</td>
           </tr>
           <tr>

spec/abstractops.html

@@ -525,8 +525,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>
@@ -1020,7 +1020,7 @@ <h1>Temporal.Calendar ( _id_ )</h1>
       <emu-alg>
         1. If NewTarget is *undefined*, then
           1. Throw a *TypeError* exception.
-        1. Set _id_ to ? ToString(_id_).
+        1. If _id_ is not a String, throw a *TypeError* exception.
         1. If IsBuiltinCalendar(_id_) is *false*, then
           1. Throw a *RangeError* exception.
         1. Return ? CreateTemporalCalendar(_id_, NewTarget).

spec/calendar.html

@@ -912,8 +912,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.

spec/duration.html

@@ -518,8 +518,10 @@ <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. NOTE: This use of ToPrimitive allows Instant-like objects to be converted.
+          1. Set _item_ to ? ToPrimitive(_item_, ~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>

spec/instant.html

@@ -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"*.

spec/plaindate.html

@@ -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]].