From 33c9b61b26250e9fe39e098f9f131549d9aa516f Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Mon, 7 Mar 2022 20:34:49 -0800 Subject: [PATCH] ICU-21871 Make behavior consistent on list format of empty strings --- icu4c/source/i18n/formattedval_sbimpl.cpp | 5 + icu4c/source/test/cintltst/ulistfmttest.c | 102 ++++++++++++++++++ .../test/intltest/listformattertest.cpp | 27 +++++ .../source/test/intltest/listformattertest.h | 1 + .../dev/test/format/ListFormatterTest.java | 24 +++++ 5 files changed, 159 insertions(+) diff --git a/icu4c/source/i18n/formattedval_sbimpl.cpp b/icu4c/source/i18n/formattedval_sbimpl.cpp index 70ffacac4b7..72197cdd8c7 100644 --- a/icu4c/source/i18n/formattedval_sbimpl.cpp +++ b/icu4c/source/i18n/formattedval_sbimpl.cpp @@ -230,6 +230,11 @@ bool FormattedValueStringBuilderImpl::nextPositionImpl(ConstrainedFieldPosition& if (si + 1 < spanIndicesCount) { nextSpanStart = spanIndices[si + 1].start; } + if (length == 0) { + // ICU-21871: Don't return fields on empty spans + i--; + continue; + } if (cfpos.matchesField(spanCategory, spanValue)) { fieldStart = i - fString.fZero; int32_t end = fieldStart + length; diff --git a/icu4c/source/test/cintltst/ulistfmttest.c b/icu4c/source/test/cintltst/ulistfmttest.c index 7fe549f43ba..b0a1f6bc349 100644 --- a/icu4c/source/test/cintltst/ulistfmttest.c +++ b/icu4c/source/test/cintltst/ulistfmttest.c @@ -20,6 +20,8 @@ static void TestUListFmt(void); static void TestUListFmtToValue(void); static void TestUListOpenStyled(void); +static void TestUList21871_A(void); +static void TestUList21871_B(void); void addUListFmtTest(TestNode** root); @@ -30,6 +32,8 @@ void addUListFmtTest(TestNode** root) TESTCASE(TestUListFmt); TESTCASE(TestUListFmtToValue); TESTCASE(TestUListOpenStyled); + TESTCASE(TestUList21871_A); + TESTCASE(TestUList21871_B); } static const UChar str0[] = { 0x41,0 }; /* "A" */ @@ -250,5 +254,103 @@ static void TestUListOpenStyled() { ulistfmt_closeResult(fl); } +#include + +static void TestUList21871_A() { + UErrorCode status = U_ZERO_ERROR; + UListFormatter *fmt = ulistfmt_openForType("en", ULISTFMT_TYPE_AND, ULISTFMT_WIDTH_WIDE, &status); + assertSuccess("ulistfmt_openForType", &status); + + const UChar *strs[] = {u"A", u""}; + const int32_t lens[] = {1, 0}; + + UFormattedList *fl = ulistfmt_openResult(&status); + assertSuccess("ulistfmt_openResult", &status); + + ulistfmt_formatStringsToResult(fmt, strs, lens, 2, fl, &status); + assertSuccess("ulistfmt_formatStringsToResult", &status); + + const UFormattedValue *value = ulistfmt_resultAsValue(fl, &status); + assertSuccess("ulistfmt_resultAsValue", &status); + + { + int32_t len; + const UChar *str = ufmtval_getString(value, &len, &status); + assertUEquals("TEST ufmtval_getString", u"A and ", str); + } + + UConstrainedFieldPosition *fpos = ucfpos_open(&status); + assertSuccess("ucfpos_open", &status); + + ucfpos_constrainField(fpos, UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, &status); + assertSuccess("ucfpos_constrainField", &status); + + bool hasMore = ufmtval_nextPosition(value, fpos, &status); + assertSuccess("ufmtval_nextPosition", &status); + assertTrue("hasMore 1", hasMore); + + int32_t beginIndex, endIndex; + ucfpos_getIndexes(fpos, &beginIndex, &endIndex, &status); + assertSuccess("ufmtval_nextPosition", &status); + assertIntEquals("TEST beginIndex", 0, beginIndex); + assertIntEquals("TEST endIndex", 1, endIndex); + + hasMore = ufmtval_nextPosition(value, fpos, &status); + assertSuccess("ufmtval_nextPosition", &status); + assertTrue("hasMore 2", !hasMore); + + ucfpos_close(fpos); + ulistfmt_closeResult(fl); + ulistfmt_close(fmt); +} + +static void TestUList21871_B() { + UErrorCode status = U_ZERO_ERROR; + UListFormatter *fmt = ulistfmt_openForType("en", ULISTFMT_TYPE_AND, ULISTFMT_WIDTH_WIDE, &status); + assertSuccess("ulistfmt_openForType", &status); + + const UChar *strs[] = {u"", u"B"}; + const int32_t lens[] = {0, 1}; + + UFormattedList *fl = ulistfmt_openResult(&status); + assertSuccess("ulistfmt_openResult", &status); + + ulistfmt_formatStringsToResult(fmt, strs, lens, 2, fl, &status); + assertSuccess("ulistfmt_formatStringsToResult", &status); + + const UFormattedValue *value = ulistfmt_resultAsValue(fl, &status); + assertSuccess("ulistfmt_resultAsValue", &status); + + { + int32_t len; + const UChar *str = ufmtval_getString(value, &len, &status); + assertUEquals("TEST ufmtval_getString", u" and B", str); + } + + UConstrainedFieldPosition *fpos = ucfpos_open(&status); + assertSuccess("ucfpos_open", &status); + + ucfpos_constrainField(fpos, UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD, &status); + assertSuccess("ucfpos_constrainField", &status); + + bool hasMore = ufmtval_nextPosition(value, fpos, &status); + assertSuccess("ufmtval_nextPosition", &status); + assertTrue("hasMore 1", hasMore); + + int32_t beginIndex, endIndex; + ucfpos_getIndexes(fpos, &beginIndex, &endIndex, &status); + assertSuccess("ucfpos_getIndexes", &status); + assertIntEquals("TEST beginIndex", 5, beginIndex); + assertIntEquals("TEST endIndex", 6, endIndex); + + hasMore = ufmtval_nextPosition(value, fpos, &status); + assertSuccess("ufmtval_nextPosition", &status); + assertTrue("hasMore 2", !hasMore); + + ucfpos_close(fpos); + ulistfmt_closeResult(fl); + ulistfmt_close(fmt); +} + #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/listformattertest.cpp b/icu4c/source/test/intltest/listformattertest.cpp index 5e3a63b0166..1a4ec97f37d 100644 --- a/icu4c/source/test/intltest/listformattertest.cpp +++ b/icu4c/source/test/intltest/listformattertest.cpp @@ -49,6 +49,7 @@ void ListFormatterTest::runIndexedTest(int32_t index, UBool exec, TESTCASE_AUTO(TestContextual); TESTCASE_AUTO(TestNextPosition); TESTCASE_AUTO(TestInt32Overflow); + TESTCASE_AUTO(Test21871); TESTCASE_AUTO_END; } @@ -840,4 +841,30 @@ void ListFormatterTest::TestInt32Overflow() { status.expectErrorAndReset(U_INPUT_TOO_LONG_ERROR); } + +void ListFormatterTest::Test21871() { + IcuTestErrorCode status(*this, "Test21871"); + LocalPointer fmt(ListFormatter::createInstance("en", status), status); + { + UnicodeString strings[] = {{u"A"}, {u""}}; + FormattedList result = fmt->formatStringsToValue(strings, 2, status); + ConstrainedFieldPosition cfp; + cfp.constrainField(UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD); + assertTrue("nextPosition 1", result.nextPosition(cfp, status)); + assertEquals("start", cfp.getStart(), 0); + assertEquals("limit", cfp.getLimit(), 1); + assertFalse("nextPosition 2", result.nextPosition(cfp, status)); + } + { + UnicodeString strings[] = {{u""}, {u"B"}}; + FormattedList result = fmt->formatStringsToValue(strings, 2, status); + ConstrainedFieldPosition cfp; + cfp.constrainField(UFIELD_CATEGORY_LIST, ULISTFMT_ELEMENT_FIELD); + assertTrue("nextPosition 1", result.nextPosition(cfp, status)); + assertEquals("start", cfp.getStart(), 5); + assertEquals("limit", cfp.getLimit(), 6); + assertFalse("nextPosition 2", result.nextPosition(cfp, status)); + } +} + #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/test/intltest/listformattertest.h b/icu4c/source/test/intltest/listformattertest.h index d6bf3bc0d68..0f0390010e4 100644 --- a/icu4c/source/test/intltest/listformattertest.h +++ b/icu4c/source/test/intltest/listformattertest.h @@ -56,6 +56,7 @@ class ListFormatterTest : public IntlTestWithFieldPosition { void TestContextual(); void TestNextPosition(); void TestInt32Overflow(); + void Test21871(); private: void CheckFormatting( 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 6bfa5551e6c..3c68db7d256 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 @@ -18,6 +18,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import com.ibm.icu.dev.test.TestFmwk; +import com.ibm.icu.text.ConstrainedFieldPosition; import com.ibm.icu.text.ListFormatter; import com.ibm.icu.text.ListFormatter.FormattedList; import com.ibm.icu.text.ListFormatter.Type; @@ -432,4 +433,27 @@ public class ListFormatterTest extends TestFmwk { } } } + + @Test + public void Test21871() { + ListFormatter fmt = ListFormatter.getInstance(ULocale.ENGLISH, Type.AND, Width.WIDE); + { + FormattedList result = fmt.formatToValue("A", ""); + ConstrainedFieldPosition cfp = new ConstrainedFieldPosition(); + cfp.constrainField(ListFormatter.Field.ELEMENT); + assertTrue("nextPosition 1", result.nextPosition(cfp)); + assertEquals("start", cfp.getStart(), 0); + assertEquals("limit", cfp.getLimit(), 1); + assertFalse("nextPosition 2", result.nextPosition(cfp)); + } + { + FormattedList result = fmt.formatToValue("", "B"); + ConstrainedFieldPosition cfp = new ConstrainedFieldPosition(); + cfp.constrainField(ListFormatter.Field.ELEMENT); + assertTrue("nextPosition 1", result.nextPosition(cfp)); + assertEquals("start", cfp.getStart(), 5); + assertEquals("limit", cfp.getLimit(), 6); + assertFalse("nextPosition 2", result.nextPosition(cfp)); + } + } }