Skip to content

Commit 7d6b8d6

Browse files
mihnitamihnita
committed
ICU-22894 MF2, ICU4J: implements configuring the error handling behavior
Implements the decision at unicode-org/message-format-wg#879
1 parent 09c5aa1 commit 7d6b8d6

File tree

5 files changed

+183
-18
lines changed

5 files changed

+183
-18
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,9 @@ public FormattedPlaceholder format(Object toFormat, Map<String, Object> variable
353353
toFormat, new PlainStringFormattedValue("{|" + toFormat + "|}"));
354354
}
355355
} else if (toFormat instanceof Clock) {
356-
toFormat = JavaTimeConverters.temporalToCalendar((Clock) toFormat);
356+
toFormat = JavaTimeConverters.temporalToCalendar((Clock) toFormat);
357357
} else if (toFormat instanceof Temporal) {
358-
toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat);
358+
toFormat = JavaTimeConverters.temporalToCalendar((Temporal) toFormat);
359359
}
360360
// Not an else-if here, because the `Clock` & `Temporal` conditions before make `toFormat` a `Calendar`
361361
if (toFormat instanceof Calendar) {

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

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.ibm.icu.message2.MFDataModel.StringPart;
3131
import com.ibm.icu.message2.MFDataModel.VariableRef;
3232
import com.ibm.icu.message2.MFDataModel.Variant;
33+
import com.ibm.icu.message2.MessageFormatter.ErrorHandlingBehavior;
3334
import com.ibm.icu.util.Calendar;
3435
import com.ibm.icu.util.CurrencyAmount;
3536

@@ -40,15 +41,21 @@
4041
// TODO: move this in the MessageFormatter?
4142
class MFDataModelFormatter {
4243
private final Locale locale;
44+
private final ErrorHandlingBehavior errorHandlingBehavior;
4345
private final MFDataModel.Message dm;
4446

4547
private final MFFunctionRegistry standardFunctions;
4648
private final MFFunctionRegistry customFunctions;
4749
private static final MFFunctionRegistry EMPTY_REGISTY = MFFunctionRegistry.builder().build();
4850

4951
MFDataModelFormatter(
50-
MFDataModel.Message dm, Locale locale, MFFunctionRegistry customFunctionRegistry) {
52+
MFDataModel.Message dm,
53+
Locale locale,
54+
ErrorHandlingBehavior errorHandlingBehavior,
55+
MFFunctionRegistry customFunctionRegistry) {
5156
this.locale = locale;
57+
this.errorHandlingBehavior = errorHandlingBehavior == null
58+
? ErrorHandlingBehavior.BEST_EFFORT : errorHandlingBehavior;
5259
this.dm = dm;
5360
this.customFunctions =
5461
customFunctionRegistry == null ? EMPTY_REGISTY : customFunctionRegistry;
@@ -96,21 +103,23 @@ String format(Map<String, Object> arguments) {
96103
if (dm instanceof MFDataModel.PatternMessage) {
97104
MFDataModel.PatternMessage pm = (MFDataModel.PatternMessage) dm;
98105
variables = resolveDeclarations(pm.declarations, arguments);
106+
if (pm.pattern == null) {
107+
fatalFormattingError("The PatternMessage is null.");
108+
}
99109
patternToRender = pm.pattern;
100110
} else if (dm instanceof MFDataModel.SelectMessage) {
101111
MFDataModel.SelectMessage sm = (MFDataModel.SelectMessage) dm;
102112
variables = resolveDeclarations(sm.declarations, arguments);
103113
patternToRender = findBestMatchingPattern(sm, variables, arguments);
114+
if (patternToRender == null) {
115+
fatalFormattingError("Cannor find a match for the selector.");
116+
}
104117
} else {
105118
fatalFormattingError("Unknown message type.");
106119
// formattingError throws, so the return does not actually happen
107120
return "ERROR!";
108121
}
109122

110-
if (patternToRender == null) {
111-
return "ERROR!";
112-
}
113-
114123
StringBuilder result = new StringBuilder();
115124
for (MFDataModel.PatternPart part : patternToRender.parts) {
116125
if (part instanceof MFDataModel.StringPart) {
@@ -177,15 +186,15 @@ private Pattern findBestMatchingPattern(
177186
// spec: Append `rv` as the last element of the list `res`.
178187
res.add(rs);
179188
} else {
180-
throw new IllegalArgumentException("Unknown selector type: " + functionName);
189+
fatalFormattingError("Unknown selector type: " + functionName);
181190
}
182191
}
183192

184193
// This should not be possible, we added one function for each selector,
185194
// or we have thrown an exception.
186195
// But just in case someone removes the throw above?
187196
if (res.size() != selectors.size()) {
188-
throw new IllegalArgumentException(
197+
fatalFormattingError(
189198
"Something went wrong, not enough selector functions, "
190199
+ res.size() + " vs. " + selectors.size());
191200
}
@@ -324,8 +333,7 @@ private Pattern findBestMatchingPattern(
324333
// And should do that only once, when building the data model.
325334
if (patternToRender == null) {
326335
// If there was a case with all entries in the keys `*` this should not happen
327-
throw new IllegalArgumentException(
328-
"The selection went wrong, cannot select any option.");
336+
fatalFormattingError("The selection went wrong, cannot select any option.");
329337
}
330338

331339
return patternToRender;
@@ -396,7 +404,7 @@ public ResolvedSelector(
396404
}
397405
}
398406

399-
private static void fatalFormattingError(String message) {
407+
private static void fatalFormattingError(String message) throws IllegalArgumentException {
400408
throw new IllegalArgumentException(message);
401409
}
402410

@@ -414,7 +422,7 @@ private FormatterFactory getFormattingFunctionFactoryByName(
414422
functionName = customFunctions.getDefaultFormatterNameForType(clazz);
415423
}
416424
if (functionName == null) {
417-
throw new IllegalArgumentException(
425+
fatalFormattingError(
418426
"Object to format without a function, and unknown type: "
419427
+ toFormat.getClass().getName());
420428
}
@@ -530,11 +538,17 @@ private FormattedPlaceholder formatExpression(
530538

531539
FormatterFactory funcFactory = getFormattingFunctionFactoryByName(toFormat, functionName);
532540
if (funcFactory == null) {
541+
if (errorHandlingBehavior == ErrorHandlingBehavior.STRICT) {
542+
fatalFormattingError("unable to find function at " + fallbackString);
543+
}
533544
return new FormattedPlaceholder(expression, new PlainStringFormattedValue(fallbackString));
534545
}
535546
Formatter ff = funcFactory.createFormatter(locale, options);
536547
String res = ff.formatToString(toFormat, arguments);
537548
if (res == null) {
549+
if (errorHandlingBehavior == ErrorHandlingBehavior.STRICT) {
550+
fatalFormattingError("unable to format string at " + fallbackString);
551+
}
538552
res = fallbackString;
539553
}
540554

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

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,15 @@
143143
public class MessageFormatter {
144144
private final Locale locale;
145145
private final String pattern;
146+
private final ErrorHandlingBehavior errorHandlingBehavior;
146147
private final MFFunctionRegistry functionRegistry;
147148
private final MFDataModel.Message dataModel;
148149
private final MFDataModelFormatter modelFormatter;
149150

150151
private MessageFormatter(Builder builder) {
151152
this.locale = builder.locale;
152153
this.functionRegistry = builder.functionRegistry;
154+
this.errorHandlingBehavior = builder.errorHandlingBehavior;
153155
if ((builder.pattern == null && builder.dataModel == null)
154156
|| (builder.pattern != null && builder.dataModel != null)) {
155157
throw new IllegalArgumentException(
@@ -171,7 +173,7 @@ private MessageFormatter(Builder builder) {
171173
+ "Error: " + pe.getMessage() + "\n");
172174
}
173175
}
174-
modelFormatter = new MFDataModelFormatter(dataModel, locale, functionRegistry);
176+
modelFormatter = new MFDataModelFormatter(dataModel, locale, errorHandlingBehavior, functionRegistry);
175177
}
176178

177179
/**
@@ -201,6 +203,20 @@ public Locale getLocale() {
201203
return locale;
202204
}
203205

206+
/**
207+
* Get the {@link ErrorHandlingBehavior} to use when encountering errors in
208+
* the current {@code MessageFormatter}.
209+
*
210+
* @return the error handling behavior.
211+
*
212+
* @internal ICU 76 technology preview
213+
* @deprecated This API is for technology preview only.
214+
*/
215+
@Deprecated
216+
public ErrorHandlingBehavior getErrorHandlingBehavior() {
217+
return errorHandlingBehavior;
218+
}
219+
204220
/**
205221
* Get the pattern (the serialized message in MessageFormat 2 syntax) of
206222
* the current {@code MessageFormatter}.
@@ -271,6 +287,37 @@ public FormattedMessage format(Map<String, Object> arguments) {
271287
throw new RuntimeException("Not yet implemented.");
272288
}
273289

290+
/**
291+
* Determines how the formatting errors will be handled at runtime.
292+
*
293+
* <p>Parsing errors always throw and will not be affected by this setting.<br>
294+
* But formatting errors will either try to fallback (if possible) or throw,
295+
* depending on this setting.</p>
296+
*
297+
* <p>Used in conjunction with the
298+
* {@link MessageFormatter.Builder#setErrorHandlingBehavior(ErrorHandlingBehavior)} method.</p>
299+
*
300+
* @internal ICU 76 technology preview
301+
* @deprecated This API is for technology preview only.
302+
*/
303+
@Deprecated
304+
public static enum ErrorHandlingBehavior {
305+
/**
306+
* Suppress errors and return best-effort output.
307+
*
308+
* @internal ICU 76 technology preview
309+
* @deprecated This API is for technology preview only.
310+
*/
311+
BEST_EFFORT,
312+
/**
313+
* Signal all {@code MessageFormat} errors by throwing a {@link RuntimeException}.
314+
*
315+
* @internal ICU 76 technology preview
316+
* @deprecated This API is for technology preview only.
317+
*/
318+
STRICT
319+
}
320+
274321
/**
275322
* A {@code Builder} used to build instances of {@link MessageFormatter}.
276323
*
@@ -281,6 +328,7 @@ public FormattedMessage format(Map<String, Object> arguments) {
281328
public static class Builder {
282329
private Locale locale = Locale.getDefault(Locale.Category.FORMAT);
283330
private String pattern = null;
331+
private ErrorHandlingBehavior errorHandlingBehavior = ErrorHandlingBehavior.BEST_EFFORT;
284332
private MFFunctionRegistry functionRegistry = MFFunctionRegistry.builder().build();
285333
private MFDataModel.Message dataModel = null;
286334

@@ -319,6 +367,26 @@ public Builder setPattern(String pattern) {
319367
return this;
320368
}
321369

370+
/**
371+
* Sets the {@link ErrorHandlingBehavior} to use when encountering errors at formatting time.
372+
*
373+
* <p>Parsing errors always throw and not affected by this setting.
374+
* But formatting errors will try to fallback (if possible) or throw, depending on this setting.</p>
375+
*
376+
* <p>The default value is {@code ErrorHandlingBehavior.BEST_EFFORT}, trying to fallback.</p>
377+
*
378+
* @param the error handling behavior to use.
379+
* @return the builder, for fluent use.
380+
*
381+
* @internal ICU 76 technology preview
382+
* @deprecated This API is for technology preview only.
383+
*/
384+
@Deprecated
385+
public Builder setErrorHandlingBehavior(ErrorHandlingBehavior errorHandlingBehavior) {
386+
this.errorHandlingBehavior = errorHandlingBehavior;
387+
return this;
388+
}
389+
322390
/**
323391
* Sets an instance of {@link MFFunctionRegistry} that should register any
324392
* custom functions used by the message.

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

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -574,4 +574,85 @@ public void testVariableOptionsInSelectorWithLocalVar() {
574574
assertEquals("test local vars loop", "Count = 23, OffCount = 21, and delta=2.",
575575
mfVar2.formatToString(Args.of("count", 23, "delta", 2)));
576576
}
577+
578+
// Needs more tests. Ported from the equivalent test in ICU4C
579+
@Test
580+
public void testFormatterAPI() {
581+
String result;
582+
Map<String, Object> messageArguments = new HashMap<>();
583+
584+
// Check that constructing the formatter fails
585+
// if there's a syntax error
586+
String pattern = "{{}";
587+
MessageFormatter.Builder mfBuilder = MessageFormatter.builder();
588+
MessageFormatter mf;
589+
try {
590+
mf = mfBuilder
591+
// This shouldn't matter, since there's a syntax error
592+
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
593+
.setPattern(pattern)
594+
.build();
595+
errln("error expected");
596+
} catch (IllegalArgumentException e) {
597+
assertTrue("", e.getMessage().contains("Parse error"));
598+
}
599+
600+
/*
601+
Parsing is done when setPattern() is called,
602+
so setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT) or setSuppressErrors must be called
603+
_before_ setPattern() to get the right behavior,
604+
and if either method is called after setting a pattern,
605+
setPattern() has to be called again.
606+
*/
607+
608+
// Should get the same behavior with strict errors
609+
try {
610+
mf = mfBuilder.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
611+
// Force re-parsing, as above comment
612+
.setPattern(pattern)
613+
.build();
614+
errln("error expected");
615+
} catch (IllegalArgumentException e) {
616+
assertTrue("", e.getMessage().contains("Parse error"));
617+
}
618+
619+
// Try the same thing for a pattern with a resolution error
620+
pattern = "{{{$x}}}";
621+
// Check that a pattern with a resolution error gives fallback output
622+
mf = mfBuilder
623+
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
624+
.setPattern(pattern)
625+
.build();
626+
result = mf.formatToString(messageArguments);
627+
assertEquals("", "{$x}", result);
628+
629+
try {
630+
// Check that we do get an error with strict errors
631+
mf = mfBuilder
632+
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
633+
.build();
634+
// U_ASSERT(U_SUCCESS(errorCode));
635+
result = mf.formatToString(messageArguments);
636+
errln("error expected");
637+
} catch (IllegalArgumentException e) {
638+
assertTrue("", e.getMessage().contains("unable to find function"));
639+
}
640+
641+
// Finally, check a valid pattern
642+
pattern = "hello";
643+
mf = mfBuilder
644+
.setPattern(pattern)
645+
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.BEST_EFFORT)
646+
.build();
647+
result = mf.formatToString(messageArguments);
648+
assertEquals("", "hello", result);
649+
650+
// Check that behavior is the same with strict errors
651+
mf = mfBuilder
652+
.setErrorHandlingBehavior(MessageFormatter.ErrorHandlingBehavior.STRICT)
653+
.build();
654+
result = mf.formatToString(messageArguments);
655+
assertEquals("", "hello", result);
656+
}
657+
577658
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,9 @@ static void rewriteDates(Param[] params) {
8585
// "params": [{"name": "exp"}, { "value": { "date": 1722746637000 } }]
8686
for (int i = 0; i < params.length; i++) {
8787
Param pair = params[i];
88-
if (pair.value instanceof Map) {
89-
Map innerMap = (Map) pair.value;
88+
if (pair.value instanceof Map<?, ?>) {
89+
@SuppressWarnings("unchecked")
90+
Map<String, Object> innerMap = (Map<String, Object>) pair.value;
9091
if (innerMap.size() == 1 && innerMap.containsKey("date") && innerMap.get("date") instanceof Double) {
9192
Long dateValue = Double.valueOf((Double) innerMap.get("date")).longValue();
9293
params[i] = new Param(pair.name, new Date(dateValue));
@@ -103,8 +104,9 @@ static void rewriteDecimals(Param[] params) {
103104
// "params": [{"name": "val"}, {"value": {"decimal": "1234567890123456789.987654321"}}]
104105
for (int i = 0; i < params.length; i++) {
105106
Param pair = params[i];
106-
if (pair.value instanceof Map) {
107-
Map innerMap = (Map) pair.value;
107+
if (pair.value instanceof Map<?, ?>) {
108+
@SuppressWarnings("unchecked")
109+
Map<String, Object> innerMap = (Map<String, Object>) pair.value;
108110
if (innerMap.size() == 1 && innerMap.containsKey("decimal")
109111
&& innerMap.get("decimal") instanceof String) {
110112
String decimalValue = (String) innerMap.get("decimal");

0 commit comments

Comments
 (0)