From: Shane Carr Date: Sat, 7 Apr 2018 11:10:08 +0000 (+0000) Subject: ICU-13634 Fixing various issues in order to make currencies round-trip in strict... X-Git-Tag: release-62-rc~200^2~46 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b9925e084eccdb7ff3b422e9c40d5e1ac704abab;p=icu ICU-13634 Fixing various issues in order to make currencies round-trip in strict mode. X-SVN-Rev: 41212 --- diff --git a/icu4c/source/i18n/numparse_affixes.cpp b/icu4c/source/i18n/numparse_affixes.cpp index 013cc01ff87..0ac1fe7c0e9 100644 --- a/icu4c/source/i18n/numparse_affixes.cpp +++ b/icu4c/source/i18n/numparse_affixes.cpp @@ -304,7 +304,7 @@ void AffixMatcherWarehouse::createAffixMatchers(const AffixPatternProvider& patt UnicodeString sb; bool includeUnpaired = 0 != (parseFlags & PARSE_FLAG_INCLUDE_UNPAIRED_AFFIXES); UNumberSignDisplay signDisplay = (0 != (parseFlags & PARSE_FLAG_PLUS_SIGN_ALLOWED)) ? UNUM_SIGN_ALWAYS - : UNUM_SIGN_NEVER; + : UNUM_SIGN_AUTO; int32_t numAffixMatchers = 0; int32_t numAffixPatternMatchers = 0; diff --git a/icu4c/source/i18n/numparse_compositions.cpp b/icu4c/source/i18n/numparse_compositions.cpp index 06aa476a29b..8c7b7d251cd 100644 --- a/icu4c/source/i18n/numparse_compositions.cpp +++ b/icu4c/source/i18n/numparse_compositions.cpp @@ -40,6 +40,11 @@ bool SeriesMatcher::match(StringSegment& segment, ParsedNumber& result, UErrorCo } else if (success) { // Match succeeded, and this is NOT a flexible matcher. Proceed to the next matcher. it++; + // Small hack: if there is another matcher coming, do not accept trailing weak chars. + // Needed for proper handling of currency spacing. + if (it < end() && segment.getOffset() != result.charEnd && result.charEnd > matcherOffset) { + segment.setOffset(result.charEnd); + } } else if (isFlexible) { // Match failed, and this is a flexible matcher. Try again with the next matcher. it++; diff --git a/icu4c/source/i18n/numparse_impl.cpp b/icu4c/source/i18n/numparse_impl.cpp index b8240e7f401..54609151bcd 100644 --- a/icu4c/source/i18n/numparse_impl.cpp +++ b/icu4c/source/i18n/numparse_impl.cpp @@ -81,12 +81,23 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr const DecimalFormatSymbols& symbols, bool parseCurrency, UErrorCode& status) { Locale locale = symbols.getLocale(); - PropertiesAffixPatternProvider patternInfo(properties, status); + PropertiesAffixPatternProvider localPAPP; + CurrencyPluralInfoAffixProvider localCPIAP; + AffixPatternProvider* affixProvider; + if (properties.currencyPluralInfo.fPtr.isNull()) { + localPAPP.setTo(properties, status); + affixProvider = &localPAPP; + } else { + localCPIAP.setTo(*properties.currencyPluralInfo.fPtr, status); + affixProvider = &localCPIAP; + } + if (affixProvider == nullptr || U_FAILURE(status)) { return nullptr; } CurrencyUnit currency = resolveCurrency(properties, locale, status); CurrencySymbols currencySymbols(currency, locale, symbols, status); bool isStrict = properties.parseMode.getOrDefault(PARSE_MODE_STRICT) == PARSE_MODE_STRICT; Grouper grouper = Grouper::forProperties(properties); int parseFlags = 0; + if (affixProvider == nullptr || U_FAILURE(status)) { return nullptr; } // Fraction grouping is disabled by default because it has never been supported in DecimalFormat parseFlags |= PARSE_FLAG_FRACTION_GROUPING_DISABLED; if (!properties.parseCaseSensitive) { @@ -109,7 +120,7 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr if (grouper.getPrimary() <= 0) { parseFlags |= PARSE_FLAG_GROUPING_DISABLED; } - if (parseCurrency || patternInfo.hasCurrencySign()) { + if (parseCurrency || affixProvider->hasCurrencySign()) { parseFlags |= PARSE_FLAG_MONETARY_SEPARATORS; } @@ -129,13 +140,13 @@ NumberParserImpl::createParserFromProperties(const number::impl::DecimalFormatPr parser->fLocalMatchers.affixTokenMatcherWarehouse = {&affixSetupData}; parser->fLocalMatchers.affixMatcherWarehouse = {&parser->fLocalMatchers.affixTokenMatcherWarehouse}; parser->fLocalMatchers.affixMatcherWarehouse.createAffixMatchers( - patternInfo, *parser, ignorables, parseFlags, status); + *affixProvider, *parser, ignorables, parseFlags, status); //////////////////////// /// CURRENCY MATCHER /// //////////////////////// - if (parseCurrency || patternInfo.hasCurrencySign()) { + if (parseCurrency || affixProvider->hasCurrencySign()) { parser->addMatcher(parser->fLocalMatchers.currency = {currencySymbols, symbols, status}); } diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp index 54817637b4e..bf070fa75ac 100644 --- a/icu4c/source/test/intltest/numfmtst.cpp +++ b/icu4c/source/test/intltest/numfmtst.cpp @@ -3987,10 +3987,10 @@ NumberFormatTest::TestCurrencyIsoPluralFormat() { {"en_US", "1", "USD", "$1.00", "USD\\u00A01.00", "1.00 US dollars"}, {"en_US", "1234.56", "USD", "$1,234.56", "USD\\u00A01,234.56", "1,234.56 US dollars"}, {"en_US", "-1234.56", "USD", "-$1,234.56", "-USD\\u00A01,234.56", "-1,234.56 US dollars"}, - {"zh_CN", "1", "USD", "US$1.00", "USD\\u00A01.00", "1.00\\u7F8E\\u5143"}, - {"zh_CN", "1234.56", "USD", "US$1,234.56", "USD\\u00A01,234.56", "1,234.56\\u7F8E\\u5143"}, - {"zh_CN", "1", "CNY", "\\uFFE51.00", "CNY\\u00A01.00", "1.00\\u4EBA\\u6C11\\u5E01"}, - {"zh_CN", "1234.56", "CNY", "\\uFFE51,234.56", "CNY\\u00A01,234.56", "1,234.56\\u4EBA\\u6C11\\u5E01"}, + {"zh_CN", "1", "USD", "US$1.00", "USD\\u00A01.00", "1.00\\u00A0\\u7F8E\\u5143"}, + {"zh_CN", "1234.56", "USD", "US$1,234.56", "USD\\u00A01,234.56", "1,234.56\\u00A0\\u7F8E\\u5143"}, + {"zh_CN", "1", "CNY", "\\uFFE51.00", "CNY\\u00A01.00", "1.00\\u00A0\\u4EBA\\u6C11\\u5E01"}, + {"zh_CN", "1234.56", "CNY", "\\uFFE51,234.56", "CNY\\u00A01,234.56", "1,234.56\\u00A0\\u4EBA\\u6C11\\u5E01"}, {"ru_RU", "1", "RUB", "1,00\\u00A0\\u20BD", "1,00\\u00A0RUB", "1,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, {"ru_RU", "2", "RUB", "2,00\\u00A0\\u20BD", "2,00\\u00A0RUB", "2,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, {"ru_RU", "5", "RUB", "5,00\\u00A0\\u20BD", "5,00\\u00A0RUB", "5,00 \\u0440\\u043E\\u0441\\u0441\\u0438\\u0439\\u0441\\u043A\\u043E\\u0433\\u043E \\u0440\\u0443\\u0431\\u043B\\u044F"}, @@ -4040,7 +4040,9 @@ NumberFormatTest::TestCurrencyIsoPluralFormat() { errln("FAIL: Expected " + formatResult + " actual: " + strBuf); } // test parsing, and test parsing for all currency formats. - for (int j = 3; j < 6; ++j) { + // NOTE: ICU 62 requires that the currency format match the pattern in strict mode. + //for (int j = 3; j < 6; ++j) { + for (int j = 3 + kIndex; j <= 3 + kIndex; j++) { // DATA[i][3] is the currency format result using // CURRENCYSTYLE formatter. // DATA[i][4] is the currency format result using diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/AffixMatcher.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/AffixMatcher.java index 04bbadbc228..f3c1f9410a1 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/AffixMatcher.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/AffixMatcher.java @@ -92,7 +92,7 @@ public class AffixMatcher implements NumberParseMatcher { boolean includeUnpaired = 0 != (parseFlags & ParsingUtils.PARSE_FLAG_INCLUDE_UNPAIRED_AFFIXES); SignDisplay signDisplay = (0 != (parseFlags & ParsingUtils.PARSE_FLAG_PLUS_SIGN_ALLOWED)) ? SignDisplay.ALWAYS - : SignDisplay.NEVER; + : SignDisplay.AUTO; AffixPatternMatcher posPrefix = null; AffixPatternMatcher posSuffix = null; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java index 72ab98e4e31..957a5933452 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/NumberParserImpl.java @@ -9,6 +9,7 @@ import java.util.List; import com.ibm.icu.impl.StringSegment; import com.ibm.icu.impl.number.AffixPatternProvider; +import com.ibm.icu.impl.number.CurrencyPluralInfoAffixProvider; import com.ibm.icu.impl.number.CustomSymbolCurrency; import com.ibm.icu.impl.number.DecimalFormatProperties; import com.ibm.icu.impl.number.DecimalFormatProperties.ParseMode; @@ -134,7 +135,12 @@ public class NumberParserImpl { boolean parseCurrency) { ULocale locale = symbols.getULocale(); - AffixPatternProvider patternInfo = new PropertiesAffixPatternProvider(properties); + AffixPatternProvider affixProvider; + if (properties.getCurrencyPluralInfo() == null) { + affixProvider = new PropertiesAffixPatternProvider(properties); + } else { + affixProvider = new CurrencyPluralInfoAffixProvider(properties.getCurrencyPluralInfo()); + } Currency currency = CustomSymbolCurrency.resolve(properties.getCurrency(), locale, symbols); boolean isStrict = properties.getParseMode() == ParseMode.STRICT; Grouper grouper = Grouper.forProperties(properties); @@ -161,7 +167,7 @@ public class NumberParserImpl { if (grouper.getPrimary() <= 0) { parseFlags |= ParsingUtils.PARSE_FLAG_GROUPING_DISABLED; } - if (parseCurrency || patternInfo.hasCurrencySign()) { + if (parseCurrency || affixProvider.hasCurrencySign()) { parseFlags |= ParsingUtils.PARSE_FLAG_MONETARY_SEPARATORS; } IgnorablesMatcher ignorables = isStrict ? IgnorablesMatcher.STRICT : IgnorablesMatcher.DEFAULT; @@ -179,13 +185,13 @@ public class NumberParserImpl { ////////////////////// // Set up a pattern modifier with mostly defaults to generate AffixMatchers. - AffixMatcher.createMatchers(patternInfo, parser, factory, ignorables, parseFlags); + AffixMatcher.createMatchers(affixProvider, parser, factory, ignorables, parseFlags); //////////////////////// /// CURRENCY MATCHER /// //////////////////////// - if (parseCurrency || patternInfo.hasCurrencySign()) { + if (parseCurrency || affixProvider.hasCurrencySign()) { parser.addMatcher(CombinedCurrencyMatcher.getInstance(currency, symbols)); } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/SeriesMatcher.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/SeriesMatcher.java index f12a81fd351..466be0e648c 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/SeriesMatcher.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/SeriesMatcher.java @@ -65,6 +65,11 @@ public class SeriesMatcher implements NumberParseMatcher { } else if (success) { // Match succeeded, and this is NOT a flexible matcher. Proceed to the next matcher. i++; + // Small hack: if there is another matcher coming, do not accept trailing weak chars. + // Needed for proper handling of currency spacing. + if (i < matchers.size() && segment.getOffset() != result.charEnd && result.charEnd > matcherOffset) { + segment.setOffset(result.charEnd); + } } else if (isFlexible) { // Match failed, and this is a flexible matcher. Try again with the next matcher. i++; diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java index 679031eea23..639744aed43 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java @@ -5994,4 +5994,27 @@ public class NumberFormatTest extends TestFmwk { assertEquals("Should parse successfully", 0.08, percentage.doubleValue()); assertEquals("Should consume whole string", 3, ppos.getIndex()); } + + @Test + public void testStrictParseCurrencyLongNames() { + DecimalFormat df = (DecimalFormat) DecimalFormat.getInstance(ULocale.ENGLISH, DecimalFormat.PLURALCURRENCYSTYLE); + df.setParseStrict(true); + df.setCurrency(Currency.getInstance("USD")); + double input = 514.23; + String formatted = df.format(input); + assertEquals("Should format as expected", "514.23 US dollars", formatted); + ParsePosition ppos = new ParsePosition(0); + CurrencyAmount ca = df.parseCurrency(formatted, ppos); + assertEquals("Should consume whole number", ppos.getIndex(), 17); + assertEquals("Number should round-trip", ca.getNumber().doubleValue(), input); + assertEquals("Should get correct currency", ca.getCurrency().getCurrencyCode(), "USD"); + } + + @Test + public void testStrictParseCurrencySpacing() { + DecimalFormat df = new DecimalFormat("¤ 0", DecimalFormatSymbols.getInstance(ULocale.ROOT)); + df.setCurrency(Currency.getInstance("USD")); + df.setParseStrict(true); + expect2(df, -51.42, "-US$ 51.42"); + } }