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 c6010ed
Show file tree
Hide file tree
Showing 25 changed files with 461 additions and 359 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 @@ -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 @@ -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,7 @@ static Map<String, Object> paramsToMap(Param[] params) {

static boolean expectsErrors(DefaultTestProperties defaults, Unit unit) {
return (unit.expErrors != null && !unit.expErrors.isEmpty())
|| (defaults.expErrors != null && defaults.expErrors.length > 0);
|| (defaults.getExpErrors().length > 0);
}

static void runTestCase(DefaultTestProperties defaults, Unit unit) {
Expand All @@ -154,8 +154,8 @@ static void runTestCase(DefaultTestProperties defaults, Unit unit, Param[] param
MessageFormatter.builder().setPattern(pattern.toString());
if (unit.locale != null && !unit.locale.isEmpty()) {
mfBuilder.setLocale(Locale.forLanguageTag(unit.locale));
} else if (defaults.locale != null) {
mfBuilder.setLocale(Locale.forLanguageTag(defaults.locale));
} else if (defaults.getLocale() != null) {
mfBuilder.setLocale(Locale.forLanguageTag(defaults.getLocale()));
} else {
mfBuilder.setLocale(Locale.US);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class Unit {
this.expErrors = expErrors;
}

class Error {
static class Error {
final String name;
final String type;

Expand Down Expand Up @@ -62,6 +62,9 @@ public String toString() {
if (exp != null) {
result.add("exp=" + escapeString(exp));
}
if (ignoreJava != null) {
result.add("ignoreJava=" + ignoreJava);
}
return result.toString();
}

Expand Down
4 changes: 2 additions & 2 deletions testdata/message2/alias-selector-annotations.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
},
"tests": [
{
"src": ".local $one = {|The one| :string}\n .match {$one}\n 1 {{Value is one}}\n * {{Value is not one}}",
"src": ".local $one = {|The one| :string}\n .match $one\n 1 {{Value is one}}\n * {{Value is not one}}",
"exp": "Value is not one"
},
{
"src": ".local $one = {|The one| :string}\n .local $two = {$one}\n .match {$two}\n 1 {{Value is one}}\n * {{Value is not one}}",
"src": ".local $one = {|The one| :string}\n .local $two = {$one}\n .match $two\n 1 {{Value is one}}\n * {{Value is not one}}",
"exp": "Value is not one"
}
]
Expand Down
24 changes: 12 additions & 12 deletions testdata/message2/icu-test-previous-release.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,50 +31,50 @@
"exp": "bar foo"
},
{
"src": ".match {$foo :number} 1 {{one}} * {{other}}",
"src": ".input {$foo :number} .match $foo 1 {{one}} * {{other}}",
"params": [{ "name": "foo", "value": "1" }],
"exp": "one",
"ignoreJava": "See ICU-22809"
},
{
"src": ".match {$foo :string} 1 {{one}} * {{other}}",
"src": ".input {$foo :string} .match $foo 1 {{one}} * {{other}}",
"params": [{ "name": "foo", "value": "1" }],
"exp": "one"
},
{
"src": ".match {$foo :number} 1 {{one}} * {{other}}",
"src": ".input {$foo :number} .match $foo 1 {{one}} * {{other}}",
"params": [{ "name": "foo", "value": 1 }],
"exp": "one"
},
{
"ignoreJava": "Can't pass null in a map",
"ignoreCpp": "Same as Java",
"src": ".match {$foo} 1 {{one}} * {{other}}",
"src": ".match $foo 1 {{one}} * {{other}}",
"params": [{ "name": "foo", "value": null }],
"exp": "other"
},
{
"src": ".match {$foo :number} 1 {{one}} * {{other}}",
"src": ".input {$foo :number} .match $foo 1 {{one}} * {{other}}",
"exp": "other",
"expErrors": [{ "type": "unresolved-variable" }]
},
{
"src": ".local $foo = {$bar} .match {$foo :number} one {{one}} * {{other}}",
"src": ".local $foo = {$bar :number} .match $foo one {{one}} * {{other}}",
"params": [{ "name": "bar", "value": 1 }],
"exp": "one"
},
{
"src": ".local $foo = {$bar} .match {$foo :number} one {{one}} * {{other}}",
"src": ".local $foo = {$bar :number} .match $foo one {{one}} * {{other}}",
"params": [{ "name": "bar", "value": 2 }],
"exp": "other"
},
{
"src": ".local $bar = {$none} .match {$foo :number} one {{one}} * {{{$bar}}}",
"src": ".local $bar = {$none} .input {$foo :number} .match $foo one {{one}} * {{{$bar}}}",
"params": [{ "name": "foo", "value": 1 }, {"name": "none", "value": "" }],
"exp": "one"
},
{
"src": ".local $bar = {$none :number} .match {$foo :string} one {{one}} * {{{$bar}}}",
"src": ".local $bar = {$none :number} .input {$foo :string} .match $foo one {{one}} * {{{$bar}}}",
"params": [{ "name": "foo", "value": 2 }],
"exp": "{$none}",
"expErrors": [{ "type": "unresolved-variable" }],
Expand Down Expand Up @@ -120,17 +120,17 @@
"ignoreCpp": "Fallback is unclear. See https://github.com/unicode-org/message-format-wg/issues/703"
},
{
"src": ".match {|foo| :string} *{{foo}}",
"src": ".local $f = {|foo| :string} .match $f *{{foo}}",
"exp": "foo"
},
{
"src": ".match {$foo :string} * * {{foo}}",
"src": ".input {$foo :string} .match $foo * * {{foo}}",
"exp": "foo",
"expErrors": [{ "type": "variant-key-mismatch" }, { "type": "unresolved-variable" }],
"ignoreCpp": "Fallback is unclear. See https://github.com/unicode-org/message-format-wg/issues/735"
},
{
"src": ".match {$foo :string} {$bar :string} * {{foo}}",
"src": ".input {$foo :string} .input {$bar :string} .match $foo $bar * {{foo}}",
"exp": "foo",
"expErrors": [{ "type": "variant-key-mismatch" }, { "type": "unresolved-variable" }],
"ignoreCpp": "Fallback is unclear. See https://github.com/unicode-org/message-format-wg/issues/735"
Expand Down
Loading

0 comments on commit c6010ed

Please sign in to comment.