From b36dbedf348ab041a5aa43b51cb805d544e7cd10 Mon Sep 17 00:00:00 2001
From: Shane Carr <shane@unicode.org>
Date: Thu, 14 Feb 2019 20:20:20 -0800
Subject: [PATCH] ICU-20417 Adding parity between NoUnit and MeasureUnit
 Percent for short forms.

- Use Percent pattern for MeasureUnit Percent instead of the short or narrow form pattern from CLDR.
---
 icu4c/source/i18n/number_formatimpl.cpp       | 11 +++--
 icu4c/source/test/intltest/numbertest.h       |  1 +
 icu4c/source/test/intltest/numbertest_api.cpp | 42 ++++++++++++++++---
 .../intltest/numbertest_decimalquantity.cpp   |  5 ++-
 .../ibm/icu/number/NumberFormatterImpl.java   | 11 +++--
 .../dev/test/number/ExhaustiveNumberTest.java | 21 ++++++++++
 6 files changed, 76 insertions(+), 15 deletions(-)

diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp
index a7f162ecc63..08b833beb7a 100644
--- a/icu4c/source/i18n/number_formatimpl.cpp
+++ b/icu4c/source/i18n/number_formatimpl.cpp
@@ -172,9 +172,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
     // Pre-compute a few values for efficiency.
     bool isCurrency = utils::unitIsCurrency(macros.unit);
     bool isNoUnit = utils::unitIsNoUnit(macros.unit);
-    bool isPercent = isNoUnit && utils::unitIsPercent(macros.unit);
-    bool isPermille = isNoUnit && utils::unitIsPermille(macros.unit);
-    bool isCldrUnit = !isCurrency && !isNoUnit;
+    bool isPercent = utils::unitIsPercent(macros.unit);
+    bool isPermille = utils::unitIsPermille(macros.unit);
     bool isAccounting =
             macros.sign == UNUM_SIGN_ACCOUNTING || macros.sign == UNUM_SIGN_ACCOUNTING_ALWAYS ||
             macros.sign == UNUM_SIGN_ACCOUNTING_EXCEPT_ZERO;
@@ -194,6 +193,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
     if (macros.unitWidth != UNUM_UNIT_WIDTH_COUNT) {
         unitWidth = macros.unitWidth;
     }
+    bool isCldrUnit = !isCurrency && !isNoUnit &&
+        (unitWidth == UNUM_UNIT_WIDTH_FULL_NAME || !(isPercent || isPermille));
 
     // Select the numbering system.
     LocalPointer<const NumberingSystem> nsLocal;
