RFR: JDK-6801704: ChoiceFormat::applyPattern inconsistency for invalid patterns [v5]

[Naoto Sato]

On Thu, 29 Feb 2024 20:07:14 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8317756) which defines the behavior for creating ChoiceFormats with incorrect patterns. The wording is added to both the ChoiceFormat constructor and ChoiceFormat::applyPattern method.
>> 
>> While ideally the inconsistent behavior itself could be fixed, this behavior has been long-standing for 20+ years and the benefit of consistent error handling does not outweigh the risk of breaking applications that may be relying on the "expected" incorrect behavior.
>> 
>> Examples of the range of behavior, (all examples violate the pattern syntax defined in the class description)
>> 
>> 
>> // no limit -> throws an expected IllegalArgumentException
>> var a = new ChoiceFormat("#foo");
>> // no limit or relation in the last subPattern -> discards the incorrect portion, 'baz' and continues
>> var b = new ChoiceFormat("0#foo|1#bar|baz"); 
>> b.format(2); // returns 'bar'
>> // no relation or limit -> discards the incorrect portion, 'foo' and continues
>> var c = new ChoiceFormat("foo");
>> c.format(1); // throws AIOOBE
>
> Justin Lu has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - include ascending order stipulation in constuctor as well
>  - include apiSpec wording, move to patterns section

LGTM

src/java.base/share/classes/java/text/ChoiceFormat.java line 406:

> 404:      * based on the pattern. The syntax and error related caveats for the
> 405:      * ChoiceFormat pattern can be found in the {@linkplain ##patterns Patterns}
> 406:      * section. Unlike {@link #ChoiceFormat(double[], String[])} this method will

Nit: `constructor` better suited than `method`.

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

Marked as reviewed by naoto (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17856#pullrequestreview-1910070004
PR Review Comment: https://git.openjdk.org/jdk/pull/17856#discussion_r1508322149