On Tue, 9 Jul 2024 18:42:25 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR which corrects a case in NumberFormat integer only
> parsing.
>
> [JDK-8333755](https://bugs.openjdk.org/browse/JDK-8333755) fixed integer only
> parsing when the value has a suffix, although it caused incorrect behavior
> for the following case: when the parsed string does not contain an integer
> portion, and the format has integer only parsing, parsing should fail,
> instead of 0 being returned. For example,
>
>
> var fmt = NumberFormat.getIntegerInstance();
> fmt.parse(".5", new ParsePosition(0)); // should return null, not 0
>
>
> The changes to the _badParseStrings_ data provider in _StrictParseTest.java_
> are needed since those cases _should_ fail in different indexes depending on
> if integer parsing is enabled. Thus, they were updated to ensure they fail
> for both integer and non-integer parsing with the same errorIndex.
>
> In the fix itself, I also updated the initial value of `intIndex` to -1 from
> 0, to provide better clarity.
The fix looks good. A couple of comments on tests.
test/jdk/java/text/Format/NumberFormat/LenientParseTest.java line 146:
> 144: @EnabledIfSystemProperty(named = "user.language", matches = "en")
> 145: public void compactIntegerParseOnlyFractionOnlyTest() {
> 146: var fmt = NumberFormat.getIntegerInstance();
Should it get a compact number instance, instead of integer?
test/jdk/java/text/Format/NumberFormat/StrictParseTest.java line 221:
> 219: @EnabledIfSystemProperty(named = "user.language", matches = "en")
> 220: public void compactIntegerParseOnlyFractionOnlyTest() {
> 221: var fmt = NumberFormat.getIntegerInstance();
same here
-------------
PR Review: https://git.openjdk.org/jdk/pull/20101#pullrequestreview-2167273474
PR Review Comment: https://git.openjdk.org/jdk/pull/20101#discussion_r1671085244
PR Review Comment: https://git.openjdk.org/jdk/pull/20101#discussion_r1671086182