@@ -243,7 +244,9 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
     }
     if (pattern == nullptr) {
         CldrPatternStyle patternStyle;
-        if (isPercent || isPermille) {
+        if (isCldrUnit) {
+            patternStyle = CLDR_PATTERN_STYLE_DECIMAL;
+        } else if (isPercent || isPermille) {
             patternStyle = CLDR_PATTERN_STYLE_PERCENT;
         } else if (!isCurrency || unitWidth == UNUM_UNIT_WIDTH_FULL_NAME) {
             patternStyle = CLDR_PATTERN_STYLE_DECIMAL;
diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h
index 35463612e0c..c07fe9e37ec 100644
--- a/icu4c/source/test/intltest/numbertest.h
+++ b/icu4c/source/test/intltest/numbertest.h
@@ -56,6 +56,7 @@ class NumberFormatterApiTest : public IntlTestWithFieldPosition {
     void unitCompoundMeasure();
     void unitCurrency();
     void unitPercent();
+    void percentParity();
     void roundingFraction();
     void roundingFigures();
     void roundingFractionFigures();
diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp
index 4e6f7f5d741..2d925eee5be 100644
--- a/icu4c/source/test/intltest/numbertest_api.cpp
+++ b/icu4c/source/test/intltest/numbertest_api.cpp
@@ -70,6 +70,10 @@ void NumberFormatterApiTest::runIndexedTest(int32_t index, UBool exec, const cha
         TESTCASE_AUTO(unitCompoundMeasure);
         TESTCASE_AUTO(unitCurrency);
         TESTCASE_AUTO(unitPercent);
+        if (!quick) {
+            // Slow test: run in exhaustive mode only
+            TESTCASE_AUTO(percentParity);
+        }
         TESTCASE_AUTO(roundingFraction);
         TESTCASE_AUTO(roundingFigures);
         TESTCASE_AUTO(roundingFractionFigures);
@@ -89,7 +93,11 @@ void NumberFormatterApiTest::runIndexedTest(int32_t index, UBool exec, const cha
         TESTCASE_AUTO(fieldPositionCoverage);
         TESTCASE_AUTO(toFormat);
         TESTCASE_AUTO(errors);
-        TESTCASE_AUTO(validRanges);
+        if (!quick) {
+            // Slow test: run in exhaustive mode only
+            // (somewhat slow to check all permutations of settings)
+            TESTCASE_AUTO(validRanges);
+        }
         TESTCASE_AUTO(copyMove);
         TESTCASE_AUTO(localPointerCAPI);
         TESTCASE_AUTO(toObject);
@@ -830,6 +838,33 @@ void NumberFormatterApiTest::unitPercent() {
             u"-98.765432%");
 }
 
+void NumberFormatterApiTest::percentParity() {
+    IcuTestErrorCode status(*this, "percentParity");
+    UnlocalizedNumberFormatter uNoUnitPercent = NumberFormatter::with().unit(NoUnit::percent());
+    UnlocalizedNumberFormatter uNoUnitPermille = NumberFormatter::with().unit(NoUnit::permille());
+    UnlocalizedNumberFormatter uMeasurePercent = NumberFormatter::with().unit(MeasureUnit::getPercent());
+    UnlocalizedNumberFormatter uMeasurePermille = NumberFormatter::with().unit(MeasureUnit::getPermille());
+
+    int32_t localeCount;
+    auto locales = Locale::getAvailableLocales(localeCount);
+    for (int32_t i=0; i<localeCount; i++) {
+        auto& locale = locales[i];
+        UnicodeString sNoUnitPercent = uNoUnitPercent.locale(locale)
+            .formatDouble(50, status).toString(status);
+        UnicodeString sNoUnitPermille = uNoUnitPermille.locale(locale)
+            .formatDouble(50, status).toString(status);
+        UnicodeString sMeasurePercent = uMeasurePercent.locale(locale)
+            .formatDouble(50, status).toString(status);
+        UnicodeString sMeasurePermille = uMeasurePermille.locale(locale)
+            .formatDouble(50, status).toString(status);
+
+        assertEquals(u"Percent, locale " + UnicodeString(locale.getName()),
+            sNoUnitPercent, sMeasurePercent);
+        assertEquals(u"Permille, locale " + UnicodeString(locale.getName()),
+            sNoUnitPermille, sMeasurePermille);
+    }
+}
+
 void NumberFormatterApiTest::roundingFraction() {
     assertFormatDescending(
             u"Integer",
@@ -2639,11 +2674,6 @@ void NumberFormatterApiTest::errors() {
 }
 
 void NumberFormatterApiTest::validRanges() {
-    if (quick) {
-        // Do not run this test except in exhaustive mode.
-        // (somewhat slow to check all permutations of settings)
-        return;
-    }
 
 #define EXPECTED_MAX_INT_FRAC_SIG 999
 
diff --git a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp
index 090398e8ef3..9e9b49be9a4 100644
--- a/icu4c/source/test/intltest/numbertest_decimalquantity.cpp
+++ b/icu4c/source/test/intltest/numbertest_decimalquantity.cpp
@@ -21,7 +21,10 @@ void DecimalQuantityTest::runIndexedTest(int32_t index, UBool exec, const char *
         TESTCASE_AUTO(testSwitchStorage);;
         TESTCASE_AUTO(testCopyMove);
         TESTCASE_AUTO(testAppend);
-        TESTCASE_AUTO(testConvertToAccurateDouble);
+        if (!quick) {
+            // Slow test: run in exhaustive mode only
+            TESTCASE_AUTO(testConvertToAccurateDouble);
+        }
         TESTCASE_AUTO(testUseApproximateDoubleWhenAble);
         TESTCASE_AUTO(testHardDoubleConversion);
         TESTCASE_AUTO(testToDouble);
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java
index b6b74f8e3ed..4adf9589d23 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java
@@ -185,9 +185,8 @@ class NumberFormatterImpl {
         // Pre-compute a few values for efficiency.
         boolean isCurrency = unitIsCurrency(macros.unit);
         boolean isNoUnit = unitIsNoUnit(macros.unit);
-        boolean isPercent = isNoUnit && unitIsPercent(macros.unit);
-        boolean isPermille = isNoUnit && unitIsPermille(macros.unit);
-        boolean isCldrUnit = !isCurrency && !isNoUnit;
+        boolean isPercent = unitIsPercent(macros.unit);
+        boolean isPermille = unitIsPermille(macros.unit);
         boolean isAccounting = macros.sign == SignDisplay.ACCOUNTING
                 || macros.sign == SignDisplay.ACCOUNTING_ALWAYS
                 || macros.sign == SignDisplay.ACCOUNTING_EXCEPT_ZERO;
@@ -196,6 +195,8 @@ class NumberFormatterImpl {
         if (macros.unitWidth != null) {
             unitWidth = macros.unitWidth;
         }
+        boolean isCldrUnit = !isCurrency && !isNoUnit &&
+            (unitWidth == UnitWidth.FULL_NAME || !(isPercent || isPermille));
         PluralRules rules = macros.rules;
 
         // Select the numbering system.
@@ -231,7 +232,9 @@ class NumberFormatterImpl {
         }
         if (pattern == null) {
             int patternStyle;
-            if (isPercent || isPermille) {
+            if (isCldrUnit) {
+                patternStyle = NumberFormat.NUMBERSTYLE;
+            } else if (isPercent || isPermille) {
                 patternStyle = NumberFormat.PERCENTSTYLE;
             } else if (!isCurrency || unitWidth == UnitWidth.FULL_NAME) {
                 patternStyle = NumberFormat.NUMBERSTYLE;
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/ExhaustiveNumberTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/ExhaustiveNumberTest.java
index 450f08ce725..6a679eff2b3 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/ExhaustiveNumberTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/ExhaustiveNumberTest.java
@@ -15,9 +15,12 @@ import com.ibm.icu.impl.StaticUnicodeSets.Key;
 import com.ibm.icu.impl.number.DecimalQuantity_DualStorageBCD;
 import com.ibm.icu.lang.UCharacter;
 import com.ibm.icu.number.NumberFormatter;
+import com.ibm.icu.number.UnlocalizedNumberFormatter;
 import com.ibm.icu.number.Precision;
 import com.ibm.icu.text.DecimalFormatSymbols;
 import com.ibm.icu.text.UnicodeSet;
+import com.ibm.icu.util.MeasureUnit;
+import com.ibm.icu.util.NoUnit;
 import com.ibm.icu.util.ULocale;
 
 /**
@@ -87,6 +90,24 @@ public class ExhaustiveNumberTest extends TestFmwk {
                 set.contains(cp));
     }
 
+    @Test
+    public void test20417_PercentParity() {
+        UnlocalizedNumberFormatter uNoUnitPercent = NumberFormatter.with().unit(NoUnit.PERCENT);
+        UnlocalizedNumberFormatter uNoUnitPermille = NumberFormatter.with().unit(NoUnit.PERMILLE);
+        UnlocalizedNumberFormatter uMeasurePercent = NumberFormatter.with().unit(MeasureUnit.PERCENT);
+        UnlocalizedNumberFormatter uMeasurePermille = NumberFormatter.with().unit(MeasureUnit.PERMILLE);
+
+        for (ULocale locale : ULocale.getAvailableLocales()) {
+            String sNoUnitPercent = uNoUnitPercent.locale(locale).format(50).toString();
+            String sNoUnitPermille = uNoUnitPermille.locale(locale).format(50).toString();
+            String sMeasurePercent = uMeasurePercent.locale(locale).format(50).toString();
+            String sMeasurePermille = uMeasurePermille.locale(locale).format(50).toString();
+
+            assertEquals("Percent, locale " + locale, sNoUnitPercent, sMeasurePercent);
+            assertEquals("Permille, locale " + locale, sNoUnitPermille, sMeasurePermille);
+        }
+    }
+
     @Test
     public void unlimitedRoundingBigDecimal() {
         BigDecimal ten10000 = BigDecimal.valueOf(10).pow(10000);
-- 
2.50.1