ICU-22319 Fix number range semanticallyEquivalent

See #2385
This commit is contained in:
Shane F. Carr 2023-10-05 22:20:40 +00:00 committed by Markus Scherer
parent aa3e49fa9a
commit 71b9b88200
16 changed files with 122 additions and 32 deletions

View file

@ -60,6 +60,41 @@ Modifier::Parameters::Parameters(
const ModifierStore* _obj, Signum _signum, StandardPlural::Form _plural)
: obj(_obj), signum(_signum), plural(_plural) {}
bool Modifier::semanticallyEquivalent(const Modifier& other) const {
Parameters paramsThis;
Parameters paramsOther;
getParameters(paramsThis);
other.getParameters(paramsOther);
if (paramsThis.obj == nullptr && paramsOther.obj == nullptr) {
return strictEquals(other);
} else if (paramsThis.obj == nullptr || paramsOther.obj == nullptr) {
return false;
}
for (size_t i=0; i<SIGNUM_COUNT; i++) {
auto signum = static_cast<Signum>(i);
for (size_t j=0; j<StandardPlural::COUNT; j++) {
auto plural = static_cast<StandardPlural::Form>(j);
auto mod1 = paramsThis.obj->getModifier(signum, plural);
auto mod2 = paramsOther.obj->getModifier(signum, plural);
if (mod1 == mod2) {
// Equal pointers
continue;
} else if (mod1 == nullptr || mod2 == nullptr) {
// One pointer is null but not the other
return false;
} else if (!mod1->strictEquals(*mod2)) {
// The modifiers are NOT equivalent
return false;
} else {
// The modifiers are equivalent
continue;
}
}
}
return true;
}
ModifierStore::~ModifierStore() = default;
AdoptingSignumModifierStore::~AdoptingSignumModifierStore() {
@ -110,7 +145,7 @@ void ConstantAffixModifier::getParameters(Parameters& output) const {
UPRV_UNREACHABLE_EXIT;
}
bool ConstantAffixModifier::semanticallyEquivalent(const Modifier& other) const {
bool ConstantAffixModifier::strictEquals(const Modifier& other) const {
auto* _other = dynamic_cast<const ConstantAffixModifier*>(&other);
if (_other == nullptr) {
return false;
@ -197,14 +232,11 @@ void SimpleModifier::getParameters(Parameters& output) const {
output = fParameters;
}
bool SimpleModifier::semanticallyEquivalent(const Modifier& other) const {
bool SimpleModifier::strictEquals(const Modifier& other) const {
auto* _other = dynamic_cast<const SimpleModifier*>(&other);
if (_other == nullptr) {
return false;
}
if (fParameters.obj != nullptr) {
return fParameters.obj == _other->fParameters.obj;
}
return fCompiledPattern == _other->fCompiledPattern
&& fField == _other->fField
&& fStrong == _other->fStrong;
@ -327,14 +359,11 @@ void ConstantMultiFieldModifier::getParameters(Parameters& output) const {
output = fParameters;
}
bool ConstantMultiFieldModifier::semanticallyEquivalent(const Modifier& other) const {
bool ConstantMultiFieldModifier::strictEquals(const Modifier& other) const {
auto* _other = dynamic_cast<const ConstantMultiFieldModifier*>(&other);
if (_other == nullptr) {
return false;
}
if (fParameters.obj != nullptr) {
return fParameters.obj == _other->fParameters.obj;
}
return fPrefix.contentEquals(_other->fPrefix)
&& fSuffix.contentEquals(_other->fSuffix)
&& fOverwrite == _other->fOverwrite

View file

@ -41,7 +41,7 @@ class U_I18N_API ConstantAffixModifier : public Modifier, public UObject {
void getParameters(Parameters& output) const override;
bool semanticallyEquivalent(const Modifier& other) const override;
bool strictEquals(const Modifier& other) const override;
private:
UnicodeString fPrefix;
@ -77,7 +77,7 @@ class U_I18N_API SimpleModifier : public Modifier, public UMemory {
void getParameters(Parameters& output) const override;
bool semanticallyEquivalent(const Modifier& other) const override;
bool strictEquals(const Modifier& other) const override;
/**
* TODO: This belongs in SimpleFormatterImpl. The only reason I haven't moved it there yet is because
@ -170,7 +170,7 @@ class U_I18N_API ConstantMultiFieldModifier : public Modifier, public UMemory {
void getParameters(Parameters& output) const override;
bool semanticallyEquivalent(const Modifier& other) const override;
bool strictEquals(const Modifier& other) const override;
protected:
// NOTE: In Java, these are stored as array pointers. In C++, the FormattedStringBuilder is stored by
@ -264,7 +264,7 @@ class U_I18N_API EmptyModifier : public Modifier, public UMemory {
output.obj = nullptr;
}
bool semanticallyEquivalent(const Modifier& other) const override {
bool strictEquals(const Modifier& other) const override {
return other.getCodePointCount() == 0;
}

View file

@ -258,7 +258,7 @@ void MutablePatternModifier::getParameters(Parameters& output) const {
UPRV_UNREACHABLE_EXIT;
}
bool MutablePatternModifier::semanticallyEquivalent(const Modifier& other) const {
bool MutablePatternModifier::strictEquals(const Modifier& other) const {
(void)other;
// This method is not currently used.
UPRV_UNREACHABLE_EXIT;

View file

@ -191,7 +191,7 @@ class U_I18N_API MutablePatternModifier
void getParameters(Parameters& output) const override;
bool semanticallyEquivalent(const Modifier& other) const override;
bool strictEquals(const Modifier& other) const override;
/**
* Returns the string that substitutes a given symbol type in a pattern.

View file

@ -104,7 +104,7 @@ void ScientificModifier::getParameters(Parameters& output) const {
output.obj = nullptr;
}
bool ScientificModifier::semanticallyEquivalent(const Modifier& other) const {
bool ScientificModifier::strictEquals(const Modifier& other) const {
auto* _other = dynamic_cast<const ScientificModifier*>(&other);
if (_other == nullptr) {
return false;

View file

@ -34,7 +34,7 @@ class U_I18N_API ScientificModifier : public UMemory, public Modifier {
void getParameters(Parameters& output) const override;
bool semanticallyEquivalent(const Modifier& other) const override;
bool strictEquals(const Modifier& other) const override;
private:
int32_t fExponent;

View file

@ -224,11 +224,16 @@ class U_I18N_API Modifier {
*/
virtual void getParameters(Parameters& output) const = 0;
/**
* Returns whether this Modifier equals another Modifier.
*/
virtual bool strictEquals(const Modifier& other) const = 0;
/**
* Returns whether this Modifier is *semantically equivalent* to the other Modifier;
* in many cases, this is the same as equal, but parameters should be ignored.
*/
virtual bool semanticallyEquivalent(const Modifier& other) const = 0;
bool semanticallyEquivalent(const Modifier& other) const;
};

View file

@ -331,6 +331,7 @@ class NumberRangeFormatterTest : public IntlTestWithFieldPosition {
void test21358_SignPosition();
void test21683_StateLeak();
void testCreateLNRFFromNumberingSystemInSkeleton();
void test22288_DifferentStartEndSettings();
void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0) override;

View file

@ -59,6 +59,7 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c
TESTCASE_AUTO(test21358_SignPosition);
TESTCASE_AUTO(test21683_StateLeak);
TESTCASE_AUTO(testCreateLNRFFromNumberingSystemInSkeleton);
TESTCASE_AUTO(test22288_DifferentStartEndSettings);
TESTCASE_AUTO_END;
}
@ -1159,6 +1160,27 @@ cleanup:
ucfpos_close(fpos);
}
void NumberRangeFormatterTest::test22288_DifferentStartEndSettings() {
IcuTestErrorCode status(*this, "test22288_DifferentStartEndSettings");
LocalizedNumberRangeFormatter lnrf(NumberRangeFormatter
::withLocale("en")
.collapse(UNUM_RANGE_COLLAPSE_UNIT)
.numberFormatterFirst(
NumberFormatter::with()
.unit(CurrencyUnit("USD", status))
.unitWidth(UNUM_UNIT_WIDTH_FULL_NAME)
.precision(Precision::integer())
.roundingMode(UNUM_ROUND_FLOOR))
.numberFormatterSecond(
NumberFormatter::with()
.unit(CurrencyUnit("USD", status))
.unitWidth(UNUM_UNIT_WIDTH_FULL_NAME)
.precision(Precision::integer())
.roundingMode(UNUM_ROUND_CEILING)));
FormattedNumberRange result = lnrf.formatFormattableRange(2.5, 2.5, status);
assertEquals("Should format successfully", u"23 US dollars", result.toString(status));
}
void NumberRangeFormatterTest::assertFormatRange(
const char16_t* message,
const UnlocalizedNumberRangeFormatter& f,

View file

@ -89,7 +89,7 @@ public class ConstantAffixModifier implements Modifier {
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
if (!(other instanceof ConstantAffixModifier)) {
return false;
}

View file

@ -96,14 +96,11 @@ public class ConstantMultiFieldModifier implements Modifier {
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
if (!(other instanceof ConstantMultiFieldModifier)) {
return false;
}
ConstantMultiFieldModifier _other = (ConstantMultiFieldModifier) other;
if (parameters != null && _other.parameters != null && parameters.obj == _other.parameters.obj) {
return true;
}
return Arrays.equals(prefixChars, _other.prefixChars) && Arrays.equals(prefixFields, _other.prefixFields)
&& Arrays.equals(suffixChars, _other.suffixChars) && Arrays.equals(suffixFields, _other.suffixFields)
&& overwrite == _other.overwrite && strong == _other.strong;

View file

@ -24,6 +24,7 @@ public interface Modifier {
POS;
static final int COUNT = Signum.values().length;
public static final Signum[] VALUES = Signum.values();
};
/**
@ -83,9 +84,46 @@ public interface Modifier {
*/
public Parameters getParameters();
/**
* Returns whether this Modifier equals another Modifier.
*/
public boolean strictEquals(Modifier other);
/**
* Returns whether this Modifier is *semantically equivalent* to the other Modifier;
* in many cases, this is the same as equal, but parameters should be ignored.
*/
public boolean semanticallyEquivalent(Modifier other);
default boolean semanticallyEquivalent(Modifier other) {
Parameters paramsThis = this.getParameters();
Parameters paramsOther = other.getParameters();
if (paramsThis == null && paramsOther == null) {
return this.strictEquals(other);
} else if (paramsThis == null || paramsOther == null) {
return false;
} else if (paramsThis.obj == null && paramsOther.obj == null) {
return this.strictEquals(other);
} else if (paramsThis.obj == null || paramsOther.obj == null) {
return false;
}
for (Signum signum : Signum.VALUES) {
for (StandardPlural plural : StandardPlural.VALUES) {
Modifier mod1 = paramsThis.obj.getModifier(signum, plural);
Modifier mod2 = paramsOther.obj.getModifier(signum, plural);
if (mod1 == mod2) {
// Equal pointers
continue;
} else if (mod1 == null || mod2 == null) {
// One pointer is null but not the other
return false;
} else if (!mod1.strictEquals(mod2)) {
// The modifiers are NOT equivalent
return false;
} else {
// The modifiers are equivalent
continue;
}
}
}
return true;
}
}

View file

@ -347,7 +347,7 @@ public class MutablePatternModifier implements Modifier, SymbolProvider, MicroPr
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
// This method is not currently used. (unsafe path not used in range formatting)
assert false;
return false;

View file

@ -71,14 +71,11 @@ public class SimpleModifier implements Modifier {
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
if (!(other instanceof SimpleModifier)) {
return false;
}
SimpleModifier _other = (SimpleModifier) other;
if (parameters != null && _other.parameters != null && parameters.obj == _other.parameters.obj) {
return true;
}
return compiledPattern.equals(_other.compiledPattern) && field == _other.field && strong == _other.strong;
}

View file

@ -17,6 +17,8 @@ import com.ibm.icu.impl.number.MacroProps;
import com.ibm.icu.impl.number.MicroProps;
import com.ibm.icu.impl.number.Modifier;
import com.ibm.icu.impl.number.SimpleModifier;
import com.ibm.icu.impl.number.Modifier.Parameters;
import com.ibm.icu.impl.number.Modifier.Signum;
import com.ibm.icu.impl.number.range.PrefixInfixSuffixLengthHelper;
import com.ibm.icu.impl.number.range.RangeMacroProps;
import com.ibm.icu.impl.number.range.StandardPluralRanges;
@ -437,5 +439,4 @@ class NumberRangeFormatterImpl {
assert mod != null;
return mod;
}
}

View file

@ -255,7 +255,7 @@ public class ScientificNotation extends Notation {
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
// This method is not currently used. (unsafe path not used in range formatting)
assert false;
return false;
@ -334,7 +334,7 @@ public class ScientificNotation extends Notation {
}
@Override
public boolean semanticallyEquivalent(Modifier other) {
public boolean strictEquals(Modifier other) {
if (!(other instanceof ScientificModifier)) {
return false;
}