RFR: 8327640: Allow NumberFormat strict parsing [v6]

[Naoto Sato]

On Wed, 10 Apr 2024 20:20:36 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR and associated [CSR](https://bugs.openjdk.org/browse/JDK-8327703) which introduces strict parsing for NumberFormat.
>> 
>> The concrete subclasses that will utilize this leniency value are `DecimalFormat` and `CompactNumberFormat`. Strict leniency allows for parsing to be used as an input validation technique for localized formatted numbers. The default leniency mode will remain lenient, and can only be set to strict through an intentional `setLenient(boolean)` method call. Leniency operates differently based off the values returned by `isGroupingUsed()`, `getGroupingSize()`, and `isParseIntegerOnly()`.
>> 
>> Below is an example of the change, the CSR can be viewed for further detail.
>> 
>> 
>> DecimalFormat fmt = (DecimalFormat) NumberFormat.getNumberInstance(Locale.US);
>> fmt.parse("1,,,0,,,00,.23Zabced45");  // returns 1000.23
>> fmt.setLenient(false);
>> fmt.parse("1,,,0,,,00,.23Zabced45"); // Now throws a ParseException
>> fmt.setGroupingSize(2);
>> fmt.parse("12,34,567"); // throws ParseException
>> fmt.setParseIntegerOnly(true)
>> fmt.parse("12,34.56"); // throws ParseException
>> fmt.parse("12,34"); // successfully returns the Number 1234
>> 
>> 
>> The associated tests are all localized, and the data is converted properly during runtime.
>
> Justin Lu has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 14 additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8327640-NumberFormat-strictParsing
>  - improve wording consistency and see tags
>  - Merge branch 'master' into JDK-8327640-NumberFormat-strictParsing
>  - revert cleanup changes; (to go into a separate change)
>  - use directed 'inheritDoc' tag
>  - update example for lenient parsing
>  - replace protected field for private fields in subclasses for consistency
>  - drop Format implNote as well
>  - setStrict and isStrict should be optional in NumberFormat
>    - specify and throw UOE as default
>    - override and implement in dFmt and cmpctNFmt
>    parseStrict should be protected, and utilized by subclasses. The public methods should just
>    serve as API.
>  - Re-work specification wording from Format down to subclasses
>  - ... and 4 more: https://git.openjdk.org/jdk/compare/645f53d4...aa1c284b

Looks good overall. Left some minor comments.
As to the tests, good to see those corner cases, but they should have unit tests for the new methods, i.e, `isStrict()` and `setStrict()`. Also I think equality/serialization for those methods should be examined.

src/java.base/share/classes/java/text/DecimalFormat.java line 43:

> 41: import sun.util.locale.provider.LocaleProviderAdapter;
> 42: import sun.util.locale.provider.ResourceBundleBasedAdapter;
> 43: 

Internal packages typically follow public packages.

src/java.base/share/classes/java/text/DecimalFormat.java line 2653:

> 2651:     }
> 2652: 
> 2653:     // Checks to make sure grouping size is not violated. Used when strict.

If it is only supposed to be used in `strict`, might be helpful to put an assertion here.

src/java.base/share/classes/java/text/DecimalFormat.java line 2663:

> 2661: 
> 2662:     // Calculates the index that violated the grouping size
> 2663:     // Calculation is determined whether it was an under or over violation

Not quite sure what `under/over violation` means here. Need more comments?

src/java.base/share/classes/java/text/NumberFormat.java line 43:

> 41: import sun.util.locale.provider.LocaleProviderAdapter;
> 42: import sun.util.locale.provider.LocaleServiceProviderPool;
> 43: 

Same as above

src/java.base/share/classes/java/text/NumberFormat.java line 493:

> 491:      */
> 492:     public boolean isStrict() {
> 493:         throw new UnsupportedOperationException();

Some message might be helpful.

src/java.base/share/classes/java/text/NumberFormat.java line 512:

> 510:      */
> 511:     public void setStrict(boolean strict) {
> 512:         throw new UnsupportedOperationException();

Same here

-------------

PR Review: https://git.openjdk.org/jdk/pull/18339#pullrequestreview-1993112743
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560317735
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560323356
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560326734
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560330147
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560332122
PR Review Comment: https://git.openjdk.org/jdk/pull/18339#discussion_r1560332205