From a283daaab78a46ab4f8dc19ffbfd0ba8cbe76de2 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 30 Jan 2023 10:36:47 -0500 Subject: [PATCH] LibJS: Refactor the Intl.NumberFormat GetStringOrBooleanOption AO This is an editorial change in the Intl.NumberFormat V3 spec. See: https://github.com/tc39/proposal-intl-numberformat-v3/commit/654bfad --- .../LibJS/Runtime/Intl/AbstractOperations.cpp | 32 +++++++------------ .../LibJS/Runtime/Intl/AbstractOperations.h | 6 ++-- .../Runtime/Intl/NumberFormatConstructor.cpp | 25 +++++++++++---- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.cpp index 3fb00376a8..97ce07651e 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.cpp @@ -608,8 +608,8 @@ ThrowCompletionOr coerce_options_to_object(VM& vm, Value options) // NOTE: 9.2.13 GetOption has been removed and is being pulled in from ECMA-262 in the Temporal proposal. -// 1.2.12 GetStringOrBooleanOption ( options, property, values, trueValue, falsyValue, fallback ), https://tc39.es/proposal-intl-numberformat-v3/out/negotiation/proposed.html#sec-getstringorbooleanoption -ThrowCompletionOr get_string_or_boolean_option(VM& vm, Object const& options, PropertyKey const& property, Span values, StringOrBoolean true_value, StringOrBoolean falsy_value, StringOrBoolean fallback) +// 1.2.14 GetBooleanOrStringNumberFormatOption ( options, property, stringValues, fallback ), https://tc39.es/proposal-intl-numberformat-v3/out/negotiation/proposed.html#sec-getbooleanorstringnumberformatoption +ThrowCompletionOr get_boolean_or_string_number_format_option(VM& vm, Object const& options, PropertyKey const& property, Span string_values, StringOrBoolean fallback) { // 1. Let value be ? Get(options, property). auto value = TRY(options.get(property)); @@ -618,31 +618,23 @@ ThrowCompletionOr get_string_or_boolean_option(VM& vm, Object c if (value.is_undefined()) return fallback; - // 3. If value is true, return trueValue. + // 3. If value is true, return true. if (value.is_boolean() && value.as_bool()) - return true_value; + return StringOrBoolean { true }; - // 4. Let valueBoolean be ToBoolean(value). - auto value_boolean = value.to_boolean(); + // 4. If ToBoolean(value) is false, return false. + if (!value.to_boolean()) + return StringOrBoolean { false }; - // 5. If valueBoolean is false, return falsyValue. - if (!value_boolean) - return falsy_value; - - // 6. Let value be ? ToString(value). + // 5. Let value be ? ToString(value). auto value_string = TRY(value.to_string(vm)); - // 7. NOTE: For historical reasons, the strings "true" and "false" are treated the same as the boolean value true. - // 8. If value is "true" or "false", return fallback. - if (value_string.is_one_of("true"sv, "false"sv)) - return fallback; - - // 9. If values does not contain an element equal to value, throw a RangeError exception. - auto it = find(values.begin(), values.end(), value_string.bytes_as_string_view()); - if (it == values.end()) + // 6. If stringValues does not contain value, throw a RangeError exception. + auto it = find(string_values.begin(), string_values.end(), value_string.bytes_as_string_view()); + if (it == string_values.end()) return vm.throw_completion(ErrorType::OptionIsNotValidValue, value_string, property.as_string()); - // 10. Return value. + // 7. Return value. return StringOrBoolean { *it }; } diff --git a/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.h b/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.h index 9334fe10b4..2f13e1a01d 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.h +++ b/Userland/Libraries/LibJS/Runtime/Intl/AbstractOperations.h @@ -89,15 +89,15 @@ ThrowCompletionOr insert_unicode_extension_and_canonicalize(VM&, ::Local ThrowCompletionOr resolve_locale(VM&, Vector const& requested_locales, LocaleOptions const& options, Span relevant_extension_keys); ThrowCompletionOr supported_locales(VM&, Vector const& requested_locales, Value options); ThrowCompletionOr coerce_options_to_object(VM&, Value options); -ThrowCompletionOr get_string_or_boolean_option(VM&, Object const& options, PropertyKey const& property, Span values, StringOrBoolean true_value, StringOrBoolean falsy_value, StringOrBoolean fallback); +ThrowCompletionOr get_boolean_or_string_number_format_option(VM& vm, Object const& options, PropertyKey const& property, Span string_values, StringOrBoolean fallback); ThrowCompletionOr> default_number_option(VM&, Value value, int minimum, int maximum, Optional fallback); ThrowCompletionOr> get_number_option(VM&, Object const& options, PropertyKey const& property, int minimum, int maximum, Optional fallback); ThrowCompletionOr> partition_pattern(VM&, StringView pattern); template -ThrowCompletionOr get_string_or_boolean_option(VM& vm, Object const& options, PropertyKey const& property, StringView const (&values)[Size], StringOrBoolean true_value, StringOrBoolean falsy_value, StringOrBoolean fallback) +ThrowCompletionOr get_boolean_or_string_number_format_option(VM& vm, Object const& options, PropertyKey const& property, StringView const (&string_values)[Size], StringOrBoolean fallback) { - return get_string_or_boolean_option(vm, options, property, Span { values }, move(true_value), move(falsy_value), move(fallback)); + return get_boolean_or_string_number_format_option(vm, options, property, Span { string_values }, move(fallback)); } // NOTE: ECMA-402's GetOption is being removed in favor of a shared ECMA-262 GetOption in the Temporal proposal. diff --git a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormatConstructor.cpp b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormatConstructor.cpp index 2e5943afd0..d253e52391 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormatConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormatConstructor.cpp @@ -185,19 +185,32 @@ ThrowCompletionOr initialize_number_format(VM& vm, NumberFormat& default_use_grouping = "min2"sv; } - // 24. Let useGrouping be ? GetStringOrBooleanOption(options, "useGrouping", « "min2", "auto", "always" », "always", false, defaultUseGrouping). - auto use_grouping = TRY(get_string_or_boolean_option(vm, *options, vm.names.useGrouping, { "min2"sv, "auto"sv, "always"sv }, "always"sv, false, default_use_grouping)); + // 24. NOTE: For historical reasons, the strings "true" and "false" are accepted and replaced with the default value. + // 25. Let useGrouping be ? GetBooleanOrStringNumberFormatOption(options, "useGrouping", « "min2", "auto", "always", "true", "false" », defaultUseGrouping). + auto use_grouping = TRY(get_boolean_or_string_number_format_option(vm, *options, vm.names.useGrouping, { "min2"sv, "auto"sv, "always"sv, "true"sv, "false"sv }, default_use_grouping)); - // 25. Set numberFormat.[[UseGrouping]] to useGrouping. + // 26. If useGrouping is "true" or useGrouping is "false", set useGrouping to defaultUseGrouping. + if (auto const* use_grouping_string = use_grouping.get_pointer()) { + if (use_grouping_string->is_one_of("true"sv, "false"sv)) + use_grouping = default_use_grouping; + } + + // 27. If useGrouping is true, set useGrouping to "always". + if (auto const* use_grouping_boolean = use_grouping.get_pointer()) { + if (*use_grouping_boolean) + use_grouping = "always"sv; + } + + // 28. Set numberFormat.[[UseGrouping]] to useGrouping. number_format.set_use_grouping(use_grouping); - // 26. Let signDisplay be ? GetOption(options, "signDisplay", string, « "auto", "never", "always", "exceptZero, "negative" », "auto"). + // 29. Let signDisplay be ? GetOption(options, "signDisplay", string, « "auto", "never", "always", "exceptZero, "negative" », "auto"). auto sign_display = TRY(get_option(vm, *options, vm.names.signDisplay, OptionType::String, { "auto"sv, "never"sv, "always"sv, "exceptZero"sv, "negative"sv }, "auto"sv)); - // 27. Set numberFormat.[[SignDisplay]] to signDisplay. + // 30. Set numberFormat.[[SignDisplay]] to signDisplay. number_format.set_sign_display(TRY(sign_display.as_string().utf8_string_view())); - // 28. Return numberFormat. + // 31. Return numberFormat. return &number_format; }