RFR: 8341923: java.util.Locale class specification improvements

Mon Nov 18 21:06:00 UTC 2024

On Mon, 18 Nov 2024 07:41:31 GMT, Justin Lu <jlu at openjdk.org> wrote:

> Please review this PR and corresponding CSR which includes a wide range of specification improvements for java.util.Locale. See the CSR for further detail. Other changes/suggestions are welcomed to be included as part of this change. Alternatively the diffs can be viewed on the API diff link attached on the CSR.

Would be nice if you can render the APIDiff and javadoc and share on cr.openjdk.org :)

src/java.base/share/classes/java/util/Locale.java line 94:

> 92:  * <p> A {@code Locale} is composed of the bolded fields described below; note that a
> 93:  * {@code Locale} need not have all such fields. For example, {@link
> 94:  * Locale#ENGLISH Locale.ENGLISH} is only comprised of the <em>language</em> field.

Side remark: in the `Locale` class itself, you can just use `{@link #ENGLISH Locale.ENGLISH}` to generate the link.

src/java.base/share/classes/java/util/Locale.java line 95:

> 93:  * {@code Locale} need not have all such fields. For example, {@link
> 94:  * Locale#ENGLISH Locale.ENGLISH} is only comprised of the <em>language</em> field.
> 95:  * Contrarily, a {@code Locale} such as the one returned by {@code

"Contrarily" is fine, but I think we use "In contrast" more often.

src/java.base/share/classes/java/util/Locale.java line 100:

> 98:  * the United States using the Latin alphabet and numerics for use in POSIX
> 99:  * environments.
> 100:  * {@code Locale} implements IETF BCP 47 and any deviations should be observed

I recommend a paragraph break here with `<p>`

src/java.base/share/classes/java/util/Locale.java line 120:

> 118:  *
> 119:  *   <dd> <em>Syntax:</em> Well-formed {@code language} values have the form {@code [a-zA-Z]{2,8}}.
> 120:  *   BCP 47 deviation: this is not the full BCP 47 language production, since it excludes

Should we add `<br>` before these deviations for more straightforward rendering?

src/java.base/share/classes/java/util/Locale.java line 210:

> 208:  * only checks if an individual field satisfies the syntactic
> 209:  * requirement (is well-formed), but does not validate the value
> 210:  * itself. Conversely, {@link #of(String, String, String)} and its overloads do not

Suggestion:

 * itself. Conversely, {@link #of(String, String, String) Locale::of} and its overloads do not

I think javadoc tends to render those arguments as full-blown `of(java.lang.String, java.lang.String, java.lang.String)`, which is most likely not what you want.

src/java.base/share/classes/java/util/Locale.java line 333:

> 331:  *  {@link Locale#US Locale.US} is the {@code Locale} object for the United States.</dd>
> 332:  *  <dt><b>Factory Methods</b></dt>
> 333:  *  <dd>The method {@link #of(String, String, String)} and its overloads obtain a

Same link remarks.  Also for `forLanguageTag` below.

src/java.base/share/classes/java/util/Locale.java line 343:

> 341:  *
> 342:  * {@snippet lang=java :
> 343:  *     // The following are all equivalent

Suggestion:

 * <p>The following are all equivalent:
 * {@snippet lang=java :

src/java.base/share/classes/java/util/Locale.java line 376:

> 374:  * Locale.Category#FORMAT FORMAT} locale :
> 375:  * {@snippet lang=java :
> 376:  *     // The following are equivalent

Still recommend moving this comment to official javadoc, so the last sentence can be

The latter uses the default {@link Locale.Category#FORMAT FORMAT} locale, so
the following are equivalent:

src/java.base/share/classes/java/util/Locale.java line 399:

> 397:  * resources for multiple locales, it sometimes needs to find one or more
> 398:  * locales (or language tags) which meet each user's specific preferences. Note
> 399:  * that the term "language tag" is used interchangeably with "locale" in the following

Should we add an index to "language tag", like:
Suggestion:

 * that the term "<dfn>{@index "language tag"}</dfn>" is used interchangeably with "locale" in the following

src/java.base/share/classes/java/util/Locale.java line 476:

> 474:  *
> 475:  * <h2>Compatibility</h2>
> 476:  * @implNote

I believe `@implNote` is a higher-level tag than a `<h2>`; all following contents will be indented. Recommended moving the `<h2>` into the `@implNote`.

src/java.base/share/classes/java/util/Locale.java line 500:

> 498:  * <p>Because of these issues, it is recommended that apps migrate
> 499:  * away from constructing non-conforming locales and use the
> 500:  * {@link #forLanguageTag} and {@link Locale.Builder} APIs instead.

Same remarks for links, add a override render text to prevent javadoc from generating long parameter lists.

src/java.base/share/classes/java/util/Locale.java line 2106:

> 2104:      * is en_US, getDisplayCountry() will return "France"; if the locale is en_US and
> 2105:      * inLocale is fr_FR, getDisplayCountry() will return "Etats-Unis".
> 2106:      * If the name returned cannot be localized according to inLocale.

Suggestion:

     * If the name returned cannot be localized according to inLocale,

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

PR Review: https://git.openjdk.org/jdk/pull/22192#pullrequestreview-2443672580
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847236829
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847240545
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847243076
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847253411
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847259756
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847261730
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847263166
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847266745
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847267850
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847270260
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847271943
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847272982