Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ICU-22747 MF2: Validate values of integer-valued options for :number and :integer #2973

Draft
wants to merge 1 commit into
base: maint/maint-75
Choose a base branch
from
Draft

ICU-22747 MF2: Validate values of integer-valued options for :number and :integer #2973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 30 additions & 51 deletions icu4c/source/i18n/messageformat2_function_registry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,11 @@ MFFunctionRegistry::~MFFunctionRegistry() {
}
}

int32_t maxSignificantDigits = number.maximumSignificantDigits(opts);
int32_t maxSignificantDigits = number.maximumSignificantDigits(opts, status);
if (!isInteger) {
int32_t minFractionDigits = number.minimumFractionDigits(opts);
int32_t maxFractionDigits = number.maximumFractionDigits(opts);
int32_t minSignificantDigits = number.minimumSignificantDigits(opts);
int32_t minFractionDigits = number.minimumFractionDigits(opts, status);
int32_t maxFractionDigits = number.maximumFractionDigits(opts, status);
int32_t minSignificantDigits = number.minimumSignificantDigits(opts, status);
Precision p = Precision::minMaxFraction(minFractionDigits, maxFractionDigits);
if (minSignificantDigits > 0) {
p = p.minSignificantDigits(minSignificantDigits);
Expand All @@ -314,7 +314,7 @@ MFFunctionRegistry::~MFFunctionRegistry() {
}

// All other options apply to both `:number` and `:integer`
int32_t minIntegerDigits = number.minimumIntegerDigits(opts);
int32_t minIntegerDigits = number.minimumIntegerDigits(opts, status);
nf = nf.integerWidth(IntegerWidth::zeroFillTo(minIntegerDigits));

// signDisplay
Expand Down Expand Up @@ -405,83 +405,62 @@ static FormattedPlaceholder stringAsNumber(const number::LocalizedNumberFormatte
return FormattedPlaceholder(input, FormattedValue(std::move(result)));
}

int32_t StandardFunctions::Number::maximumFractionDigits(const FunctionOptions& opts) const {
int32_t StandardFunctions::Number::intValuedOption(const FunctionOptions& opts,
const UnicodeString& optionName,
int32_t defaultVal,
UErrorCode& status) const {
Formattable opt;

if (opts.getFunctionOption(optionName, opt)) {
return static_cast<int32_t>(getInt64Value(locale, opt, status));
}
return defaultVal;
}

int32_t StandardFunctions::Number::maximumFractionDigits(const FunctionOptions& opts, UErrorCode& status) const {
Formattable opt;

if (isInteger) {
return 0;
}

if (opts.getFunctionOption(UnicodeString("maximumFractionDigits"), opt)) {
UErrorCode localErrorCode = U_ZERO_ERROR;
int64_t val = getInt64Value(locale, opt, localErrorCode);
if (U_SUCCESS(localErrorCode)) {
return static_cast<int32_t>(val);
}
}
return number::impl::kMaxIntFracSig;
return intValuedOption(opts, UnicodeString("maximumFractionDigits"), number::impl::kMaxIntFracSig, status);
}

int32_t StandardFunctions::Number::minimumFractionDigits(const FunctionOptions& opts) const {
int32_t StandardFunctions::Number::minimumFractionDigits(const FunctionOptions& opts, UErrorCode& status) const {
Formattable opt;

if (!isInteger) {
if (opts.getFunctionOption(UnicodeString("minimumFractionDigits"), opt)) {
UErrorCode localErrorCode = U_ZERO_ERROR;
int64_t val = getInt64Value(locale, opt, localErrorCode);
if (U_SUCCESS(localErrorCode)) {
return static_cast<int32_t>(val);
}
}
if (isInteger) {
return 0;
}
return 0;
return intValuedOption(opts, UnicodeString("minimumFractionDigits"), 0, status);
}

int32_t StandardFunctions::Number::minimumIntegerDigits(const FunctionOptions& opts) const {
int32_t StandardFunctions::Number::minimumIntegerDigits(const FunctionOptions& opts, UErrorCode& status) const {
Formattable opt;

if (opts.getFunctionOption(UnicodeString("minimumIntegerDigits"), opt)) {
UErrorCode localErrorCode = U_ZERO_ERROR;
int64_t val = getInt64Value(locale, opt, localErrorCode);
if (U_SUCCESS(localErrorCode)) {
return static_cast<int32_t>(val);
}
}
return 0;
return intValuedOption(opts, UnicodeString("minimumIntegerDigits"), 0, status);
}

int32_t StandardFunctions::Number::minimumSignificantDigits(const FunctionOptions& opts) const {
int32_t StandardFunctions::Number::minimumSignificantDigits(const FunctionOptions& opts, UErrorCode& status) const {
Formattable opt;

if (!isInteger) {
if (opts.getFunctionOption(UnicodeString("minimumSignificantDigits"), opt)) {
UErrorCode localErrorCode = U_ZERO_ERROR;
int64_t val = getInt64Value(locale, opt, localErrorCode);
if (U_SUCCESS(localErrorCode)) {
return static_cast<int32_t>(val);
}
}
return intValuedOption(opts, UnicodeString("minimumSignificantDigits"), 0, status);
}
// Returning 0 indicates that the option wasn't provided or was a non-integer.
// The caller needs to check for that case, since passing 0 to Precision::minSignificantDigits()
// is an error.
return 0;
}

int32_t StandardFunctions::Number::maximumSignificantDigits(const FunctionOptions& opts) const {
int32_t StandardFunctions::Number::maximumSignificantDigits(const FunctionOptions& opts, UErrorCode& status) const {
Formattable opt;

if (opts.getFunctionOption(UnicodeString("maximumSignificantDigits"), opt)) {
UErrorCode localErrorCode = U_ZERO_ERROR;
int64_t val = getInt64Value(locale, opt, localErrorCode);
if (U_SUCCESS(localErrorCode)) {
return static_cast<int32_t>(val);
}
}
// Returning 0 indicates that the option wasn't provided or was a non-integer.
// Returning 0 indicates that the option wasn't provided.
// The caller needs to check for that case, since passing 0 to Precision::maxSignificantDigits()
// is an error.
return 0; // Not a valid value for Precision; has to be checked
return intValuedOption(opts, UnicodeString("maximumSignificantDigits"), 0, status);
}

bool StandardFunctions::Number::usePercent(const FunctionOptions& opts) const {
Expand Down
11 changes: 6 additions & 5 deletions icu4c/source/i18n/messageformat2_function_registry_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ namespace message2 {
static Number integer(const Locale& loc);

// These options have their own accessor methods, since they have different default values.
int32_t maximumFractionDigits(const FunctionOptions& options) const;
int32_t minimumFractionDigits(const FunctionOptions& options) const;
int32_t minimumSignificantDigits(const FunctionOptions& options) const;
int32_t maximumSignificantDigits(const FunctionOptions& options) const;
int32_t minimumIntegerDigits(const FunctionOptions& options) const;
int32_t intValuedOption(const FunctionOptions&, const UnicodeString&, int32_t, UErrorCode&) const;
int32_t maximumFractionDigits(const FunctionOptions& options, UErrorCode& status) const;
int32_t minimumFractionDigits(const FunctionOptions& options, UErrorCode& status) const;
int32_t minimumSignificantDigits(const FunctionOptions& options, UErrorCode& status) const;
int32_t maximumSignificantDigits(const FunctionOptions& options, UErrorCode& status) const;
int32_t minimumIntegerDigits(const FunctionOptions& options, UErrorCode& status) const;

bool usePercent(const FunctionOptions& options) const;
const Locale& locale;
Expand Down
30 changes: 30 additions & 0 deletions icu4c/source/test/intltest/messageformat2test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,36 @@ void TestMessageFormat2::testResolutionErrors() {
testRuntimeErrorPattern(++i, ".local $bar = {|42| ~plural} .match {|horse| :string} * {{{$bar}}}",
U_MF_UNSUPPORTED_EXPRESSION_ERROR);

// Bad options. The spec is unclear about this -- see https://github.com/unicode-org/message-format-wg/issues/738
// The current behavior is to set a U_MF_FORMATTING_ERROR for any invalid options.
testRuntimeErrorPattern(++i,
".local $foo = {1 :number minimumIntegerDigits=-1} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :number minimumIntegerDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :number minimumFractionDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :number maximumFractionDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :number minimumSignificantDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :number maximumSignificantDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :integer minimumIntegerDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);
testRuntimeErrorPattern(++i,
".local $foo = {1 :integer maximumSignificantDigits=foo} {{bar {$foo}}}",
U_MF_FORMATTING_ERROR);

// Ideally this would cover all the options, but since there are no spec tests yet for other options,
// they are omitted. See https://github.com/unicode-org/message-format-wg/issues/752

// Selector error
// Here, the plural selector returns "no match" so the * variant matches
testRuntimeWarningPattern(++i, ".match {|horse| :number}\n\
Expand Down