Skip to content

Commit

Permalink
ICU-22908 MF2 ICU4J: Update spec tests and update implementation for …
Browse files Browse the repository at this point in the history
…recent spec changes
  • Loading branch information
mihnita committed Sep 22, 2024
1 parent 2f348f4 commit 242d586
Show file tree
Hide file tree
Showing 32 changed files with 33,503 additions and 386 deletions.
32,998 changes: 32,998 additions & 0 deletions icu4j/main/a

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ private boolean validateDeclarations(List<Declaration> declarations) throws MFPa
private void validateExpression(Expression expression, boolean fromInput)
throws MFParseException {
String argName = null;
boolean wasLiteral = false;
Annotation annotation = null;
if (expression instanceof Literal) {
// ...{foo}... or ...{|foo|}... or ...{123}...
Expand All @@ -148,6 +149,7 @@ private void validateExpression(Expression expression, boolean fromInput)
LiteralExpression le = (LiteralExpression) expression;
argName = le.arg.value;
annotation = le.annotation;
wasLiteral = true;
} else if (expression instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) expression;
// ...{$foo :bar opt1=|str| opt2=$x opt3=$y}...
Expand Down Expand Up @@ -184,7 +186,10 @@ private void validateExpression(Expression expression, boolean fromInput)
addVariableDeclaration(argName);
} else {
// Remember that we've seen it, to complain if there is a declaration later
declaredVars.add(argName);
if (!wasLiteral) {
// We don't consider {|bar| :func} to be a declaration of a "bar" variable
declaredVars.add(argName);
}
}
}
}
Expand Down
25 changes: 13 additions & 12 deletions icu4j/main/core/src/main/java/com/ibm/icu/message2/MFParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -559,9 +559,9 @@ private MFDataModel.Message getComplexMessage() throws MFParseException {
}
}

