ICU-20348 Fixing DecimalFormat set affix currency behavior.

- Includes minor changes to tests.
This commit is contained in:
Shane F. Carr 2019-01-14 13:27:45 -08:00
parent 33d7868d45
commit ac359112a1
11 changed files with 142 additions and 88 deletions

View file

@ -4,7 +4,7 @@ fr_CH{
NumberElements{
latn{
patterns{
currencyFormat{"#,##0.00 ¤;-#,##0.00 ¤"}
currencyFormat{"#,##0.00 ¤"}
percentFormat{"#,##0%"}
}
symbols{

View file

@ -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;

View file

@ -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 {

View file

@ -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;

View file

@ -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;

View file

@ -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<DecimalFormat> fmt(static_cast<DecimalFormat*>(
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 */

View file

@ -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);

View file

@ -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();

View file

@ -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

View file

@ -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));
}
}

View file

@ -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) {