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

Justin Lu jlu at openjdk.org
Thu Feb 22 17:55:18 UTC 2024


> 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.
> 
> 
> This PR includes a lot of cleanup to the applyPattern implementation, 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:

  null check at public level. spacing adjustment.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/17883/files
  - new: https://git.openjdk.org/jdk/pull/17883/files/edfbe7bf..d57fa91a

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17883&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17883&range=00-01

  Stats: 5 lines in 1 file changed: 2 ins; 2 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/17883.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17883/head:pull/17883

PR: https://git.openjdk.org/jdk/pull/17883


More information about the i18n-dev mailing list