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 4396b04
Show file tree
Hide file tree
Showing 31 changed files with 505 additions and 386 deletions.
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 4396b04

Please sign in to comment.