Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 36235a5

Browse files
committedSep 23, 2024·
ICU-22908 MF2 ICU4J: Update spec tests and update implementation for recent spec changes
1 parent 2f348f4 commit 36235a5

32 files changed

+514
-391
lines changed
 

‎icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/message2/Mf2FeaturesTest.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ public void testAllKindOfNumbers() {
328328
public void testSpecialPluralWithDecimals() {
329329
String message;
330330
message = ".local $amount = {$count :number}\n"
331-
+ ".match {$amount :number}\n"
331+
+ ".match $amount\n"
332332
+ " 1 {{I have {$amount} dollar.}}\n"
333333
+ " * {{I have {$amount} dollars.}}";
334334
TestUtils.runTestCase(new TestCase.Builder()
@@ -338,7 +338,7 @@ public void testSpecialPluralWithDecimals() {
338338
.expected("I have 1 dollar.")
339339
.build());
340340
message = ".local $amount = {$count :number minimumFractionDigits=2}\n"
341-
+ ".match {$amount :number minimumFractionDigits=2}\n"
341+
+ ".match $amount\n"
342342
+ " one {{I have {$amount} dollar.}}\n"
343343
+ " * {{I have {$amount} dollars.}}";
344344
TestUtils.runTestCase(new TestCase.Builder()
@@ -367,7 +367,8 @@ public void testDefaultFunctionAndOptions() {
367367

368368
@Test
369369
public void testSimpleSelection() {
370-
String message = ".match {$count :number}\n"
370+
String message = ".input {$count :number}\n"
371+
+ ".match $count\n"
371372
+ " 1 {{You have one notification.}}\n"
372373
+ " * {{You have {$count} notifications.}}";
373374

@@ -386,7 +387,9 @@ public void testSimpleSelection() {
386387
@Test
387388
public void testComplexSelection() {
388389
String message = ""
389-
+ ".match {$photoCount :number} {$userGender :string}\n"
390+
+ ".input {$photoCount :number}\n"
391+
+ ".input {$userGender :string}\n"
392+
+ ".match $photoCount $userGender\n"
390393
+ " 1 masculine {{{$userName} added a new photo to his album.}}\n"
391394
+ " 1 feminine {{{$userName} added a new photo to her album.}}\n"
392395
+ " 1 * {{{$userName} added a new photo to their album.}}\n"
@@ -437,8 +440,9 @@ public void testSimpleLocaleVariable() {
437440
@Test
438441
public void testLocaleVariableWithSelect() {
439442
String message = ""
443+
+ ".input {$count :number}\n"
440444
+ ".local $exp = {$expDate :date year=numeric month=short day=numeric weekday=short}\n"
441-
+ ".match {$count :number}\n"
445+
+ ".match $count\n"
442446
+ " 1 {{Your ticket expires on {$exp}.}}\n"
443447
+ " * {{Your {$count} tickets expire on {$exp}.}}";
444448

‎icu4j/main/core/src/main/java/com/ibm/icu/message2/MFDataModelValidator.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ private boolean validateDeclarations(List<Declaration> declarations) throws MFPa
140140
private void validateExpression(Expression expression, boolean fromInput)
141141
throws MFParseException {
142142
String argName = null;
143+
boolean wasLiteral = false;
143144
Annotation annotation = null;
144145
if (expression instanceof Literal) {
145146
// ...{foo}... or ...{|foo|}... or ...{123}...
@@ -148,6 +149,7 @@ private void validateExpression(Expression expression, boolean fromInput)
148149
LiteralExpression le = (LiteralExpression) expression;
149150
argName = le.arg.value;
150151
annotation = le.annotation;
152+
wasLiteral = true;
151153
} else if (expression instanceof VariableExpression) {
152154
VariableExpression ve = (VariableExpression) expression;
153155
// ...{$foo :bar opt1=|str| opt2=$x opt3=$y}...
@@ -184,7 +186,10 @@ private void validateExpression(Expression expression, boolean fromInput)
184186
addVariableDeclaration(argName);
185187
} else {
186188
// Remember that we've seen it, to complain if there is a declaration later
187-
declaredVars.add(argName);
189+
if (!wasLiteral) {
190+
// We don't consider {|bar| :func} to be a declaration of a "bar" variable
191+
declaredVars.add(argName);
192+
}
188193
}
189194
}
190195
}

‎icu4j/main/core/src/main/java/com/ibm/icu/message2/MFParser.java

+13-12
Original file line numberDiff line numberDiff line change
@@ -559,9 +559,9 @@ private MFDataModel.Message getComplexMessage() throws MFParseException {
559559
}
560560
}
561561

562-
// abnf: matcher = match-statement 1*([s] variant)
563-
// abnf: match-statement = match 1*([s] selector)
564-
// abnf: selector = expression
562+
// abnf: matcher = match-statement s variant *([s] variant)
563+
// abnf: match-statement = match 1*(s selector)
564+
// abnf: selector = variable
565565
// abnf: variant = key *(s key) [s] quoted-pattern
566566
// abnf: key = literal / "*"
567567
// abnf: match = %s".match"
@@ -571,17 +571,18 @@ private MFDataModel.SelectMessage getMatch(List<MFDataModel.Declaration> declara
571571
// Look for selectors
572572
List<MFDataModel.Expression> expressions = new ArrayList<>();
573573
while (true) {
574-
// Whitespace not required between selectors:
575-
// match 1*([s] selector)
576-
// Whitespace not required before first variant:
577-
// matcher = match-statement 1*([s] variant)
578-
skipOptionalWhitespaces();
579-
MFDataModel.Expression expression = getPlaceholder();
580-
if (expression == null) {
574+
// Whitespace required between selectors but not required before first variant.
575+
skipMandatoryWhitespaces();
576+
int cp = input.peekChar();
577+
if (cp != '$') {
581578
break;
582579
}
583-
checkCondition(
584-
!(expression instanceof MFDataModel.Markup), "Cannot do selection on markup");
580+
MFDataModel.VariableRef variableRef = getVariableRef();
581+
if (variableRef == null) {
582+
break;
583+
}
584+
MFDataModel.Expression expression =
585+
new MFDataModel.VariableExpression(variableRef, null, new ArrayList<>());
585586
expressions.add(expression);
586587
}
587588

‎icu4j/main/core/src/main/java/com/ibm/icu/message2/MFSerializer.java

+9-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,15 @@ private void selectMessageToString(SelectMessage message) {
7979
result.append(".match");
8080
for (Expression selector : message.selectors) {
8181
result.append(' ');
82-
expressionToString(selector);
82+
if (selector instanceof VariableExpression) {
83+
VariableExpression ve = (VariableExpression) selector;
84+
literalOrVariableRefToString(ve.arg);
85+
} else {
86+
// TODO: we have a (valid?) data model, so do we really want to fail?
87+
// It is very close to release, so I am a bit reluctant to add a throw.
88+
// I tried, and none of the unit tests fail (as expected). But still feels unsafe.
89+
expressionToString(selector);
90+
}
8391
}
8492
for (Variant variant : message.variants) {
8593
variantToString(variant);

‎icu4j/main/core/src/main/java/com/ibm/icu/message2/NumberFormatterFactory.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,16 @@ public FormattedPlaceholder format(Object toFormat, Map<String, Object> variable
163163
private static class PluralSelectorImpl implements Selector {
164164
private static final String NO_MATCH = "\uFFFDNO_MATCH\uFFFE"; // Unlikely to show in a key
165165
private final PluralRules rules;
166-
private Map<String, Object> fixedOptions;
167-
private LocalizedNumberFormatter icuFormatter;
166+
private final Map<String, Object> fixedOptions;
167+
private final LocalizedNumberFormatter icuFormatter;
168+
private final String kind;
168169

169170
private PluralSelectorImpl(
170171
Locale locale, PluralRules rules, Map<String, Object> fixedOptions, String kind) {
171172
this.rules = rules;
172173
this.fixedOptions = fixedOptions;
173174
this.icuFormatter = formatterForOptions(locale, fixedOptions, kind);
175+
this.kind = kind;
174176
}
175177

176178
/**
@@ -252,6 +254,9 @@ private boolean matches(Object value, String key, Map<String, Object> variableOp
252254
} else {
253255
return false;
254256
}
257+
if ("integer".equals(kind)) {
258+
valToCheck = valToCheck.longValue();
259+
}
255260

256261
Number keyNrVal = OptUtils.asNumber(key);
257262
if (keyNrVal != null && valToCheck.doubleValue() == keyNrVal.doubleValue()) {

‎icu4j/main/core/src/test/java/com/ibm/icu/dev/test/message2/CoreTest.java

+33-31
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,39 @@
1414
@SuppressWarnings({"static-method", "javadoc"})
1515
@RunWith(JUnit4.class)
1616
public class CoreTest extends CoreTestFmwk {
17-
private static final String[] JSON_FILES = {"alias-selector-annotations.json",
18-
"duplicate-declarations.json",
19-
"icu-parser-tests.json",
20-
"icu-test-functions.json",
21-
"icu-test-previous-release.json",
22-
"icu-test-selectors.json",
23-
"invalid-number-literals-diagnostics.json",
24-
"invalid-options.json",
25-
"markup.json",
26-
"matches-whitespace.json",
27-
"more-data-model-errors.json",
28-
"more-functions.json",
29-
"resolution-errors.json",
30-
"runtime-errors.json",
31-
"spec/data-model-errors.json",
32-
"spec/syntax-errors.json",
33-
"spec/syntax.json",
34-
"spec/functions/date.json",
35-
"spec/functions/datetime.json",
36-
"spec/functions/integer.json",
37-
"spec/functions/number.json",
38-
"spec/functions/string.json",
39-
"spec/functions/time.json",
40-
"syntax-errors-diagnostics.json",
41-
"syntax-errors-diagnostics-multiline.json",
42-
"syntax-errors-end-of-input.json",
43-
"syntax-errors-reserved.json",
44-
"tricky-declarations.json",
45-
"unsupported-expressions.json",
46-
"unsupported-statements.json",
47-
"valid-tests.json"};
17+
private static final String[] JSON_FILES = {
18+
"alias-selector-annotations.json",
19+
"duplicate-declarations.json",
20+
"icu-parser-tests.json",
21+
"icu-test-functions.json",
22+
"icu-test-previous-release.json",
23+
"icu-test-selectors.json",
24+
"invalid-number-literals-diagnostics.json",
25+
"invalid-options.json",
26+
"markup.json",
27+
"matches-whitespace.json",
28+
"more-data-model-errors.json",
29+
"more-functions.json",
30+
"resolution-errors.json",
31+
"runtime-errors.json",
32+
"spec/data-model-errors.json",
33+
"spec/syntax-errors.json",
34+
"spec/syntax.json",
35+
"spec/functions/date.json",
36+
"spec/functions/datetime.json",
37+
"spec/functions/integer.json",
38+
"spec/functions/number.json",
39+
"spec/functions/string.json",
40+
"spec/functions/time.json",
41+
"syntax-errors-diagnostics.json",
42+
"syntax-errors-diagnostics-multiline.json",
43+
"syntax-errors-end-of-input.json",
44+
"syntax-errors-reserved.json",
45+
"tricky-declarations.json",
46+
"unsupported-expressions.json",
47+
"unsupported-statements.json",
48+
"valid-tests.json"
49+
};
4850

4951
@Test
5052
public void test() throws Exception {

‎icu4j/main/core/src/test/java/com/ibm/icu/dev/test/message2/CustomFormatterMessageRefTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ public String formatToString(Object toFormat, Map<String, Object> variableOption
7979

8080
@BeforeClass
8181
public static void beforeClass() {
82-
PROPERTIES.put("firefox", ".match {$gcase :string} genitive {{Firefoxin}} * {{Firefox}}");
83-
PROPERTIES.put("chrome", ".match {$gcase :string} genitive {{Chromen}} * {{Chrome}}");
84-
PROPERTIES.put("safari", ".match {$gcase :string} genitive {{Safarin}} * {{Safari}}");
82+
PROPERTIES.put("firefox", ".input {$gcase :string} .match $gcase genitive {{Firefoxin}} * {{Firefox}}");
83+
PROPERTIES.put("chrome", ".input {$gcase :string} .match $gcase genitive {{Chromen}} * {{Chrome}}");
84+
PROPERTIES.put("safari", ".input {$gcase :string} .match $gcase genitive {{Safarin}} * {{Safari}}");
8585
}
8686

8787
@Test

‎icu4j/main/core/src/test/java/com/ibm/icu/dev/test/message2/CustomFormatterPersonTest.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -146,11 +146,13 @@ public void testCustomFunctionsComplexMessage() {
146146
Person malePerson = new Person("Mr.", "John", "Doe");
147147
Person unknownPerson = new Person("Mr./Ms.", "Anonymous", "Doe");
148148
String message = ""
149+
+ ".input {$hostGender :string}\n"
150+
+ ".input {$guestCount :number}\n"
149151
+ ".local $hostName = {$host :person length=long}\n"
150152
+ ".local $guestName = {$guest :person length=long}\n"
151153
+ ".local $guestsOther = {$guestCount :number icu:offset=1}\n"
152154
// + "\n"
153-
+ ".match {$hostGender :icu:gender} {$guestCount :number}\n"
155+
+ ".match $hostGender $guestCount\n"
154156
// + "\n"
155157
+ " female 0 {{{$hostName} does not give a party.}}\n"
156158
+ " female 1 {{{$hostName} invites {$guestName} to her party.}}\n"

‎icu4j/main/core/src/test/java/com/ibm/icu/dev/test/message2/DefaultTestProperties.java

+23-3
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,37 @@
33

44
package com.ibm.icu.dev.test.message2;
55

6+
import com.google.gson.JsonArray;
7+
import com.google.gson.JsonElement;
8+
69
// See https://github.com/unicode-org/conformance/blob/main/schema/message_fmt2/testgen_schema.json
710

811
// Class corresponding to the json test files.
912
// Since this is serialized by Gson, the field names should match the keys in the .json files.
1013
class DefaultTestProperties {
14+
private static final Object[] NO_ERRORS = {};
1115
// Unused fields ignored
12-
final String locale;
13-
final Object[] expErrors;
16+
private final String locale;
17+
private final JsonElement expErrors;
1418

15-
DefaultTestProperties(String locale, Object[] expErrors) {
19+
DefaultTestProperties(String locale, JsonElement expErrors) {
1620
this.locale = locale;
1721
this.expErrors = expErrors;
1822
}
23+
24+
String getLocale() {
25+
return this.locale;
26+
}
27+
28+
Object[] getExpErrors() {
29+
if (expErrors == null || !expErrors.isJsonArray()) {
30+
return NO_ERRORS;
31+
}
32+
JsonArray arr = expErrors.getAsJsonArray();
33+
Object [] result = new Object[arr.size()];
34+
for (int i = 0; i < result.length; i++) {
35+
result[i] = arr.get(i);
36+
}
37+
return result;
38+
}
1939
}

0 commit comments

Comments
 (0)
Please sign in to comment.