<i18n dev> RFR: JDK-8325898: ChoiceFormat returns erroneous result when formatting bad pattern [v5]

Naoto Sato naoto at openjdk.org
Fri Feb 23 18:55:54 UTC 2024


On Fri, 23 Feb 2024 17:29:18 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR which handles an edge case pattern bug with ChoiceFormat.
>> 
>> 
>> var d = new ChoiceFormat("0#foo|1#bar|baz|") // creates cFmt equivalent to "0.0#foo|1.0#bar|1.0#"
>> d.format(1) // unexpectedly returns ""
>> 
>> 
>> Not only does this lead to faulty formatting results, but breaks the ChoiceFormat class invariant of duplicate limits.
>> 
>> It would have been preferable if either an exception was thrown when the ChoiceFormat was initially created, or the ChoiceFormat formatting 1 returned a value of "bar".
>> 
>> For comparison,
>> 
>> var d = new ChoiceFormat("0#foo|1#bar|baz") // creates cFmt equivalent to "0.0#foo|1.0#bar"
>> d.format(1) // returns "bar"
>> 
>> 
>> After this change, the first code snippet now returns "bar" when formatting 1 and discards the "baz" as the second code snippet does.
>> 
>> 
>> The specific fix for this bug is addressed with the following, on line 305,
>> 
>> if (seg != Segment.FORMAT) {
>>     // Discard incorrect portion and finish building cFmt
>>     break;
>> }
>> 
>> This prevents a limit/format entry from being built when the current parsing mode has not yet processed the limit and relation. The previous implementation would build an additional limit/format of 1 with an empty string. The `break` allows for the discarding of the incorrect portion and continuing. Alternatively an exception could have been thrown. For consistency with the second code snippet, the former was picked.
>> 
>> https://github.com/openjdk/jdk/pull/17856 may be of interest, which is a semi-related specification fix.
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   use enhanced switch in stringToNum

Marked as reviewed by naoto (Reviewer).

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

PR Review: https://git.openjdk.org/jdk/pull/17883#pullrequestreview-1898741418


More information about the i18n-dev mailing list