On Fri, 5 Jun 2026 00:05:04 GMT, Naoto Sato <[email protected]> wrote:
>> Made changes to the invalid placeholder handling in >> `ListFormat.getInstance(String[])`, stemming from >> [JDK-8385736](https://bugs.openjdk.org/browse/JDK-8385736). >> >> This rejects duplicate placeholders and `"{2}"` outside the `three` pattern, >> with spec and test updates. Also includes minor Javadoc formatting cleanups. >> The invalid-long-pattern test added with >> [JDK-8385736](https://bugs.openjdk.org/browse/JDK-8385736) has been >> repurposed to cover the newly specified invalid-placeholder cases, because >> the repeated-placeholder input is now rejected immediately by >> duplicate-placeholder validation. >> >> Since this changes the behavior of the method, a corresponding CSR has been >> drafted. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflects review comments src/java.base/share/classes/java/text/ListFormat.java line 280: > 278: * {@code end}, {@code two}, or {@code three} fails, including > duplicate > 279: * placeholders, "{2}" in patterns other than the {@code three} > element > 280: * pattern, or any use of "{" or "}" other than "{0}", "{1}", or > "{2}", We won't have support for literal braces which seems reasonable for these list patterns. There could always be an escaping mechanism for such support, but seems unnecessary for now unless there was a genuine request/demand. src/java.base/share/classes/java/text/ListFormat.java line 744: > 742: */ > 743: private static boolean validatePlaceholders(String pattern) { > 744: for (int i = 0; i < pattern.length(); i++) { Since `validatePlaceholders` already iterates through the entire pattern string to validate braces, I think we can just merge the logic of `findPlaceholders` and `validatePlaceholders` into this main loop to do validation and placeholder recording. Instead of var positions = new int[3]; for (int i = 0; i < positions.length; i++) { var ph = "{" + i + "}"; positions[i] = pattern.indexOf(ph); if (positions[i] >= 0) { // Check for duplicate placeholders if (pattern.indexOf(ph, positions[i] + PLACEHOLDER_LENGTH) >= 0) { return null; } } } I think we can just initialize `positions` as `new int[] {-1, -1, -1};` and if `positions[index] != -1` we can fail. We can derive the correct `index` via `pattern.charAt(i + 1) - '0'` (given this code occurs within the `ch == '{'` block). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31377#discussion_r3364043238 PR Review Comment: https://git.openjdk.org/jdk/pull/31377#discussion_r3364114941
