ICU-20741 Changing SimpleDateTimeFormat::subFormat to only include 1 field at the...

diff --git a/icu4c/source/i18n/smpdtfmt.cpp b/icu4c/source/i18n/smpdtfmt.cpp
index 5fcbb5875b2bfb07569761ee69924a5189b7306d..6246e8f270f9ce5392c9092146b7b2084b4eabb5 100644 (file)
--- a/icu4c/source/i18n/smpdtfmt.cpp
+++ b/icu4c/source/i18n/smpdtfmt.cpp
@@ -996,7 +996,8 @@ SimpleDateFormat::_format(Calendar& cal, UnicodeString& appendTo,
         // Use subFormat() to format a repeated pattern character
         // when a different pattern or non-pattern character is seen
         if (ch != prevCh && count > 0) {
-            subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++, handler, *workCal, status);
+            subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++,
+                      prevCh, handler, *workCal, status);
             count = 0;
         }
         if (ch == QUOTE) {
@@ -1023,7 +1024,8 @@ SimpleDateFormat::_format(Calendar& cal, UnicodeString& appendTo,
 
     // Format the last item in the pattern, if any
     if (count > 0) {
-        subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++, handler, *workCal, status);
+        subFormat(appendTo, prevCh, count, capitalizationContext, fieldNum++,
+                  prevCh, handler, *workCal, status);
     }
 
     if (calClone != NULL) {
@@ -1402,10 +1404,11 @@ SimpleDateFormat::processOverrideString(const Locale &locale, const UnicodeStrin
 //---------------------------------------------------------------------
 void
 SimpleDateFormat::subFormat(UnicodeString &appendTo,
-                            UChar ch,
+                            char16_t ch,
                             int32_t count,
                             UDisplayContext capitalizationContext,
                             int32_t fieldNum,
+                            char16_t fieldToOutput,
                             FieldPositionHandler& handler,
                             Calendar& cal,
                             UErrorCode& status) const
@@ -1853,8 +1856,11 @@ SimpleDateFormat::subFormat(UnicodeString &appendTo,
         // In either case, fall back to am/pm.
         if (toAppend == NULL || toAppend->isBogus()) {
             // Reformat with identical arguments except ch, now changed to 'a'.
-            subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
-                      handler, cal, status);
+            // We are passing a different fieldToOutput because we want to add
+            // 'b' to field position. This makes this fallback stable when
+            // there is a data change on locales.
+            subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'b', handler, cal, status);
+            return;
         } else {
             appendTo += *toAppend;
         }
@@ -1874,9 +1880,11 @@ SimpleDateFormat::subFormat(UnicodeString &appendTo,
         if (ruleSet == NULL) {
             // Data doesn't exist for the locale we're looking for.
             // Falling back to am/pm.
-            subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
-                      handler, cal, status);
-            break;
+            // We are passing a different fieldToOutput because we want to add
+            // 'B' to field position. This makes this fallback stable when
+            // there is a data change on locales.
+            subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'B', handler, cal, status);
+            return;
         }
 
         // Get current display time.
@@ -1945,8 +1953,11 @@ SimpleDateFormat::subFormat(UnicodeString &appendTo,
         if (periodType == DayPeriodRules::DAYPERIOD_AM ||
             periodType == DayPeriodRules::DAYPERIOD_PM ||
             toAppend->isBogus()) {
-            subFormat(appendTo, 0x61, count, capitalizationContext, fieldNum,
-                      handler, cal, status);
+            // We are passing a different fieldToOutput because we want to add
+            // 'B' to field position iterator. This makes this fallback stable when
+            // there is a data change on locales.
+            subFormat(appendTo, u'a', count, capitalizationContext, fieldNum, u'B', handler, cal, status);
+            return;
         }
         else {
             appendTo += *toAppend;
@@ -1990,7 +2001,7 @@ SimpleDateFormat::subFormat(UnicodeString &appendTo,
     }
 #endif
 
-    handler.addAttribute(fgPatternIndexToDateFormatField[patternCharIndex], beginOffset, appendTo.length());
+    handler.addAttribute(DateFormatSymbols::getPatternCharIndex(fieldToOutput), beginOffset, appendTo.length());
 }
 
 //----------------------------------------------------------------------

diff --git a/icu4c/source/i18n/unicode/smpdtfmt.h b/icu4c/source/i18n/unicode/smpdtfmt.h
index 79fa817d5afb893d19736d6d7ade26358715d2ab..b4b0e5fb3908015237c275fd3176df2240eec866 100644 (file)
--- a/icu4c/source/i18n/unicode/smpdtfmt.h
+++ b/icu4c/source/i18n/unicode/smpdtfmt.h
@@ -1274,6 +1274,7 @@ private:
                    int32_t count,
                    UDisplayContext capitalizationContext,
                    int32_t fieldNum,
+                   char16_t fieldToOutput,
                    FieldPositionHandler& handler,
                    Calendar& cal,
                    UErrorCode& status) const; // in case of illegal argument

diff --git a/icu4c/source/test/intltest/dtfmttst.cpp b/icu4c/source/test/intltest/dtfmttst.cpp
index 93aa95165e91affa4b6b40061d9d191f7a5ddad2..d4fca1c88d6eb4e3b01218dc1955577077e9af29 100644 (file)
--- a/icu4c/source/test/intltest/dtfmttst.cpp
+++ b/icu4c/source/test/intltest/dtfmttst.cpp
@@ -5579,34 +5579,39 @@ void DateFormatTest::Test20741_ABFields() {
 
     const char16_t timeZone[] = u"PST8PDT";
 
-    UnicodeString skeleton = u"EEEEEBBBBB"; 
-    int32_t count = 0;
-    const Locale* locales = Locale::getAvailableLocales(count);
-    for (int32_t i = 0; i < count; i++) {
-        if (quick && (i % 17) != 0) { continue; }
-
-        const Locale locale = locales[i];
-        LocalPointer<DateTimePatternGenerator> gen(DateTimePatternGenerator::createInstance(locale, status));
-        UnicodeString pattern = gen->getBestPattern(skeleton, status);
-
-        SimpleDateFormat dateFormat(pattern, locale, status);
-        FieldPositionIterator fpositer;
-        UnicodeString result;
-        LocalPointer<Calendar> calendar(Calendar::createInstance(TimeZone::createTimeZone(timeZone), status));
-        calendar->setTime(UDate(0), status);
-        dateFormat.format(*calendar, result, &fpositer, status);
-
-        FieldPosition curFieldPosition;
-        FieldPosition lastFieldPosition;
-        lastFieldPosition.setBeginIndex(-1);
-        lastFieldPosition.setEndIndex(-1);
-        while(fpositer.next(curFieldPosition)) {
-            if (curFieldPosition.getBeginIndex() == lastFieldPosition.getBeginIndex() && curFieldPosition.getEndIndex() == lastFieldPosition.getEndIndex()) {
-                if (logKnownIssue("20741")) continue;
-                assertEquals("Different fields at same position", 'B', PATTERN_CHARS[lastFieldPosition.getField()]);   
-            }
+    UnicodeString skeletons[] = {u"EEEEEBBBBB", u"EEEEEbbbbb"};
+
+    for (int32_t j = 0; j < 2; j++) {
+        UnicodeString skeleton = skeletons[j];
+
+        int32_t count = 0;
+        const Locale* locales = Locale::getAvailableLocales(count);
+        for (int32_t i = 0; i < count; i++) {
+            if (quick && (i % 17) != 0) { continue; }
+
+            const Locale locale = locales[i];
+            LocalPointer<DateTimePatternGenerator> gen(DateTimePatternGenerator::createInstance(locale, status));
+            UnicodeString pattern = gen->getBestPattern(skeleton, status);
+
+            SimpleDateFormat dateFormat(pattern, locale, status);
+            FieldPositionIterator fpositer;
+            UnicodeString result;
+            LocalPointer<Calendar> calendar(Calendar::createInstance(TimeZone::createTimeZone(timeZone), status));
+            calendar->setTime(UDate(0), status);
+            dateFormat.format(*calendar, result, &fpositer, status);
+
+            FieldPosition curFieldPosition;
+            FieldPosition lastFieldPosition;
+            lastFieldPosition.setBeginIndex(-1);
+            lastFieldPosition.setEndIndex(-1);
+            while(fpositer.next(curFieldPosition)) {
+                assertFalse("Field missing on pattern", pattern.indexOf(PATTERN_CHARS[curFieldPosition.getField()]) == -1);
+                if (curFieldPosition.getBeginIndex() == lastFieldPosition.getBeginIndex() && curFieldPosition.getEndIndex() == lastFieldPosition.getEndIndex()) {
+                    assertEquals("Different fields at same position", PATTERN_CHARS[curFieldPosition.getField()], PATTERN_CHARS[lastFieldPosition.getField()]);
+                }
 
-            lastFieldPosition = curFieldPosition;
+                lastFieldPosition = curFieldPosition;
+            }
         }
     }
 }

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
index 735fac6606db6b81c01e03f9333598fae0132389..a101504ddc5061863ea5df53578ce0bcddc4bec5 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/ChineseDateFormat.java
@@ -126,12 +126,13 @@ public class ChineseDateFormat extends SimpleDateFormat {
                              char ch, int count, int beginOffset,
                              int fieldNum, DisplayContext capitalizationContext,
                              FieldPosition pos,
+                             char patternCharToOutput,
                              Calendar cal) {
 
         // Logic to handle 'G' for chinese calendar is moved into SimpleDateFormat,
         // and obsolete pattern char 'l' is now ignored in SimpleDateFormat, so we
         // just use its implementation
-        super.subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+        super.subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, patternCharToOutput, cal);
 
         // The following is no longer an issue for this subclass...
         // TODO: add code to set FieldPosition for 'G' and 'l' fields. This

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
index 090e9564cfa7174313c421c430534e3726ec0f23..733e5406285d2a7f51723ba80a1f8be6f1b46eaa 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/SimpleDateFormat.java
@@ -1395,10 +1395,10 @@ public class SimpleDateFormat extends DateFormat {
                 }
                 if (useFastFormat) {
                     subFormat(toAppendTo, item.type, item.length, toAppendTo.length(),
-                              i, capitalizationContext, pos, cal);
+                              i, capitalizationContext, pos, item.type, cal);
                 } else {
                     toAppendTo.append(subFormat(item.type, item.length, toAppendTo.length(),
-                                                i, capitalizationContext, pos, cal));
+                                                i, capitalizationContext, pos, item.type, cal));
                 }
                 if (attributes != null) {
                     // Check the sub format length
@@ -1547,7 +1547,7 @@ public class SimpleDateFormat extends DateFormat {
         throws IllegalArgumentException
     {
         // Note: formatData is ignored
-        return subFormat(ch, count, beginOffset, 0, DisplayContext.CAPITALIZATION_NONE, pos, cal);
+        return subFormat(ch, count, beginOffset, 0, DisplayContext.CAPITALIZATION_NONE, pos, ch, cal);
     }
 
      /**
@@ -1561,10 +1561,11 @@ public class SimpleDateFormat extends DateFormat {
     protected String subFormat(char ch, int count, int beginOffset,
                                int fieldNum, DisplayContext capitalizationContext,
                                FieldPosition pos,
+                               char patternCharToOutput,
                                Calendar cal)
     {
         StringBuffer buf = new StringBuffer();
-        subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+        subFormat(buf, ch, count, beginOffset, fieldNum, capitalizationContext, pos, patternCharToOutput, cal);
         return buf.toString();
     }
 
@@ -1586,6 +1587,7 @@ public class SimpleDateFormat extends DateFormat {
                              char ch, int count, int beginOffset,
                              int fieldNum, DisplayContext capitalizationContext,
                              FieldPosition pos,
+                             char patternCharToOutput,
                              Calendar cal) {
 
         final int maxIntCount = Integer.MAX_VALUE;
@@ -1944,7 +1946,10 @@ public class SimpleDateFormat extends DateFormat {
             if (toAppend == null) {
                 // Time isn't exactly midnight or noon (as displayed) or localized string doesn't
                 // exist for requested period. Fall back to am/pm instead.
-                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+                // We are passing a different patternCharToOutput because we want to add
+                // 'b' to field position. This makes this fallback stable when
+                // there is a data change on locales.
+                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'b', cal);
             } else {
                 buf.append(toAppend);
             }
@@ -1959,8 +1964,11 @@ public class SimpleDateFormat extends DateFormat {
             if (ruleSet == null) {
                 // Data doesn't exist for the locale we're looking for.
                 // Fall back to am/pm.
-                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
-                break;
+                // We are passing a different patternCharToOutput because we want to add
+                // 'B' to field position. This makes this fallback stable when
+                // there is a data change on locales.
+                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'B', cal);
+                return;
             }
 
             // Get current display time.
@@ -2025,7 +2033,11 @@ public class SimpleDateFormat extends DateFormat {
             if (periodType == DayPeriodRules.DayPeriod.AM ||
                     periodType == DayPeriodRules.DayPeriod.PM ||
                     toAppend == null) {
-                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, cal);
+                // We are passing a different patternCharToOutput because we want to add
+                // 'B' to field position. This makes this fallback stable when
+                // there is a data change on locales.
+                subFormat(buf, 'a', count, beginOffset, fieldNum, capitalizationContext, pos, 'B', cal);
+                return;
             }
             else {
                 buf.append(toAppend);
@@ -2089,12 +2101,13 @@ public class SimpleDateFormat extends DateFormat {
         }
 
         // Set the FieldPosition (for the first occurrence only)
+        int outputCharIndex = getIndexFromChar(patternCharToOutput);
         if (pos.getBeginIndex() == pos.getEndIndex()) {
-            if (pos.getField() == PATTERN_INDEX_TO_DATE_FORMAT_FIELD[patternCharIndex]) {
+            if (pos.getField() == PATTERN_INDEX_TO_DATE_FORMAT_FIELD[outputCharIndex]) {
                 pos.setBeginIndex(beginOffset);
                 pos.setEndIndex(beginOffset + buf.length() - bufstart);
             } else if (pos.getFieldAttribute() ==
-                       PATTERN_INDEX_TO_DATE_FORMAT_ATTRIBUTE[patternCharIndex]) {
+                       PATTERN_INDEX_TO_DATE_FORMAT_ATTRIBUTE[outputCharIndex]) {
                 pos.setBeginIndex(beginOffset);
                 pos.setEndIndex(beginOffset + buf.length() - bufstart);
             }
@@ -4368,10 +4381,10 @@ public class SimpleDateFormat extends DateFormat {
                 PatternItem item = (PatternItem)items[i];
                 if (useFastFormat) {
                     subFormat(appendTo, item.type, item.length, appendTo.length(),
-                              i, capSetting, pos, fromCalendar);
+                              i, capSetting, pos, item.type, fromCalendar);
                 } else {
                     appendTo.append(subFormat(item.type, item.length, appendTo.length(),
-                                              i, capSetting, pos, fromCalendar));
+                                              i, capSetting, pos, item.type, fromCalendar));
                 }
             }
         }
@@ -4386,10 +4399,10 @@ public class SimpleDateFormat extends DateFormat {
                 PatternItem item = (PatternItem)items[i];
                 if (useFastFormat) {
                     subFormat(appendTo, item.type, item.length, appendTo.length(),
-                              i, capSetting, pos, toCalendar);
+                              i, capSetting, pos, item.type, toCalendar);
                 } else {
                     appendTo.append(subFormat(item.type, item.length, appendTo.length(),
-                                              i, capSetting, pos, toCalendar));
+                                              i, capSetting, pos, item.type, toCalendar));
                 }
             }
         }

diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
index 5b28a76d33435b71ce538d33e1ceb807864990f2..42b9ed33038659d625b44f6b01d4716eb4f4132e 100644 (file)
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/DateFormatTest.java
@@ -5462,30 +5462,36 @@ public class DateFormatTest extends TestFmwk {
 
     @Test
     public void test20741_ABFields() {
+        String [] skeletons = {"EEEEEBBBBB", "EEEEEbbbbb"};
         ULocale[] locales = ULocale.getAvailableLocales();
-        for (int i = 0; i < locales.length; i++) {
-            ULocale locale = locales[i];
-            if (isQuick() && (i % 17 != 0)) continue;
-
-            DateTimePatternGenerator gen = DateTimePatternGenerator.getInstance(locale);
-            String pattern = gen.getBestPattern("EEEEEBBBBB");
-            SimpleDateFormat dateFormat = new SimpleDateFormat(pattern, locale);
-            Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("PST8PDT"));
-            calendar.setTime(new Date(0));
-
-            FieldPosition pos_c = new FieldPosition(DateFormat.Field.DAY_OF_WEEK);
-            dateFormat.format(calendar, new StringBuffer(""), pos_c);
-            assertFalse("'Day of week' field was not found", pos_c.getBeginIndex() == 0 && pos_c.getEndIndex() == 0);
-
-            FieldPosition pos_B = new FieldPosition(DateFormat.Field.FLEXIBLE_DAY_PERIOD);
-            dateFormat.format(calendar, new StringBuffer(""), pos_B);
-            assertFalse("'Flexible day period' field was not found", pos_B.getBeginIndex() == 0 && pos_B.getEndIndex() == 0);
-
-            FieldPosition pos_a = new FieldPosition(DateFormat.Field.AM_PM);
-            dateFormat.format(calendar, new StringBuffer(""), pos_a);
-            if (pos_B.getBeginIndex() == pos_a.getBeginIndex() && pos_B.getEndIndex() == pos_a.getEndIndex()) {
-                if (logKnownIssue("20741", "Format Fields reports same field position as \"a\" and \"B\"")) continue;
-                fail("Duplicated field found");
+        for (String skeleton : skeletons) {
+            for (int i = 0; i < locales.length; i++) {
+                ULocale locale = locales[i];
+                if (isQuick() && (i % 17 != 0)) continue;
+
+                DateTimePatternGenerator gen = DateTimePatternGenerator.getInstance(locale);
+                String pattern = gen.getBestPattern(skeleton);
+                SimpleDateFormat dateFormat = new SimpleDateFormat(pattern, locale);
+                Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("PST8PDT"));
+                calendar.setTime(new Date(0));
+
+                FieldPosition pos_c = new FieldPosition(DateFormat.Field.DAY_OF_WEEK);
+                dateFormat.format(calendar, new StringBuffer(""), pos_c);
+                assertFalse("'Day of week' field was not found", pos_c.getBeginIndex() == 0 && pos_c.getEndIndex() == 0);
+
+                if (skeleton.equals("EEEEEBBBBB")) {
+                    FieldPosition pos_B = new FieldPosition(DateFormat.Field.FLEXIBLE_DAY_PERIOD);
+                    dateFormat.format(calendar, new StringBuffer(""), pos_B);
+                    assertFalse("'Flexible day period' field was not found", pos_B.getBeginIndex() == 0 && pos_B.getEndIndex() == 0);
+                } else {
+                    FieldPosition pos_b = new FieldPosition(DateFormat.Field.AM_PM_MIDNIGHT_NOON);
+                    dateFormat.format(calendar, new StringBuffer(""), pos_b);
+                    assertFalse("'AM/PM/Midnight/Noon' field was not found", pos_b.getBeginIndex() == 0 && pos_b.getEndIndex() == 0);
+                }
+
+                FieldPosition pos_a = new FieldPosition(DateFormat.Field.AM_PM);
+                dateFormat.format(calendar, new StringBuffer(""), pos_a);
+                assertTrue("'AM/PM' field was found", pos_a.getBeginIndex() == 0 && pos_a.getEndIndex() == 0);
             }
         }
     }