From 3720f4bd8fea7ff92c1e721a89b3682ca79eb07c Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Mon, 27 Jun 2022 23:18:10 +0100 Subject: [PATCH] LibJS: Fix production for time zone name in time zone strings This is a normative change in the Temporal spec. See: - https://github.com/tc39/proposal-temporal/commit/caf013a - https://github.com/tc39/proposal-temporal/commit/fb7cfa6 --- .../Runtime/Temporal/AbstractOperations.cpp | 4 ++-- .../Libraries/LibJS/Runtime/Temporal/ISO8601.cpp | 12 +++--------- .../Libraries/LibJS/Runtime/Temporal/ISO8601.h | 2 +- .../LibJS/Runtime/Temporal/TimeZone.cpp | 16 ++-------------- .../builtins/Temporal/TimeZone/TimeZone.from.js | 6 +++--- 5 files changed, 11 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp index f57717f354..2b72ac541a 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp @@ -1695,10 +1695,10 @@ ThrowCompletionOr parse_temporal_time_zone_string(GlobalObject if (!parse_result.has_value()) return vm.throw_completion(global_object, ErrorType::TemporalInvalidTimeZoneString, iso_string); - // 3. Let each of z, offsetString, and name be the source text matched by the respective UTCDesignator, TimeZoneNumericUTCOffset, and TimeZoneIANAName Parse Node contained within parseResult, or an empty sequence of code points if not present. + // 3. Let each of z, offsetString, and name be the source text matched by the respective UTCDesignator, TimeZoneNumericUTCOffset, and TimeZoneIdentifier Parse Nodes contained within parseResult, or an empty sequence of code points if not present. auto z = parse_result->utc_designator; auto offset_string = parse_result->time_zone_numeric_utc_offset; - auto name = parse_result->time_zone_iana_name; + auto name = parse_result->time_zone_identifier; // 4. If name is empty, then // a. Set name to undefined. diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.cpp index a83965ad25..de2746ee04 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.cpp @@ -935,15 +935,8 @@ bool ISO8601Parser::parse_time_zone_iana_name() transaction.commit(); return true; }; - StateTransaction transaction { *this }; - if (parse_etc_gmt_with_offset()) { - // no-op. - } else if (!parse_time_zone_iana_name_tail()) { - return false; - } - m_state.parse_result.time_zone_iana_name = transaction.parsed_string_view(); - transaction.commit(); - return true; + return parse_etc_gmt_with_offset() + || parse_time_zone_iana_name_tail(); } // https://tc39.es/proposal-temporal/#prod-TimeZoneIdentifier @@ -958,6 +951,7 @@ bool ISO8601Parser::parse_time_zone_identifier() } else if (!parse_time_zone_utc_offset_name()) { return false; } + m_state.parse_result.time_zone_identifier = transaction.parsed_string_view(); transaction.commit(); return true; } diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.h b/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.h index b875f22edf..8f49063878 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.h +++ b/Userland/Libraries/LibJS/Runtime/Temporal/ISO8601.h @@ -37,7 +37,7 @@ struct ParseResult { Optional time_zone_utc_offset_minute; Optional time_zone_utc_offset_second; Optional time_zone_utc_offset_fraction; - Optional time_zone_iana_name; + Optional time_zone_identifier; Optional duration_years; Optional duration_months; Optional duration_weeks; diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/TimeZone.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/TimeZone.cpp index e228803f01..9de2f86a61 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/TimeZone.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/TimeZone.cpp @@ -449,25 +449,13 @@ ThrowCompletionOr to_temporal_time_zone(GlobalObject& global_object, Va // 3. Let parseResult be ? ParseTemporalTimeZoneString(identifier). auto parse_result = TRY(parse_temporal_time_zone_string(global_object, identifier)); - // TODO: This currently cannot be tested as ParseTemporalTimeZoneString only considers - // TimeZoneIANAName for the returned [[Name]] slot, not TimeZoneUTCOffsetName. - // So when we provide a numeric time zone offset, this branch won't be executed, - // and if we provide an IANA name, it won't be a valid TimeZoneNumericUTCOffset. - // This should be fixed by: https://github.com/tc39/proposal-temporal/pull/2200 - // 4. If parseResult.[[Name]] is not undefined, then if (parse_result.name.has_value()) { // a. Let name be parseResult.[[Name]]. auto name = parse_result.name.release_value(); - // b. If ParseText(StringToCodePoints(name, TimeZoneNumericUTCOffset)) is not a List of errors, then - if (is_valid_time_zone_numeric_utc_offset_syntax(name)) { - // i. If parseResult.[[OffsetString]] is not undefined, and ! ParseTimeZoneOffsetString(parseResult.[[OffsetString]]) ≠ ! ParseTimeZoneOffsetString(name), throw a RangeError exception. - if (parse_result.offset_string.has_value() && (MUST(parse_time_zone_offset_string(global_object, *parse_result.offset_string)) != MUST(parse_time_zone_offset_string(global_object, name)))) - return vm.throw_completion(global_object, ErrorType::TemporalTimeZoneOffsetStringMismatch, *parse_result.offset_string, name); - } - // c. Else, - else { + // b. If ParseText(StringToCodePoints(name, TimeZoneNumericUTCOffset)) is a List of errors, then + if (!is_valid_time_zone_numeric_utc_offset_syntax(name)) { // i. If IsValidTimeZoneName(name) is false, throw a RangeError exception. if (!is_valid_time_zone_name(name)) return vm.throw_completion(global_object, ErrorType::TemporalInvalidTimeZoneName, name); diff --git a/Userland/Libraries/LibJS/Tests/builtins/Temporal/TimeZone/TimeZone.from.js b/Userland/Libraries/LibJS/Tests/builtins/Temporal/TimeZone/TimeZone.from.js index 9ac8382fa2..7b071d0b52 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Temporal/TimeZone/TimeZone.from.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Temporal/TimeZone/TimeZone.from.js @@ -25,18 +25,18 @@ describe("normal behavior", () => { ["Europe/London", "Europe/London"], ["Europe/Isle_of_Man", "Europe/London"], ["1970-01-01+01", "+01:00"], - ["1970-01-01+01[-12:34]", "+01:00"], + ["1970-01-01+01[-12:34]", "-12:34"], ["1970-01-01T00:00:00+01", "+01:00"], ["1970-01-01T00:00:00.000000000+01", "+01:00"], ["1970-01-01T00:00:00.000000000+01:00:00", "+01:00"], ["1970-01-01+12:34", "+12:34"], ["1970-01-01+12:34:56", "+12:34:56"], ["1970-01-01+12:34:56.789", "+12:34:56.789"], - ["1970-01-01+12:34:56.789[-01:00]", "+12:34:56.789"], + ["1970-01-01+12:34:56.789[-01:00]", "-01:00"], ["1970-01-01-12:34", "-12:34"], ["1970-01-01-12:34:56", "-12:34:56"], ["1970-01-01-12:34:56.789", "-12:34:56.789"], - ["1970-01-01-12:34:56.789[+01:00]", "-12:34:56.789"], + ["1970-01-01-12:34:56.789[+01:00]", "+01:00"], ]; for (const [arg, expected] of values) { expect(Temporal.TimeZone.from(arg).id).toBe(expected);