From b186f2cff6ac5b9648c1e46b446f6d2beb46b40c Mon Sep 17 00:00:00 2001 From: "Shane F. Carr" Date: Sat, 7 Dec 2019 11:47:39 -0800 Subject: [PATCH] ICU-20912 Make C/J Currency consistent on lowercase/uppercase currency equality - Adds additional tests for Currency equality behavior --- icu4c/source/common/cstring.h | 2 ++ icu4c/source/common/ustr_imp.h | 12 ++++++++ icu4c/source/common/ustring.cpp | 8 +++++ icu4c/source/i18n/currunit.cpp | 29 +++++++++++-------- icu4c/source/test/intltest/numfmtst.cpp | 25 ++++++++++++++++ .../ibm/icu/dev/test/util/CurrencyTest.java | 7 +++++ 6 files changed, 71 insertions(+), 12 deletions(-) diff --git a/icu4c/source/common/cstring.h b/icu4c/source/common/cstring.h index ed0b1a7c8b..3a14e4216c 100644 --- a/icu4c/source/common/cstring.h +++ b/icu4c/source/common/cstring.h @@ -52,6 +52,8 @@ U_CAPI UBool U_EXPORT2 uprv_isASCIILetter(char c); +// NOTE: For u_asciiToUpper that takes a UChar, see ustr_imp.h + U_CAPI char U_EXPORT2 uprv_toupper(char c); diff --git a/icu4c/source/common/ustr_imp.h b/icu4c/source/common/ustr_imp.h index c555ee37ea..5d137c0921 100644 --- a/icu4c/source/common/ustr_imp.h +++ b/icu4c/source/common/ustr_imp.h @@ -46,6 +46,18 @@ ustr_hashCharsN(const char *str, int32_t length); U_CAPI int32_t U_EXPORT2 ustr_hashICharsN(const char *str, int32_t length); +/** + * Convert an ASCII-range lowercase character to uppercase. + * + * @param c A UChar. + * @return If UChar is a lowercase ASCII character, returns the uppercase version. + * Otherwise, returns the input character. + */ +U_CAPI UChar U_EXPORT2 +u_asciiToUpper(UChar c); + +// TODO: Add u_asciiToLower if/when there is a need for it. + /** * NUL-terminate a UChar * string if possible. * If length < destCapacity then NUL-terminate. diff --git a/icu4c/source/common/ustring.cpp b/icu4c/source/common/ustring.cpp index 67cb4544b9..de43d22ccc 100644 --- a/icu4c/source/common/ustring.cpp +++ b/icu4c/source/common/ustring.cpp @@ -1451,6 +1451,14 @@ u_unescape(const char *src, UChar *dest, int32_t destCapacity) { } \ } UPRV_BLOCK_MACRO_END +U_CAPI UChar U_EXPORT2 +u_asciiToUpper(UChar c) { + if (u'a' <= c && c <= u'z') { + c = c + u'A' - u'a'; + } + return c; +} + U_CAPI int32_t U_EXPORT2 u_terminateUChars(UChar *dest, int32_t destCapacity, int32_t length, UErrorCode *pErrorCode) { __TERMINATE_STRING(dest, destCapacity, length, pErrorCode); diff --git a/icu4c/source/i18n/currunit.cpp b/icu4c/source/i18n/currunit.cpp index 280bd563e5..92bcf1268a 100644 --- a/icu4c/source/i18n/currunit.cpp +++ b/icu4c/source/i18n/currunit.cpp @@ -16,9 +16,11 @@ #include "unicode/currunit.h" #include "unicode/ustring.h" +#include "unicode/uchar.h" #include "cstring.h" #include "uinvchar.h" #include "charstr.h" +#include "ustr_imp.h" #include "measunit_impl.h" U_NAMESPACE_BEGIN @@ -29,22 +31,25 @@ CurrencyUnit::CurrencyUnit(ConstChar16Ptr _isoCode, UErrorCode& ec) { // non-NUL-terminated string to be passed as an argument, so it is not possible to check length. // However, we allow a NUL-terminated empty string, which should have the same behavior as nullptr. // Consider NUL-terminated strings of length 1 or 2 as invalid. - const char16_t* isoCodeToUse; + bool useDefault = false; if (U_FAILURE(ec) || _isoCode == nullptr || _isoCode[0] == 0) { - isoCodeToUse = kDefaultCurrency; + useDefault = true; } else if (_isoCode[1] == 0 || _isoCode[2] == 0) { - isoCodeToUse = kDefaultCurrency; + useDefault = true; ec = U_ILLEGAL_ARGUMENT_ERROR; } else if (!uprv_isInvariantUString(_isoCode, 3)) { // TODO: Perform a more strict ASCII check like in ICU4J isAlpha3Code? - isoCodeToUse = kDefaultCurrency; + useDefault = true; ec = U_INVARIANT_CONVERSION_ERROR; } else { - isoCodeToUse = _isoCode; + for (int32_t i=0; i<3; i++) { + isoCode[i] = u_asciiToUpper(_isoCode[i]); + } + isoCode[3] = 0; + } + if (useDefault) { + uprv_memcpy(isoCode, kDefaultCurrency, sizeof(UChar) * 4); } - // TODO: Perform uppercasing here like in ICU4J Currency.getInstance()? - uprv_memcpy(isoCode, isoCodeToUse, sizeof(UChar) * 3); - isoCode[3] = 0; char simpleIsoCode[4]; u_UCharsToChars(isoCode, simpleIsoCode, 4); initCurrency(simpleIsoCode); @@ -64,13 +69,13 @@ CurrencyUnit::CurrencyUnit(StringPiece _isoCode, UErrorCode& ec) { ec = U_INVARIANT_CONVERSION_ERROR; } else { // Have to use isoCodeBuffer to ensure the string is NUL-terminated - uprv_strncpy(isoCodeBuffer, _isoCode.data(), 3); + for (int32_t i=0; i<3; i++) { + isoCodeBuffer[i] = uprv_toupper(_isoCode.data()[i]); + } isoCodeBuffer[3] = 0; isoCodeToUse = isoCodeBuffer; } - // TODO: Perform uppercasing here like in ICU4J Currency.getInstance()? - u_charsToUChars(isoCodeToUse, isoCode, 3); - isoCode[3] = 0; + u_charsToUChars(isoCodeToUse, isoCode, 4); initCurrency(isoCodeToUse); } diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 316a74396e..62e161f570 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -2172,6 +2172,10 @@ void NumberFormatTest::TestCurrencyUnit(void){ static const char INV8[] = "{$%"; static const UChar ZZZ[] = u"zz"; static const char ZZZ8[] = "zz"; + static const UChar JPY[] = u"JPY"; + static const char JPY8[] = "JPY"; + static const UChar jpy[] = u"jpy"; + static const char jpy8[] = "jpy"; UChar* EUR = (UChar*) malloc(6); EUR[0] = u'E'; @@ -2289,6 +2293,27 @@ void NumberFormatTest::TestCurrencyUnit(void){ assertEquals("Copying from meter should fail", ec, U_ILLEGAL_ARGUMENT_ERROR); assertEquals("Copying should not give uninitialized ISO code", u"", failure.getISOCurrency()); + // Test equality + ec = U_ZERO_ERROR; + assertFalse("FAIL: USD == JPY", CurrencyUnit(USD, ec) == CurrencyUnit(JPY, ec)); + assertTrue("FAIL: USD != USD", CurrencyUnit(USD, ec) == CurrencyUnit(USD, ec)); + assertTrue("FAIL: JPY != jpy", CurrencyUnit(JPY, ec) == CurrencyUnit(jpy, ec)); + assertTrue("FAIL: jpy != JPY", CurrencyUnit(jpy, ec) == CurrencyUnit(JPY, ec)); + + // Test equality with system charset instances + assertFalse("FAIL: USD8 == JPY8", CurrencyUnit(USD8, ec) == CurrencyUnit(JPY8, ec)); + assertTrue("FAIL: USD8 != USD8", CurrencyUnit(USD8, ec) == CurrencyUnit(USD8, ec)); + assertTrue("FAIL: JPY8 != jpy8", CurrencyUnit(JPY8, ec) == CurrencyUnit(jpy8, ec)); + assertTrue("FAIL: jpy8 != JPY8", CurrencyUnit(jpy8, ec) == CurrencyUnit(JPY8, ec)); + + // Test equality between UTF-16 and system charset instances + assertTrue("FAIL: USD != USD8", CurrencyUnit(USD, ec) == CurrencyUnit(USD8, ec)); + assertTrue("FAIL: USD8 != USD", CurrencyUnit(USD8, ec) == CurrencyUnit(USD, ec)); + assertTrue("FAIL: JPY != jpy8", CurrencyUnit(JPY, ec) == CurrencyUnit(jpy8, ec)); + assertTrue("FAIL: JPY8 != jpy", CurrencyUnit(JPY8, ec) == CurrencyUnit(jpy, ec)); + assertTrue("FAIL: jpy != JPY8", CurrencyUnit(jpy, ec) == CurrencyUnit(JPY8, ec)); + assertTrue("FAIL: jpy8 != JPY", CurrencyUnit(jpy8, ec) == CurrencyUnit(JPY, ec)); + free(EUR); free(EUR8); } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/CurrencyTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/CurrencyTest.java index fefe945847..058dfaa9a7 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/CurrencyTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/util/CurrencyTest.java @@ -52,6 +52,7 @@ public class CurrencyTest extends TestFmwk { Currency usd = Currency.getInstance("USD"); /*int hash = */usd.hashCode(); Currency jpy = Currency.getInstance("JPY"); + Currency jpy2 = Currency.getInstance("jpy"); if (usd.equals(jpy)) { errln("FAIL: USD == JPY"); } @@ -64,6 +65,12 @@ public class CurrencyTest extends TestFmwk { if (!usd.equals(usd)) { errln("FAIL: USD != USD"); } + if (!jpy.equals(jpy2)) { + errln("FAIL: JPY != jpy"); + } + if (!jpy2.equals(jpy)) { + errln("FAIL: jpy != JPY"); + } try { Currency nullCurrency = Currency.getInstance((String)null);