diff --git a/icu4c/source/i18n/formattedval_impl.h b/icu4c/source/i18n/formattedval_impl.h index 4cfb0f078afe..c0dec83ba1eb 100644 --- a/icu4c/source/i18n/formattedval_impl.h +++ b/icu4c/source/i18n/formattedval_impl.h @@ -174,8 +174,8 @@ class U_I18N_API FormattedValueStringBuilderImpl : public UMemory, public Format * spanValue: the index of the list item, for example. * length: the length of the span, used to split adjacent fields. */ - void appendSpanInfo(int32_t spanValue, int32_t length); - void prependSpanInfo(int32_t spanValue, int32_t length); + void appendSpanInfo(int32_t spanValue, int32_t length, UErrorCode& status); + void prependSpanInfo(int32_t spanValue, int32_t length, UErrorCode& status); private: FormattedStringBuilder fString; diff --git a/icu4c/source/i18n/formattedval_sbimpl.cpp b/icu4c/source/i18n/formattedval_sbimpl.cpp index aad4438360ae..84c2d00666c2 100644 --- a/icu4c/source/i18n/formattedval_sbimpl.cpp +++ b/icu4c/source/i18n/formattedval_sbimpl.cpp @@ -219,19 +219,35 @@ bool FormattedValueStringBuilderImpl::nextPositionImpl(ConstrainedFieldPosition& } U_ASSERT(currField == kUndefinedField); + // Always set the position to the end so that we don't revisit previous sections + cfpos.setState( + cfpos.getCategory(), + cfpos.getField(), + fString.fLength, + fString.fLength); return false; } -void FormattedValueStringBuilderImpl::appendSpanInfo(int32_t spanValue, int32_t length) { - if (spanIndices.getCapacity() <= spanValue) { - spanIndices.resize(spanValue * 2); +void FormattedValueStringBuilderImpl::appendSpanInfo(int32_t spanValue, int32_t length, UErrorCode& status) { + if (U_FAILURE(status)) { return; } + U_ASSERT(spanIndices.getCapacity() >= spanValue); + if (spanIndices.getCapacity() == spanValue) { + if (!spanIndices.resize(spanValue * 2, spanValue)) { + status = U_MEMORY_ALLOCATION_ERROR; + return; + } } spanIndices[spanValue] = {spanValue, length}; } -void FormattedValueStringBuilderImpl::prependSpanInfo(int32_t spanValue, int32_t length) { - if (spanIndices.getCapacity() <= spanValue) { - spanIndices.resize(spanValue * 2); +void FormattedValueStringBuilderImpl::prependSpanInfo(int32_t spanValue, int32_t length, UErrorCode& status) { + if (U_FAILURE(status)) { return; } + U_ASSERT(spanIndices.getCapacity() >= spanValue); + if (spanIndices.getCapacity() == spanValue) { + if (!spanIndices.resize(spanValue * 2, spanValue)) { + status = U_MEMORY_ALLOCATION_ERROR; + return; + } } for (int32_t i = spanValue - 1; i >= 0; i--) { spanIndices[i+1] = spanIndices[i]; diff --git a/icu4c/source/i18n/listformatter.cpp b/icu4c/source/i18n/listformatter.cpp index 2ee646f0171e..be0d16bc7f52 100644 --- a/icu4c/source/i18n/listformatter.cpp +++ b/icu4c/source/i18n/listformatter.cpp @@ -567,7 +567,7 @@ class FormattedListBuilder { start, {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD}, status); - data->appendSpanInfo(0, start.length()); + data->appendSpanInfo(0, start.length(), status); } } @@ -603,7 +603,7 @@ class FormattedListBuilder { next, {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD}, status); - data->appendSpanInfo(position, next.length()); + data->appendSpanInfo(position, next.length(), status); data->getStringRef().append( temp.tempSubString(offsets[1]), {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD}, @@ -622,7 +622,7 @@ class FormattedListBuilder { next, {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD}, status); - data->prependSpanInfo(position, next.length()); + data->prependSpanInfo(position, next.length(), status); data->getStringRef().insert( 0, temp.tempSubStringBetween(0, offsets[1]), diff --git a/icu4c/source/test/intltest/formattedvaluetest.cpp b/icu4c/source/test/intltest/formattedvaluetest.cpp index 8de225dfd00d..0edf42086737 100644 --- a/icu4c/source/test/intltest/formattedvaluetest.cpp +++ b/icu4c/source/test/intltest/formattedvaluetest.cpp @@ -237,6 +237,8 @@ void IntlTestWithFieldPosition::checkMixedFormattedValue( } UBool afterLoopResult = fv.nextPosition(cfpos, status); assertFalse(baseMessage + u"A after loop: " + CFPosToUnicodeString(cfpos), afterLoopResult); + afterLoopResult = fv.nextPosition(cfpos, status); + assertFalse(baseMessage + u"A after loop again: " + CFPosToUnicodeString(cfpos), afterLoopResult); // Check nextPosition constrained over each category one at a time for (int32_t category=0; category fmt(ListFormatter::createInstance("en", ULISTFMT_TYPE_UNITS, ULISTFMT_WIDTH_SHORT, status)); + if (status.errIfFailureAndReset()) { return; } + const char16_t* message = u"ICU-21383 Long list"; + const char16_t* expectedString = u"a, b, c, d, e, f, g, h, i"; + const UnicodeString inputs[] = { + u"a", + u"b", + u"c", + u"d", + u"e", + u"f", + u"g", + u"h", + u"i", + }; + FormattedList result = fmt->formatStringsToValue(inputs, UPRV_LENGTHOF(inputs), status); + static const UFieldPositionWithCategory expectedFieldPositions[] = { + // field, begin index, end index + {UFIELD_CATEGORY_LIST_SPAN, 0, 0, 1}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 0, 1}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 1, 3}, + {UFIELD_CATEGORY_LIST_SPAN, 1, 3, 4}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 3, 4}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 4, 6}, + {UFIELD_CATEGORY_LIST_SPAN, 2, 6, 7}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 6, 7}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 7, 9}, + {UFIELD_CATEGORY_LIST_SPAN, 3, 9, 10}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 9, 10}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 10, 12}, + {UFIELD_CATEGORY_LIST_SPAN, 4, 12, 13}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 12, 13}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 13, 15}, + {UFIELD_CATEGORY_LIST_SPAN, 5, 15, 16}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 15, 16}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 16, 18}, + {UFIELD_CATEGORY_LIST_SPAN, 6, 18, 19}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 18, 19}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 19, 21}, + {UFIELD_CATEGORY_LIST_SPAN, 7, 21, 22}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 21, 22}, + {UFIELD_CATEGORY_LIST, ULISTFMT_LITERAL_FIELD, 22, 24}, + {UFIELD_CATEGORY_LIST_SPAN, 8, 24, 25}, + {UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, 24, 25}, + }; + checkMixedFormattedValue( + message, + result, + expectedString, + expectedFieldPositions, + UPRV_LENGTHOF(expectedFieldPositions)); + } } void ListFormatterTest::DoTheRealListStyleTesting(Locale locale, diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java index e6c54a57afb9..0c82d263254f 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/FormattedValueStringBuilderImpl.java @@ -226,6 +226,12 @@ else if (cfpos.matchesField((Field) _field, null)) { } assert currField == null; + // Always set the position to the end so that we don't revisit previous sections + cfpos.setState( + cfpos.getField(), + cfpos.getFieldValue(), + self.length, + self.length); return false; } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/ConstrainedFieldPosition.java b/icu4j/main/classes/core/src/com/ibm/icu/text/ConstrainedFieldPosition.java index fd69112468ba..fac9453d6170 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/text/ConstrainedFieldPosition.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/text/ConstrainedFieldPosition.java @@ -295,7 +295,9 @@ public void setInt64IterationContext(long context) { */ public void setState(Field field, Object value, int start, int limit) { // Check matchesField only as an assertion (debug build) - assert matchesField(field, value); + if (field != null) { + assert matchesField(field, value); + } fField = field; fValue = value; diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java index 1c155cb98d08..51a39f47ed2a 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/FormattedValueTest.java @@ -215,6 +215,8 @@ public static void checkFormattedValue(String message, FormattedValue fv, String } boolean afterLoopResult = fv.nextPosition(cfpos); assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult); + afterLoopResult = fv.nextPosition(cfpos); + assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult); // Check nextPosition constrained over each class one at a time for (Class classConstraint : uniqueFieldClasses) { @@ -238,6 +240,8 @@ public static void checkFormattedValue(String message, FormattedValue fv, String } afterLoopResult = fv.nextPosition(cfpos); assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult); + afterLoopResult = fv.nextPosition(cfpos); + assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult); } // Check nextPosition constrained over an unrelated class @@ -267,6 +271,8 @@ public static void checkFormattedValue(String message, FormattedValue fv, String } afterLoopResult = fv.nextPosition(cfpos); assertFalse(baseMessage + "after loop: " + cfpos, afterLoopResult); + afterLoopResult = fv.nextPosition(cfpos); + assertFalse(baseMessage + "after loop again: " + cfpos, afterLoopResult); } } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java index b9c7b74b2d6d..92977524bc86 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/ListFormatterTest.java @@ -217,9 +217,8 @@ private boolean isDefaultLocaleEnglishLike() { @Test public void TestFormattedValue() { - ListFormatter fmt = ListFormatter.getInstance(ULocale.ENGLISH); - { + ListFormatter fmt = ListFormatter.getInstance(ULocale.ENGLISH); String message = "Field position test 1"; String expectedString = "hello, wonderful, and world"; String[] inputs = { @@ -244,6 +243,85 @@ public void TestFormattedValue() { expectedString, expectedFieldPositions); } + + { + ListFormatter fmt = ListFormatter.getInstance(ULocale.CHINESE, Type.UNITS, Width.SHORT); + String message = "Field position test 2 (ICU-21340)"; + String expectedString = "aabbbbbbbccc"; + String inputs[] = { + "aa", + "bbbbbbb", + "ccc" + }; + FormattedList result = fmt.formatToValue(Arrays.asList(inputs)); + Object[][] expectedFieldPositions = { + // field, begin index, end index + {ListFormatter.SpanField.LIST_SPAN, 0, 2, 0}, + {ListFormatter.Field.ELEMENT, 0, 2}, + {ListFormatter.SpanField.LIST_SPAN, 2, 9, 1}, + {ListFormatter.Field.ELEMENT, 2, 9}, + {ListFormatter.SpanField.LIST_SPAN, 9, 12, 2}, + {ListFormatter.Field.ELEMENT, 9, 12}}; + if (!logKnownIssue("21351", "Java still coalesces adjacent elements")) { + FormattedValueTest.checkFormattedValue( + message, + result, + expectedString, + expectedFieldPositions); + } + } + + { + ListFormatter fmt = ListFormatter.getInstance(ULocale.ENGLISH, Type.UNITS, Width.SHORT); + String message = "ICU-21383 Long list"; + String expectedString = "a, b, c, d, e, f, g, h, i"; + String inputs[] = { + "a", + "b", + "c", + "d", + "e", + "f", + "g", + "h", + "i", + }; + FormattedList result = fmt.formatToValue(Arrays.asList(inputs)); + Object[][] expectedFieldPositions = { + // field, begin index, end index + {ListFormatter.SpanField.LIST_SPAN, 0, 1, 0}, + {ListFormatter.Field.ELEMENT, 0, 1}, + {ListFormatter.Field.LITERAL, 1, 3}, + {ListFormatter.SpanField.LIST_SPAN, 3, 4, 1}, + {ListFormatter.Field.ELEMENT, 3, 4}, + {ListFormatter.Field.LITERAL, 4, 6}, + {ListFormatter.SpanField.LIST_SPAN, 6, 7, 2}, + {ListFormatter.Field.ELEMENT, 6, 7}, + {ListFormatter.Field.LITERAL, 7, 9}, + {ListFormatter.SpanField.LIST_SPAN, 9, 10, 3}, + {ListFormatter.Field.ELEMENT, 9, 10}, + {ListFormatter.Field.LITERAL, 10, 12}, + {ListFormatter.SpanField.LIST_SPAN, 12, 13, 4}, + {ListFormatter.Field.ELEMENT, 12, 13}, + {ListFormatter.Field.LITERAL, 13, 15}, + {ListFormatter.SpanField.LIST_SPAN, 15, 16, 5}, + {ListFormatter.Field.ELEMENT, 15, 16}, + {ListFormatter.Field.LITERAL, 16, 18}, + {ListFormatter.SpanField.LIST_SPAN, 18, 19, 6}, + {ListFormatter.Field.ELEMENT, 18, 19}, + {ListFormatter.Field.LITERAL, 19, 21}, + {ListFormatter.SpanField.LIST_SPAN, 21, 22, 7}, + {ListFormatter.Field.ELEMENT, 21, 22}, + {ListFormatter.Field.LITERAL, 22, 24}, + {ListFormatter.SpanField.LIST_SPAN, 24, 25, 8}, + {ListFormatter.Field.ELEMENT, 24, 25}, + }; + FormattedValueTest.checkFormattedValue( + message, + result, + expectedString, + expectedFieldPositions); + } } @Test