From 63dbee9f666925b25e4470a1916277efaeb2297a Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Thu, 10 Aug 2023 22:09:16 -0700 Subject: [PATCH] fixup! Polyfill: Simplify time zone parsing --- polyfill/lib/ecmascript.mjs | 50 ++++++++++++++++++---------------- polyfill/test/validStrings.mjs | 13 +++++---- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/polyfill/lib/ecmascript.mjs b/polyfill/lib/ecmascript.mjs index 14cc299801..b53bfadf08 100644 --- a/polyfill/lib/ecmascript.mjs +++ b/polyfill/lib/ecmascript.mjs @@ -562,45 +562,49 @@ export function ParseTemporalMonthDayString(isoString) { const TIMEZONE_IDENTIFIER = new RegExp(`^${PARSE.timeZoneID.source}$`, 'i'); const OFFSET_IDENTIFIER = new RegExp(`^${PARSE.offsetIdentifier.source}$`); +function throwBadTimeZoneStringError(timeZoneString) { + // Offset identifiers only support minute precision, but offsets in ISO + // strings support nanosecond precision. If the identifier is invalid but + // it's a valid ISO offset, then it has sub-minute precision. Show a clearer + // error message in that case. + const msg = OFFSET.test(timeZoneString) ? 'Seconds not allowed in offset time zone' : 'Invalid time zone'; + throw new RangeError(`${msg}: ${timeZoneString}`); +} + export function ParseTimeZoneIdentifier(identifier) { if (!TIMEZONE_IDENTIFIER.test(identifier)) { - // Offset identifiers only support minute precision, but offsets in ISO - // strings support nanosecond precision. If the identifier is invalid but - // it's a valid ISO offset, then it has sub-minute precision. Show a clearer - // error message in that case. - const msg = OFFSET.test(identifier) ? 'Seconds not allowed in offset time zone' : 'Invalid time zone identifier'; - throw new RangeError(`${msg}: ${identifier}`); + throwBadTimeZoneStringError(identifier); } if (OFFSET_IDENTIFIER.test(identifier)) { - // The regex limits the input to minutes precision const offsetNanoseconds = ParseDateTimeUTCOffset(identifier); + // The regex limits the input to minutes precision, so we know that the + // division below will result in an integer. return { offsetMinutes: offsetNanoseconds / 60e9 }; } return { tzName: identifier }; } -// This operation is separated out of ParseTemporalTimeZoneStringIntoIdentifier -// for testability reasons. See comment above that function. -export function ParseTemporalTimeZoneString(stringIdent) { - const bareID = new RegExp(`^${PARSE.timeZoneID.source}$`, 'i'); - if (bareID.test(stringIdent)) return { tzAnnotation: stringIdent }; +// This operation doesn't exist in the spec, but in the polyfill it's split from +// ParseTemporalTimeZoneString so that parsing can be tested separately from the +// logic of converting parsed values into a named or offset identifier. +export function ParseTemporalTimeZoneStringRaw(timeZoneString) { + if (TIMEZONE_IDENTIFIER.test(timeZoneString)) { + return { tzAnnotation: timeZoneString, offset: undefined, z: false }; + } try { // Try parsing ISO string instead - const result = ParseISODateTime(stringIdent); - if (result.z || result.tzAnnotation || result.offset) { - return result; + const { tzAnnotation, offset, z } = ParseISODateTime(timeZoneString); + if (z || tzAnnotation || offset) { + return { tzAnnotation, offset, z }; } } catch { // fall through } - throw new RangeError(`Invalid time zone: ${stringIdent}`); + throwBadTimeZoneStringError(timeZoneString); } -// This operation is just called ParseTemporalTimeZoneString in the spec, but we -// separate it in order to be able to test only the text parsing. This function -// contains logic, while ParseTemporalTimeZoneString performs only the parsing. -export function ParseTemporalTimeZoneStringIntoIdentifier(stringIdent) { - const { tzAnnotation, offset, z } = ParseTemporalTimeZoneString(stringIdent); +export function ParseTemporalTimeZoneString(stringIdent) { + const { tzAnnotation, offset, z } = ParseTemporalTimeZoneStringRaw(stringIdent); if (tzAnnotation) return ParseTimeZoneIdentifier(tzAnnotation); if (z) return ParseTimeZoneIdentifier('UTC'); if (offset) return ParseTimeZoneIdentifier(offset); @@ -2157,9 +2161,9 @@ export function ToTemporalTimeZoneSlotValue(temporalTimeZoneLike) { } return temporalTimeZoneLike; } - const identifier = RequireString(temporalTimeZoneLike); + const timeZoneString = RequireString(temporalTimeZoneLike); - const { tzName, offsetMinutes } = ParseTemporalTimeZoneStringIntoIdentifier(identifier); + const { tzName, offsetMinutes } = ParseTemporalTimeZoneString(timeZoneString); if (offsetMinutes !== undefined) { return FormatOffsetTimeZoneIdentifier(offsetMinutes); } diff --git a/polyfill/test/validStrings.mjs b/polyfill/test/validStrings.mjs index 3c446a413c..1e4fd16db9 100644 --- a/polyfill/test/validStrings.mjs +++ b/polyfill/test/validStrings.mjs @@ -203,7 +203,7 @@ const timeDesignator = character('Tt'); const weeksDesignator = character('Ww'); const yearsDesignator = character('Yy'); const utcDesignator = withCode(character('Zz'), (data) => { - data.z = 'Z'; + data.z = true; }); const annotationCriticalFlag = character('!'); const fraction = seq(decimalSeparator, between(1, 9, digit())); @@ -426,7 +426,7 @@ const goals = { const dateItems = ['year', 'month', 'day']; const timeItems = ['hour', 'minute', 'second', 'millisecond', 'microsecond', 'nanosecond']; const comparisonItems = { - Instant: [...dateItems, ...timeItems, 'offset'], + Instant: [...dateItems, ...timeItems, 'offset', 'z'], Date: [...dateItems, 'calendar'], DateTime: [...dateItems, ...timeItems, 'calendar'], Duration: [ @@ -443,9 +443,9 @@ const comparisonItems = { ], MonthDay: ['month', 'day', 'calendar'], Time: [...timeItems], - TimeZone: ['offset', 'tzAnnotation'], + TimeZone: ['offset', 'tzAnnotation', 'z'], YearMonth: ['year', 'month', 'calendar'], - ZonedDateTime: [...dateItems, ...timeItems, 'offset', 'tzAnnotation', 'calendar'] + ZonedDateTime: [...dateItems, ...timeItems, 'offset', 'z', 'tzAnnotation', 'calendar'] }; const plainModes = ['Date', 'DateTime', 'MonthDay', 'Time', 'YearMonth']; @@ -458,10 +458,11 @@ function fuzzMode(mode) { fuzzed = goals[mode].generate(generatedData); } while (plainModes.includes(mode) && /[0-9][zZ]/.test(fuzzed)); try { - const parsed = ES[`ParseTemporal${mode}String`](fuzzed); + const parsingMethod = ES[`ParseTemporal${mode}StringRaw`] ?? ES[`ParseTemporal${mode}String`]; + const parsed = parsingMethod(fuzzed); for (let prop of comparisonItems[mode]) { let expected = generatedData[prop]; - if (!['tzAnnotation', 'offset', 'calendar'].includes(prop)) expected ??= 0; + if (!['tzAnnotation', 'offset', 'calendar'].includes(prop)) expected ??= prop === 'z' ? false : 0; if (prop === 'offset') { const parsedResult = parsed[prop] === undefined ? undefined : ES.ParseDateTimeUTCOffset(parsed[prop]); assert.equal(parsedResult, expected, prop);