On Wed, 12 Jun 2024 17:06:25 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8333920) >> which corrects a bug where NumberFormat cannot successfully parse values in >> integer only mode, when the format expects a suffix. >> >> For example, >> >> // a format that expects a currency suffix >> var fmt = NumberFormat.getCurrencyInstance(Locale.FRANCE); >> fmt.setParseIntegerOnly(true); >> failFmt.parse("5,00 €"); // throws ParseException when you would have >> expected 5 returned >> >> >> When parsing in integer only mode, instead of breaking upon a decimal symbol >> encounter, we should store the index but continue to fully parse so that we >> can verify the entire string and increment our position to search for and >> match the suffix. Upon a successful suffix match, we can then set >> `ParsePosition.index` back to the stored decimal index. >> >> It should be noted that CompactNumberFormat did previously have code that >> would traverse the rest of the String, to allow matching of the suffix. The >> difference is that NumberFormat returns the index upon decimal symbol >> encounter, while CompactNumberFormat was implemented to return the index >> reflected by the entire string traversal. Such differences cannot be >> standardized due to behavioral compatibility concerns. >> >> Thus while parsing in integer only, CNF sets index to the >> `Position.fullPos()`, while DF sets index to the `Position.intPos()`. > > Justin Lu has updated the pull request incrementally with one additional > commit since the last revision: > > specification improvements Looks good overall. Some minor nits follow. src/java.base/share/classes/java/text/DecimalFormat.java line 2446: > 2444: // To track the full parse index as well as the integer portion > 2445: record Position(int intPos, int fullPos) {} > 2446: The record name may be more descriptive. Also I would expect more description here, such as what it is and those params are. src/java.base/share/classes/java/text/DecimalFormat.java line 2461: > 2459: * @return returns the position of the first unparseable character > or > 2460: * -1 in case of no valid number parsed > 2461: */ Needs modification accommodating `Position`. test/jdk/java/text/Format/NumberFormat/StrictParseTest.java line 320: > 318: } > 319: > 320: // Overloaded method used for integer only parsing, where the > expected Is this method for `integer only` only? ------------- PR Review: https://git.openjdk.org/jdk/pull/19664#pullrequestreview-2114129703 PR Review Comment: https://git.openjdk.org/jdk/pull/19664#discussion_r1637021751 PR Review Comment: https://git.openjdk.org/jdk/pull/19664#discussion_r1637024103 PR Review Comment: https://git.openjdk.org/jdk/pull/19664#discussion_r1637040736
