ICU-20741 Changing SimpleDateTimeFormat::subFormat to only include 1 field at the same position when there is a data fallback

This commit is contained in:
Caio Lima 2019-12-10 13:10:00 -08:00 committed by Shane F. Carr
parent 7d7449bcbe
commit 7c147e4e85
6 changed files with 106 additions and 69 deletions

View file

@ -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());
}
//----------------------------------------------------------------------

View file

@ -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

View file

@ -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; }
UnicodeString skeletons[] = {u"EEEEEBBBBB", u"EEEEEbbbbb"};
const Locale locale = locales[i];
LocalPointer<DateTimePatternGenerator> gen(DateTimePatternGenerator::createInstance(locale, status));
UnicodeString pattern = gen->getBestPattern(skeleton, status);
for (int32_t j = 0; j < 2; j++) {
UnicodeString skeleton = skeletons[j];
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);
int32_t count = 0;
const Locale* locales = Locale::getAvailableLocales(count);
for (int32_t i = 0; i < count; i++) {
if (quick && (i % 17) != 0) { continue; }
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()]);
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;
}
}
}

View file

@ -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

View file

@ -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));
}
}
}

View file

@ -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;
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("EEEEEBBBBB");
SimpleDateFormat dateFormat = new SimpleDateFormat(pattern, locale);
Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("PST8PDT"));
calendar.setTime(new Date(0));
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);
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);
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);
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");
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);
}
}
}