ICU-13525 Fixing NumberFormatter behavior when unit pattern does not contain an argument.

X-SVN-Rev: 40770
This commit is contained in:
Shane Carr 2018-01-10 02:44:23 +00:00
parent a111b4ed90
commit 0344ea5118
12 changed files with 301 additions and 48 deletions

View File

@ -233,7 +233,7 @@ void LongNameHandler::simpleFormatsToModifiers(const UnicodeString *simpleFormat
for (int32_t i = 0; i < StandardPlural::Form::COUNT; i++) {
UnicodeString simpleFormat = getWithPlural(simpleFormats, i, status);
if (U_FAILURE(status)) { return; }
SimpleFormatter compiledFormatter(simpleFormat, 1, 1, status);
SimpleFormatter compiledFormatter(simpleFormat, 0, 1, status);
if (U_FAILURE(status)) { return; }
output[i] = SimpleModifier(compiledFormatter, field, false);
}
@ -249,7 +249,7 @@ void LongNameHandler::multiSimpleFormatsToModifiers(const UnicodeString *leadFor
UnicodeString compoundFormat;
trailCompiled.format(leadFormat, compoundFormat, status);
if (U_FAILURE(status)) { return; }
SimpleFormatter compoundCompiled(compoundFormat, 1, 1, status);
SimpleFormatter compoundCompiled(compoundFormat, 0, 1, status);
if (U_FAILURE(status)) { return; }
output[i] = SimpleModifier(compoundCompiled, field, false);
}

View File

@ -74,19 +74,29 @@ bool ConstantAffixModifier::isStrong() const {
SimpleModifier::SimpleModifier(const SimpleFormatter &simpleFormatter, Field field, bool strong)
: fCompiledPattern(simpleFormatter.compiledPattern), fField(field), fStrong(strong) {
U_ASSERT(1 ==
SimpleFormatter::getArgumentLimit(fCompiledPattern.getBuffer(), fCompiledPattern.length()));
if (fCompiledPattern.charAt(1) != 0) {
int32_t argLimit = SimpleFormatter::getArgumentLimit(
fCompiledPattern.getBuffer(), fCompiledPattern.length());
if (argLimit == 0) {
// No arguments in compiled pattern
fPrefixLength = fCompiledPattern.charAt(1) - ARG_NUM_LIMIT;
fSuffixOffset = 3 + fPrefixLength;
} else {
fPrefixLength = 0;
fSuffixOffset = 2;
}
if (3 + fPrefixLength < fCompiledPattern.length()) {
fSuffixLength = fCompiledPattern.charAt(fSuffixOffset) - ARG_NUM_LIMIT;
} else {
U_ASSERT(2 + fPrefixLength == fCompiledPattern.length());
// Set suffixOffset = -1 to indicate no arguments in compiled pattern.
fSuffixOffset = -1;
fSuffixLength = 0;
} else {
U_ASSERT(argLimit == 1);
if (fCompiledPattern.charAt(1) != 0) {
fPrefixLength = fCompiledPattern.charAt(1) - ARG_NUM_LIMIT;
fSuffixOffset = 3 + fPrefixLength;
} else {
fPrefixLength = 0;
fSuffixOffset = 2;
}
if (3 + fPrefixLength < fCompiledPattern.length()) {
fSuffixLength = fCompiledPattern.charAt(fSuffixOffset) - ARG_NUM_LIMIT;
} else {
fSuffixLength = 0;
}
}
}
@ -123,19 +133,24 @@ bool SimpleModifier::isStrong() const {
int32_t
SimpleModifier::formatAsPrefixSuffix(NumberStringBuilder &result, int32_t startIndex, int32_t endIndex,
Field field, UErrorCode &status) const {
if (fPrefixLength > 0) {
result.insert(startIndex, fCompiledPattern, 2, 2 + fPrefixLength, field, status);
if (fSuffixOffset == -1) {
// There is no argument for the inner number; overwrite the entire segment with our string.
return result.splice(startIndex, endIndex, fCompiledPattern, 2, 2 + fPrefixLength, field, status);
} else {
if (fPrefixLength > 0) {
result.insert(startIndex, fCompiledPattern, 2, 2 + fPrefixLength, field, status);
}
if (fSuffixLength > 0) {
result.insert(
endIndex + fPrefixLength,
fCompiledPattern,
1 + fSuffixOffset,
1 + fSuffixOffset + fSuffixLength,
field,
status);
}
return fPrefixLength + fSuffixLength;
}
if (fSuffixLength > 0) {
result.insert(
endIndex + fPrefixLength,
fCompiledPattern,
1 + fSuffixOffset,
1 + fSuffixOffset + fSuffixLength,
field,
status);
}
return fPrefixLength + fSuffixLength;
}
int32_t ConstantMultiFieldModifier::apply(NumberStringBuilder &output, int leftIndex, int rightIndex,

View File

@ -191,6 +191,30 @@ NumberStringBuilder::insert(int32_t index, const UnicodeString &unistr, int32_t
return count;
}
int32_t
NumberStringBuilder::splice(int32_t startThis, int32_t endThis, const UnicodeString &unistr,
int32_t startOther, int32_t endOther, Field field, UErrorCode& status) {
int32_t thisLength = endThis - startThis;
int32_t otherLength = endOther - startOther;
int32_t count = otherLength - thisLength;
int32_t position;
if (count > 0) {
// Overall, chars need to be added.
position = prepareForInsert(startThis, count, status);
} else {
// Overall, chars need to be removed or kept the same.
position = remove(startThis, -count);
}
if (U_FAILURE(status)) {
return count;
}
for (int32_t i = 0; i < otherLength; i++) {
getCharPtr()[position + i] = unistr.charAt(startOther + i);
getFieldPtr()[position + i] = field;
}
return count;
}
int32_t NumberStringBuilder::append(const NumberStringBuilder &other, UErrorCode &status) {
return insert(fLength, other, status);
}
@ -296,6 +320,19 @@ int32_t NumberStringBuilder::prepareForInsertHelper(int32_t index, int32_t count
return fZero + index;
}
int32_t NumberStringBuilder::remove(int32_t index, int32_t count) {
// TODO: Reset the heap here? (If the string after removal can fit on stack?)
int32_t position = index + fZero;
uprv_memmove2(getCharPtr() + position,
getCharPtr() + position + count,
sizeof(char16_t) * (fLength - index - count));
uprv_memmove2(getFieldPtr() + position,
getFieldPtr() + position + count,
sizeof(Field) * (fLength - index - count));
fLength -= count;
return position;
}
UnicodeString NumberStringBuilder::toUnicodeString() const {
return UnicodeString(getCharPtr() + fZero, fLength);
}

View File

@ -77,6 +77,9 @@ class U_I18N_API NumberStringBuilder : public UMemory {
int32_t insert(int32_t index, const UnicodeString &unistr, int32_t start, int32_t end, Field field,
UErrorCode &status);
int32_t splice(int32_t startThis, int32_t endThis, const UnicodeString &unistr,
int32_t startOther, int32_t endOther, Field field, UErrorCode& status);
int32_t append(const NumberStringBuilder &other, UErrorCode &status);
int32_t insert(int32_t index, const NumberStringBuilder &other, UErrorCode &status);
@ -123,6 +126,8 @@ class U_I18N_API NumberStringBuilder : public UMemory {
int32_t prepareForInsert(int32_t index, int32_t count, UErrorCode &status);
int32_t prepareForInsertHelper(int32_t index, int32_t count, UErrorCode &status);
int32_t remove(int32_t index, int32_t count);
};
} // namespace impl

View File

@ -81,6 +81,7 @@ class NumberFormatterApiTest : public IntlTest {
MeasureUnit SQUARE_MILE;
MeasureUnit JOULE;
MeasureUnit FURLONG;
MeasureUnit KELVIN;
NumberingSystem MATHSANB;
NumberingSystem LATN;
@ -161,6 +162,7 @@ class PatternStringTest : public IntlTest {
class NumberStringBuilderTest : public IntlTest {
public:
void testInsertAppendUnicodeString();
void testSplice();
void testInsertAppendCodePoint();
void testCopy();
void testFields();

View File

@ -42,6 +42,7 @@ NumberFormatterApiTest::NumberFormatterApiTest(UErrorCode &status)
SQUARE_MILE = *LocalPointer<MeasureUnit>(MeasureUnit::createSquareMile(status));
JOULE = *LocalPointer<MeasureUnit>(MeasureUnit::createJoule(status));
FURLONG = *LocalPointer<MeasureUnit>(MeasureUnit::createFurlong(status));
KELVIN = *LocalPointer<MeasureUnit>(MeasureUnit::createKelvin(status));
MATHSANB = *LocalPointer<NumberingSystem>(NumberingSystem::createInstanceByName("mathsanb", status));
LATN = *LocalPointer<NumberingSystem>(NumberingSystem::createInstanceByName("latn", status));
@ -464,6 +465,25 @@ void NumberFormatterApiTest::unitMeasure() {
Locale("es-US"),
5.43,
u"5.43 °F");
assertFormatSingle(
u"MeasureUnit form without {0} in CLDR pattern",
NumberFormatter::with()
.unit(KELVIN)
.unitWidth(UNumberUnitWidth::UNUM_UNIT_WIDTH_FULL_NAME),
Locale("es-MX"),
1,
u"kelvin");
assertFormatSingle(
u"MeasureUnit form without {0} in CLDR pattern and wide base form",
NumberFormatter::with()
.rounding(Rounder::fixedFraction(20))
.unit(KELVIN)
.unitWidth(UNumberUnitWidth::UNUM_UNIT_WIDTH_FULL_NAME),
Locale("es-MX"),
1,
u"kelvin");
}
void NumberFormatterApiTest::unitCompoundMeasure() {

View File

@ -23,6 +23,7 @@ void NumberStringBuilderTest::runIndexedTest(int32_t index, UBool exec, const ch
}
TESTCASE_AUTO_BEGIN;
TESTCASE_AUTO(testInsertAppendUnicodeString);
TESTCASE_AUTO(testSplice);
TESTCASE_AUTO(testInsertAppendCodePoint);
TESTCASE_AUTO(testCopy);
TESTCASE_AUTO(testFields);
@ -75,6 +76,55 @@ void NumberStringBuilderTest::testInsertAppendUnicodeString() {
}
}
void NumberStringBuilderTest::testSplice() {
const struct TestCase {
const char16_t* input;
const int32_t startThis;
const int32_t endThis;
} cases[] = {
{ u"", 0, 0 },
{ u"abc", 0, 0 },
{ u"abc", 1, 1 },
{ u"abc", 1, 2 },
{ u"abc", 0, 2 },
{ u"abc", 0, 3 },
{ u"lorem ipsum dolor sit amet", 8, 8 },
{ u"lorem ipsum dolor sit amet", 8, 11 }, // 3 chars, equal to replacement "xyz"
{ u"lorem ipsum dolor sit amet", 8, 18 } }; // 10 chars, larger than several replacements
UErrorCode status = U_ZERO_ERROR;
UnicodeString sb1;
NumberStringBuilder sb2;
for (auto cas : cases) {
for (const char16_t* replacementPtr : EXAMPLE_STRINGS) {
UnicodeString replacement(replacementPtr);
// Test replacement with full string
sb1.remove();
sb1.append(cas.input);
sb1.replace(cas.startThis, cas.endThis - cas.startThis, replacement);
sb2.clear();
sb2.append(cas.input, UNUM_FIELD_COUNT, status);
sb2.splice(cas.startThis, cas.endThis, replacement, 0, replacement.length(), UNUM_FIELD_COUNT, status);
assertSuccess("Splicing into sb2 first time", status);
assertEqualsImpl(sb1, sb2);
// Test replacement with partial string
if (replacement.length() <= 2) {
continue;
}
sb1.remove();
sb1.append(cas.input);
sb1.replace(cas.startThis, cas.endThis - cas.startThis, UnicodeString(replacement, 1, 2));
sb2.clear();
sb2.append(cas.input, UNUM_FIELD_COUNT, status);
sb2.splice(cas.startThis, cas.endThis, replacement, 1, 3, UNUM_FIELD_COUNT, status);
assertSuccess("Splicing into sb2 second time", status);
assertEqualsImpl(sb1, sb2);
}
}
}
void NumberStringBuilderTest::testInsertAppendCodePoint() {
static const UChar32 cases[] = {
0, 1, 60, 127, 128, 0x7fff, 0x8000, 0xffff, 0x10000, 0x1f000, 0x10ffff};
@ -230,7 +280,8 @@ void NumberStringBuilderTest::assertEqualsImpl(const UnicodeString &a, const Num
for (int32_t i = 0; i < a.length(); i++) {
IntlTest::assertEquals(
UnicodeString(u"Char at position ") + Int64ToUnicodeString(i) +
UnicodeString(u" in string ") + a, a.charAt(i), b.charAt(i));
UnicodeString(u" in \"") + a + UnicodeString("\" versus \"") +
b.toUnicodeString() + UnicodeString("\""), a.charAt(i), b.charAt(i));
}
}

View File

@ -245,7 +245,7 @@ public class LongNameHandler implements MicroPropsGenerator {
StringBuilder sb = new StringBuilder();
for (StandardPlural plural : StandardPlural.VALUES) {
String simpleFormat = getWithPlural(simpleFormats, plural);
String compiled = SimpleFormatterImpl.compileToStringMinMaxArguments(simpleFormat, sb, 1, 1);
String compiled = SimpleFormatterImpl.compileToStringMinMaxArguments(simpleFormat, sb, 0, 1);
output.put(plural, new SimpleModifier(compiled, field, false));
}
}
@ -261,7 +261,7 @@ public class LongNameHandler implements MicroPropsGenerator {
String leadFormat = getWithPlural(leadFormats, plural);
String compoundFormat = SimpleFormatterImpl.formatCompiledPattern(trailCompiled, leadFormat);
String compoundCompiled = SimpleFormatterImpl
.compileToStringMinMaxArguments(compoundFormat, sb, 1, 1);
.compileToStringMinMaxArguments(compoundFormat, sb, 0, 1);
output.put(plural, new SimpleModifier(compoundCompiled, field, false));
}
}

View File

@ -171,6 +171,39 @@ public class NumberStringBuilder implements CharSequence {
return count;
}
/**
* Replaces the chars between startThis and endThis with the chars between startOther and endOther of
* the given CharSequence. Calling this method with startThis == endThis is equivalent to calling
* insert.
*
* @return The number of chars added, which may be negative if the removed segment is longer than the
* length of the CharSequence segment that was inserted.
*/
public int splice(
int startThis,
int endThis,
CharSequence sequence,
int startOther,
int endOther,
Field field) {
int thisLength = endThis - startThis;
int otherLength = endOther - startOther;
int count = otherLength - thisLength;
int position;
if (count > 0) {
// Overall, chars need to be added.
position = prepareForInsert(startThis, count);
} else {
// Overall, chars need to be removed or kept the same.
position = remove(startThis, -count);
}
for (int i = 0; i < otherLength; i++) {
chars[position + i] = sequence.charAt(startOther + i);
fields[position + i] = field;
}
return count;
}
/**
* Appends the chars in the specified char array to the end of the string, and associates them with
* the fields in the specified field array, which must have the same length as chars.
@ -313,6 +346,18 @@ public class NumberStringBuilder implements CharSequence {
return zero + index;
}
/**
* Removes the "count" chars starting at "index". Returns the position at which the chars were
* removed.
*/
private int remove(int index, int count) {
int position = index + zero;
System.arraycopy(chars, position + count, chars, position, length - index - count);
System.arraycopy(fields, position + count, fields, position, length - index - count);
length -= count;
return position;
}
private int getCapacity() {
return chars.length;
}

View File

@ -27,18 +27,28 @@ public class SimpleModifier implements Modifier {
this.field = field;
this.strong = strong;
assert SimpleFormatterImpl.getArgumentLimit(compiledPattern) == 1;
if (compiledPattern.charAt(1) != '\u0000') {
int argLimit = SimpleFormatterImpl.getArgumentLimit(compiledPattern);
if (argLimit == 0) {
// No arguments in compiled pattern
prefixLength = compiledPattern.charAt(1) - ARG_NUM_LIMIT;
suffixOffset = 3 + prefixLength;
} else {
prefixLength = 0;
suffixOffset = 2;
}
if (3 + prefixLength < compiledPattern.length()) {
suffixLength = compiledPattern.charAt(suffixOffset) - ARG_NUM_LIMIT;
} else {
assert 2 + prefixLength == compiledPattern.length();
// Set suffixOffset = -1 to indicate no arguments in compiled pattern.
suffixOffset = -1;
suffixLength = 0;
} else {
assert argLimit == 1;
if (compiledPattern.charAt(1) != '\u0000') {
prefixLength = compiledPattern.charAt(1) - ARG_NUM_LIMIT;
suffixOffset = 3 + prefixLength;
} else {
prefixLength = 0;
suffixOffset = 2;
}
if (3 + prefixLength < compiledPattern.length()) {
suffixLength = compiledPattern.charAt(suffixOffset) - ARG_NUM_LIMIT;
} else {
suffixLength = 0;
}
}
}
@ -96,16 +106,21 @@ public class SimpleModifier implements Modifier {
int startIndex,
int endIndex,
Field field) {
if (prefixLength > 0) {
result.insert(startIndex, compiledPattern, 2, 2 + prefixLength, field);
if (suffixOffset == -1) {
// There is no argument for the inner number; overwrite the entire segment with our string.
return result.splice(startIndex, endIndex, compiledPattern, 2, 2 + prefixLength, field);
} else {
if (prefixLength > 0) {
result.insert(startIndex, compiledPattern, 2, 2 + prefixLength, field);
}
if (suffixLength > 0) {
result.insert(endIndex + prefixLength,
compiledPattern,
1 + suffixOffset,
1 + suffixOffset + suffixLength,
field);
}
return prefixLength + suffixLength;
}
if (suffixLength > 0) {
result.insert(endIndex + prefixLength,
compiledPattern,
1 + suffixOffset,
1 + suffixOffset + suffixLength,
field);
}
return prefixLength + suffixLength;
}
}

View File

@ -473,6 +473,25 @@ public class NumberFormatterApiTest {
ULocale.forLanguageTag("es-US"),
5.43,
"5.43 °F");
assertFormatSingle(
"MeasureUnit form without {0} in CLDR pattern",
"",
NumberFormatter.with().unit(MeasureUnit.KELVIN).unitWidth(UnitWidth.FULL_NAME),
ULocale.forLanguageTag("es-MX"),
1,
"kelvin");
assertFormatSingle(
"MeasureUnit form without {0} in CLDR pattern and wide base form",
"",
NumberFormatter.with()
.rounding(Rounder.fixedFraction(20))
.unit(MeasureUnit.KELVIN)
.unitWidth(UnitWidth.FULL_NAME),
ULocale.forLanguageTag("es-MX"),
1,
"kelvin");
}
@Test

View File

@ -68,6 +68,50 @@ public class NumberStringBuilderTest {
}
}
@Test
public void testSplice() {
Object[][] cases = {
{ "", 0, 0 },
{ "abc", 0, 0 },
{ "abc", 1, 1 },
{ "abc", 1, 2 },
{ "abc", 0, 2 },
{ "abc", 0, 3 },
{ "lorem ipsum dolor sit amet", 8, 8 },
{ "lorem ipsum dolor sit amet", 8, 11 }, // 3 chars, equal to replacement "xyz"
{ "lorem ipsum dolor sit amet", 8, 18 } }; // 10 chars, larger than several replacements
StringBuilder sb1 = new StringBuilder();
NumberStringBuilder sb2 = new NumberStringBuilder();
for (Object[] cas : cases) {
String input = (String) cas[0];
int startThis = (Integer) cas[1];
int endThis = (Integer) cas[2];
for (String replacement : EXAMPLE_STRINGS) {
// Test replacement with full string
sb1.setLength(0);
sb1.append(input);
sb1.replace(startThis, endThis, replacement);
sb2.clear();
sb2.append(input, null);
sb2.splice(startThis, endThis, replacement, 0, replacement.length(), null);
assertCharSequenceEquals(sb1, sb2);
// Test replacement with partial string
if (replacement.length() <= 2) {
continue;
}
sb1.setLength(0);
sb1.append(input);
sb1.replace(startThis, endThis, replacement.substring(1, 3));
sb2.clear();
sb2.append(input, null);
sb2.splice(startThis, endThis, replacement, 1, 3, null);
assertCharSequenceEquals(sb1, sb2);
}
}
}
@Test
public void testInsertAppendCodePoint() {
int[] cases = { 0, 1, 60, 127, 128, 0x7fff, 0x8000, 0xffff, 0x10000, 0x1f000, 0x10ffff };