Skip to content

Commit

Permalink
ICU-22762 MF2: Add builder method to control error handling behavior
Browse files Browse the repository at this point in the history 
Add MessageFormatter::Builder::setErrorHandlingBehavior() method
and a new enum type MessageFormatter::UMFErrorHandlingBehavior
to denote strict or best-effort behavior.

The reason for adding a single method that takes an enum is to allow
for the possibility of more error handling modes in the future.

Co-authored-by: Markus Scherer <[email protected]>
  • Loading branch information
@catamorphism @markusicu
catamorphism and markusicu committed Sep 19, 2024
1 parent ac737da commit 23edf9c
Show file tree
Hide file tree
Showing 10 changed files with 290 additions and 73 deletions.
9 changes: 7 additions & 2 deletions 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 Expand Up @@ -869,7 +874,7 @@ void MessageFormatter::checkDeclarations(MessageContext& context, Environment*&
CHECK_ERROR(status);

const Binding* decls = getDataModel().getLocalVariablesInternal();
U_ASSERT(env != nullptr && decls != nullptr);
U_ASSERT(env != nullptr && (decls != nullptr || getDataModel().bindingsLen == 0));

for (int32_t i = 0; i < getDataModel().bindingsLen; i++) {
const Binding& decl = decls[i];
Expand Down
26 changes: 16 additions & 10 deletions icu4c/source/i18n/messageformat2_data_model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -930,9 +930,10 @@ const Pattern& MFDataModel::getPattern() const {
return *(std::get_if<Pattern>(&body));
}

// Returns nullptr if no bindings
const Binding* MFDataModel::getLocalVariablesInternal() const {
U_ASSERT(!bogus);
U_ASSERT(bindings.isValid());
U_ASSERT(bindingsLen == 0 || bindings.isValid());
return bindings.getAlias();
}

Expand All @@ -948,9 +949,10 @@ const Variant* MFDataModel::getVariantsInternal() const {
return std::get_if<Matcher>(&body)->variants.getAlias();
}

// Returns nullptr if no unsupported statements
const UnsupportedStatement* MFDataModel::getUnsupportedStatementsInternal() const {
U_ASSERT(!bogus);
U_ASSERT(unsupportedStatements.isValid());
U_ASSERT(unsupportedStatementsLen == 0 || unsupportedStatements != nullptr);
return unsupportedStatements.getAlias();
}

Expand Down Expand Up @@ -1056,7 +1058,6 @@ MFDataModel::MFDataModel(const MFDataModel& other) : body(Pattern()) {
UErrorCode localErrorCode = U_ZERO_ERROR;

if (other.hasPattern()) {
// body.emplace<Pattern>(Pattern(*std::get_if<Pattern>(&other.body)));
body = *std::get_if<Pattern>(&other.body);
} else {
const Expression* otherSelectors = other.getSelectorsInternal();
Expand All @@ -1069,17 +1070,17 @@ MFDataModel::MFDataModel(const MFDataModel& other) : body(Pattern()) {
bogus = true;
return;
}
// body.emplace<Matcher>(Matcher(copiedSelectors, numSelectors, copiedVariants, numVariants));
body = Matcher(copiedSelectors, numSelectors, copiedVariants, numVariants);
}

bindingsLen = other.bindingsLen;
bindings.adoptInstead(copyArray(other.bindings.getAlias(), bindingsLen, localErrorCode));
if (U_FAILURE(localErrorCode)) {
bogus = true;
if (bindingsLen > 0) {
bindings.adoptInstead(copyArray(other.bindings.getAlias(), bindingsLen, localErrorCode));
}
unsupportedStatementsLen = other.unsupportedStatementsLen;
unsupportedStatements.adoptInstead(copyArray(other.unsupportedStatements.getAlias(), unsupportedStatementsLen, localErrorCode));
if (unsupportedStatementsLen > 0) {
unsupportedStatements.adoptInstead(copyArray(other.unsupportedStatements.getAlias(), unsupportedStatementsLen, localErrorCode));
}
if (U_FAILURE(localErrorCode)) {
bogus = true;
}
Expand All @@ -1106,9 +1107,14 @@ MFDataModel::MFDataModel(const MFDataModel::Builder& builder, UErrorCode& errorC

U_ASSERT(builder.bindings != nullptr);
bindingsLen = builder.bindings->size();
bindings.adoptInstead(copyVectorToArray<Binding>(*builder.bindings, errorCode));
if (bindingsLen > 0) {
bindings.adoptInstead(copyVectorToArray<Binding>(*builder.bindings, errorCode));
}
unsupportedStatementsLen = builder.unsupportedStatements->size();
unsupportedStatements.adoptInstead(copyVectorToArray<UnsupportedStatement>(*builder.unsupportedStatements, errorCode));
if (unsupportedStatementsLen > 0) {
unsupportedStatements.adoptInstead(copyVectorToArray<UnsupportedStatement>(*builder.unsupportedStatements,
errorCode));
}
if (U_FAILURE(errorCode)) {
bogus = true;
}
Expand Down
81 changes: 45 additions & 36 deletions icu4c/source/i18n/messageformat2_errors.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,41 +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::DuplicateVariant: {
status = U_MF_DUPLICATE_VARIANT_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 @@ -183,7 +152,6 @@ namespace message2 {
break;
}
}
}
}

void StaticErrors::addSyntaxError(UErrorCode& status) {
Expand Down Expand Up @@ -277,6 +245,47 @@ 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::DuplicateVariant: {
status = U_MF_DUPLICATE_VARIANT_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 @@ -100,8 +100,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
35 changes: 31 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,31 @@ 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);
// Check that the checkErrors() method set the error code
U_ASSERT(U_FAILURE(errorCode));
}

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

MessageFormatter::Builder& MessageFormatter::Builder::setDataModel(MFDataModel&& newDataModel) {
normalizedInput.remove();
delete errors;
errors = nullptr;
clearState();
hasPattern = false;
hasDataModel = true;
dataModel = std::move(newDataModel);

return *this;
}

MessageFormatter::Builder&
MessageFormatter::Builder::setErrorHandlingBehavior(
MessageFormatter::UMFErrorHandlingBehavior type) {
signalErrors = type == U_MF_STRICT;
return *this;
}

/*
This build() method is non-destructive, which entails the risk that
its borrowed MFFunctionRegistry and (if the setDataModel() method was called)
Expand All @@ -86,6 +110,7 @@ namespace message2 {
MessageFormatter::Builder::~Builder() {
if (errors != nullptr) {
delete errors;
errors = nullptr;
}
}

Expand Down Expand Up @@ -116,6 +141,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 @@ -162,6 +188,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
4 changes: 2 additions & 2 deletions icu4c/source/i18n/messageformat2_serializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void Serializer::emit(const Pattern& pat) {

void Serializer::serializeDeclarations() {
const Binding* bindings = dataModel.getLocalVariablesInternal();
U_ASSERT(bindings != nullptr);
U_ASSERT(dataModel.bindingsLen == 0 || bindings != nullptr);

for (int32_t i = 0; i < dataModel.bindingsLen; i++) {
const Binding& b = bindings[i];
Expand All @@ -272,7 +272,7 @@ void Serializer::serializeDeclarations() {

void Serializer::serializeUnsupported() {
const UnsupportedStatement* statements = dataModel.getUnsupportedStatementsInternal();
U_ASSERT(statements != nullptr);
U_ASSERT(dataModel.unsupportedStatementsLen == 0 || statements != nullptr);

for (int32_t i = 0; i < dataModel.unsupportedStatementsLen; i++) {
const UnsupportedStatement& s = statements[i];
Expand Down
Loading

0 comments on commit 23edf9c

Please sign in to comment.