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 23, 2024
1 parent 2f348f4 commit 1abe585
Show file tree
Hide file tree
Showing 32 changed files with 514 additions and 391 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ public void testAllKindOfNumbers() {
public void testSpecialPluralWithDecimals() {
String message;
message = ".local $amount = {$count :number}\n"
+ ".match {$amount :number}\n"
+ ".match $amount\n"
+ " 1 {{I have {$amount} dollar.}}\n"
+ " * {{I have {$amount} dollars.}}";
TestUtils.runTestCase(new TestCase.Builder()
Expand All @@ -338,7 +338,7 @@ public void testSpecialPluralWithDecimals() {
.expected("I have 1 dollar.")
.build());
message = ".local $amount = {$count :number minimumFractionDigits=2}\n"
+ ".match {$amount :number minimumFractionDigits=2}\n"
+ ".match $amount\n"
+ " one {{I have {$amount} dollar.}}\n"
+ " * {{I have {$amount} dollars.}}";
TestUtils.runTestCase(new TestCase.Builder()
Expand Down Expand Up @@ -367,7 +367,8 @@ public void testDefaultFunctionAndOptions() {

@Test
public void testSimpleSelection() {
String message = ".match {$count :number}\n"
String message = ".input {$count :number}\n"
+ ".match $count\n"
+ " 1 {{You have one notification.}}\n"
+ " * {{You have {$count} notifications.}}";

Expand All @@ -386,7 +387,9 @@ public void testSimpleSelection() {
@Test
public void testComplexSelection() {
String message = ""
+ ".match {$photoCount :number} {$userGender :string}\n"
+ ".input {$photoCount :number}\n"
+ ".input {$userGender :string}\n"
+ ".match $photoCount $userGender\n"
+ " 1 masculine {{{$userName} added a new photo to his album.}}\n"
+ " 1 feminine {{{$userName} added a new photo to her album.}}\n"
+ " 1 * {{{$userName} added a new photo to their album.}}\n"
Expand Down Expand Up @@ -437,8 +440,9 @@ public void testSimpleLocaleVariable() {
@Test
public void testLocaleVariableWithSelect() {
String message = ""
+ ".input {$count :number}\n"
+ ".local $exp = {$expDate :date year=numeric month=short day=numeric weekday=short}\n"
+ ".match {$count :number}\n"
+ ".match $count\n"
+ " 1 {{Your ticket expires on {$exp}.}}\n"
+ " * {{Your {$count} tickets expire on {$exp}.}}";

Expand Down
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;
}
}
Loading

0 comments on commit 1abe585

Please sign in to comment.