RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v3]

Archie Cobbs acobbs at openjdk.org
Wed Jan 24 00:21:46 UTC 2024


On Tue, 23 Jan 2024 22:15:40 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add @implNote to Javadoc for toPattern().
>
> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line 90:
> 
>> 88:         // A few more test cases from the PR#17416 discussion
>> 89:         testRoundTrip(new MessageFormat("Test: {0,number,foo'{'#.00}"));
>> 90:         testRoundTrip(new MessageFormat("Test: {0,number,foo'}'#.00}"));
> 
> Can we add some more concrete test cases, for example, escaped quotes within a MessageFormat pattern (represented by doubled single quotes). 
> 
> Another funny case is something like a MessageFormat created by `"{0,number,' abc }'' ' 0.00}"` which becomes `"{0,number, abc '}'''  #0.00}"` after created from the value returned by toPattern() from the first MessageFormat. The Strings appear really different but format equivalently, it would be nice to have suspicious cases like these explicitly defined.

Sure thing - added in 58e8cc68f45.

> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line 154:
> 
>> 152:     // Generate a "random" MessageFormat. We do this by creating a MessageFormat with "{0}" placeholders
>> 153:     // and then substituting in random DecimalFormat, DateFormat, and ChoiceFormat subformats. The goal here
>> 154:     // is to avoid using pattern strings to construct formats, because they're what we're trying to check.
> 
> Can you add a general example of a randomly generated MessageFormat in the comments, (I know they vary by quite a lot), but it would be hard for someone to piece if together without digging into the code. And unfortunately the current toString() of MessageFormat is not implemented, so although JUnit prints the MessageFormat out, it's just gibberish.

Added in 58e8cc68f45 as a test case.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1464143414
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1464143622


More information about the core-libs-dev mailing list