Skip to content

Commit

Permalink
ICU-22762 MF2: Add methods to control error handling behavior
Browse files Browse the repository at this point in the history 
Add two new methods to MessageFormatter::Builder,
setStrictErrors() and setSuppressErrors(),
to control whether MF2 errors are signaled or best-effort
output is provided.
  • Loading branch information
@catamorphism
catamorphism committed Aug 2, 2024
1 parent 3e0aaa3 commit db75954
Show file tree
Hide file tree
Showing 8 changed files with 238 additions and 56 deletions.
7 changes: 6 additions & 1 deletion icu4c/source/i18n/messageformat2.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,12 @@ UnicodeString MessageFormatter::formatToString(const MessageArguments& arguments
}
}
// Update status according to all errors seen while formatting
context.checkErrors(status);
if (signalErrors) {
context.checkErrors(status);
}
if (U_FAILURE(status)) {
result.remove();
}
return result;
}

Expand Down
73 changes: 41 additions & 32 deletions icu4c/source/i18n/messageformat2_errors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,37 +121,10 @@ namespace message2 {
if (count() == 0) {
return;
}
if (staticErrors.syntaxAndDataModelErrors->size() > 0) {
switch (staticErrors.first().type) {
case StaticErrorType::DuplicateDeclarationError: {
status = U_MF_DUPLICATE_DECLARATION_ERROR;
break;
}
case StaticErrorType::DuplicateOptionName: {
status = U_MF_DUPLICATE_OPTION_NAME_ERROR;
break;
}
case StaticErrorType::VariantKeyMismatchError: {
status = U_MF_VARIANT_KEY_MISMATCH_ERROR;
break;
}
case StaticErrorType::NonexhaustivePattern: {
status = U_MF_NONEXHAUSTIVE_PATTERN_ERROR;
break;
}
case StaticErrorType::MissingSelectorAnnotation: {
status = U_MF_MISSING_SELECTOR_ANNOTATION_ERROR;
break;
}
case StaticErrorType::SyntaxError: {
status = U_MF_SYNTAX_ERROR;
break;
}
case StaticErrorType::UnsupportedStatementError: {
status = U_MF_UNSUPPORTED_STATEMENT_ERROR;
}
}
} else {
staticErrors.checkErrors(status);
if (U_FAILURE(status)) {
return;
}
U_ASSERT(resolutionAndFormattingErrors->size() > 0);
switch (first().type) {
case DynamicErrorType::UnknownFunction: {
Expand Down Expand Up @@ -179,7 +152,6 @@ namespace message2 {
break;
}
}
}
}

void StaticErrors::addSyntaxError(UErrorCode& status) {
Expand Down Expand Up @@ -269,6 +241,43 @@ namespace message2 {
}
}

void StaticErrors::checkErrors(UErrorCode& status) const {
if (U_FAILURE(status)) {
return;
}
if (syntaxAndDataModelErrors->size() > 0) {
switch (first().type) {
case StaticErrorType::DuplicateDeclarationError: {
status = U_MF_DUPLICATE_DECLARATION_ERROR;
break;
}
case StaticErrorType::DuplicateOptionName: {
status = U_MF_DUPLICATE_OPTION_NAME_ERROR;
break;
}
case StaticErrorType::VariantKeyMismatchError: {
status = U_MF_VARIANT_KEY_MISMATCH_ERROR;
break;
}
case StaticErrorType::NonexhaustivePattern: {
status = U_MF_NONEXHAUSTIVE_PATTERN_ERROR;
break;
}
case StaticErrorType::MissingSelectorAnnotation: {
status = U_MF_MISSING_SELECTOR_ANNOTATION_ERROR;
break;
}
case StaticErrorType::SyntaxError: {
status = U_MF_SYNTAX_ERROR;
break;
}
case StaticErrorType::UnsupportedStatementError: {
status = U_MF_UNSUPPORTED_STATEMENT_ERROR;
}
}
}
}

const StaticError& StaticErrors::first() const {
U_ASSERT(syntaxAndDataModelErrors.isValid() && syntaxAndDataModelErrors->size() > 0);
return *static_cast<StaticError*>(syntaxAndDataModelErrors->elementAt(0));
Expand Down
3 changes: 2 additions & 1 deletion icu4c/source/i18n/messageformat2_errors.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ namespace message2 {
bool hasSyntaxError() const { return syntaxError; }
bool hasMissingSelectorAnnotationError() const { return missingSelectorAnnotationError; }
void addError(StaticError&&, UErrorCode&);
void checkErrors(UErrorCode&);
void checkErrors(UErrorCode&) const;

void clear();
const StaticError& first() const;
StaticErrors(const StaticErrors&, UErrorCode&);
StaticErrors(StaticErrors&&) noexcept;
Expand Down
26 changes: 22 additions & 4 deletions icu4c/source/i18n/messageformat2_formatter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,30 @@ namespace message2 {
// -------------------------------------
// Creates a MessageFormat instance based on the pattern.

MessageFormatter::Builder& MessageFormatter::Builder::setPattern(const UnicodeString& pat, UParseError& parseError, UErrorCode& errorCode) {
void MessageFormatter::Builder::clearState() {
normalizedInput.remove();
delete errors;
errors = nullptr;
}

MessageFormatter::Builder& MessageFormatter::Builder::setPattern(const UnicodeString& pat,
UParseError& parseError,
UErrorCode& errorCode) {
clearState();
// Create errors
errors = create<StaticErrors>(StaticErrors(errorCode), errorCode);
THIS_ON_ERROR(errorCode);

// Parse the pattern
MFDataModel::Builder tree(errorCode);
Parser(pat, tree, *errors, normalizedInput).parse(parseError, errorCode);

// Fail on syntax errors
if (errors->hasSyntaxError()) {
errors->checkErrors(errorCode);
U_ASSERT(U_FAILURE(errorCode));
}

// Build the data model based on what was parsed
dataModel = tree.build(errorCode);
hasDataModel = true;
Expand All @@ -55,9 +73,7 @@ namespace message2 {
}

MessageFormatter::Builder& MessageFormatter::Builder::setDataModel(MFDataModel&& newDataModel) {
normalizedInput.remove();
delete errors;
errors = nullptr;
clearState();
hasPattern = false;
hasDataModel = true;
dataModel = std::move(newDataModel);
Expand Down Expand Up @@ -117,6 +133,7 @@ namespace message2 {
standardMFFunctionRegistry.checkStandard();

normalizedInput = builder.normalizedInput;
signalErrors = builder.signalErrors;

// Build data model
// First, check that there is a data model
Expand Down Expand Up @@ -163,6 +180,7 @@ namespace message2 {
customMFFunctionRegistry = other.customMFFunctionRegistry;
dataModel = std::move(other.dataModel);
normalizedInput = std::move(other.normalizedInput);
signalErrors = other.signalErrors;
errors = other.errors;
other.errors = nullptr;
return *this;
Expand Down
51 changes: 49 additions & 2 deletions icu4c/source/i18n/unicode/messageformat2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ namespace message2 {
Locale locale;
// Not owned
const MFFunctionRegistry* customMFFunctionRegistry;
// Error behavior; see comment in `MessageFormatter` class
bool signalErrors = false;

void clearState();
public:
/**
* Sets the locale to use for formatting.
Expand Down Expand Up @@ -218,6 +221,43 @@ namespace message2 {
* @deprecated This API is for technology preview only.
*/
Builder& setDataModel(MFDataModel&& dataModel);
/**
* Set this formatter to handle errors "strictly",
* meaning that formatting methods will set their
* UErrorCode arguments to signal MessageFormat data model,
* resolution, and runtime errors. Syntax errors are
* always signaled.
* This doesn't re-trigger parsing of an existing pattern,
* so it has to be called before parsing the pattern
* whose error behavior should be affected.
*
* The default is to suppress all MessageFormat errors
* and return best-effort output.
*
* @return A reference to the builder.
*
* @internal ICU 75 technology preview
* @deprecated This API is for technology preview only.
*/
Builder& setStrictErrors() { signalErrors = true; return *this; }
/**
* Set this formatter to suppress errors
* meaning that formatting methods will _not_ set their
* UErrorCode arguments to signal MessageFormat data model,
* resolution, or runtime errors. Best-effort output
* will be returned. Syntax errors are always signaled.
* This is the default behavior.
*
* This doesn't re-trigger parsing of an existing pattern,
* so it has to be called before parsing the pattern
* whose error behavior should be affected.
*
* @return A reference to the builder.
*
* @internal ICU 75 technology preview
* @deprecated This API is for technology preview only.
*/
Builder& setSuppressErrors() { signalErrors = false; return *this; }
/**
* Constructs a new immutable MessageFormatter using the pattern or data model
* that was previously set, and the locale (if it was previously set)
Expand Down Expand Up @@ -378,8 +418,15 @@ namespace message2 {
// Must be a raw pointer to avoid including the internal header file
// defining StaticErrors
// Owned by `this`
StaticErrors* errors;

StaticErrors* errors = nullptr;

// Error handling behavior.
// If true, then formatting methods set their UErrorCode arguments
// to signal MessageFormat errors, and no useful output is returned.
// If false, then MessageFormat errors are not signaled and the
// formatting methods return best-effort output.
// The default is false.
bool signalErrors = false;
}; // class MessageFormatter

} // namespace message2
Expand Down
78 changes: 75 additions & 3 deletions icu4c/source/test/intltest/messageformat2test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ TestMessageFormat2::runIndexedTest(int32_t index, UBool exec,
TESTCASE_AUTO(testAPI);
TESTCASE_AUTO(testAPISimple);
TESTCASE_AUTO(testDataModelAPI);
TESTCASE_AUTO(testFormatterAPI);
TESTCASE_AUTO(dataDrivenTests);
TESTCASE_AUTO_END;
}
Expand Down Expand Up @@ -64,6 +65,75 @@ void TestMessageFormat2::testDataModelAPI() {
assertEquals("testDataModelAPI", i, 3);
}

// Needs more tests
void TestMessageFormat2::testFormatterAPI() {
IcuTestErrorCode errorCode(*this, "testFormatterAPI");
UnicodeString result;
UParseError parseError;

// Check that constructing the formatter fails
// if there's a syntax error
UnicodeString pattern = "{{}";
MessageFormatter::Builder mfBuilder(errorCode);
mfBuilder.setSuppressErrors(); // This shouldn't matter, since there's a syntax error
mfBuilder.setPattern(pattern, parseError, errorCode);
MessageFormatter mf = mfBuilder.build(errorCode);
U_ASSERT(errorCode == U_MF_SYNTAX_ERROR);

/*
Parsing is done when setPattern() is called,
so setStrictErrors() or setSuppressErrors must be called
_before_ setPattern() to get the right behavior,
and if either method is called after setting a pattern,
setPattern() has to be called again.
*/
// Should get the same behavior with strict errors
errorCode.reset();
mfBuilder.setStrictErrors();
// Force re-parsing, as above comment
mfBuilder.setPattern(pattern, parseError, errorCode);
mf = mfBuilder.build(errorCode);
U_ASSERT(errorCode == U_MF_SYNTAX_ERROR);

// Try the same thing for a pattern with a resolution error
pattern = "{{{$x}}}";
errorCode.reset();
// Check that a pattern with a resolution error gives fallback output
mfBuilder.setSuppressErrors();
mfBuilder.setPattern(pattern, parseError, errorCode);
mf = mfBuilder.build(errorCode);
U_ASSERT(U_SUCCESS(errorCode));
result = mf.formatToString(MessageArguments(), errorCode);
U_ASSERT(U_SUCCESS(errorCode));
U_ASSERT(result == "{$x}");

// Check that we do get an error with strict errors
mfBuilder.setStrictErrors();
mf = mfBuilder.build(errorCode);
U_ASSERT(U_SUCCESS(errorCode));
result = mf.formatToString(MessageArguments(), errorCode);
U_ASSERT(errorCode == U_MF_UNRESOLVED_VARIABLE_ERROR);

// Finally, check a valid pattern
errorCode.reset();
pattern = "hello";
mfBuilder.setPattern(pattern, parseError, errorCode);
mfBuilder.setSuppressErrors();
mf = mfBuilder.build(errorCode);
U_ASSERT(U_SUCCESS(errorCode));
result = mf.formatToString(MessageArguments(), errorCode);
U_ASSERT(U_SUCCESS(errorCode));
U_ASSERT(result == "hello");

// Check that behavior is the same with strict errors
mfBuilder.setStrictErrors();
mf = mfBuilder.build(errorCode);
U_ASSERT(U_SUCCESS(errorCode));
result = mf.formatToString(MessageArguments(), errorCode);
U_ASSERT(U_SUCCESS(errorCode));
U_ASSERT(result == "hello");
}

// Example for design doc -- version without null and error checks
void TestMessageFormat2::testAPISimple() {
IcuTestErrorCode errorCode1(*this, "testAPI");
Expand Down Expand Up @@ -216,9 +286,11 @@ void TestMessageFormat2::testAPICustomFunctions() {
MessageFormatter::Builder mfBuilder(errorCode);
UnicodeString result;
// This fails, because we did not provide a function registry:
MessageFormatter mf = mfBuilder.setPattern("Hello {$name :person formality=informal}", parseError, errorCode)
.setLocale(locale)
.build(errorCode);
MessageFormatter mf = mfBuilder.setStrictErrors()
.setPattern("Hello {$name :person formality=informal}",
parseError, errorCode)
.setLocale(locale)
.build(errorCode);
result = mf.formatToString(arguments, errorCode);
assertEquals("testAPICustomFunctions", U_MF_UNKNOWN_FUNCTION_ERROR, errorCode);

Expand Down
2 changes: 2 additions & 0 deletions icu4c/source/test/intltest/messageformat2test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class TestMessageFormat2: public IntlTest {
void testCustomFunctions(void);
// Test the data model API
void testDataModelAPI(void);
// Test the formatting API
void testFormatterAPI(void);
void testAPI(void);
void testAPISimple(void);

Expand Down
Loading

0 comments on commit db75954

Please sign in to comment.