From: Shane Carr Date: Thu, 6 Sep 2018 06:04:32 +0000 (-0700) Subject: ICU-11276 Adding test cases and more API coverage. X-Git-Tag: release-63-rc~63^2~19 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dfd13867b2ffa28b40172f5292a1a6ce69814285;p=icu ICU-11276 Adding test cases and more API coverage. --- diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp index 9d80e3349cb..760914e26f8 100644 --- a/icu4c/source/i18n/number_decimalquantity.cpp +++ b/icu4c/source/i18n/number_decimalquantity.cpp @@ -1155,7 +1155,7 @@ const char16_t* DecimalQuantity::checkHealth() const { bool DecimalQuantity::operator==(const DecimalQuantity& other) const { // FIXME: Make a faster implementation. - return toString() == other.toString(); + return toScientificString() == other.toScientificString(); } UnicodeString DecimalQuantity::toString() const { diff --git a/icu4c/source/i18n/number_modifiers.cpp b/icu4c/source/i18n/number_modifiers.cpp index 07adce16c49..6d0096055c8 100644 --- a/icu4c/source/i18n/number_modifiers.cpp +++ b/icu4c/source/i18n/number_modifiers.cpp @@ -280,10 +280,7 @@ bool ConstantMultiFieldModifier::isStrong() const { } bool ConstantMultiFieldModifier::containsField(UNumberFormatFields field) const { - (void)field; - // This method is not currently used. - U_ASSERT(false); - return false; + return fPrefix.containsField(field) || fSuffix.containsField(field); } bool ConstantMultiFieldModifier::operator==(const Modifier& other) const { diff --git a/icu4c/source/i18n/number_stringbuilder.cpp b/icu4c/source/i18n/number_stringbuilder.cpp index 2806fefbaa9..74ba33fbbc1 100644 --- a/icu4c/source/i18n/number_stringbuilder.cpp +++ b/icu4c/source/i18n/number_stringbuilder.cpp @@ -488,4 +488,13 @@ void NumberStringBuilder::getAllFieldPositions(FieldPositionIteratorHandler& fpi } } +bool NumberStringBuilder::containsField(Field field) const { + for (int32_t i = 0; i < fLength; i++) { + if (field == fieldAt(i)) { + return true; + } + } + return false; +} + #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/i18n/number_stringbuilder.h b/icu4c/source/i18n/number_stringbuilder.h index cd8ce2f805e..b14ad9ede2f 100644 --- a/icu4c/source/i18n/number_stringbuilder.h +++ b/icu4c/source/i18n/number_stringbuilder.h @@ -106,6 +106,8 @@ class U_I18N_API NumberStringBuilder : public UMemory { void getAllFieldPositions(FieldPositionIteratorHandler& fpih, UErrorCode& status) const; + bool containsField(Field field) const; + private: bool fUsingHeap = false; ValueOrHeapArray fChars; diff --git a/icu4c/source/i18n/numrange_fluent.cpp b/icu4c/source/i18n/numrange_fluent.cpp index d2c5edcd2bf..ee81cb7aa07 100644 --- a/icu4c/source/i18n/numrange_fluent.cpp +++ b/icu4c/source/i18n/numrange_fluent.cpp @@ -114,6 +114,34 @@ Derived NumberRangeFormatterSettings::numberFormatterSecond(Unlocalized return move; } +template +Derived NumberRangeFormatterSettings::collapse(UNumberRangeCollapse collapse) const& { + Derived copy(*this); + copy.fMacros.collapse = collapse; + return copy; +} + +template +Derived NumberRangeFormatterSettings::collapse(UNumberRangeCollapse collapse) && { + Derived move(std::move(*this)); + move.fMacros.collapse = collapse; + return move; +} + +template +Derived NumberRangeFormatterSettings::identityFallback(UNumberRangeIdentityFallback identityFallback) const& { + Derived copy(*this); + copy.fMacros.identityFallback = identityFallback; + return copy; +} + +template +Derived NumberRangeFormatterSettings::identityFallback(UNumberRangeIdentityFallback identityFallback) && { + Derived move(std::move(*this)); + move.fMacros.identityFallback = identityFallback; + return move; +} + // Declare all classes that implement NumberRangeFormatterSettings // See https://stackoverflow.com/a/495056/1407170 template diff --git a/icu4c/source/i18n/numrange_impl.cpp b/icu4c/source/i18n/numrange_impl.cpp index 6e52d3498b0..be6d38c5b6f 100644 --- a/icu4c/source/i18n/numrange_impl.cpp +++ b/icu4c/source/i18n/numrange_impl.cpp @@ -21,7 +21,7 @@ using namespace icu::number::impl; namespace { // Helper function for 2-dimensional switch statement -constexpr int8_t identity2d(UNumberIdentityFallback a, UNumberRangeIdentityResult b) { +constexpr int8_t identity2d(UNumberRangeIdentityFallback a, UNumberRangeIdentityResult b) { return static_cast(a) | (static_cast(b) << 4); } @@ -111,10 +111,18 @@ void NumberRangeFormatterImpl::format(UFormattedNumberRangeData& data, bool equa formatterImpl1.preProcess(data.quantity1, micros1, status); formatterImpl1.preProcess(data.quantity2, micros2, status); } else { - // If the formatters are different, an identity is not possible. - // Always use formatRange(). formatterImpl1.preProcess(data.quantity1, micros1, status); formatterImpl2.preProcess(data.quantity2, micros2, status); + } + + // If any of the affixes are different, an identity is not possible + // and we must use formatRange(). + // TODO: Write this as MicroProps operator==() ? + // TODO: Avoid the redundancy of these equality operations with the + // ones in formatRange? + if (!(*micros1.modInner == *micros2.modInner) + || !(*micros1.modMiddle == *micros2.modMiddle) + || !(*micros1.modOuter == *micros2.modOuter)) { formatRange(data, micros1, micros2, status); return; } @@ -174,7 +182,8 @@ void NumberRangeFormatterImpl::formatSingleValue(UFormattedNumberRangeData& data UErrorCode& status) const { if (U_FAILURE(status)) { return; } if (fSameFormatters) { - formatterImpl1.format(data.quantity1, data.string, status); + int32_t length = formatterImpl1.writeNumber(micros1, data.quantity1, data.string, 0, status); + formatterImpl1.writeAffixes(micros1, data.string, 0, length, status); } else { formatRange(data, micros1, micros2, status); } @@ -186,7 +195,8 @@ void NumberRangeFormatterImpl::formatApproximately (UFormattedNumberRangeData& d UErrorCode& status) const { if (U_FAILURE(status)) { return; } if (fSameFormatters) { - int32_t length = formatterImpl1.format(data.quantity1, data.string, status); + int32_t length = formatterImpl1.writeNumber(micros1, data.quantity1, data.string, 0, status); + length += formatterImpl1.writeAffixes(micros1, data.string, 0, length, status); fApproximatelyModifier.apply(data.string, 0, length, status); } else { formatRange(data, micros1, micros2, status); @@ -242,8 +252,8 @@ void NumberRangeFormatterImpl::formatRange(UFormattedNumberRangeData& data, collapseMiddle = false; } } else if (fCollapse == UNUM_RANGE_COLLAPSE_AUTO) { - // Heuristic as of ICU 63: collapse only if the modifier is exactly one code point. - if (mm->getCodePointCount() != 1) { + // Heuristic as of ICU 63: collapse only if the modifier is more than one code point. + if (mm->getCodePointCount() <= 1) { collapseMiddle = false; } } @@ -273,6 +283,8 @@ void NumberRangeFormatterImpl::formatRange(UFormattedNumberRangeData& data, int32_t lengthInfix = 0; int32_t length2 = 0; int32_t lengthSuffix = 0; + + // Use #define so that these are evaluated at the call site. #define UPRV_INDEX_0 (lengthPrefix) #define UPRV_INDEX_1 (lengthPrefix + length1) #define UPRV_INDEX_2 (lengthPrefix + length1 + lengthInfix) @@ -291,6 +303,7 @@ void NumberRangeFormatterImpl::formatRange(UFormattedNumberRangeData& data, // SPACING HEURISTIC // Add spacing unless all modifiers are collapsed. + // TODO: add API to control this? { bool repeatInner = !collapseInner && micros1.modInner->getCodePointCount() > 0; bool repeatMiddle = !collapseMiddle && micros1.modMiddle->getCodePointCount() > 0; diff --git a/icu4c/source/i18n/unicode/numberrangeformatter.h b/icu4c/source/i18n/unicode/numberrangeformatter.h index 583361df1da..30a7c1e8050 100644 --- a/icu4c/source/i18n/unicode/numberrangeformatter.h +++ b/icu4c/source/i18n/unicode/numberrangeformatter.h @@ -116,7 +116,7 @@ typedef enum UNumberRangeIdentityFallback { * @draft ICU 63 */ UNUM_IDENTITY_FALLBACK_RANGE -} UNumberIdentityFallback; +} UNumberRangeIdentityFallback; /** * Used in the result class FormattedNumberRange to indicate to the user whether the numbers formatted in the range @@ -199,7 +199,7 @@ struct U_I18N_API RangeMacroProps : public UMemory { UNumberRangeCollapse collapse = UNUM_RANGE_COLLAPSE_AUTO; /** @internal */ - UNumberIdentityFallback identityFallback = UNUM_IDENTITY_FALLBACK_APPROXIMATELY; + UNumberRangeIdentityFallback identityFallback = UNUM_IDENTITY_FALLBACK_APPROXIMATELY; /** @internal */ Locale locale; @@ -414,7 +414,7 @@ class U_I18N_API NumberRangeFormatterSettings { * @return The fluent chain. * @draft ICU 63 */ - Derived identityFallback(UNumberIdentityFallback identityFallback) const &; + Derived identityFallback(UNumberRangeIdentityFallback identityFallback) const &; /** * Overload of identityFallback() for use on an rvalue reference. @@ -425,7 +425,7 @@ class U_I18N_API NumberRangeFormatterSettings { * @see #identityFallback * @draft ICU 63 */ - Derived identityFallback(UNumberIdentityFallback identityFallback) &&; + Derived identityFallback(UNumberRangeIdentityFallback identityFallback) &&; /** * Sets the UErrorCode if an error occurred in the fluent chain. diff --git a/icu4c/source/test/intltest/numbertest.h b/icu4c/source/test/intltest/numbertest.h index 74e5987156d..03ef809d6e5 100644 --- a/icu4c/source/test/intltest/numbertest.h +++ b/icu4c/source/test/intltest/numbertest.h @@ -253,6 +253,9 @@ class NumberRangeFormatterTest : public IntlTest { void testSanity(); void testBasic(); + void testCollapse(); + void testIdentity(); + void testDifferentFormatters(); void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = 0); diff --git a/icu4c/source/test/intltest/numbertest_range.cpp b/icu4c/source/test/intltest/numbertest_range.cpp index 6257e03f968..7d5cb0467a5 100644 --- a/icu4c/source/test/intltest/numbertest_range.cpp +++ b/icu4c/source/test/intltest/numbertest_range.cpp @@ -44,6 +44,9 @@ void NumberRangeFormatterTest::runIndexedTest(int32_t index, UBool exec, const c TESTCASE_AUTO_BEGIN; TESTCASE_AUTO(testSanity); TESTCASE_AUTO(testBasic); + TESTCASE_AUTO(testCollapse); + TESTCASE_AUTO(testIdentity); + TESTCASE_AUTO(testDifferentFormatters); TESTCASE_AUTO_END; } @@ -106,6 +109,376 @@ void NumberRangeFormatterTest::testBasic() { u"5,000 m – 5,000,000 km"); } +void NumberRangeFormatterTest::testCollapse() { + assertFormatRange( + u"Default collapse on currency (default rounding)", + NumberRangeFormatter::with() + .numberFormatterBoth(NumberFormatter::with().unit(USD)), + Locale("en-us"), + u"$1.00 – $5.00", + u"~$5.00", + u"~$5.00", + u"$0.00 – $3.00", + u"~$0.00", + u"$3.00 – $3,000.00", + u"$3,000.00 – $5,000.00", + u"$4,999.00 – $5,001.00", + u"~$5,000.00", + u"$5,000.00 – $5,000,000.00"); + + assertFormatRange( + u"Default collapse on currency", + NumberRangeFormatter::with() + .numberFormatterBoth(NumberFormatter::with().unit(USD).precision(Precision::integer())), + Locale("en-us"), + u"$1 – $5", + u"~$5", + u"~$5", + u"$0 – $3", + u"~$0", + u"$3 – $3,000", + u"$3,000 – $5,000", + u"$4,999 – $5,001", + u"~$5,000", + u"$5,000 – $5,000,000"); + + assertFormatRange( + u"No collapse on currency", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_NONE) + .numberFormatterBoth(NumberFormatter::with().unit(USD).precision(Precision::integer())), + Locale("en-us"), + u"$1 – $5", + u"~$5", + u"~$5", + u"$0 – $3", + u"~$0", + u"$3 – $3,000", + u"$3,000 – $5,000", + u"$4,999 – $5,001", + u"~$5,000", + u"$5,000 – $5,000,000"); + + assertFormatRange( + u"Unit collapse on currency", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_UNIT) + .numberFormatterBoth(NumberFormatter::with().unit(USD).precision(Precision::integer())), + Locale("en-us"), + u"$1–5", + u"~$5", + u"~$5", + u"$0–3", + u"~$0", + u"$3–3,000", + u"$3,000–5,000", + u"$4,999–5,001", + u"~$5,000", + u"$5,000–5,000,000"); + + assertFormatRange( + u"All collapse on currency", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_ALL) + .numberFormatterBoth(NumberFormatter::with().unit(USD).precision(Precision::integer())), + Locale("en-us"), + u"$1–5", + u"~$5", + u"~$5", + u"$0–3", + u"~$0", + u"$3–3,000", + u"$3,000–5,000", + u"$4,999–5,001", + u"~$5,000", + u"$5,000–5,000,000"); + + assertFormatRange( + u"Default collapse on currency ISO code", + NumberRangeFormatter::with() + .numberFormatterBoth(NumberFormatter::with() + .unit(GBP) + .unitWidth(UNUM_UNIT_WIDTH_ISO_CODE) + .precision(Precision::integer())), + Locale("en-us"), + u"GBP 1–5", + u"~GBP 5", // TODO: Fix this at some point + u"~GBP 5", + u"GBP 0–3", + u"~GBP 0", + u"GBP 3–3,000", + u"GBP 3,000–5,000", + u"GBP 4,999–5,001", + u"~GBP 5,000", + u"GBP 5,000–5,000,000"); + + assertFormatRange( + u"No collapse on currency ISO code", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_NONE) + .numberFormatterBoth(NumberFormatter::with() + .unit(GBP) + .unitWidth(UNUM_UNIT_WIDTH_ISO_CODE) + .precision(Precision::integer())), + Locale("en-us"), + u"GBP 1 – GBP 5", + u"~GBP 5", // TODO: Fix this at some point + u"~GBP 5", + u"GBP 0 – GBP 3", + u"~GBP 0", + u"GBP 3 – GBP 3,000", + u"GBP 3,000 – GBP 5,000", + u"GBP 4,999 – GBP 5,001", + u"~GBP 5,000", + u"GBP 5,000 – GBP 5,000,000"); + + assertFormatRange( + u"Unit collapse on currency ISO code", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_UNIT) + .numberFormatterBoth(NumberFormatter::with() + .unit(GBP) + .unitWidth(UNUM_UNIT_WIDTH_ISO_CODE) + .precision(Precision::integer())), + Locale("en-us"), + u"GBP 1–5", + u"~GBP 5", // TODO: Fix this at some point + u"~GBP 5", + u"GBP 0–3", + u"~GBP 0", + u"GBP 3–3,000", + u"GBP 3,000–5,000", + u"GBP 4,999–5,001", + u"~GBP 5,000", + u"GBP 5,000–5,000,000"); + + assertFormatRange( + u"All collapse on currency ISO code", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_ALL) + .numberFormatterBoth(NumberFormatter::with() + .unit(GBP) + .unitWidth(UNUM_UNIT_WIDTH_ISO_CODE) + .precision(Precision::integer())), + Locale("en-us"), + u"GBP 1–5", + u"~GBP 5", // TODO: Fix this at some point + u"~GBP 5", + u"GBP 0–3", + u"~GBP 0", + u"GBP 3–3,000", + u"GBP 3,000–5,000", + u"GBP 4,999–5,001", + u"~GBP 5,000", + u"GBP 5,000–5,000,000"); + + // Default collapse on measurement unit is in testBasic() + + assertFormatRange( + u"No collapse on measurement unit", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_NONE) + .numberFormatterBoth(NumberFormatter::with().unit(METER)), + Locale("en-us"), + u"1 m – 5 m", + u"~5 m", + u"~5 m", + u"0 m – 3 m", + u"~0 m", + u"3 m – 3,000 m", + u"3,000 m – 5,000 m", + u"4,999 m – 5,001 m", + u"~5,000 m", + u"5,000 m – 5,000,000 m"); + + assertFormatRange( + u"Unit collapse on measurement unit", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_UNIT) + .numberFormatterBoth(NumberFormatter::with().unit(METER)), + Locale("en-us"), + u"1–5 m", + u"~5 m", + u"~5 m", + u"0–3 m", + u"~0 m", + u"3–3,000 m", + u"3,000–5,000 m", + u"4,999–5,001 m", + u"~5,000 m", + u"5,000–5,000,000 m"); + + assertFormatRange( + u"All collapse on measurement unit", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_ALL) + .numberFormatterBoth(NumberFormatter::with().unit(METER)), + Locale("en-us"), + u"1–5 m", + u"~5 m", + u"~5 m", + u"0–3 m", + u"~0 m", + u"3–3,000 m", + u"3,000–5,000 m", + u"4,999–5,001 m", + u"~5,000 m", + u"5,000–5,000,000 m"); + + assertFormatRange( + u"Default collapse on measurement unit with compact-short notation", + NumberRangeFormatter::with() + .numberFormatterBoth(NumberFormatter::with().notation(Notation::compactShort()).unit(METER)), + Locale("en-us"), + u"1–5 m", + u"~5 m", + u"~5 m", + u"0–3 m", + u"~0 m", + u"3–3K m", + u"3K – 5K m", + u"~5K m", + u"~5K m", + u"5K – 5M m"); + + assertFormatRange( + u"No collapse on measurement unit with compact-short notation", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_NONE) + .numberFormatterBoth(NumberFormatter::with().notation(Notation::compactShort()).unit(METER)), + Locale("en-us"), + u"1 m – 5 m", + u"~5 m", + u"~5 m", + u"0 m – 3 m", + u"~0 m", + u"3 m – 3K m", + u"3K m – 5K m", + u"~5K m", + u"~5K m", + u"5K m – 5M m"); + + assertFormatRange( + u"Unit collapse on measurement unit with compact-short notation", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_UNIT) + .numberFormatterBoth(NumberFormatter::with().notation(Notation::compactShort()).unit(METER)), + Locale("en-us"), + u"1–5 m", + u"~5 m", + u"~5 m", + u"0–3 m", + u"~0 m", + u"3–3K m", + u"3K – 5K m", + u"~5K m", + u"~5K m", + u"5K – 5M m"); + + assertFormatRange( + u"All collapse on measurement unit with compact-short notation", + NumberRangeFormatter::with() + .collapse(UNUM_RANGE_COLLAPSE_ALL) + .numberFormatterBoth(NumberFormatter::with().notation(Notation::compactShort()).unit(METER)), + Locale("en-us"), + u"1–5 m", + u"~5 m", + u"~5 m", + u"0–3 m", + u"~0 m", + u"3–3K m", + u"3–5K m", // this one is the key use case for ALL + u"~5K m", + u"~5K m", + u"5K – 5M m"); + + // TODO: Test compact currency? + // The code is not smart enough to differentiate the notation from the unit. +} + +void NumberRangeFormatterTest::testIdentity() { + assertFormatRange( + u"Identity fallback Range", + NumberRangeFormatter::with().identityFallback(UNUM_IDENTITY_FALLBACK_RANGE), + Locale("en-us"), + u"1–5", + u"5–5", + u"5–5", + u"0–3", + u"0–0", + u"3–3,000", + u"3,000–5,000", + u"4,999–5,001", + u"5,000–5,000", + u"5,000–5,000,000"); + + assertFormatRange( + u"Identity fallback Approximately or Single Value", + NumberRangeFormatter::with().identityFallback(UNUM_IDENTITY_FALLBACK_APPROXIMATELY_OR_SINGLE_VALUE), + Locale("en-us"), + u"1–5", + u"~5", + u"5", + u"0–3", + u"0", + u"3–3,000", + u"3,000–5,000", + u"4,999–5,001", + u"5,000", + u"5,000–5,000,000"); + + assertFormatRange( + u"Identity fallback Single Value", + NumberRangeFormatter::with().identityFallback(UNUM_IDENTITY_FALLBACK_SINGLE_VALUE), + Locale("en-us"), + u"1–5", + u"5", + u"5", + u"0–3", + u"0", + u"3–3,000", + u"3,000–5,000", + u"4,999–5,001", + u"5,000", + u"5,000–5,000,000"); + + assertFormatRange( + u"Identity fallback Approximately or Single Value with compact notation", + NumberRangeFormatter::with() + .identityFallback(UNUM_IDENTITY_FALLBACK_APPROXIMATELY_OR_SINGLE_VALUE) + .numberFormatterBoth(NumberFormatter::with().notation(Notation::compactShort())), + Locale("en-us"), + u"1–5", + u"~5", + u"5", + u"0–3", + u"0", + u"3–3K", + u"3K – 5K", + u"~5K", + u"5K", + u"5K – 5M"); +} + +void NumberRangeFormatterTest::testDifferentFormatters() { + assertFormatRange( + u"Different rounding rules", + NumberRangeFormatter::with() + .numberFormatterFirst(NumberFormatter::with().precision(Precision::integer())) + .numberFormatterSecond(NumberFormatter::with().precision(Precision::fixedDigits(2))), + Locale("en-us"), + u"1–5.0", + u"5–5.0", + u"5–5.0", + u"0–3.0", + u"0–0.0", + u"3–3,000", + u"3,000–5,000", + u"4,999–5,000", + u"5,000–5,000", // TODO: Should this one be ~5,000? + u"5,000–5,000,000"); +} + void NumberRangeFormatterTest::assertFormatRange( const char16_t* message, const UnlocalizedNumberRangeFormatter& f,