Skip to content

Commit 58e2f6f

Browse files
macchiatimacchiati
authored
CLDR-18921 Fix attribute order, add test (#5012)
1 parent 530111f commit 58e2f6f

File tree

3 files changed

+138
-6
lines changed

3 files changed

+138
-6
lines changed

‎common/dtd/ldmlSupplemental.dtd‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -994,15 +994,15 @@ CLDR data files are interpreted according to the LDML specification (http://unic
994994
<!--@MATCH:time/yyyy-MM-dd HH:mm-->
995995
<!ATTLIST usesMetazone to CDATA #IMPLIED >
996996
<!--@MATCH:time/yyyy-MM-dd HH:mm-->
997+
<!ATTLIST usesMetazone mzone NMTOKEN #REQUIRED >
998+
<!--@MATCH:metazone-->
999+
<!--@VALUE-->
9971000
<!ATTLIST usesMetazone stdOffset CDATA #IMPLIED >
9981001
<!--@MATCH:regex/[+-][0-9]{2}(:[0-9]{2})?-->
9991002
<!--@VALUE-->
10001003
<!ATTLIST usesMetazone dstOffset CDATA #IMPLIED >
10011004
<!--@MATCH:regex/[+-][0-9]{2}(:[0-9]{2})?-->
10021005
<!--@VALUE-->
1003-
<!ATTLIST usesMetazone mzone NMTOKEN #REQUIRED >
1004-
<!--@MATCH:metazone-->
1005-
<!--@VALUE-->
10061006

10071007
<!ELEMENT plurals ( pluralRules*, pluralRanges* ) >
10081008
<!ATTLIST plurals type (ordinal | cardinal) #IMPLIED >

‎tools/cldr-code/src/main/java/org/unicode/cldr/util/DtdData.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,10 @@ public static String getShortName(AttributeStatus status) {
9999

100100
public enum Mode {
101101
REQUIRED("#REQUIRED"),
102-
OPTIONAL("#IMPLIED"),
103102
FIXED("#FIXED"),
104-
NULL("null");
103+
NULL("null"), // there is a default value
104+
OPTIONAL("#IMPLIED") // ordered after the others (normally)
105+
;
105106

106107
public final String source;
107108

@@ -116,7 +117,7 @@ public static Mode forString(String mode) {
116117
}
117118
}
118119
if (mode == null) {
119-
return NULL;
120+
return Mode.NULL;
120121
}
121122
throw new IllegalArgumentException(mode);
122123
}

‎tools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestDtdData.java‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.Arrays;
1010
import java.util.Collection;
1111
import java.util.Collections;
12+
import java.util.Comparator;
1213
import java.util.HashSet;
1314
import java.util.Iterator;
1415
import java.util.LinkedHashSet;
@@ -24,7 +25,10 @@
2425
import org.unicode.cldr.util.DtdData.Element;
2526
import org.unicode.cldr.util.DtdData.Element.ValueConstraint;
2627
import org.unicode.cldr.util.DtdData.ElementType;
28+
import org.unicode.cldr.util.DtdData.Mode;
2729
import org.unicode.cldr.util.DtdType;
30+
import org.unicode.cldr.util.Joiners;
31+
import org.unicode.cldr.util.MapComparator;
2832
import org.unicode.cldr.util.MatchValue;
2933
import org.unicode.cldr.util.MatchValue.EnumParser;
3034
import org.unicode.cldr.util.Pair;
@@ -936,4 +940,131 @@ public void testEmptyPcdata() {
936940
assertEquals(path, expected, actual);
937941
}
938942
}
943+
944+
final Set<String> attributeOrderGrandfathered =
945+
Set.of(
946+
"//ldml…/version[@ number < cldrVersion", // Status: metadata ≠ value Mode:
947+
// REQUIRED ≠ FIXED
948+
"//ldml…/pattern[@ numbers < count", // Status: value ≠ distinguished Mode:
949+
// OPTIONAL
950+
"//ldml…/symbols[@ references < numberSystem", // Status: metadata ≠
951+
// distinguished Mode: OPTIONAL
952+
"//supplementalData…/languagePopulation[@ writingPercent < populationPercent", // Status: value Mode: OPTIONAL ≠ REQUIRED
953+
"//supplementalData…/metazoneId[@ longId < deprecated", // Status: value
954+
// Mode: OPTIONAL ≠ NULL
955+
"//supplementalData…/version[@ number < cldrVersion", // Status: metadata ≠
956+
// value Mode: REQUIRED ≠
957+
// FIXED
958+
"//supplementalData…/coverageLevel[@ value < match", // Status: value ≠
959+
// distinguished Mode:
960+
// REQUIRED
961+
"//supplementalData…/parentLocale[@ localeRules < locales", // Status: value
962+
// Mode: OPTIONAL ≠
963+
// REQUIRED
964+
"//supplementalData…/group[@ grouping < status", // Status: value ≠
965+
// distinguished Mode:
966+
// OPTIONAL
967+
"//supplementalData…/approvalRequirement[@ votes < locales", // Status: value ≠
968+
// distinguished
969+
// Mode: REQUIRED
970+
"//supplementalData…/mapZone[@ type < other", // Status: value ≠ distinguished
971+
// Mode: REQUIRED
972+
"//supplementalData…/transform[@ variant < direction", // Status: distinguished
973+
// Mode: OPTIONAL ≠
974+
// NULL
975+
"//supplementalData…/transform[@ backwardAlias < visibility", // Status: value
976+
// Mode: OPTIONAL
977+
// ≠ NULL
978+
"//supplementalData…/numberingSystem[@ type < id", // Status: value ≠
979+
// distinguished Mode:
980+
// REQUIRED
981+
"//supplementalData…/variable[@ type < id", // Status: value ≠ distinguished
982+
// Mode: OPTIONAL ≠ REQUIRED
983+
"//ldmlBCP47…/type[@ since < iana", // Status: metadata ≠ value Mode: OPTIONAL
984+
"//ldmlBCP47…/version[@ number < cldrVersion", // Status: metadata ≠ value Mode:
985+
// REQUIRED ≠ FIXED
986+
"//ldmlBCP47…/key[@ extension < name", // Status: distinguished Mode:
987+
// OPTIONAL ≠ REQUIRED
988+
"//ldmlBCP47…/key[@ description < deprecated", // Status: value Mode:
989+
// OPTIONAL ≠ NULL
990+
"//keyboard3…/reorder[@ before < from", // Status: value Mode: OPTIONAL ≠
991+
// REQUIRED
992+
"//keyboardTest3…/repertoire[@ type < name", // Status: value ≠ distinguished
993+
// Mode: OPTIONAL ≠ REQUIRED
994+
"//keyboardTest3…/info[@ author < name" // Status: metadata ≠ distinguished
995+
// Mode: OPTIONAL ≠ REQUIRED
996+
);
997+
998+
public void testAttributeOrder() {
999+
for (DtdType dtdType : DtdType.values()) {
1000+
if (dtdType == DtdType.ldmlICU) {
1001+
continue;
1002+
}
1003+
DtdData dtdData = DtdData.getInstance(dtdType);
1004+
for (Element element : dtdData.getElements()) {
1005+
if (element.isDeprecated()) {
1006+
continue;
1007+
}
1008+
Attribute lastAttribute = null;
1009+
for (Attribute attribute : element.getAttributes().keySet()) {
1010+
if (attribute.isDeprecated() || attribute.name.equals("alt")) {
1011+
continue;
1012+
}
1013+
if (lastAttribute != null) {
1014+
String header =
1015+
Joiners.ES.join(
1016+
"//",
1017+
dtdType,
1018+
"…/",
1019+
element.name,
1020+
"[@ ",
1021+
lastAttribute.name,
1022+
" < ",
1023+
attribute.name);
1024+
int diffToLast =
1025+
attributeStatusModeComparator.compare(attribute, lastAttribute);
1026+
if (diffToLast < 0) {
1027+
String message =
1028+
Joiners.ES.join(
1029+
"\"",
1030+
header,
1031+
"\",\t// Status: ",
1032+
showDiff(
1033+
lastAttribute.getStatus(),
1034+
attribute.getStatus()),
1035+
"\tMode: ",
1036+
showDiff(lastAttribute.mode, attribute.mode));
1037+
if (!attributeOrderGrandfathered.contains(header)) {
1038+
errln(message);
1039+
} else if (isVerbose()) {
1040+
warnln(message);
1041+
}
1042+
}
1043+
}
1044+
lastAttribute = attribute;
1045+
}
1046+
}
1047+
MapComparator<String> comp = dtdData.getAttributeComparator();
1048+
comp.getOrder();
1049+
}
1050+
warnln("Use -v to see overrides");
1051+
}
1052+
1053+
private String showDiff(Object a, Object b) {
1054+
return a == b ? a.toString() : a + " ≠ " + b;
1055+
}
1056+
1057+
final Comparator<Attribute> attributeStatusModeComparator =
1058+
new Comparator<>() {
1059+
@Override
1060+
public int compare(Attribute o1, Attribute o2) {
1061+
return Comparator.comparing(
1062+
Attribute::getStatus) // @distinguished before @value before
1063+
// @metadata;
1064+
.thenComparing(x -> x.mode != Mode.OPTIONAL ? 0 : 1)
1065+
// #optional must come after all others
1066+
// (which can be intermixed)
1067+
.compare(o1, o2);
1068+
}
1069+
};
9391070
}

0 commit comments

Comments
 (0)