Skip to content

CLDR-19106 check for empty datetimeformats #5219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
macchiati merged 3 commits into from
Dec 9, 2025
+113 −20
Merged

CLDR-19106 check for empty datetimeformats #5219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a93a3e
CLDR-19106 Check datetimeformats
macchiati Dec 9, 2025
e04a896
CLDR-19106 Disallow empty datetimeformats
macchiati Dec 9, 2025
8943223
CLDR-19106 Spotless
macchiati Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions common/main/yue.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2388,7 +2388,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<dateTimeFormats>
<dateTimeFormatLength type="full">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>↑↑↑</pattern>
Expand All @@ -2402,7 +2402,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -2413,7 +2413,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand Down Expand Up @@ -2988,7 +2988,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -2999,7 +2999,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -3010,7 +3010,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand Down
12 changes: 6 additions & 6 deletions common/main/yue_Hans.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2389,7 +2389,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<dateTimeFormats>
<dateTimeFormatLength type="full">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>↑↑↑</pattern>
Expand All @@ -2403,7 +2403,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -2414,7 +2414,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand Down Expand Up @@ -2989,7 +2989,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -3000,7 +3000,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand All @@ -3011,7 +3011,7 @@ CLDR data files are interpreted according to the LDML specification (http://unic
<pattern>↑↑↑</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand Down
14 changes: 7 additions & 7 deletions common/main/zh_Hant.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4471,7 +4471,7 @@ Warnings: All cp values have U+FE0F characters removed. See /annotationsDerived/
<dateTimeFormats>
<dateTimeFormatLength type="full">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>↑↑↑</pattern>
Expand Down Expand Up @@ -5068,32 +5068,32 @@ Warnings: All cp values have U+FE0F characters removed. See /annotationsDerived/
<dateTimeFormats>
<dateTimeFormatLength type="full">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
</dateTimeFormat>
</dateTimeFormatLength>
<dateTimeFormatLength type="long">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
</dateTimeFormat>
</dateTimeFormatLength>
<dateTimeFormatLength type="medium">
<dateTimeFormat>
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="atTime">
<pattern>{1}{0}</pattern>
<pattern>{1} {0}</pattern>
</dateTimeFormat>
<dateTimeFormat type="relative">
<pattern>{1}{0}</pattern>
Expand Down
95 changes: 94 additions & 1 deletion tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckDates.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.unicode.cldr.util.XPathParts;

public class CheckDates extends FactoryCheckCLDR {
private static final boolean DISABLE = true;
private static final boolean DEBUG = false;
private static final boolean DISABLE_DATE_ORDER = true;

Expand Down Expand Up @@ -272,6 +273,8 @@ public CheckCLDR handleCheck(
return this;
}

XPathParts parts = XPathParts.getFrozenInstance(fullPath);

if (!path.contains("/dates") || path.endsWith("/default") || path.endsWith("/alias")) {
return this;
}
Expand Down Expand Up @@ -466,6 +469,10 @@ && isTooMuchWiderThan(value, abbrValue)
final String collisionPrefix = "//ldml/dates/calendars/calendar";
main:
if (path.startsWith(collisionPrefix)) {
if (parts.containsElement("dateTimeFormats")
&& !parts.containsElement("appendItems")) {
checkDateTimeFormats(path, parts, value, result);
Copy link
Member

@sffc sffc Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: this means we're checking all of the datetime glue patterns (short/medium/long/full and standard/atTime), correct?

Copy link
Member Author

@macchiati macchiati Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right.

Copy link
Contributor

@pedberg-icu pedberg-icu Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm,, I had approved this when it was still draft, but somehow that got erased. Re-approving now.

}
int pos = path.indexOf("\"]"); // end of first type
if (pos < 0 || skipPath(path)) { // skip narrow, no-calendar
break main;
Expand Down Expand Up @@ -495,7 +502,6 @@ && isTooMuchWiderThan(value, abbrValue)
DayPeriod dayPeriod = null;
final boolean isDayPeriod = path.contains("dayPeriod");
if (isDayPeriod) {
XPathParts parts = XPathParts.getFrozenInstance(fullPath);
type =
Type.fromString(
parts.getAttributeValue(5, "type")); // format, stand-alone
Expand Down Expand Up @@ -680,6 +686,93 @@ && isTooMuchWiderThan(value, abbrValue)
return this;
}

private static final Pattern datePatternDoesntEndsWithDigits =
Pattern.compile(".*(MMM|LLL|[^yd])");
private static final XPathParts timeParts =
XPathParts.getFrozenInstance(
"//ldml/dates/calendars/calendar[@type=\"gregorian\"]/timeFormats/timeFormatLength[@type=\"short\"]/timeFormat[@type=\"standard\"]/pattern[@type=\"standard\"]");
private static final XPathParts dateParts =
XPathParts.getFrozenInstance(
"//ldml/dates/calendars/calendar[@type=\"gregorian\"]/dateFormats/dateFormatLength[@type=\"short\"]/dateFormat[@type=\"standard\"]/pattern[@type=\"standard\"]");
private static final XPathParts availableParts =
XPathParts.getFrozenInstance(
"//ldml/dates/calendars/calendar[@type=\"generic\"]/dateTimeFormats/availableFormats/dateFormatItem[@id=\"yyyyQQQ\"]");

private static final int calendarIndex = timeParts.findElement("calendar");
private static final int dateFormatLengthIndex = timeParts.findElement("dateFormatLength");
private static final int dateFormatIndex = timeParts.findElement("timeFormat");

// NOTE: the timeFormatLength is always at the same position as the dateFormatLength, but just
// for consistency...

private void checkDateTimeFormats(
String path, XPathParts parts, String value, List<CheckStatus> result) {

// Make sure that if the format has adjacent placeholders, we don't run numbers together
if (!value.contains("}{")) { // only worry about adjacents
return;
}
// we assume that relative dates don't end with digits
if ("relative".equals(parts.getAttributeValue(dateFormatIndex, "type"))) {
return;
}

String length = parts.getAttributeValue(dateFormatLengthIndex, "type");
String calendar = parts.getAttributeValue(calendarIndex, "type");

// By the spec, the dateTimeFormatLength will be the same as the dateFormatLength
// So see if the corresponding date ends with digits
// However, this only accounts for the stock formats; not combinations of available.

CLDRFile resolvedCldrFile = getResolvedCldrFileToCheck();

XPathParts currentDateParts =
dateParts
.cloneAsThawed()
.setAttribute(calendarIndex, "type", calendar)
.setAttribute(dateFormatLengthIndex, "type", length);
String dateValue = resolvedCldrFile.getStringValue(currentDateParts.toString());
if (dateValue == null) {
throw new IllegalArgumentException(
"Missing value: " + currentDateParts); // should never occur
}

// TODO: This is a first pass, to correct the bad cases.
// We should check later to see if we can allow more cases.
// IN THAT CASE, the following code could be the start of that, so it is left here, but
// inoperative

if (!DISABLE) {
// all current formats are date + time, not reverse
if (value.contains("{0}{1}")) {
throw new IllegalArgumentException("Enhance checks to handle {0}{1}");
}

// TODO: also check the available formats that are for a date
Copy link
Member

@sffc sffc Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an important TODO. We use these glue patterns to assemble date+time patterns from availableFormats.

Copy link
Member Author

@macchiati macchiati Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. At first I wasn't checking for this, and just the short datetime pattern needed to be fixed. But then I realized that it needed to be extended.


if (datePatternDoesntEndsWithDigits.matcher(dateValue).matches()) {
return;
}
}

// My first version of this checked the corresponding time formats to see which could start
// with an integer.
// However, when we count available formats, essentially all locales have a time format
// starting with HH

String mergedPattern = value.replace("{1}", dateValue).replace("{0}", "HH…");
CheckStatus item =
new CheckStatus()
.setCause(this)
.setMainType(CheckStatus.errorType)
.setSubtype(Subtype.illegalDatePattern)
.setMessage(
"The date and time must not have adjacent integers. "
+ "This pattern could produce {0}",
mergedPattern);
result.add(item);
}

private void checkTimeFormatMatchesRegion(String value, List<CheckStatus> result) {
String localeID = getResolvedCldrFileToCheck().getLocaleID();
if (LocaleNames.ROOT.equals(localeID)) {
Expand Down
Loading