From: Shane Carr Date: Sat, 15 Feb 2020 03:18:43 +0000 (-0800) Subject: ICU-20961 Return correct currency plural pattern from DecimalFormat X-Git-Tag: release-67-rc~81 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e572de551683db2231b19ed3fd540350c1c89394;p=icu ICU-20961 Return correct currency plural pattern from DecimalFormat --- diff --git a/icu4c/source/i18n/number_mapper.cpp b/icu4c/source/i18n/number_mapper.cpp index f0d313fad87..b7308873d32 100644 --- a/icu4c/source/i18n/number_mapper.cpp +++ b/icu4c/source/i18n/number_mapper.cpp @@ -63,17 +63,8 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert // AFFIXES // ///////////// - AffixPatternProvider* affixProvider; - if (properties.currencyPluralInfo.fPtr.isNull()) { - warehouse.currencyPluralInfoAPP.setToBogus(); - warehouse.propertiesAPP.setTo(properties, status); - affixProvider = &warehouse.propertiesAPP; - } else { - warehouse.currencyPluralInfoAPP.setTo(*properties.currencyPluralInfo.fPtr, properties, status); - warehouse.propertiesAPP.setToBogus(); - affixProvider = &warehouse.currencyPluralInfoAPP; - } - macros.affixProvider = affixProvider; + warehouse.affixProvider.setTo(properties, status); + macros.affixProvider = &warehouse.affixProvider.get(); /////////// // UNITS // @@ -83,7 +74,7 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert !properties.currency.isNull() || !properties.currencyPluralInfo.fPtr.isNull() || !properties.currencyUsage.isNull() || - affixProvider->hasCurrencySign()); + warehouse.affixProvider.get().hasCurrencySign()); CurrencyUnit currency = resolveCurrency(properties, locale, status); UCurrencyUsage currencyUsage = properties.currencyUsage.getOrDefault(UCURR_USAGE_STANDARD); if (useCurrency) { diff --git a/icu4c/source/i18n/number_mapper.h b/icu4c/source/i18n/number_mapper.h index de7d9c3865c..88e0cb74b8b 100644 --- a/icu4c/source/i18n/number_mapper.h +++ b/icu4c/source/i18n/number_mapper.h @@ -20,6 +20,10 @@ namespace number { namespace impl { +class AutoAffixPatternProvider; +class CurrencyPluralInfoAffixProvider; + + class PropertiesAffixPatternProvider : public AffixPatternProvider, public UMemory { public: bool isBogus() const { @@ -32,12 +36,6 @@ class PropertiesAffixPatternProvider : public AffixPatternProvider, public UMemo void setTo(const DecimalFormatProperties& properties, UErrorCode& status); - PropertiesAffixPatternProvider() = default; // puts instance in valid but undefined state - - PropertiesAffixPatternProvider(const DecimalFormatProperties& properties, UErrorCode& status) { - setTo(properties, status); - } - // AffixPatternProvider Methods: char16_t charAt(int32_t flags, int32_t i) const U_OVERRIDE; @@ -65,9 +63,14 @@ class PropertiesAffixPatternProvider : public AffixPatternProvider, public UMemo UnicodeString negSuffix; bool isCurrencyPattern; + PropertiesAffixPatternProvider() = default; // puts instance in valid but undefined state + const UnicodeString& getStringInternal(int32_t flags) const; bool fBogus{true}; + + friend class AutoAffixPatternProvider; + friend class CurrencyPluralInfoAffixProvider; }; @@ -107,7 +110,43 @@ class CurrencyPluralInfoAffixProvider : public AffixPatternProvider, public UMem private: PropertiesAffixPatternProvider affixesByPlural[StandardPlural::COUNT]; + CurrencyPluralInfoAffixProvider() = default; + bool fBogus{true}; + + friend class AutoAffixPatternProvider; +}; + + +class AutoAffixPatternProvider { + public: + inline AutoAffixPatternProvider() = default; + + inline AutoAffixPatternProvider(const DecimalFormatProperties& properties, UErrorCode& status) { + setTo(properties, status); + } + + inline void setTo(const DecimalFormatProperties& properties, UErrorCode& status) { + if (properties.currencyPluralInfo.fPtr.isNull()) { + propertiesAPP.setTo(properties, status); + currencyPluralInfoAPP.setToBogus(); + } else { + propertiesAPP.setToBogus(); + currencyPluralInfoAPP.setTo(*properties.currencyPluralInfo.fPtr, properties, status); + } + } + + inline const AffixPatternProvider& get() const { + if (!currencyPluralInfoAPP.isBogus()) { + return currencyPluralInfoAPP; + } else { + return propertiesAPP; + } + } + + private: + PropertiesAffixPatternProvider propertiesAPP; + CurrencyPluralInfoAffixProvider currencyPluralInfoAPP; }; @@ -115,8 +154,7 @@ class CurrencyPluralInfoAffixProvider : public AffixPatternProvider, public UMem * A struct for ownership of a few objects needed for formatting. */ struct DecimalFormatWarehouse { - PropertiesAffixPatternProvider propertiesAPP; - CurrencyPluralInfoAffixProvider currencyPluralInfoAPP; + AutoAffixPatternProvider affixProvider; CurrencySymbols currencySymbols; }; diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp index 55b83f1ac63..9d845056069 100644 --- a/icu4c/source/i18n/number_patternstring.cpp +++ b/icu4c/source/i18n/number_patternstring.cpp @@ -686,10 +686,10 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP int32_t exponentDigits = uprv_min(properties.minimumExponentDigits, dosMax); bool exponentShowPlusSign = properties.exponentSignAlwaysShown; - PropertiesAffixPatternProvider affixes(properties, status); + AutoAffixPatternProvider affixProvider(properties, status); // Prefixes - sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_PREFIX)); + sb.append(affixProvider.get().getString(AffixPatternProvider::AFFIX_POS_PREFIX)); int32_t afterPrefixPos = sb.length(); // Figure out the grouping sizes. @@ -778,7 +778,7 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP // Suffixes int32_t beforeSuffixPos = sb.length(); - sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_SUFFIX)); + sb.append(affixProvider.get().getString(AffixPatternProvider::AFFIX_POS_SUFFIX)); // Resolve Padding if (paddingWidth > 0 && !paddingLocation.isNull()) { @@ -814,16 +814,16 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP // Negative affixes // Ignore if the negative prefix pattern is "-" and the negative suffix is empty - if (affixes.hasNegativeSubpattern()) { + if (affixProvider.get().hasNegativeSubpattern()) { sb.append(u';'); - sb.append(affixes.getString(AffixPatternProvider::AFFIX_NEG_PREFIX)); + sb.append(affixProvider.get().getString(AffixPatternProvider::AFFIX_NEG_PREFIX)); // Copy the positive digit format into the negative. // This is optional; the pattern is the same as if '#' were appended here instead. // NOTE: It is not safe to append the UnicodeString to itself, so we need to copy. // See http://bugs.icu-project.org/trac/ticket/13707 UnicodeString copy(sb); sb.append(copy, afterPrefixPos, beforeSuffixPos - afterPrefixPos); - sb.append(affixes.getString(AffixPatternProvider::AFFIX_NEG_SUFFIX)); + sb.append(affixProvider.get().getString(AffixPatternProvider::AFFIX_NEG_SUFFIX)); } return sb; diff --git a/icu4c/source/i18n/numparse_impl.cpp b/icu4c/source/i18n/numparse_impl.cpp index bf5829061a1..4b76da1c149 100644 --- a/icu4c/source/i18n/numparse_impl.cpp +++ b/icu4c/source/i18n/numparse_impl.cpp @@ -83,23 +83,14 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr const DecimalFormatSymbols& symbols, bool parseCurrency, UErrorCode& status) { Locale locale = symbols.getLocale(); - PropertiesAffixPatternProvider localPAPP; - CurrencyPluralInfoAffixProvider localCPIAP; - AffixPatternProvider* affixProvider; - if (properties.currencyPluralInfo.fPtr.isNull()) { - localPAPP.setTo(properties, status); - affixProvider = &localPAPP; - } else { - localCPIAP.setTo(*properties.currencyPluralInfo.fPtr, properties, status); - affixProvider = &localCPIAP; - } - if (affixProvider == nullptr || U_FAILURE(status)) { return nullptr; } + AutoAffixPatternProvider affixProvider(properties, status); + if (U_FAILURE(status)) { return nullptr; } CurrencyUnit currency = resolveCurrency(properties, locale, status); CurrencySymbols currencySymbols(currency, locale, symbols, status); bool isStrict = properties.parseMode.getOrDefault(PARSE_MODE_STRICT) == PARSE_MODE_STRICT; Grouper grouper = Grouper::forProperties(properties); int parseFlags = 0; - if (affixProvider == nullptr || U_FAILURE(status)) { return nullptr; } + if (U_FAILURE(status)) { return nullptr; } if (!properties.parseCaseSensitive) { parseFlags |= PARSE_FLAG_IGNORE_CASE; } @@ -121,7 +112,7 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr if (grouper.getPrimary() <= 0) { parseFlags |= PARSE_FLAG_GROUPING_DISABLED; } - if (parseCurrency || affixProvider->hasCurrencySign()) { + if (parseCurrency || affixProvider.get().hasCurrencySign()) { parseFlags |= PARSE_FLAG_MONETARY_SEPARATORS; } if (!parseCurrency) { @@ -143,13 +134,13 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr parser->fLocalMatchers.affixTokenMatcherWarehouse = {&affixSetupData}; parser->fLocalMatchers.affixMatcherWarehouse = {&parser->fLocalMatchers.affixTokenMatcherWarehouse}; parser->fLocalMatchers.affixMatcherWarehouse.createAffixMatchers( - *affixProvider, *parser, ignorables, parseFlags, status); + affixProvider.get(), *parser, ignorables, parseFlags, status); //////////////////////// /// CURRENCY MATCHER /// //////////////////////// - if (parseCurrency || affixProvider->hasCurrencySign()) { + if (parseCurrency || affixProvider.get().hasCurrencySign()) { parser->addMatcher(parser->fLocalMatchers.currency = {currencySymbols, symbols, parseFlags, status}); } @@ -159,10 +150,10 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr // ICU-TC meeting, April 11, 2018: accept percent/permille only if it is in the pattern, // and to maintain regressive behavior, divide by 100 even if no percent sign is present. - if (!isStrict && affixProvider->containsSymbolType(AffixPatternType::TYPE_PERCENT, status)) { + if (!isStrict && affixProvider.get().containsSymbolType(AffixPatternType::TYPE_PERCENT, status)) { parser->addMatcher(parser->fLocalMatchers.percent = {symbols}); } - if (!isStrict && affixProvider->containsSymbolType(AffixPatternType::TYPE_PERMILLE, status)) { + if (!isStrict && affixProvider.get().containsSymbolType(AffixPatternType::TYPE_PERMILLE, status)) { parser->addMatcher(parser->fLocalMatchers.permille = {symbols}); } diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 8b0f7e0a77b..b4fc3a04e25 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -243,6 +243,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n TESTCASE_AUTO(Test20499_CurrencyVisibleDigitsPlural); TESTCASE_AUTO(Test13735_GroupingSizeGetter); TESTCASE_AUTO(Test13734_StrictFlexibleWhitespace); + TESTCASE_AUTO(Test20961_CurrencyPluralPattern); TESTCASE_AUTO_END; } @@ -9749,4 +9750,15 @@ void NumberFormatTest::Test13734_StrictFlexibleWhitespace() { } } +void NumberFormatTest::Test20961_CurrencyPluralPattern() { + IcuTestErrorCode status(*this, "Test20961_CurrencyPluralPattern"); + { + LocalPointer decimalFormat(static_cast( + NumberFormat::createInstance("en-US", UNUM_CURRENCY_PLURAL, status))); + UnicodeString result; + decimalFormat->toPattern(result); + assertEquals("Currency pattern", u"#,##0.00 ¤¤¤", result); + } +} + #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h index 80abc7166d4..71b1a77c5de 100644 --- a/icu4c/source/test/intltest/numfmtst.h +++ b/icu4c/source/test/intltest/numfmtst.h @@ -299,6 +299,7 @@ class NumberFormatTest: public CalendarTimeZoneTest { void Test20499_CurrencyVisibleDigitsPlural(); void Test13735_GroupingSizeGetter(); void Test13734_StrictFlexibleWhitespace(); + void Test20961_CurrencyPluralPattern(); private: UBool testFormattableAsUFormattable(const char *file, int line, Formattable &f); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringUtils.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringUtils.java index 601f1c86103..1ef37812df9 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringUtils.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PatternStringUtils.java @@ -93,7 +93,7 @@ public class PatternStringUtils { boolean alwaysShowDecimal = properties.getDecimalSeparatorAlwaysShown(); int exponentDigits = Math.min(properties.getMinimumExponentDigits(), dosMax); boolean exponentShowPlusSign = properties.getExponentSignAlwaysShown(); - PropertiesAffixPatternProvider affixes = new PropertiesAffixPatternProvider(properties); + AffixPatternProvider affixes = PropertiesAffixPatternProvider.forProperties(properties); // Prefixes sb.append(affixes.getString(AffixPatternProvider.FLAG_POS_PREFIX)); diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PropertiesAffixPatternProvider.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PropertiesAffixPatternProvider.java index e55e9701426..115d1b9b792 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PropertiesAffixPatternProvider.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/PropertiesAffixPatternProvider.java @@ -9,7 +9,15 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { private final String negSuffix; private final boolean isCurrencyPattern; - public PropertiesAffixPatternProvider(DecimalFormatProperties properties) { + public static AffixPatternProvider forProperties(DecimalFormatProperties properties) { + if (properties.getCurrencyPluralInfo() == null) { + return new PropertiesAffixPatternProvider(properties); + } else { + return new CurrencyPluralInfoAffixProvider(properties.getCurrencyPluralInfo(), properties); + } + } + + PropertiesAffixPatternProvider(DecimalFormatProperties properties) { // There are two ways to set affixes in DecimalFormat: via the pattern string (applyPattern), and via the // explicit setters (setPositivePrefix and friends). The way to resolve the settings is as follows: // diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java index c347f90b452..86755c2d54c 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java @@ -10,7 +10,6 @@ import java.util.List; import com.ibm.icu.impl.StringSegment; import com.ibm.icu.impl.number.AffixPatternProvider; import com.ibm.icu.impl.number.AffixUtils; -import com.ibm.icu.impl.number.CurrencyPluralInfoAffixProvider; import com.ibm.icu.impl.number.CustomSymbolCurrency; import com.ibm.icu.impl.number.DecimalFormatProperties; import com.ibm.icu.impl.number.DecimalFormatProperties.ParseMode; @@ -138,12 +137,7 @@ public class NumberParserImpl { boolean parseCurrency) { ULocale locale = symbols.getULocale(); - AffixPatternProvider affixProvider; - if (properties.getCurrencyPluralInfo() == null) { - affixProvider = new PropertiesAffixPatternProvider(properties); - } else { - affixProvider = new CurrencyPluralInfoAffixProvider(properties.getCurrencyPluralInfo(), properties); - } + AffixPatternProvider affixProvider = PropertiesAffixPatternProvider.forProperties(properties); Currency currency = CustomSymbolCurrency.resolve(properties.getCurrency(), locale, symbols); ParseMode parseMode = properties.getParseMode(); if (parseMode == null) { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java index 82f75d57fd9..3a5fcdcda0c 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberPropertyMapper.java @@ -6,7 +6,6 @@ import java.math.BigDecimal; import java.math.MathContext; import com.ibm.icu.impl.number.AffixPatternProvider; -import com.ibm.icu.impl.number.CurrencyPluralInfoAffixProvider; import com.ibm.icu.impl.number.CustomSymbolCurrency; import com.ibm.icu.impl.number.DecimalFormatProperties; import com.ibm.icu.impl.number.Grouper; @@ -104,12 +103,7 @@ final class NumberPropertyMapper { // AFFIXES // ///////////// - AffixPatternProvider affixProvider; - if (properties.getCurrencyPluralInfo() == null) { - affixProvider = new PropertiesAffixPatternProvider(properties); - } else { - affixProvider = new CurrencyPluralInfoAffixProvider(properties.getCurrencyPluralInfo(), properties); - } + AffixPatternProvider affixProvider = PropertiesAffixPatternProvider.forProperties(properties); macros.affixProvider = affixProvider; /////////// diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java index 1729f2adaa2..e555bb6887a 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java @@ -6711,4 +6711,10 @@ public class NumberFormatTest extends TestFmwk { assertEquals("result: ", null, result); } } + + @Test + public void test20961_CurrencyPluralPattern() { + DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getInstance(ULocale.US, NumberFormat.PLURALCURRENCYSTYLE); + assertEquals("Currency pattern", "#,##0.00 ¤¤¤", decimalFormat.toPattern()); + } }