diff --git a/icu4c/source/data/locales/fr_CH.txt b/icu4c/source/data/locales/fr_CH.txt index bc4ff1cb604..631a9a0275f 100644 --- a/icu4c/source/data/locales/fr_CH.txt +++ b/icu4c/source/data/locales/fr_CH.txt @@ -4,7 +4,7 @@ fr_CH{ NumberElements{ latn{ patterns{ - currencyFormat{"#,##0.00 ¤;-#,##0.00 ¤"} + currencyFormat{"#,##0.00 ¤"} percentFormat{"#,##0%"} } symbols{ diff --git a/icu4c/source/i18n/decimfmt.cpp b/icu4c/source/i18n/decimfmt.cpp index edd8910d9d4..0470db0ffa8 100644 --- a/icu4c/source/i18n/decimfmt.cpp +++ b/icu4c/source/i18n/decimfmt.cpp @@ -74,7 +74,8 @@ DecimalFormat::DecimalFormat(const UnicodeString& pattern, DecimalFormatSymbols* UNumberFormatStyle style, UErrorCode& status) : DecimalFormat(symbolsToAdopt, status) { // If choice is a currency type, ignore the rounding information. - if (style == UNumberFormatStyle::UNUM_CURRENCY || style == UNumberFormatStyle::UNUM_CURRENCY_ISO || + if (style == UNumberFormatStyle::UNUM_CURRENCY || + style == UNumberFormatStyle::UNUM_CURRENCY_ISO || style == UNumberFormatStyle::UNUM_CURRENCY_ACCOUNTING || style == UNumberFormatStyle::UNUM_CASH_CURRENCY || style == UNumberFormatStyle::UNUM_CURRENCY_STANDARD || @@ -933,12 +934,14 @@ UnicodeString& DecimalFormat::toPattern(UnicodeString& result) const { // TODO: Consider putting this logic in number_patternstring.cpp instead. ErrorCode localStatus; DecimalFormatProperties tprops(*fields->properties); - bool useCurrency = ((!tprops.currency.isNull()) || !tprops.currencyPluralInfo.fPtr.isNull() || - !tprops.currencyUsage.isNull() || AffixUtils::hasCurrencySymbols( - tprops.positivePrefixPattern, localStatus) || AffixUtils::hasCurrencySymbols( - tprops.positiveSuffixPattern, localStatus) || AffixUtils::hasCurrencySymbols( - tprops.negativePrefixPattern, localStatus) || AffixUtils::hasCurrencySymbols( - tprops.negativeSuffixPattern, localStatus)); + bool useCurrency = ( + !tprops.currency.isNull() || + !tprops.currencyPluralInfo.fPtr.isNull() || + !tprops.currencyUsage.isNull() || + AffixUtils::hasCurrencySymbols(tprops.positivePrefixPattern, localStatus) || + AffixUtils::hasCurrencySymbols(tprops.positiveSuffixPattern, localStatus) || + AffixUtils::hasCurrencySymbols(tprops.negativePrefixPattern, localStatus) || + AffixUtils::hasCurrencySymbols(tprops.negativeSuffixPattern, localStatus)); if (useCurrency) { tprops.minimumFractionDigits = fields->exportedProperties->minimumFractionDigits; tprops.maximumFractionDigits = fields->exportedProperties->maximumFractionDigits; diff --git a/icu4c/source/i18n/number_mapper.cpp b/icu4c/source/i18n/number_mapper.cpp index 83a72895a97..5dca9df310c 100644 --- a/icu4c/source/i18n/number_mapper.cpp +++ b/icu4c/source/i18n/number_mapper.cpp @@ -80,8 +80,10 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert /////////// bool useCurrency = ( - !properties.currency.isNull() || !properties.currencyPluralInfo.fPtr.isNull() || - !properties.currencyUsage.isNull() || affixProvider->hasCurrencySign()); + !properties.currency.isNull() || + !properties.currencyPluralInfo.fPtr.isNull() || + !properties.currencyUsage.isNull() || + affixProvider->hasCurrencySign()); CurrencyUnit currency = resolveCurrency(properties, locale, status); UCurrencyUsage currencyUsage = properties.currencyUsage.getOrDefault(UCURR_USAGE_STANDARD); if (useCurrency) { @@ -317,7 +319,7 @@ MacroProps NumberPropertyMapper::oldToNew(const DecimalFormatProperties& propert } -void PropertiesAffixPatternProvider::setTo(const DecimalFormatProperties& properties, UErrorCode&) { +void PropertiesAffixPatternProvider::setTo(const DecimalFormatProperties& properties, UErrorCode& status) { fBogus = false; // There are two ways to set affixes in DecimalFormat: via the pattern string (applyPattern), and via the @@ -327,9 +329,7 @@ void PropertiesAffixPatternProvider::setTo(const DecimalFormatProperties& proper // 2) Otherwise, follows UTS 35 rules based on the pattern string. // // Importantly, the explicit setters affect only the one field they override. If you set the positive - // prefix, that should not affect the negative prefix. Since it is impossible for the user of this class - // to know whether the origin for a string was the override or the pattern, we have to say that we always - // have a negative subpattern and perform all resolution logic here. + // prefix, that should not affect the negative prefix. // Convenience: Extract the properties into local variables. // Variables are named with three chars: [p/n][p/s][o/p] @@ -381,6 +381,14 @@ void PropertiesAffixPatternProvider::setTo(const DecimalFormatProperties& proper // UTS 35: Default negative prefix is the positive prefix. negSuffix = psp.isBogus() ? u"" : psp; } + + // For declaring if this is a currency pattern, we need to look at the + // original pattern, not at any user-specified overrides. + isCurrencyPattern = ( + AffixUtils::hasCurrencySymbols(ppp, status) || + AffixUtils::hasCurrencySymbols(psp, status) || + AffixUtils::hasCurrencySymbols(npp, status) || + AffixUtils::hasCurrencySymbols(nsp, status)); } char16_t PropertiesAffixPatternProvider::charAt(int flags, int i) const { @@ -417,8 +425,11 @@ bool PropertiesAffixPatternProvider::positiveHasPlusSign() const { } bool PropertiesAffixPatternProvider::hasNegativeSubpattern() const { - // See comments in the constructor for more information on why this is always true. - return true; + return ( + (negSuffix != posSuffix) || + negPrefix.tempSubString(1) != posPrefix || + negPrefix.charAt(0) != u'-' + ); } bool PropertiesAffixPatternProvider::negativeHasMinusSign() const { @@ -428,11 +439,7 @@ bool PropertiesAffixPatternProvider::negativeHasMinusSign() const { } bool PropertiesAffixPatternProvider::hasCurrencySign() const { - ErrorCode localStatus; - return AffixUtils::hasCurrencySymbols(posPrefix, localStatus) || - AffixUtils::hasCurrencySymbols(posSuffix, localStatus) || - AffixUtils::hasCurrencySymbols(negPrefix, localStatus) || - AffixUtils::hasCurrencySymbols(negSuffix, localStatus); + return isCurrencyPattern; } bool PropertiesAffixPatternProvider::containsSymbolType(AffixPatternType type, UErrorCode& status) const { diff --git a/icu4c/source/i18n/number_mapper.h b/icu4c/source/i18n/number_mapper.h index 82c5711c8d0..2f06fc85484 100644 --- a/icu4c/source/i18n/number_mapper.h +++ b/icu4c/source/i18n/number_mapper.h @@ -63,6 +63,7 @@ class PropertiesAffixPatternProvider : public AffixPatternProvider, public UMemo UnicodeString posSuffix; UnicodeString negPrefix; UnicodeString negSuffix; + bool isCurrencyPattern; const UnicodeString& getStringInternal(int32_t flags) const; diff --git a/icu4c/source/i18n/number_patternstring.cpp b/icu4c/source/i18n/number_patternstring.cpp index 37e107a6688..2e04333af6b 100644 --- a/icu4c/source/i18n/number_patternstring.cpp +++ b/icu4c/source/i18n/number_patternstring.cpp @@ -15,6 +15,7 @@ #include "unicode/utf16.h" #include "number_utils.h" #include "number_roundingutils.h" +#include "number_mapper.h" using namespace icu; using namespace icu::number; @@ -664,20 +665,11 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP bool alwaysShowDecimal = properties.decimalSeparatorAlwaysShown; int exponentDigits = uprv_min(properties.minimumExponentDigits, dosMax); bool exponentShowPlusSign = properties.exponentSignAlwaysShown; - UnicodeString pp = properties.positivePrefix; - UnicodeString ppp = properties.positivePrefixPattern; - UnicodeString ps = properties.positiveSuffix; - UnicodeString psp = properties.positiveSuffixPattern; - UnicodeString np = properties.negativePrefix; - UnicodeString npp = properties.negativePrefixPattern; - UnicodeString ns = properties.negativeSuffix; - UnicodeString nsp = properties.negativeSuffixPattern; + + PropertiesAffixPatternProvider affixes(properties, status); // Prefixes - if (!ppp.isBogus()) { - sb.append(ppp); - } - sb.append(AffixUtils::escape(pp)); + sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_PREFIX)); int afterPrefixPos = sb.length(); // Figure out the grouping sizes. @@ -771,10 +763,7 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP // Suffixes int beforeSuffixPos = sb.length(); - if (!psp.isBogus()) { - sb.append(psp); - } - sb.append(AffixUtils::escape(ps)); + sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_SUFFIX)); // Resolve Padding if (paddingWidth != -1 && !paddingLocation.isNull()) { @@ -810,23 +799,16 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP // Negative affixes // Ignore if the negative prefix pattern is "-" and the negative suffix is empty - if (!np.isBogus() || !ns.isBogus() || (npp.isBogus() && !nsp.isBogus()) || - (!npp.isBogus() && (npp.length() != 1 || npp.charAt(0) != u'-' || nsp.length() != 0))) { + if (affixes.hasNegativeSubpattern()) { sb.append(u';'); - if (!npp.isBogus()) { - sb.append(npp); - } - sb.append(AffixUtils::escape(np)); + sb.append(affixes.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); - if (!nsp.isBogus()) { - sb.append(nsp); - } - sb.append(AffixUtils::escape(ns)); + sb.append(affixes.getString(AffixPatternProvider::AFFIX_NEG_SUFFIX)); } return sb; diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 3594c730d18..3cb3ea9af17 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -220,6 +220,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n TESTCASE_AUTO(Test20037_ScientificIntegerOverflow); TESTCASE_AUTO(Test13840_ParseLongStringCrash); TESTCASE_AUTO(Test13850_EmptyStringCurrency); + TESTCASE_AUTO(Test20348_CurrencyPrefixOverride); TESTCASE_AUTO_END; } @@ -9328,4 +9329,40 @@ void NumberFormatTest::Test13850_EmptyStringCurrency() { } } +void NumberFormatTest::Test20348_CurrencyPrefixOverride() { + IcuTestErrorCode status(*this, "Test20348_CurrencyPrefixOverride"); + // LocalUNumberFormatPointer fmt(unum_open(UNUM_CURRENCY, NULL, 0, "en", NULL, status)); + LocalPointer fmt(static_cast( + NumberFormat::createCurrencyInstance("en", status))); + UnicodeString result; + assertEquals("Initial pattern", + u"¤#,##0.00", fmt->toPattern(result.remove())); + assertEquals("Initial prefix", + u"¤", fmt->getPositivePrefix(result.remove())); + assertEquals("Initial suffix", + u"-¤", fmt->getNegativePrefix(result.remove())); + assertEquals("Initial format", + u"\u00A4100.00", fmt->format(100, result.remove(), NULL, status)); + + fmt->setPositivePrefix(u"$"); + assertEquals("Set positive prefix pattern", + u"$#,##0.00;-\u00A4#,##0.00", fmt->toPattern(result.remove())); + assertEquals("Set positive prefix prefix", + u"$", fmt->getPositivePrefix(result.remove())); + assertEquals("Set positive prefix suffix", + u"-¤", fmt->getNegativePrefix(result.remove())); + assertEquals("Set positive prefix format", + u"$100.00", fmt->format(100, result.remove(), NULL, status)); + + fmt->setNegativePrefix(u"-$"); + assertEquals("Set negative prefix pattern", + u"$#,##0.00;'-'$#,##0.00", fmt->toPattern(result.remove())); + assertEquals("Set negative prefix prefix", + u"$", fmt->getPositivePrefix(result.remove())); + assertEquals("Set negative prefix suffix", + u"-$", fmt->getNegativePrefix(result.remove())); + assertEquals("Set negative prefix format", + u"$100.00", fmt->format(100, result.remove(), NULL, status)); +} + #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h index 5a07a9a1808..c2d9e83b8f5 100644 --- a/icu4c/source/test/intltest/numfmtst.h +++ b/icu4c/source/test/intltest/numfmtst.h @@ -285,6 +285,7 @@ class NumberFormatTest: public CalendarTimeZoneTest { void Test20037_ScientificIntegerOverflow(); void Test13840_ParseLongStringCrash(); void Test13850_EmptyStringCurrency(); + void Test20348_CurrencyPrefixOverride(); 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 d423c68b89a..d5c76a20e7a 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 @@ -46,20 +46,10 @@ public class PatternStringUtils { boolean alwaysShowDecimal = properties.getDecimalSeparatorAlwaysShown(); int exponentDigits = Math.min(properties.getMinimumExponentDigits(), dosMax); boolean exponentShowPlusSign = properties.getExponentSignAlwaysShown(); - String pp = properties.getPositivePrefix(); - String ppp = properties.getPositivePrefixPattern(); - String ps = properties.getPositiveSuffix(); - String psp = properties.getPositiveSuffixPattern(); - String np = properties.getNegativePrefix(); - String npp = properties.getNegativePrefixPattern(); - String ns = properties.getNegativeSuffix(); - String nsp = properties.getNegativeSuffixPattern(); + PropertiesAffixPatternProvider affixes = new PropertiesAffixPatternProvider(properties); // Prefixes - if (ppp != null) { - sb.append(ppp); - } - AffixUtils.escape(pp, sb); + sb.append(affixes.getString(AffixPatternProvider.FLAG_POS_PREFIX)); int afterPrefixPos = sb.length(); // Figure out the grouping sizes. @@ -150,10 +140,7 @@ public class PatternStringUtils { // Suffixes int beforeSuffixPos = sb.length(); - if (psp != null) { - sb.append(psp); - } - AffixUtils.escape(ps, sb); + sb.append(affixes.getString(AffixPatternProvider.FLAG_POS_SUFFIX)); // Resolve Padding if (paddingWidth != -1) { @@ -188,20 +175,13 @@ public class PatternStringUtils { // Negative affixes // Ignore if the negative prefix pattern is "-" and the negative suffix is empty - if (np != null - || ns != null - || (npp == null && nsp != null) - || (npp != null && (npp.length() != 1 || npp.charAt(0) != '-' || nsp.length() != 0))) { + if (affixes.hasNegativeSubpattern()) { sb.append(';'); - if (npp != null) - sb.append(npp); - AffixUtils.escape(np, sb); + sb.append(affixes.getString(AffixPatternProvider.FLAG_NEG_PREFIX)); // Copy the positive digit format into the negative. // This is optional; the pattern is the same as if '#' were appended here instead. sb.append(sb, afterPrefixPos, beforeSuffixPos); - if (nsp != null) - sb.append(nsp); - AffixUtils.escape(ns, sb); + sb.append(affixes.getString(AffixPatternProvider.FLAG_NEG_SUFFIX)); } return sb.toString(); 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 6d2eab65c80..e55e9701426 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 @@ -7,6 +7,7 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { private final String posSuffix; private final String negPrefix; private final String negSuffix; + private final boolean isCurrencyPattern; public PropertiesAffixPatternProvider(DecimalFormatProperties properties) { // There are two ways to set affixes in DecimalFormat: via the pattern string (applyPattern), and via the @@ -16,9 +17,7 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { // 2) Otherwise, follows UTS 35 rules based on the pattern string. // // Importantly, the explicit setters affect only the one field they override. If you set the positive - // prefix, that should not affect the negative prefix. Since it is impossible for the user of this class - // to know whether the origin for a string was the override or the pattern, we have to say that we always - // have a negative subpattern and perform all resolution logic here. + // prefix, that should not affect the negative prefix. // Convenience: Extract the properties into local variables. // Variables are named with three chars: [p/n][p/s][o/p] @@ -70,6 +69,14 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { // UTS 35: Default negative prefix is the positive prefix. negSuffix = psp == null ? "" : psp; } + + // For declaring if this is a currency pattern, we need to look at the + // original pattern, not at any user-specified overrides. + isCurrencyPattern = ( + AffixUtils.hasCurrencySymbols(ppp) || + AffixUtils.hasCurrencySymbols(psp) || + AffixUtils.hasCurrencySymbols(npp) || + AffixUtils.hasCurrencySymbols(nsp)); } @Override @@ -105,8 +112,12 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { @Override public boolean hasNegativeSubpattern() { - // See comments in the constructor for more information on why this is always true. - return true; + return ( + negSuffix != posSuffix || + negPrefix.length() != posPrefix.length() + 1 || + !negPrefix.regionMatches(1, posPrefix, 0, posPrefix.length()) || + negPrefix.charAt(0) != '-' + ); } @Override @@ -117,8 +128,7 @@ public class PropertiesAffixPatternProvider implements AffixPatternProvider { @Override public boolean hasCurrencySign() { - return AffixUtils.hasCurrencySymbols(posPrefix) || AffixUtils.hasCurrencySymbols(posSuffix) - || AffixUtils.hasCurrencySymbols(negPrefix) || AffixUtils.hasCurrencySymbols(negSuffix); + return isCurrencyPattern; } @Override 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 dce8d327b4d..63a52e846c8 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 @@ -6373,4 +6373,37 @@ public class NumberFormatTest extends TestFmwk { assertEquals("Should round-trip without crashing", expectedUString, actualUString); } + + @Test + public void test20348_CurrencyPrefixOverride() { + DecimalFormat fmt = (DecimalFormat) NumberFormat.getCurrencyInstance(ULocale.ENGLISH); + assertEquals("Initial pattern", + "¤#,##0.00", fmt.toPattern()); + assertEquals("Initial prefix", + "¤", fmt.getPositivePrefix()); + assertEquals("Initial suffix", + "-¤", fmt.getNegativePrefix()); + assertEquals("Initial format", + "\u00A4100.00", fmt.format(100)); + + fmt.setPositivePrefix("$"); + assertEquals("Set positive prefix pattern", + "$#,##0.00;-\u00A4#,##0.00", fmt.toPattern()); + assertEquals("Set positive prefix prefix", + "$", fmt.getPositivePrefix()); + assertEquals("Set positive prefix suffix", + "-¤", fmt.getNegativePrefix()); + assertEquals("Set positive prefix format", + "$100.00", fmt.format(100)); + + fmt.setNegativePrefix("-$"); + assertEquals("Set negative prefix pattern", + "$#,##0.00;'-'$#,##0.00", fmt.toPattern()); + assertEquals("Set negative prefix prefix", + "$", fmt.getPositivePrefix()); + assertEquals("Set negative prefix suffix", + "-$", fmt.getNegativePrefix()); + assertEquals("Set negative prefix format", + "$100.00", fmt.format(100)); + } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/PatternStringTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/PatternStringTest.java index e5823804f28..aaf2d8f4962 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/PatternStringTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/PatternStringTest.java @@ -72,19 +72,19 @@ public class PatternStringTest { @Test public void testToPatternWithProperties() { Object[][] cases = { - { new DecimalFormatProperties().setPositivePrefix("abc"), "abc#" }, - { new DecimalFormatProperties().setPositiveSuffix("abc"), "#abc" }, + { new DecimalFormatProperties().setPositivePrefix("abc"), "abc#;-#" }, + { new DecimalFormatProperties().setPositiveSuffix("abc"), "#abc;-#" }, { new DecimalFormatProperties().setPositivePrefixPattern("abc"), "abc#" }, { new DecimalFormatProperties().setPositiveSuffixPattern("abc"), "#abc" }, { new DecimalFormatProperties().setNegativePrefix("abc"), "#;abc#" }, - { new DecimalFormatProperties().setNegativeSuffix("abc"), "#;#abc" }, + { new DecimalFormatProperties().setNegativeSuffix("abc"), "#;-#abc" }, { new DecimalFormatProperties().setNegativePrefixPattern("abc"), "#;abc#" }, - { new DecimalFormatProperties().setNegativeSuffixPattern("abc"), "#;#abc" }, - { new DecimalFormatProperties().setPositivePrefix("+"), "'+'#" }, + { new DecimalFormatProperties().setNegativeSuffixPattern("abc"), "#;-#abc" }, + { new DecimalFormatProperties().setPositivePrefix("+"), "'+'#;-#" }, { new DecimalFormatProperties().setPositivePrefixPattern("+"), "+#" }, - { new DecimalFormatProperties().setPositivePrefix("+'"), "'+'''#" }, - { new DecimalFormatProperties().setPositivePrefix("'+"), "'''+'#" }, - { new DecimalFormatProperties().setPositivePrefix("'"), "''#" }, + { new DecimalFormatProperties().setPositivePrefix("+'"), "'+'''#;-#" }, + { new DecimalFormatProperties().setPositivePrefix("'+"), "'''+'#;-#" }, + { new DecimalFormatProperties().setPositivePrefix("'"), "''#;-#" }, { new DecimalFormatProperties().setPositivePrefixPattern("+''"), "+''#" }, }; for (Object[] cas : cases) {