On Tue, 30 Jun 2026 17:22:25 GMT, Justin Lu <[email protected]> wrote:
> This PR corrects the special case handling in `Locale.Builder.setLocale`. The
> current handling always assumes a `Locale` matching the `ja/JP/JP` and
> `th/TH/TH` special cases will contain the correct compatibility extensions. A
> user can remove the extension or create a Locale in the form of the special
> case without the correct compatibility extension.
>
> For example, the current implementation does the following,
>
>> jshell> loc = new Builder().setLocale(Locale.of("ja", "JP",
>> "JP").stripExtensions()).build()
>> loc ==> ja_JP
>>
>> jshell> loc = new
>> Builder().setLocale(Locale.forLanguageTag("ja-JP-u-ca-foobar-x-lvariant-JP")).build()
>> loc ==> ja_JP_#u-ca-foobar
>
> That is, it silently discards the ill-formed JP variant.
>
> With assertions on, this instead becomes a `NullPointerException` when the
> extensions are not present, or an `AssertionError` when the extension has the
> wrong value.
>
> For the example above, one would expect an `IllFormedLocaleException` to be
> thrown, since without a correct compatibility extension, JP is merely an
> invalid variant and `setLocale(Locale)` throws on ill-formed fields.
>
> The changes in this PR ensure that these cases throw as expected.
>
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK
> Interim AI Policy](https://openjdk.org/legal/ai).
Looks good overall. I’d expect the specification of Locale.Builder.setLocale()
to be updated, since this change causes it to throw IllformedLocaleException
when the required compatibility extension is missing or has an incorrect value
src/java.base/share/classes/sun/util/locale/InternalLocaleBuilder.java line 387:
> 385: if (language.equals("ja") && region.equals("JP") &&
> variant.equals("JP")
> 386: && localeExtensions != null
> 387: &&
> "japanese".equals(localeExtensions.getUnicodeLocaleType("ca"))) {
I think the check here should include `script.isEmpty()`, as the
`Locale.getCompatibilityExtensions()` does so.
src/java.base/share/classes/sun/util/locale/InternalLocaleBuilder.java line 394:
> 392: }
> 393: // Exception 2 - th_TH_TH
> 394: else if (language.equals("th") && region.equals("TH") &&
> variant.equals("TH") &&
Same here.
test/jdk/java/util/Locale/LocaleEnhanceTest.java line 743:
> 741: assertThrows(IllformedLocaleException.class,
> 742: () -> new
> Builder().setLocale(Locale.forLanguageTag("th-TH-u-nu-foobar-x-lvariant-TH")));
> 743:
Could check these locales throw ILE as well, as they are not legacy locales.
Locale.forLanguageTag("ja-Jpan-JP-u-ca-japanese-x-lvariant-JP")
Locale.forLanguageTag("th-Thai-TH-u-nu-thai-x-lvariant-TH")
-------------
PR Review: https://git.openjdk.org/jdk/pull/31728#pullrequestreview-4603494311
PR Review Comment: https://git.openjdk.org/jdk/pull/31728#discussion_r3501335547
PR Review Comment: https://git.openjdk.org/jdk/pull/31728#discussion_r3501339859
PR Review Comment: https://git.openjdk.org/jdk/pull/31728#discussion_r3501363162
