From 662354b78a4ee8f4cf449c47c46fcaa6ca473847 Mon Sep 17 00:00:00 2001 From: Daniel Lindenkreuz Date: Thu, 14 Nov 2024 17:18:45 -0800 Subject: [PATCH] fix NumberFormat.resolvedOptions on Android (#1565) Summary: A typo currently leads to wrong results for `NumberFormat.resolvedOptions()`: `minimumSignificantDigits` and `maximumSignificantDigits` are interchanged. This PR fixes that and adds tests. Pull Request resolved: https://github.com/facebook/hermes/pull/1565 Test Plan: Added unit tests. Reviewed By: dannysu Differential Revision: D65822468 Pulled By: neildhar fbshipit-source-id: bb5ba5d6257b9c6d7257fb9494b988b175855d77 --- .../hermes/test/HermesIntlAndroidTest.java | 12 ++++ .../facebook/hermes/intl/NumberFormat.java | 4 +- .../intl/number-format-significant-digits.js | 65 +++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 test/hermes/intl/number-format-significant-digits.js diff --git a/android/intltest/java/com/facebook/hermes/test/HermesIntlAndroidTest.java b/android/intltest/java/com/facebook/hermes/test/HermesIntlAndroidTest.java index 03637ba89e9..8b1a89bc0ae 100644 --- a/android/intltest/java/com/facebook/hermes/test/HermesIntlAndroidTest.java +++ b/android/intltest/java/com/facebook/hermes/test/HermesIntlAndroidTest.java @@ -48,6 +48,18 @@ public void testNumberFormatFractionDigitsFromAsset() throws IOException { } } + @Test + public void testNumberFormatSignificantDigitsFromAsset() throws IOException { + AssetManager assets = + InstrumentationRegistry.getInstrumentation().getTargetContext().getAssets(); + InputStream is = assets.open("number-format-significant-digits.js"); + String script = + new BufferedReader(new InputStreamReader(is)).lines().collect(Collectors.joining("\n")); + try (JSRuntime rt = JSRuntime.makeHermesRuntime()) { + rt.evaluateJavaScript(script); + } + } + @Test public void testDateTimeFormat() { try (JSRuntime rt = JSRuntime.makeHermesRuntime()) { diff --git a/lib/Platform/Intl/java/com/facebook/hermes/intl/NumberFormat.java b/lib/Platform/Intl/java/com/facebook/hermes/intl/NumberFormat.java index c83ada557fc..4433c02ab1b 100644 --- a/lib/Platform/Intl/java/com/facebook/hermes/intl/NumberFormat.java +++ b/lib/Platform/Intl/java/com/facebook/hermes/intl/NumberFormat.java @@ -626,10 +626,10 @@ public Map resolvedOptions() throws JSRangeErrorException { if (mRoundingType == IPlatformNumberFormatter.RoundingType.SIGNIFICANT_DIGITS) { if (mResolvedMaximumSignificantDigits != -1) - finalResolvedOptions.put("minimumSignificantDigits", mResolvedMaximumSignificantDigits); + finalResolvedOptions.put("maximumSignificantDigits", mResolvedMaximumSignificantDigits); if (mResolvedMinimumSignificantDigits != -1) - finalResolvedOptions.put("maximumSignificantDigits", mResolvedMinimumSignificantDigits); + finalResolvedOptions.put("minimumSignificantDigits", mResolvedMinimumSignificantDigits); } else if (mRoundingType == IPlatformNumberFormatter.RoundingType.FRACTION_DIGITS) { diff --git a/test/hermes/intl/number-format-significant-digits.js b/test/hermes/intl/number-format-significant-digits.js new file mode 100644 index 00000000000..bf90124094c --- /dev/null +++ b/test/hermes/intl/number-format-significant-digits.js @@ -0,0 +1,65 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermes %s +// REQUIRES: intl + +function assert(pred, str) { + if (!pred) { + throw new Error('assertion failed' + (str === undefined ? '' : (': ' + str))); + } +} + +let resolvedOptions; + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD' }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === undefined); +assert(resolvedOptions.maximumSignificantDigits === undefined); + +// +// Validate minimumSignificantDigits logic +// +try { new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: -1 }) } +catch (e) { assert(e.message.includes('minimumSignificantDigits value is invalid.')) } + +try { new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: 22 }) } +catch (e) { assert(e.message.includes('minimumSignificantDigits value is invalid.')) } + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: 1 }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === 1); +assert(resolvedOptions.maximumSignificantDigits === 21); + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: 21 }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === 21); +assert(resolvedOptions.maximumSignificantDigits === 21); + +// +// Validate maximumSignificantDigits logic +// +try { new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', maximumSignificantDigits: -1 }) } +catch (e) { assert(e.message.includes('maximumSignificantDigits value is invalid.')) } + +try { new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', maximumSignificantDigits: 22 }) } +catch (e) { assert(e.message.includes('maximumSignificantDigits value is invalid.')) } + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', maximumSignificantDigits: 1 }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === 1); +assert(resolvedOptions.maximumSignificantDigits === 1); + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', maximumSignificantDigits: 21 }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === 1); +assert(resolvedOptions.maximumSignificantDigits === 21); + +// +// Validate when both are set +// +try { new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: 5, maximumSignificantDigits: 2 }) } +catch (e) { assert(e.message.includes('maximumSignificantDigits value is invalid.')) } + +resolvedOptions = new Intl.NumberFormat('en-US', {style: 'currency', currency: 'USD', minimumSignificantDigits: 3, maximumSignificantDigits: 5 }).resolvedOptions(); +assert(resolvedOptions.minimumSignificantDigits === 3); +assert(resolvedOptions.maximumSignificantDigits === 5);