Clarifying record reflective support

[Remi Forax]

Hi Chris,

Please do not add another java.io.File.listFiles() !

People will test a code that use Class.getRecordComponents() only with records forgetting as usual to have negative tests and it will blow in production.

A method that returns an array should never return null.
I repeat
A method that returns an array should never return null.

If someone want to disambiguate between i've no record component because i'm a record or i've no record component because i'm not a record, isRecord is exactly the kind of method you want to call.

regards,
Rémi


----- Mail original -----
> De: "Chris Hegarty" <chris.hegarty at oracle.com>
> À: "amber-spec-experts" <amber-spec-experts at openjdk.java.net>, "joe darcy" <joe.darcy at oracle.com>, "Maurizio Cimadamore"
> <maurizio.cimadamore at oracle.com>
> Envoyé: Mardi 3 Décembre 2019 12:26:53
> Objet: Clarifying record reflective support

> A number of issues and/or concerns have been come up recently relating
> to the reflective support for records. These arose when finalizing and
> completing the runtime CSR. Taken together they seem to lead back to a
> few small, but significant, omissions in the spec that would be good to
> tighten up. It is important that it be possible to reflectively reason
> about record classes in a way that is unambiguous and provides
> certainty that the record class is well-formed.
> 
> For a class to be a record class, then:
> 
>  1) It's direct superclass must be java.lang.Record, and
>  2) It must have a Record attribute.
> 
> The entry point to the reflective API for records is the two methods:
> Class::isRecord and Class::getRecordComponents.
> 
> The isRecord method should, in it's specification, guaranteed both of
> point 1 and point 2 above. That is to say, for a class to be considered
> a record class, then its direct superclass must be java.lang.Record, and
> it must have a Record attribute. The implementation already behaves
> this way, but the specification should require it.
> 
> The getRecordComponents method currently returns an empty array for both
> a record class with no components and a non-record class. We thought it
> kinda nice to be able to avoid returning null, but with hindsight I
> think that it would be better to remove this potential ambiguity. The
> getRecordComponents method should only return a non-null value if both
> point 1 ( the class is a record class ) and point 2 above are true.
> There are many other null returning methods in Class, so this is not
> unusual or out of place. The implementation only requires a minor change
> to support this ( return null for non-record classes ).
> 
> The most significant part of the changes proposed are to the
> specification, so I've included that here inline. The proposed changes
> tightly couple the pair of methods as part of their specification,
> something that will hopefully be cleaner to do (or even unnecessary)
> when we have full pattern matching.
> 
> 
> /src/java.base/share/classes/java/lang/Class.java
> 
>     /**
>      * {@preview Associated with records, a preview feature of the Java language.
>      *
>      *           This method is associated with <i>records</i>, a preview
>      *           feature of the Java language. Preview features
>      *           may be removed in a future release, or upgraded to permanent
>      *           features of the Java language.}
>      *
>      * Returns {@code true} if and only if this class is a record class.
> -     * It returns {@code false} otherwise. Note that class {@link Record} is
> not a
> -     * record type and thus invoking this method on class {@link
> java.lang.Record}
> -     * returns {@code false}.
> -     *
> -     * @return true if and only if this class is a record class
> +     *
> +     * <p> The {@linkplain #getSuperclass() direct superclass} of a record
> +     * class is {@code java.lang.Record}. A record class has (possibly empty)
> +     * record components, that is, {@link #getRecordComponents()} returns a
> +     * non-null value.
> +     *
> +     * <p> Note that class {@link Record} is not a record type and thus
> invoking
> +     * this method on class {@code Record} returns {@code false}.
> +     *
> +     * @return true if and only if this class is a record class, otherwise
> false
>      * @jls 8.10 Record Types
>      * @since 14
>      */
>     public boolean isRecord() { ... }
> 
>     /**
>      * {@preview Associated with records, a preview feature of the Java language.
>      *
>      *           This method is associated with <i>records</i>, a preview
>      *           feature of the Java language. Preview features
>      *           may be removed in a future release, or upgraded to permanent
>      *           features of the Java language.}
>      *
> -     * Returns an array containing {@code RecordComponent} objects reflecting
> all the
> -     * declared record components of the record represented by this {@code
> Class} object.
> -     * The components are returned in the same order that they are declared in
> the
> -     * record header.
> -     *
> -     * @return  The array of {@code RecordComponent} objects representing all
> the
> -     *          record components of this record. The array is empty if this
> class
> -     *          is not a record, or if this class is a record with no
> components.
> +     * Returns an array of {@code RecordComponent} objects representing all the
> +     * record components of this record class, or {@code null} if this class is
> +     * not a record class.
> +     *
> +     * <p> The components are returned in the same order that they are declared
> +     * in the record header. The array is empty if this record class has no
> +     * components. If the class is not a record class, that is {@link
> +     * #isRecord()} returns false, then this method returns null. Conversely,
> if
> +     * {@link #isRecord()} returns true, then this method returns a non-null
> +     * value.
> +     *
> +     * @return  An array of {@code RecordComponent} objects representing all
> the
> +     *          record components of this record class, or {@code null} if this
> +     *          class is not a record class
>      * @throws  SecurityException
>      *          ...
>      *
>      * @jls 8.10 Record Types
>      * @since 14
>      */
>     public RecordComponent[] getRecordComponents() { … }
> 
> 
> -Chris.