ICU-20358 Clean up grouping resolution in DecimalFormat#toPattern().

This commit is contained in:
Shane Carr 2019-01-17 14:57:42 -08:00 committed by Shane F. Carr
parent e96724b52f
commit 1f7d8ababe
5 changed files with 117 additions and 69 deletions

View file

@ -650,54 +650,42 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP
// Convenience references
// The uprv_min() calls prevent DoS
int dosMax = 100;
int groupingSize = uprv_min(properties.secondaryGroupingSize, dosMax);
int firstGroupingSize = uprv_min(properties.groupingSize, dosMax);
int paddingWidth = uprv_min(properties.formatWidth, dosMax);
int32_t dosMax = 100;
int32_t grouping1 = uprv_max(0, uprv_min(properties.groupingSize, dosMax));
int32_t grouping2 = uprv_max(0, uprv_min(properties.secondaryGroupingSize, dosMax));
bool useGrouping = properties.groupingUsed;
int32_t paddingWidth = uprv_min(properties.formatWidth, dosMax);
NullableValue<PadPosition> paddingLocation = properties.padPosition;
UnicodeString paddingString = properties.padString;
int minInt = uprv_max(uprv_min(properties.minimumIntegerDigits, dosMax), 0);
int maxInt = uprv_min(properties.maximumIntegerDigits, dosMax);
int minFrac = uprv_max(uprv_min(properties.minimumFractionDigits, dosMax), 0);
int maxFrac = uprv_min(properties.maximumFractionDigits, dosMax);
int minSig = uprv_min(properties.minimumSignificantDigits, dosMax);
int maxSig = uprv_min(properties.maximumSignificantDigits, dosMax);
int32_t minInt = uprv_max(0, uprv_min(properties.minimumIntegerDigits, dosMax));
int32_t maxInt = uprv_min(properties.maximumIntegerDigits, dosMax);
int32_t minFrac = uprv_max(0, uprv_min(properties.minimumFractionDigits, dosMax));
int32_t maxFrac = uprv_min(properties.maximumFractionDigits, dosMax);
int32_t minSig = uprv_min(properties.minimumSignificantDigits, dosMax);
int32_t maxSig = uprv_min(properties.maximumSignificantDigits, dosMax);
bool alwaysShowDecimal = properties.decimalSeparatorAlwaysShown;
int exponentDigits = uprv_min(properties.minimumExponentDigits, dosMax);
int32_t exponentDigits = uprv_min(properties.minimumExponentDigits, dosMax);
bool exponentShowPlusSign = properties.exponentSignAlwaysShown;
PropertiesAffixPatternProvider affixes(properties, status);
// Prefixes
sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_PREFIX));
int afterPrefixPos = sb.length();
int32_t afterPrefixPos = sb.length();
// Figure out the grouping sizes.
int grouping1, grouping2, grouping;
if (groupingSize != uprv_min(dosMax, -1) && firstGroupingSize != uprv_min(dosMax, -1) &&
groupingSize != firstGroupingSize) {
grouping = groupingSize;
grouping1 = groupingSize;
grouping2 = firstGroupingSize;
} else if (groupingSize != uprv_min(dosMax, -1)) {
grouping = groupingSize;
grouping1 = 0;
grouping2 = groupingSize;
} else if (firstGroupingSize != uprv_min(dosMax, -1)) {
grouping = groupingSize;
grouping1 = 0;
grouping2 = firstGroupingSize;
} else {
grouping = 0;
if (!useGrouping) {
grouping1 = 0;
grouping2 = 0;
} else if (grouping1 == grouping2) {
grouping1 = 0;
}
int groupingLength = grouping1 + grouping2 + 1;
int32_t groupingLength = grouping1 + grouping2 + 1;
// Figure out the digits we need to put in the pattern.
double roundingInterval = properties.roundingIncrement;
UnicodeString digitsString;
int digitsStringScale = 0;
int32_t digitsStringScale = 0;
if (maxSig != uprv_min(dosMax, -1)) {
// Significant Digits.
while (digitsString.length() < minSig) {
@ -731,23 +719,31 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP
}
// Write the digits to the string builder
int m0 = uprv_max(groupingLength, digitsString.length() + digitsStringScale);
int32_t m0 = uprv_max(groupingLength, digitsString.length() + digitsStringScale);
m0 = (maxInt != dosMax) ? uprv_max(maxInt, m0) - 1 : m0 - 1;
int mN = (maxFrac != dosMax) ? uprv_min(-maxFrac, digitsStringScale) : digitsStringScale;
for (int magnitude = m0; magnitude >= mN; magnitude--) {
int di = digitsString.length() + digitsStringScale - magnitude - 1;
int32_t mN = (maxFrac != dosMax) ? uprv_min(-maxFrac, digitsStringScale) : digitsStringScale;
for (int32_t magnitude = m0; magnitude >= mN; magnitude--) {
int32_t di = digitsString.length() + digitsStringScale - magnitude - 1;
if (di < 0 || di >= digitsString.length()) {
sb.append(u'#');
} else {
sb.append(digitsString.charAt(di));
}
if (magnitude > grouping2 && grouping > 0 && (magnitude - grouping2) % grouping == 0) {
sb.append(u',');
} else if (magnitude > 0 && magnitude == grouping2) {
sb.append(u',');
} else if (magnitude == 0 && (alwaysShowDecimal || mN < 0)) {
// Decimal separator
if (magnitude == 0 && (alwaysShowDecimal || mN < 0)) {
sb.append(u'.');
}
if (!useGrouping) {
continue;
}
// Least-significant grouping separator
if (magnitude > 0 && magnitude == grouping1) {
sb.append(u',');
}
// All other grouping separators
if (magnitude > grouping1 && grouping2 > 0 && (magnitude - grouping1) % grouping2 == 0) {
sb.append(u',');
}
}
// Exponential notation
@ -756,13 +752,13 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP
if (exponentShowPlusSign) {
sb.append(u'+');
}
for (int i = 0; i < exponentDigits; i++) {
for (int32_t i = 0; i < exponentDigits; i++) {
sb.append(u'0');
}
}
// Suffixes
int beforeSuffixPos = sb.length();
int32_t beforeSuffixPos = sb.length();
sb.append(affixes.getString(AffixPatternProvider::AFFIX_POS_SUFFIX));
// Resolve Padding
@ -771,7 +767,7 @@ UnicodeString PatternStringUtils::propertiesToPatternString(const DecimalFormatP
sb.insert(afterPrefixPos, u'#');
beforeSuffixPos++;
}
int addedLength;
int32_t addedLength;
switch (paddingLocation.get(status)) {
case PadPosition::UNUM_PAD_BEFORE_PREFIX:
addedLength = escapePaddingString(paddingString, sb, 0, status);

View file

@ -221,6 +221,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(Test20358_GroupingInPattern);
TESTCASE_AUTO_END;
}
@ -9385,7 +9386,6 @@ 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;
@ -9419,4 +9419,33 @@ void NumberFormatTest::Test20348_CurrencyPrefixOverride() {
u"$100.00", fmt->format(100, result.remove(), NULL, status));
}
void NumberFormatTest::Test20358_GroupingInPattern() {
IcuTestErrorCode status(*this, "Test20358_GroupingInPattern");
LocalPointer<DecimalFormat> fmt(static_cast<DecimalFormat*>(
NumberFormat::createInstance("en", status)));
UnicodeString result;
assertEquals("Initial pattern",
u"#,##0.###", fmt->toPattern(result.remove()));
assertTrue("Initial grouping",
fmt->isGroupingUsed());
assertEquals("Initial format",
u"54,321", fmt->format(54321, result.remove(), NULL, status));
fmt->setGroupingUsed(false);
assertEquals("Set grouping false",
u"0.###", fmt->toPattern(result.remove()));
assertFalse("Set grouping false grouping",
fmt->isGroupingUsed());
assertEquals("Set grouping false format",
u"54321", fmt->format(54321, result.remove(), NULL, status));
fmt->setGroupingUsed(true);
assertEquals("Set grouping true",
u"#,##0.###", fmt->toPattern(result.remove()));
assertTrue("Set grouping true grouping",
fmt->isGroupingUsed());
assertEquals("Set grouping true format",
u"54,321", fmt->format(54321, result.remove(), NULL, status));
}
#endif /* #if !UCONFIG_NO_FORMATTING */

View file

@ -286,6 +286,7 @@ class NumberFormatTest: public CalendarTimeZoneTest {
void Test13840_ParseLongStringCrash();
void Test13850_EmptyStringCurrency();
void Test20348_CurrencyPrefixOverride();
void Test20358_GroupingInPattern();
private:
UBool testFormattableAsUFormattable(const char *file, int line, Formattable &f);

View file

@ -32,14 +32,15 @@ public class PatternStringUtils {
// Convenience references
// The Math.min() calls prevent DoS
int dosMax = 100;
int groupingSize = Math.min(properties.getSecondaryGroupingSize(), dosMax);
int firstGroupingSize = Math.min(properties.getGroupingSize(), dosMax);
int grouping1 = Math.max(0, Math.min(properties.getGroupingSize(), dosMax));
int grouping2 = Math.max(0, Math.min(properties.getSecondaryGroupingSize(), dosMax));
boolean useGrouping = properties.getGroupingUsed();
int paddingWidth = Math.min(properties.getFormatWidth(), dosMax);
PadPosition paddingLocation = properties.getPadPosition();
String paddingString = properties.getPadString();
int minInt = Math.max(Math.min(properties.getMinimumIntegerDigits(), dosMax), 0);
int minInt = Math.max(0, Math.min(properties.getMinimumIntegerDigits(), dosMax));
int maxInt = Math.min(properties.getMaximumIntegerDigits(), dosMax);
int minFrac = Math.max(Math.min(properties.getMinimumFractionDigits(), dosMax), 0);
int minFrac = Math.max(0, Math.min(properties.getMinimumFractionDigits(), dosMax));
int maxFrac = Math.min(properties.getMaximumFractionDigits(), dosMax);
int minSig = Math.min(properties.getMinimumSignificantDigits(), dosMax);
int maxSig = Math.min(properties.getMaximumSignificantDigits(), dosMax);
@ -53,25 +54,11 @@ public class PatternStringUtils {
int afterPrefixPos = sb.length();
// Figure out the grouping sizes.
int grouping1, grouping2, grouping;
if (groupingSize != Math.min(dosMax, -1)
&& firstGroupingSize != Math.min(dosMax, -1)
&& groupingSize != firstGroupingSize) {
grouping = groupingSize;
grouping1 = groupingSize;
grouping2 = firstGroupingSize;
} else if (groupingSize != Math.min(dosMax, -1)) {
grouping = groupingSize;
grouping1 = 0;
grouping2 = groupingSize;
} else if (firstGroupingSize != Math.min(dosMax, -1)) {
grouping = groupingSize;
grouping1 = 0;
grouping2 = firstGroupingSize;
} else {
grouping = 0;
if (!useGrouping) {
grouping1 = 0;
grouping2 = 0;
} else if (grouping1 == grouping2) {
grouping1 = 0;
}
int groupingLength = grouping1 + grouping2 + 1;
@ -118,13 +105,21 @@ public class PatternStringUtils {
} else {
sb.append(digitsString.charAt(di));
}
if (magnitude > grouping2 && grouping > 0 && (magnitude - grouping2) % grouping == 0) {
sb.append(',');
} else if (magnitude > 0 && magnitude == grouping2) {
sb.append(',');
} else if (magnitude == 0 && (alwaysShowDecimal || mN < 0)) {
// Decimal separator
if (magnitude == 0 && (alwaysShowDecimal || mN < 0)) {
sb.append('.');
}
if (!useGrouping) {
continue;
}
// Least-significant grouping separator
if (magnitude > 0 && magnitude == grouping1) {
sb.append(',');
}
// All other grouping separators
if (magnitude > grouping1 && grouping2 > 0 && (magnitude - grouping1) % grouping2 == 0) {
sb.append(',');
}
}
// Exponential notation

View file

@ -6406,4 +6406,31 @@ public class NumberFormatTest extends TestFmwk {
assertEquals("Set negative prefix format",
"$100.00", fmt.format(100));
}
@Test
public void test20358_GroupingInPattern() {
DecimalFormat fmt = (DecimalFormat) NumberFormat.getInstance(ULocale.ENGLISH);
assertEquals("Initial pattern",
"#,##0.###", fmt.toPattern());
assertTrue("Initial grouping",
fmt.isGroupingUsed());
assertEquals("Initial format",
"54,321", fmt.format(54321));
fmt.setGroupingUsed(false);
assertEquals("Set grouping false",
"0.###", fmt.toPattern());
assertFalse("Set grouping false grouping",
fmt.isGroupingUsed());
assertEquals("Set grouping false format",
"54321", fmt.format(54321));
fmt.setGroupingUsed(true);
assertEquals("Set grouping true",
"#,##0.###", fmt.toPattern());
assertTrue("Set grouping true grouping",
fmt.isGroupingUsed());
assertEquals("Set grouping true format",
"54,321", fmt.format(54321));
}
}