From 1e24bcd7211d3c020801ffbaac7efa7b5e1b7e21 Mon Sep 17 00:00:00 2001 From: Shane Carr Date: Fri, 6 Mar 2020 01:53:58 +0000 Subject: [PATCH] ICU-20956 Fix monetary symbol getters in DecimalFormat See #987 --- icu4c/source/i18n/dcfmtsym.cpp | 96 ++++++++++--------- icu4c/source/i18n/decimfmt.cpp | 7 +- icu4c/source/i18n/number_formatimpl.cpp | 95 ++++-------------- icu4c/source/i18n/number_mapper.cpp | 3 - icu4c/source/i18n/number_mapper.h | 2 +- icu4c/source/i18n/number_patternmodifier.cpp | 18 ++-- icu4c/source/i18n/number_patternmodifier.h | 12 ++- icu4c/source/i18n/number_skeletons.cpp | 4 - icu4c/source/i18n/unicode/dcfmtsym.h | 13 ++- icu4c/source/i18n/unicode/numberformatter.h | 3 - .../intltest/numbertest_patternmodifier.cpp | 9 +- icu4c/source/test/intltest/numfmtst.cpp | 36 ++++++- icu4c/source/test/intltest/numfmtst.h | 1 + .../ibm/icu/number/NumberFormatterImpl.java | 17 +--- .../src/com/ibm/icu/text/DecimalFormat.java | 13 ++- .../ibm/icu/text/DecimalFormatSymbols.java | 56 ++++++----- .../icu/dev/test/format/NumberFormatTest.java | 21 ++++ 17 files changed, 203 insertions(+), 203 deletions(-) diff --git a/icu4c/source/i18n/dcfmtsym.cpp b/icu4c/source/i18n/dcfmtsym.cpp index d8b1ecdbea1..15418bfe65f 100644 --- a/icu4c/source/i18n/dcfmtsym.cpp +++ b/icu4c/source/i18n/dcfmtsym.cpp @@ -167,6 +167,7 @@ DecimalFormatSymbols::operator=(const DecimalFormatSymbols& rhs) fIsCustomCurrencySymbol = rhs.fIsCustomCurrencySymbol; fIsCustomIntlCurrencySymbol = rhs.fIsCustomIntlCurrencySymbol; fCodePointZero = rhs.fCodePointZero; + currPattern = rhs.currPattern; } return *this; } @@ -453,58 +454,16 @@ DecimalFormatSymbols::initialize(const Locale& loc, UErrorCode& status, } fCodePointZero = tempCodePointZero; - // Obtain currency data from the currency API. This is strictly - // for backward compatibility; we don't use DecimalFormatSymbols - // for currency data anymore. + // Get the default currency from the currency API. UErrorCode internalStatus = U_ZERO_ERROR; // don't propagate failures out UChar curriso[4]; UnicodeString tempStr; int32_t currisoLength = ucurr_forLocale(locStr, curriso, UPRV_LENGTHOF(curriso), &internalStatus); if (U_SUCCESS(internalStatus) && currisoLength == 3) { - uprv_getStaticCurrencyName(curriso, locStr, tempStr, internalStatus); - if (U_SUCCESS(internalStatus)) { - fSymbols[kIntlCurrencySymbol].setTo(curriso, currisoLength); - fSymbols[kCurrencySymbol] = tempStr; - } + setCurrency(curriso, status); + } else { + setCurrency(nullptr, status); } - /* else use the default values. */ - - //load the currency data - UChar ucc[4]={0}; //Currency Codes are always 3 chars long - int32_t uccLen = 4; - const char* locName = loc.getName(); - UErrorCode localStatus = U_ZERO_ERROR; - uccLen = ucurr_forLocale(locName, ucc, uccLen, &localStatus); - - // TODO: Currency pattern data loading is duplicated in number_formatimpl.cpp - if(U_SUCCESS(localStatus) && uccLen > 0) { - char cc[4]={0}; - u_UCharsToChars(ucc, cc, uccLen); - /* An explicit currency was requested */ - LocalUResourceBundlePointer currencyResource(ures_open(U_ICUDATA_CURR, locStr, &localStatus)); - LocalUResourceBundlePointer currency( - ures_getByKeyWithFallback(currencyResource.getAlias(), "Currencies", NULL, &localStatus)); - ures_getByKeyWithFallback(currency.getAlias(), cc, currency.getAlias(), &localStatus); - if(U_SUCCESS(localStatus) && ures_getSize(currency.getAlias())>2) { // the length is 3 if more data is present - ures_getByIndex(currency.getAlias(), 2, currency.getAlias(), &localStatus); - int32_t currPatternLen = 0; - currPattern = - ures_getStringByIndex(currency.getAlias(), (int32_t)0, &currPatternLen, &localStatus); - UnicodeString decimalSep = - ures_getUnicodeStringByIndex(currency.getAlias(), (int32_t)1, &localStatus); - UnicodeString groupingSep = - ures_getUnicodeStringByIndex(currency.getAlias(), (int32_t)2, &localStatus); - if(U_SUCCESS(localStatus)){ - fSymbols[kMonetaryGroupingSeparatorSymbol] = groupingSep; - fSymbols[kMonetarySeparatorSymbol] = decimalSep; - //pattern.setTo(TRUE, currPattern, currPatternLen); - status = localStatus; - } - } - /* else An explicit currency was requested and is unknown or locale data is malformed. */ - /* ucurr_* API will get the correct value later on. */ - } - // else ignore the error if no currency // Currency Spacing. LocalUResourceBundlePointer currencyResource(ures_open(U_ICUDATA_CURR, locStr, &status)); @@ -553,9 +512,54 @@ DecimalFormatSymbols::initialize() { fIsCustomIntlCurrencySymbol = FALSE; fCodePointZero = 0x30; U_ASSERT(fCodePointZero == fSymbols[kZeroDigitSymbol].char32At(0)); + currPattern = nullptr; } +void DecimalFormatSymbols::setCurrency(const UChar* currency, UErrorCode& status) { + // TODO: If this method is made public: + // - Adopt ICU4J behavior of not allowing currency to be null. + // - Also verify that the length of currency is 3. + if (!currency) { + return; + } + + UnicodeString tempStr; + uprv_getStaticCurrencyName(currency, locale.getName(), tempStr, status); + if (U_SUCCESS(status)) { + fSymbols[kIntlCurrencySymbol].setTo(currency, 3); + fSymbols[kCurrencySymbol] = tempStr; + } + + char cc[4]={0}; + u_UCharsToChars(currency, cc, 3); + + /* An explicit currency was requested */ + // TODO(ICU-13297): Move this data loading logic into a centralized place + UErrorCode localStatus = U_ZERO_ERROR; + LocalUResourceBundlePointer rbTop(ures_open(U_ICUDATA_CURR, locale.getName(), &localStatus)); + LocalUResourceBundlePointer rb( + ures_getByKeyWithFallback(rbTop.getAlias(), "Currencies", NULL, &localStatus)); + ures_getByKeyWithFallback(rb.getAlias(), cc, rb.getAlias(), &localStatus); + if(U_SUCCESS(localStatus) && ures_getSize(rb.getAlias())>2) { // the length is 3 if more data is present + ures_getByIndex(rb.getAlias(), 2, rb.getAlias(), &localStatus); + int32_t currPatternLen = 0; + currPattern = + ures_getStringByIndex(rb.getAlias(), (int32_t)0, &currPatternLen, &localStatus); + UnicodeString decimalSep = + ures_getUnicodeStringByIndex(rb.getAlias(), (int32_t)1, &localStatus); + UnicodeString groupingSep = + ures_getUnicodeStringByIndex(rb.getAlias(), (int32_t)2, &localStatus); + if(U_SUCCESS(localStatus)){ + fSymbols[kMonetaryGroupingSeparatorSymbol] = groupingSep; + fSymbols[kMonetarySeparatorSymbol] = decimalSep; + //pattern.setTo(TRUE, currPattern, currPatternLen); + } + } + /* else An explicit currency was requested and is unknown or locale data is malformed. */ + /* ucurr_* API will get the correct value later on. */ +} + Locale DecimalFormatSymbols::getLocale(ULocDataLocaleType type, UErrorCode& status) const { U_LOCALE_BASED(locBased, *this); diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index 0cbaca7e099..07a41e58184 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -1502,8 +1502,11 @@ void DecimalFormat::setCurrency(const char16_t* theCurrency, UErrorCode& ec) { } NumberFormat::setCurrency(theCurrency, ec); // to set field for compatibility fields->properties.currency = currencyUnit; - // TODO: Set values in fields->symbols, too? - touchNoError(); + // In Java, the DecimalFormatSymbols is mutable. Why not in C++? + LocalPointer newSymbols(new DecimalFormatSymbols(*fields->symbols), ec); + newSymbols->setCurrency(currencyUnit.getISOCurrency(), ec); + fields->symbols.adoptInsteadAndCheckErrorCode(newSymbols.orphan(), ec); + touch(ec); } void DecimalFormat::setCurrency(const char16_t* theCurrency) { diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp index 83ee2a8bfe9..8042979bdc3 100644 --- a/icu4c/source/i18n/number_formatimpl.cpp +++ b/icu4c/source/i18n/number_formatimpl.cpp @@ -24,45 +24,6 @@ using namespace icu; using namespace icu::number; using namespace icu::number::impl; -namespace { - -struct CurrencyFormatInfoResult { - bool exists; - const char16_t* pattern; - const char16_t* decimalSeparator; - const char16_t* groupingSeparator; -}; - -CurrencyFormatInfoResult -getCurrencyFormatInfo(const Locale& locale, const char* isoCode, UErrorCode& status) { - // TODO: Load this data in a centralized location like ICU4J? - // TODO: Move this into the CurrencySymbols class? - // TODO: Parts of this same data are loaded in dcfmtsym.cpp; should clean up. - CurrencyFormatInfoResult result = {false, nullptr, nullptr, nullptr}; - if (U_FAILURE(status)) { return result; } - CharString key; - key.append("Currencies/", status); - key.append(isoCode, status); - UErrorCode localStatus = status; - LocalUResourceBundlePointer bundle(ures_open(U_ICUDATA_CURR, locale.getName(), &localStatus)); - ures_getByKeyWithFallback(bundle.getAlias(), key.data(), bundle.getAlias(), &localStatus); - if (U_SUCCESS(localStatus) && - ures_getSize(bundle.getAlias()) > 2) { // the length is 3 if more data is present - ures_getByIndex(bundle.getAlias(), 2, bundle.getAlias(), &localStatus); - int32_t dummy; - result.exists = true; - result.pattern = ures_getStringByIndex(bundle.getAlias(), 0, &dummy, &localStatus); - result.decimalSeparator = ures_getStringByIndex(bundle.getAlias(), 1, &dummy, &localStatus); - result.groupingSeparator = ures_getStringByIndex(bundle.getAlias(), 2, &dummy, &localStatus); - status = localStatus; - } else if (localStatus != U_MISSING_RESOURCE_ERROR) { - status = localStatus; - } - return result; -} - -} // namespace - MicroPropsGenerator::~MicroPropsGenerator() = default; @@ -179,14 +140,6 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, if (isCurrency) { currency = CurrencyUnit(macros.unit, status); // Restore CurrencyUnit from MeasureUnit } - const CurrencySymbols* currencySymbols; - if (macros.currencySymbols != nullptr) { - // Used by the DecimalFormat code path - currencySymbols = macros.currencySymbols; - } else { - fWarehouse.fCurrencySymbols = {currency, macros.locale, status}; - currencySymbols = &fWarehouse.fCurrencySymbols; - } UNumberUnitWidth unitWidth = UNUM_UNIT_WIDTH_SHORT; if (macros.unitWidth != UNUM_UNIT_WIDTH_COUNT) { unitWidth = macros.unitWidth; @@ -213,41 +166,26 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, if (macros.symbols.isDecimalFormatSymbols()) { fMicros.symbols = macros.symbols.getDecimalFormatSymbols(); } else { - auto newSymbols = new DecimalFormatSymbols(macros.locale, *ns, status); - if (newSymbols == nullptr) { - status = U_MEMORY_ALLOCATION_ERROR; + LocalPointer newSymbols( + new DecimalFormatSymbols(macros.locale, *ns, status), status); + if (U_FAILURE(status)) { return nullptr; } - fMicros.symbols = newSymbols; - // Give ownership to the NumberFormatterImpl. - fSymbols.adoptInstead(fMicros.symbols); + if (isCurrency) { + newSymbols->setCurrency(currency.getISOCurrency(), status); + if (U_FAILURE(status)) { + return nullptr; + } + } + fMicros.symbols = newSymbols.getAlias(); + fSymbols.adoptInstead(newSymbols.orphan()); } // Load and parse the pattern string. It is used for grouping sizes and affixes only. // If we are formatting currency, check for a currency-specific pattern. const char16_t* pattern = nullptr; - if (isCurrency) { - CurrencyFormatInfoResult info = getCurrencyFormatInfo( - macros.locale, currency.getSubtype(), status); - if (info.exists) { - pattern = info.pattern; - // It's clunky to clone an object here, but this code is not frequently executed. - auto symbols = new DecimalFormatSymbols(*fMicros.symbols); - if (symbols == nullptr) { - status = U_MEMORY_ALLOCATION_ERROR; - return nullptr; - } - fMicros.symbols = symbols; - fSymbols.adoptInstead(symbols); - symbols->setSymbol( - DecimalFormatSymbols::ENumberFormatSymbol::kMonetarySeparatorSymbol, - UnicodeString(info.decimalSeparator), - FALSE); - symbols->setSymbol( - DecimalFormatSymbols::ENumberFormatSymbol::kMonetaryGroupingSeparatorSymbol, - UnicodeString(info.groupingSeparator), - FALSE); - } + if (isCurrency && fMicros.symbols->getCurrencyPattern() != nullptr) { + pattern = fMicros.symbols->getCurrencyPattern(); } if (pattern == nullptr) { CldrPatternStyle patternStyle; @@ -375,11 +313,12 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe, if (patternModifier->needsPlurals()) { patternModifier->setSymbols( fMicros.symbols, - currencySymbols, + currency, unitWidth, - resolvePluralRules(macros.rules, macros.locale, status)); + resolvePluralRules(macros.rules, macros.locale, status), + status); } else { - patternModifier->setSymbols(fMicros.symbols, currencySymbols, unitWidth, nullptr); + patternModifier->setSymbols(fMicros.symbols, currency, unitWidth, nullptr, status); } if (safe) { fImmutablePatternModifier.adoptInstead(patternModifier->createImmutable(status)); diff --git a/icu4c/source/i18n/number_mapper.cpp b/icu4c/source/i18n/number_mapper.cpp index b7308873d32..ec617438c9a 100644 --- a/icu4c/source/i18n/number_mapper.cpp +++ b/icu4c/source/i18n/number_mapper.cpp @@ -13,7 +13,6 @@ #include "number_patternstring.h" #include "unicode/errorcode.h" #include "number_utils.h" -#include "number_currencysymbols.h" using namespace icu; using namespace icu::number; @@ -81,8 +80,6 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert // NOTE: Slicing is OK. macros.unit = currency; // NOLINT } - warehouse.currencySymbols = {currency, locale, symbols, status}; - macros.currencySymbols = &warehouse.currencySymbols; /////////////////////// // ROUNDING STRATEGY // diff --git a/icu4c/source/i18n/number_mapper.h b/icu4c/source/i18n/number_mapper.h index 88e0cb74b8b..d18b8b3c438 100644 --- a/icu4c/source/i18n/number_mapper.h +++ b/icu4c/source/i18n/number_mapper.h @@ -155,7 +155,7 @@ class AutoAffixPatternProvider { */ struct DecimalFormatWarehouse { AutoAffixPatternProvider affixProvider; - CurrencySymbols currencySymbols; + }; diff --git a/icu4c/source/i18n/number_patternmodifier.cpp b/icu4c/source/i18n/number_patternmodifier.cpp index e37dde8c175..45602942aef 100644 --- a/icu4c/source/i18n/number_patternmodifier.cpp +++ b/icu4c/source/i18n/number_patternmodifier.cpp @@ -34,11 +34,13 @@ void MutablePatternModifier::setPatternAttributes(UNumberSignDisplay signDisplay } void MutablePatternModifier::setSymbols(const DecimalFormatSymbols* symbols, - const CurrencySymbols* currencySymbols, - const UNumberUnitWidth unitWidth, const PluralRules* rules) { + const CurrencyUnit& currency, + const UNumberUnitWidth unitWidth, + const PluralRules* rules, + UErrorCode& status) { U_ASSERT((rules != nullptr) == needsPlurals()); fSymbols = symbols; - fCurrencySymbols = currencySymbols; + fCurrencySymbols = {currency, symbols->getLocale(), *symbols, status}; fUnitWidth = unitWidth; fRules = rules; } @@ -294,23 +296,23 @@ UnicodeString MutablePatternModifier::getSymbol(AffixPatternType type) const { case AffixPatternType::TYPE_CURRENCY_SINGLE: { // UnitWidth ISO and HIDDEN overrides the singular currency symbol. if (fUnitWidth == UNumberUnitWidth::UNUM_UNIT_WIDTH_ISO_CODE) { - return fCurrencySymbols->getIntlCurrencySymbol(localStatus); + return fCurrencySymbols.getIntlCurrencySymbol(localStatus); } else if (fUnitWidth == UNumberUnitWidth::UNUM_UNIT_WIDTH_HIDDEN) { return UnicodeString(); } else if (fUnitWidth == UNumberUnitWidth::UNUM_UNIT_WIDTH_NARROW) { - return fCurrencySymbols->getNarrowCurrencySymbol(localStatus); + return fCurrencySymbols.getNarrowCurrencySymbol(localStatus); } else { - return fCurrencySymbols->getCurrencySymbol(localStatus); + return fCurrencySymbols.getCurrencySymbol(localStatus); } } case AffixPatternType::TYPE_CURRENCY_DOUBLE: - return fCurrencySymbols->getIntlCurrencySymbol(localStatus); + return fCurrencySymbols.getIntlCurrencySymbol(localStatus); case AffixPatternType::TYPE_CURRENCY_TRIPLE: // NOTE: This is the code path only for patterns containing "¤¤¤". // Plural currencies set via the API are formatted in LongNameHandler. // This code path is used by DecimalFormat via CurrencyPluralInfo. U_ASSERT(fPlural != StandardPlural::Form::COUNT); - return fCurrencySymbols->getPluralName(fPlural, localStatus); + return fCurrencySymbols.getPluralName(fPlural, localStatus); case AffixPatternType::TYPE_CURRENCY_QUAD: return UnicodeString(u"\uFFFD"); case AffixPatternType::TYPE_CURRENCY_QUINT: diff --git a/icu4c/source/i18n/number_patternmodifier.h b/icu4c/source/i18n/number_patternmodifier.h index 2597afefd53..5ba842d5692 100644 --- a/icu4c/source/i18n/number_patternmodifier.h +++ b/icu4c/source/i18n/number_patternmodifier.h @@ -124,16 +124,18 @@ class U_I18N_API MutablePatternModifier * * @param symbols * The desired instance of DecimalFormatSymbols. - * @param currencySymbols - * The currency symbols to be used when substituting currency values into the affixes. + * @param currency + * The currency to be used when substituting currency values into the affixes. * @param unitWidth * The width used to render currencies. * @param rules * Required if the triple currency sign, "¤¤¤", appears in the pattern, which can be determined from the * convenience method {@link #needsPlurals()}. + * @param status + * Set if an error occurs while loading currency data. */ - void setSymbols(const DecimalFormatSymbols* symbols, const CurrencySymbols* currencySymbols, - UNumberUnitWidth unitWidth, const PluralRules* rules); + void setSymbols(const DecimalFormatSymbols* symbols, const CurrencyUnit& currency, + UNumberUnitWidth unitWidth, const PluralRules* rules, UErrorCode& status); /** * Sets attributes of the current number being processed. @@ -206,7 +208,7 @@ class U_I18N_API MutablePatternModifier // Symbol details (initialized in setSymbols) const DecimalFormatSymbols *fSymbols; UNumberUnitWidth fUnitWidth; - const CurrencySymbols *fCurrencySymbols; + CurrencySymbols fCurrencySymbols; const PluralRules *fRules; // Number details (initialized in setNumberProperties) diff --git a/icu4c/source/i18n/number_skeletons.cpp b/icu4c/source/i18n/number_skeletons.cpp index fe96279467f..e186a56e698 100644 --- a/icu4c/source/i18n/number_skeletons.cpp +++ b/icu4c/source/i18n/number_skeletons.cpp @@ -885,10 +885,6 @@ void GeneratorHelpers::generateSkeleton(const MacroProps& macros, UnicodeString& status = U_UNSUPPORTED_ERROR; return; } - if (macros.currencySymbols != nullptr) { - status = U_UNSUPPORTED_ERROR; - return; - } // Remove the trailing space if (sb.length() > 0) { diff --git a/icu4c/source/i18n/unicode/dcfmtsym.h b/icu4c/source/i18n/unicode/dcfmtsym.h index e1e0ab6b08c..582e7533a4a 100644 --- a/icu4c/source/i18n/unicode/dcfmtsym.h +++ b/icu4c/source/i18n/unicode/dcfmtsym.h @@ -291,6 +291,17 @@ public: */ void setSymbol(ENumberFormatSymbol symbol, const UnicodeString &value, const UBool propogateDigits); +#ifndef U_HIDE_INTERNAL_API + /** + * Loads symbols for the specified currency into this instance. + * + * This method is internal. If you think it should be public, file a ticket. + * + * @internal + */ + void setCurrency(const UChar* currency, UErrorCode& status); +#endif // U_HIDE_INTERNAL_API + /** * Returns the locale for which this object was constructed. * @stable ICU 2.6 @@ -374,8 +385,6 @@ private: */ void initialize(); - void setCurrencyForSymbols(); - public: #ifndef U_HIDE_INTERNAL_API diff --git a/icu4c/source/i18n/unicode/numberformatter.h b/icu4c/source/i18n/unicode/numberformatter.h index 58b13c36314..112a285b766 100644 --- a/icu4c/source/i18n/unicode/numberformatter.h +++ b/icu4c/source/i18n/unicode/numberformatter.h @@ -1415,9 +1415,6 @@ struct U_I18N_API MacroProps : public UMemory { /** @internal */ const PluralRules* rules = nullptr; // no ownership - /** @internal */ - const CurrencySymbols* currencySymbols = nullptr; // no ownership - /** @internal */ int32_t threshold = kInternalDefaultThreshold; diff --git a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp index 42d3a6f7cf4..650c526ce06 100644 --- a/icu4c/source/test/intltest/numbertest_patternmodifier.cpp +++ b/icu4c/source/test/intltest/numbertest_patternmodifier.cpp @@ -29,11 +29,10 @@ void PatternModifierTest::testBasic() { mod.setPatternInfo(&patternInfo, kUndefinedField); mod.setPatternAttributes(UNUM_SIGN_AUTO, false); DecimalFormatSymbols symbols(Locale::getEnglish(), status); - CurrencySymbols currencySymbols({u"USD", status}, "en", status); + mod.setSymbols(&symbols, {u"USD", status}, UNUM_UNIT_WIDTH_SHORT, nullptr, status); if (!assertSuccess("Spot 2", status, true)) { return; } - mod.setSymbols(&symbols, ¤cySymbols, UNUM_UNIT_WIDTH_SHORT, nullptr); mod.setNumberProperties(SIGNUM_POS, StandardPlural::Form::COUNT); assertEquals("Pattern a0b", u"a", getPrefix(mod, status)); @@ -96,11 +95,10 @@ void PatternModifierTest::testPatternWithNoPlaceholder() { mod.setPatternInfo(&patternInfo, kUndefinedField); mod.setPatternAttributes(UNUM_SIGN_AUTO, false); DecimalFormatSymbols symbols(Locale::getEnglish(), status); - CurrencySymbols currencySymbols({u"USD", status}, "en", status); + mod.setSymbols(&symbols, {u"USD", status}, UNUM_UNIT_WIDTH_SHORT, nullptr, status); if (!assertSuccess("Spot 2", status, true)) { return; } - mod.setSymbols(&symbols, ¤cySymbols, UNUM_UNIT_WIDTH_SHORT, nullptr); mod.setNumberProperties(SIGNUM_POS, StandardPlural::Form::COUNT); // Unsafe Code Path @@ -139,10 +137,9 @@ void PatternModifierTest::testMutableEqualsImmutable() { mod.setPatternInfo(&patternInfo, kUndefinedField); mod.setPatternAttributes(UNUM_SIGN_AUTO, false); DecimalFormatSymbols symbols(Locale::getEnglish(), status); - CurrencySymbols currencySymbols({u"USD", status}, "en", status); + mod.setSymbols(&symbols, {u"USD", status}, UNUM_UNIT_WIDTH_SHORT, nullptr, status); assertSuccess("Spot 2", status); if (U_FAILURE(status)) { return; } - mod.setSymbols(&symbols, ¤cySymbols, UNUM_UNIT_WIDTH_SHORT, nullptr); DecimalQuantity fq; fq.setToInt(1); diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index b4fc3a04e25..9ebd26e5462 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -238,6 +238,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n TESTCASE_AUTO(Test13840_ParseLongStringCrash); TESTCASE_AUTO(Test13850_EmptyStringCurrency); TESTCASE_AUTO(Test20348_CurrencyPrefixOverride); + TESTCASE_AUTO(Test20956_MonetarySymbolGetters); TESTCASE_AUTO(Test20358_GroupingInPattern); TESTCASE_AUTO(Test13731_DefaultCurrency); TESTCASE_AUTO(Test20499_CurrencyVisibleDigitsPlural); @@ -9568,12 +9569,12 @@ void NumberFormatTest::Test13850_EmptyStringCurrency() { const char16_t* currencyArg; UErrorCode expectedError; } cases[] = { - {u"", U_ZERO_ERROR}, + {u"", U_USING_FALLBACK_WARNING}, {u"U", U_ILLEGAL_ARGUMENT_ERROR}, {u"Us", U_ILLEGAL_ARGUMENT_ERROR}, - {nullptr, U_ZERO_ERROR}, + {nullptr, U_USING_FALLBACK_WARNING}, {u"U$D", U_INVARIANT_CONVERSION_ERROR}, - {u"Xxx", U_ZERO_ERROR} + {u"Xxx", U_USING_FALLBACK_WARNING} }; for (const auto& cas : cases) { UnicodeString message(u"with currency arg: "); @@ -9636,6 +9637,35 @@ void NumberFormatTest::Test20348_CurrencyPrefixOverride() { u"$100.00", fmt->format(100, result.remove(), NULL, status)); } +void NumberFormatTest::Test20956_MonetarySymbolGetters() { + IcuTestErrorCode status(*this, "Test20956_MonetarySymbolGetters"); + LocalPointer decimalFormat(static_cast( + NumberFormat::createCurrencyInstance("et", status))); + + decimalFormat->setCurrency(u"EEK"); + + const DecimalFormatSymbols* decimalFormatSymbols = decimalFormat->getDecimalFormatSymbols(); + assertEquals("MONETARY DECIMAL SEPARATOR", + u".", + decimalFormatSymbols->getSymbol(DecimalFormatSymbols::kMonetarySeparatorSymbol)); + assertEquals("DECIMAL SEPARATOR", + u",", + decimalFormatSymbols->getSymbol(DecimalFormatSymbols::kDecimalSeparatorSymbol)); + assertEquals("MONETARY GROUPING SEPARATOR", + u" ", + decimalFormatSymbols->getSymbol(DecimalFormatSymbols::kMonetaryGroupingSeparatorSymbol)); + assertEquals("GROUPING SEPARATOR", + u" ", + decimalFormatSymbols->getSymbol(DecimalFormatSymbols::kGroupingSeparatorSymbol)); + assertEquals("CURRENCY SYMBOL", + u"kr", + decimalFormatSymbols->getSymbol(DecimalFormatSymbols::kCurrencySymbol)); + + UnicodeString sb; + decimalFormat->format(12345.12, sb, status); + assertEquals("OUTPUT", u"12 345.12 kr", sb); +} + void NumberFormatTest::Test20358_GroupingInPattern() { IcuTestErrorCode status(*this, "Test20358_GroupingInPattern"); LocalPointer fmt(static_cast( diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h index 71b1a77c5de..068ad589344 100644 --- a/icu4c/source/test/intltest/numfmtst.h +++ b/icu4c/source/test/intltest/numfmtst.h @@ -294,6 +294,7 @@ class NumberFormatTest: public CalendarTimeZoneTest { void Test13840_ParseLongStringCrash(); void Test13850_EmptyStringCurrency(); void Test20348_CurrencyPrefixOverride(); + void Test20956_MonetarySymbolGetters(); void Test20358_GroupingInPattern(); void Test13731_DefaultCurrency(); void Test20499_CurrencyVisibleDigitsPlural(); 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 a84968533dd..e0233f2b453 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 @@ -2,8 +2,6 @@ // License & terms of use: http://www.unicode.org/copyright.html#License package com.ibm.icu.number; -import com.ibm.icu.impl.CurrencyData; -import com.ibm.icu.impl.CurrencyData.CurrencyFormatInfo; import com.ibm.icu.impl.FormattedStringBuilder; import com.ibm.icu.impl.StandardPlural; import com.ibm.icu.impl.number.CompactData.CompactType; @@ -213,21 +211,16 @@ class NumberFormatterImpl { micros.symbols = (DecimalFormatSymbols) macros.symbols; } else { micros.symbols = DecimalFormatSymbols.forNumberingSystem(macros.loc, ns); + if (isCurrency) { + micros.symbols.setCurrency(currency); + } } // Load and parse the pattern string. It is used for grouping sizes and affixes only. // If we are formatting currency, check for a currency-specific pattern. String pattern = null; - if (isCurrency) { - CurrencyFormatInfo info = CurrencyData.provider.getInstance(macros.loc, true) - .getFormatInfo(currency.getCurrencyCode()); - if (info != null) { - pattern = info.currencyPattern; - // It's clunky to clone an object here, but this code is not frequently executed. - micros.symbols = (DecimalFormatSymbols) micros.symbols.clone(); - micros.symbols.setMonetaryDecimalSeparatorString(info.monetaryDecimalSeparator); - micros.symbols.setMonetaryGroupingSeparatorString(info.monetaryGroupingSeparator); - } + if (isCurrency && micros.symbols.getCurrencyPattern() != null) { + pattern = micros.symbols.getCurrencyPattern(); } if (pattern == null) { int patternStyle; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java index 1bfeff4c8c8..a83bd1ba259 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormat.java @@ -787,7 +787,15 @@ public class DecimalFormat extends NumberFormat { */ @Override public StringBuffer format(CurrencyAmount currAmt, StringBuffer result, FieldPosition fieldPosition) { - FormattedNumber output = formatter.format(currAmt); + // We need to make localSymbols in order for monetary symbols to be initialized. + // Also, bypass the CurrencyAmount override of LocalizedNumberFormatter#format, + // because its caching mechanism will not provide any benefit here. + DecimalFormatSymbols localSymbols = (DecimalFormatSymbols) symbols.clone(); + localSymbols.setCurrency(currAmt.getCurrency()); + FormattedNumber output = formatter + .symbols(localSymbols) + .unit(currAmt.getCurrency()) + .format(currAmt.getNumber()); fieldPositionHelper(output, fieldPosition, result.length()); output.appendTo(result); return result; @@ -2043,11 +2051,8 @@ public class DecimalFormat extends NumberFormat { @Override public synchronized void setCurrency(Currency currency) { properties.setCurrency(currency); - // Backwards compatibility: also set the currency in the DecimalFormatSymbols if (currency != null) { symbols.setCurrency(currency); - String symbol = currency.getName(symbols.getULocale(), Currency.SYMBOL_NAME, null); - symbols.setCurrencySymbol(symbol); } refreshFormatter(); } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java index 8ad220ffa35..749971b937d 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/DecimalFormatSymbols.java @@ -888,9 +888,32 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { if (currency == null) { throw new NullPointerException(); } + if (currency.equals(this.currency)) { + return; + } + CurrencyDisplayInfo displayInfo = CurrencyData.provider.getInstance(ulocale, true); + setCurrencyOrNull(currency, displayInfo); + } + + private void setCurrencyOrNull(Currency currency, CurrencyDisplayInfo displayInfo) { this.currency = currency; + + if (currency == null) { + intlCurrencySymbol = "XXX"; + currencySymbol = "\u00A4"; // 'OX' currency symbol + currencyPattern = null; + return; + } + intlCurrencySymbol = currency.getCurrencyCode(); - currencySymbol = currency.getSymbol(requestedLocale); + currencySymbol = currency.getSymbol(ulocale); + + CurrencyFormatInfo formatInfo = displayInfo.getFormatInfo(currency.getCurrencyCode()); + if (formatInfo != null) { + setMonetaryDecimalSeparatorString(formatInfo.monetaryDecimalSeparator); + setMonetaryGroupingSeparatorString(formatInfo.monetaryGroupingSeparator); + currencyPattern = formatInfo.currencyPattern; + } } /** @@ -1004,11 +1027,12 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { } /** - } * Internal API for NumberFormat * @return String currency pattern string + * @deprecated This API is for ICU internal use only */ - String getCurrencyPattern() { + @Deprecated + public String getCurrencyPattern() { return currencyPattern; } @@ -1396,30 +1420,10 @@ public class DecimalFormatSymbols implements Cloneable, Serializable { padEscape = '*'; sigDigit = '@'; + CurrencyDisplayInfo displayInfo = CurrencyData.provider.getInstance(ulocale, true); + initSpacingInfo(displayInfo.getSpacingInfo()); - CurrencyDisplayInfo info = CurrencyData.provider.getInstance(locale, true); - - // Obtain currency data from the currency API. This is strictly - // for backward compatibility; we don't use DecimalFormatSymbols - // for currency data anymore. - currency = Currency.getInstance(locale); - if (currency != null) { - intlCurrencySymbol = currency.getCurrencyCode(); - currencySymbol = currency.getName(locale, Currency.SYMBOL_NAME, null); - CurrencyFormatInfo fmtInfo = info.getFormatInfo(intlCurrencySymbol); - if (fmtInfo != null) { - currencyPattern = fmtInfo.currencyPattern; - setMonetaryDecimalSeparatorString(fmtInfo.monetaryDecimalSeparator); - setMonetaryGroupingSeparatorString(fmtInfo.monetaryGroupingSeparator); - } - } else { - intlCurrencySymbol = "XXX"; - currencySymbol = "\u00A4"; // 'OX' currency symbol - } - - - // Get currency spacing data. - initSpacingInfo(info.getSpacingInfo()); + setCurrencyOrNull(Currency.getInstance(ulocale), displayInfo); } private static CacheData loadData(ULocale locale) { 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 42034df573c..5d35226a454 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 @@ -43,6 +43,7 @@ import org.junit.runners.JUnit4; import com.ibm.icu.dev.test.TestFmwk; import com.ibm.icu.dev.test.TestUtil; import com.ibm.icu.dev.test.format.IntlTestDecimalFormatAPIC.FieldContainer; +import com.ibm.icu.impl.DontCareFieldPosition; import com.ibm.icu.impl.ICUConfig; import com.ibm.icu.impl.LocaleUtility; import com.ibm.icu.impl.data.ResourceReader; @@ -6669,6 +6670,26 @@ public class NumberFormatTest extends TestFmwk { "$100.00", fmt.format(100)); } + @Test + public void test20956_MonetarySymbolGetters() { + Locale locale = new Locale.Builder().setLocale(Locale.forLanguageTag("et")).build(); + DecimalFormat decimalFormat = (DecimalFormat) NumberFormat.getCurrencyInstance(locale); + Currency currency = Currency.getInstance("EEK"); + + decimalFormat.setCurrency(currency); + + DecimalFormatSymbols decimalFormatSymbols = decimalFormat.getDecimalFormatSymbols(); + assertEquals("MONETARY DECIMAL SEPARATOR", ".", decimalFormatSymbols.getMonetaryDecimalSeparatorString()); + assertEquals("DECIMAL SEPARATOR", ",", decimalFormatSymbols.getDecimalSeparatorString()); + assertEquals("MONETARY GROUPING SEPARATOR", " ", decimalFormatSymbols.getMonetaryGroupingSeparatorString()); + assertEquals("GROUPING SEPARATOR", " ", decimalFormatSymbols.getGroupingSeparatorString()); + assertEquals("CURRENCY SYMBOL", "kr", decimalFormatSymbols.getCurrencySymbol()); + + StringBuffer sb = new StringBuffer(); + decimalFormat.format(new BigDecimal(12345.12), sb, DontCareFieldPosition.INSTANCE); + assertEquals("OUTPUT", "12 345.12 kr", sb.toString()); + } + @Test public void test20358_GroupingInPattern() { DecimalFormat fmt = (DecimalFormat) NumberFormat.getInstance(ULocale.ENGLISH);