RFR: 8306729: Add nominal descriptors of modules and packages to Constants API [v4]

[Mandy Chung]

On Tue, 25 Apr 2023 08:11:24 GMT, Adam Sotona <asotona at openjdk.org> wrote:

>> Constants API already provides models for all loadable constants to help programs manipulating class files and modelling bytecode instructions. However no models of module and package constants are provided by Constants API. Every program manipulating class files must implement own models and validation of modules and packages constants.
>> 
>> This pul request adds `java.lang.constant.ModuleDesc` and `java.lang.constant.PackageDesc` to the Constants API. 
>> 
>> Classfile API will follow up and remove its internal implementations of `PackageDesc` and `ModuleDesc`. 
>> 
>> Please review this pull request and attached CSR.
>> 
>> Thank you,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional commit since the last revision:
> 
>   added custom toString() methods

A side note: I notice that the type names are linked with `@linkplain` in this package mixed with some `@link`.  I'm not sure if it's intentional as type names are generally used `@link` or `@code`.

src/java.base/share/classes/java/lang/constant/ModuleDesc.java line 64:

> 62:      * an at-sign in a module name.
> 63:      * </ul>
> 64:      * @param name module name

Suggestion:

     * @param name the module name

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 33:

> 31:  *
> 32:  * <p>To create a {@linkplain PackageDesc} for a package, use {@link #of} or
> 33:  * {@link #ofInternalName(String)}.

Suggestion:

 * <p>To create a {@linkplain PackageDesc} for a package, use the {@link #of} or
 * {@link #ofInternalName(String)} method.

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 44:

> 42:      * given the name of the package, such as {@code "java.lang"}.
> 43:      * <p>
> 44:      * {@jls 13.1}

Do you mean to reference JLS 6.7?

Suggest to move this reference after `throws` in the see also section.

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 46:

> 44:      * {@jls 13.1}
> 45:      *
> 46:      * @param name the fully qualified (dot-separated) binary package name

JLS 13.1 does not describe a package has a binary name.   Is it correct to say "binary package name"?

Suggestion:

     * @param name the fully qualified (dot-separated) package name

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 62:

> 60:      * such as {@code "java/lang"}.
> 61:      * <p>
> 62:      * {@jvms 4.2.1} In this internal form, the ASCII periods (.) that normally

Suggest not to copy JVMS 4.2.1 here but instead just add it to see also section.

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 65:

> 63:      * separate the identifiers
> 64:      * which make up the binary name are replaced by ASCII forward slashes (/).
> 65:      * @param name the fully qualified class name, in internal (slash-separated)

Suggestion:

     * @param name the fully qualified package name, in internal (slash-separated) form

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 82:

> 80:      *
> 81:      * @return the package name, or the empty string for the
> 82:      * default package

Suggestion:

     * unnamed package

src/java.base/share/classes/java/lang/constant/PackageDesc.java line 91:

> 89:      *
> 90:      * @return the package name, or the empty string for the
> 91:      * default package

Suggestion:

     * unnamed package


See JLS 7.4.2

src/java.base/share/classes/java/lang/constant/package-info.java line 93:

> 91:  *
> 92:  * <p>Other members of this package are {@link java.lang.constant.ModuleDesc}
> 93:  * and  {@link java.lang.constant.PackageDesc}. They represent module and package

Suggestion:

 * <p>Other members of this package are {@link ModuleDesc}
 * and  {@link PackageDesc}. They represent module and package


same package and so no need to qualify.

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

PR Review: https://git.openjdk.org/jdk/pull/13615#pullrequestreview-1398780011
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176758742
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176759664
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176763362
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176770337
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176781861
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176768935
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176789226
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176787981
PR Review Comment: https://git.openjdk.org/jdk/pull/13615#discussion_r1176790549