// abnf: matcher = match-statement 1*([s] variant)
// abnf: match-statement = match 1*([s] selector)
// abnf: selector = expression
// abnf: matcher = match-statement s variant *([s] variant)
// abnf: match-statement = match 1*(s selector)
// abnf: selector = variable
// abnf: variant = key *(s key) [s] quoted-pattern
// abnf: key = literal / "*"
// abnf: match = %s".match"
Expand All @@ -571,17 +571,18 @@ private MFDataModel.SelectMessage getMatch(List<MFDataModel.Declaration> declara
// Look for selectors
List<MFDataModel.Expression> expressions = new ArrayList<>();
while (true) {
// Whitespace not required between selectors:
// match 1*([s] selector)
// Whitespace not required before first variant:
// matcher = match-statement 1*([s] variant)
skipOptionalWhitespaces();
MFDataModel.Expression expression = getPlaceholder();
if (expression == null) {
// Whitespace required between selectors but not required before first variant.
skipMandatoryWhitespaces();
int cp = input.peekChar();
if (cp != '$') {
break;
}
checkCondition(
!(expression instanceof MFDataModel.Markup), "Cannot do selection on markup");
MFDataModel.VariableRef variableRef = getVariableRef();
if (variableRef == null) {
break;
}
MFDataModel.Expression expression =
new MFDataModel.VariableExpression(variableRef, null, new ArrayList<>());
expressions.add(expression);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,15 @@ private void selectMessageToString(SelectMessage message) {
result.append(".match");
for (Expression selector : message.selectors) {
result.append(' ');
expressionToString(selector);
if (selector instanceof VariableExpression) {
VariableExpression ve = (VariableExpression) selector;
literalOrVariableRefToString(ve.arg);
} else {
// TODO: we have a (valid?) data model, so do we really want to fail?
// It is very close to release, so I am a bit reluctant to add a throw.
// I tried, and none of the unit tests fail (as expected). But still feels unsafe.
expressionToString(selector);
}
}
for (Variant variant : message.variants) {
variantToString(variant);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,16 @@ public FormattedPlaceholder format(Object toFormat, Map<String, Object> variable
private static class PluralSelectorImpl implements Selector {
private static final String NO_MATCH = "\uFFFDNO_MATCH\uFFFE"; // Unlikely to show in a key
private final PluralRules rules;
private Map<String, Object> fixedOptions;
private LocalizedNumberFormatter icuFormatter;
private final Map<String, Object> fixedOptions;
private final LocalizedNumberFormatter icuFormatter;
private final String kind;

private PluralSelectorImpl(
Locale locale, PluralRules rules, Map<String, Object> fixedOptions, String kind) {
this.rules = rules;
this.fixedOptions = fixedOptions;
this.icuFormatter = formatterForOptions(locale, fixedOptions, kind);
this.kind = kind;
}

/**
Expand Down Expand Up @@ -252,6 +254,9 @@ private boolean matches(Object value, String key, Map<String, Object> variableOp
} else {
return false;
}
if ("integer".equals(kind)) {
valToCheck = valToCheck.intValue();
}

Number keyNrVal = OptUtils.asNumber(key);
if (keyNrVal != null && valToCheck.doubleValue() == keyNrVal.doubleValue()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,37 +14,39 @@
@SuppressWarnings({"static-method", "javadoc"})
@RunWith(JUnit4.class)
public class CoreTest extends CoreTestFmwk {
private static final String[] JSON_FILES = {"alias-selector-annotations.json",
"duplicate-declarations.json",
"icu-parser-tests.json",
"icu-test-functions.json",
"icu-test-previous-release.json",
"icu-test-selectors.json",
"invalid-number-literals-diagnostics.json",
"invalid-options.json",
"markup.json",
"matches-whitespace.json",
"more-data-model-errors.json",
"more-functions.json",
"resolution-errors.json",
"runtime-errors.json",
"spec/data-model-errors.json",
"spec/syntax-errors.json",
"spec/syntax.json",
"spec/functions/date.json",
"spec/functions/datetime.json",
"spec/functions/integer.json",
"spec/functions/number.json",
"spec/functions/string.json",
"spec/functions/time.json",
"syntax-errors-diagnostics.json",
"syntax-errors-diagnostics-multiline.json",
"syntax-errors-end-of-input.json",
"syntax-errors-reserved.json",
"tricky-declarations.json",
"unsupported-expressions.json",
"unsupported-statements.json",
"valid-tests.json"};
private static final String[] JSON_FILES = {
"alias-selector-annotations.json",
"duplicate-declarations.json",
"icu-parser-tests.json",
"icu-test-functions.json",
"icu-test-previous-release.json",
"icu-test-selectors.json",
"invalid-number-literals-diagnostics.json",
"invalid-options.json",
"markup.json",
"matches-whitespace.json",
"more-data-model-errors.json",
"more-functions.json",
"resolution-errors.json",
"runtime-errors.json",
"spec/data-model-errors.json",
"spec/syntax-errors.json",
"spec/syntax.json",
"spec/functions/date.json",
"spec/functions/datetime.json",
"spec/functions/integer.json",
"spec/functions/number.json",
"spec/functions/string.json",
"spec/functions/time.json",
"syntax-errors-diagnostics.json",
"syntax-errors-diagnostics-multiline.json",
"syntax-errors-end-of-input.json",
"syntax-errors-reserved.json",
"tricky-declarations.json",
"unsupported-expressions.json",
"unsupported-statements.json",
"valid-tests.json"
};

@Test
public void test() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ public String formatToString(Object toFormat, Map<String, Object> variableOption

@BeforeClass
public static void beforeClass() {
PROPERTIES.put("firefox", ".match {$gcase :string} genitive {{Firefoxin}} * {{Firefox}}");
PROPERTIES.put("chrome", ".match {$gcase :string} genitive {{Chromen}} * {{Chrome}}");
PROPERTIES.put("safari", ".match {$gcase :string} genitive {{Safarin}} * {{Safari}}");
PROPERTIES.put("firefox", ".input {$gcase :string} .match $gcase genitive {{Firefoxin}} * {{Firefox}}");
PROPERTIES.put("chrome", ".input {$gcase :string} .match $gcase genitive {{Chromen}} * {{Chrome}}");
PROPERTIES.put("safari", ".input {$gcase :string} .match $gcase genitive {{Safarin}} * {{Safari}}");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,13 @@ public void testCustomFunctionsComplexMessage() {
Person malePerson = new Person("Mr.", "John", "Doe");
Person unknownPerson = new Person("Mr./Ms.", "Anonymous", "Doe");
String message = ""
+ ".input {$hostGender :string}\n"
+ ".input {$guestCount :number}\n"
+ ".local $hostName = {$host :person length=long}\n"
+ ".local $guestName = {$guest :person length=long}\n"
+ ".local $guestsOther = {$guestCount :number icu:offset=1}\n"
// + "\n"
+ ".match {$hostGender :icu:gender} {$guestCount :number}\n"
+ ".match $hostGender $guestCount\n"
// + "\n"
+ " female 0 {{{$hostName} does not give a party.}}\n"
+ " female 1 {{{$hostName} invites {$guestName} to her party.}}\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,37 @@

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

import com.google.gson.JsonArray;
import com.google.gson.JsonElement;

// See https://github.com/unicode-org/conformance/blob/main/schema/message_fmt2/testgen_schema.json

// Class corresponding to the json test files.
// Since this is serialized by Gson, the field names should match the keys in the .json files.
class DefaultTestProperties {
private static final Object[] NO_ERRORS = {};
// Unused fields ignored
final String locale;
final Object[] expErrors;
private final String locale;
private final JsonElement expErrors;

DefaultTestProperties(String locale, Object[] expErrors) {
DefaultTestProperties(String locale, JsonElement expErrors) {
this.locale = locale;
this.expErrors = expErrors;
}

String getLocale() {
return this.locale;
}

Object[] getExpErrors() {
if (expErrors == null || !expErrors.isJsonArray()) {
return NO_ERRORS;
}
JsonArray arr = expErrors.getAsJsonArray();
Object [] result = new Object[arr.size()];
for (int i = 0; i < result.length; i++) {
result[i] = arr.get(i);
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ public void testDateFormat() {
@Test
public void testPlural() {
String message = ""
+ ".match {$count :number}\n"
+ ".input {$count :number}\n"
+ ".match $count\n"
+ " 1 {{You have one notification.}}\n"
+ " * {{You have {$count} notifications.}}";

Expand All @@ -147,7 +148,8 @@ public void testPlural() {
@Test
public void testPluralOrdinal() {
String message = ""
+ ".match {$place :number select=ordinal}\n"
+ ".input {$place :number select=ordinal}\n"
+ ".match $place\n"
+ " 1 {{You got the gold medal}}\n"
+ " 2 {{You got the silver medal}}\n"
+ " 3 {{You got the bronze medal}}\n"
Expand Down Expand Up @@ -332,7 +334,8 @@ public void testFormatterIsCreatedOnce() {
@Test
public void testPluralWithOffset() {
String message = ""
+ ".match {$count :number icu:offset=2}\n"
+ ".input {$count :number icu:offset=2}\n"
+ ".match $count\n"
+ " 1 {{Anna}}\n"
+ " 2 {{Anna and Bob}}\n"
+ " one {{Anna, Bob, and {$count :number icu:offset=2} other guest}}\n"
Expand Down Expand Up @@ -361,7 +364,7 @@ public void testPluralWithOffset() {
public void testPluralWithOffsetAndLocalVar() {
String message = ""
+ ".local $foo = {$count :number icu:offset=2}"
+ ".match {$foo :number}\n" // should "inherit" the offset
+ ".match $foo\n" // should "inherit" the offset
+ " 1 {{Anna}}\n"
+ " 2 {{Anna and Bob}}\n"
+ " one {{Anna, Bob, and {$foo} other guest}}\n"
Expand Down Expand Up @@ -390,7 +393,7 @@ public void testPluralWithOffsetAndLocalVar() {
public void testPluralWithOffsetAndLocalVar2() {
String message = ""
+ ".local $foo = {$amount :number icu:skeleton=|.00/w|}\n"
+ ".match {$foo :number}\n" // should "inherit" the offset
+ ".match $foo\n" // should "inherit" the offset
+ " 1 {{Last dollar}}\n"
+ " one {{{$foo} dollar}}\n"
+ " * {{{$foo} dollars}}";
Expand All @@ -412,7 +415,7 @@ public void testPluralWithOffsetAndLocalVar2() {
public void testPluralWithOffsetAndLocalVar2Options() {
String message = ""
+ ".local $foo = {$amount :number minumumFractionalDigits=2}\n"
+ ".match {$foo :number}\n" // should "inherit" the offset
+ ".match $foo\n" // should "inherit" the offset
+ " 1 {{Last dollar}}\n"
+ " one {{{$foo} dollar}}\n"
+ " * {{{$foo} dollars}}";
Expand Down Expand Up @@ -447,7 +450,8 @@ public void testLoopOnLocalVars() {
@Test
public void testVariableOptionsInSelector() {
String messageVar = ""
+ ".match {$count :number icu:offset=$delta}\n"
+ ".input {$count :number icu:offset=$delta}\n"
+ ".match $count\n"
+ " 1 {{A}}\n"
+ " 2 {{A and B}}\n"
+ " one {{A, B, and {$count :number icu:offset=$delta} more character}}\n"
Expand All @@ -465,7 +469,8 @@ public void testVariableOptionsInSelector() {
mfVar.formatToString(Args.of("count", 7, "delta", 2)));

String messageVar2 = ""
+ ".match {$count :number icu:offset=$delta}\n"
+ ".input {$count :number icu:offset=$delta}\n"
+ ".match $count\n"
+ " 1 {{Exactly 1}}\n"
+ " 2 {{Exactly 2}}\n"
+ " * {{Count = {$count :number icu:offset=$delta} and delta={$delta}.}}";
Expand Down Expand Up @@ -505,7 +510,7 @@ public void testVariableOptionsInSelector() {
public void testVariableOptionsInSelectorWithLocalVar() {
String messageFix = ""
+ ".local $offCount = {$count :number icu:offset=2}"
+ ".match {$offCount :number}\n"
+ ".match $offCount\n"
+ " 1 {{A}}\n"
+ " 2 {{A and B}}\n"
+ " one {{A, B, and {$offCount} more character}}\n"
Expand All @@ -520,7 +525,7 @@ public void testVariableOptionsInSelectorWithLocalVar() {

String messageVar = ""
+ ".local $offCount = {$count :number icu:offset=$delta}"
+ ".match {$offCount :number}\n"
+ ".match $offCount\n"
+ " 1 {{A}}\n"
+ " 2 {{A and B}}\n"
+ " one {{A, B, and {$offCount} more character}}\n"
Expand All @@ -539,7 +544,7 @@ public void testVariableOptionsInSelectorWithLocalVar() {

String messageVar2 = ""
+ ".local $offCount = {$count :number icu:offset=$delta}"
+ ".match {$offCount :number}\n"
+ ".match $offCount\n"
+ " 1 {{Exactly 1}}\n"
+ " 2 {{Exactly 2}}\n"
+ " * {{Count = {$count}, OffCount = {$offCount}, and delta={$delta}.}}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ public void testSimpleFormat() {
@Test
public void testSelectFormatToPattern() {
String pattern = ""
+ ".match {$userGender :string}\n"
+ ".input {$userGender :string}\n"
+ ".match $userGender\n"
+ " female {{{$userName} est all\u00E9e \u00E0 Paris.}}"
+ " * {{{$userName} est all\u00E9 \u00E0 Paris.}}"
;
Expand Down
Loading

0 comments on commit 242d586

Please sign in to comment.