JDK 14 RFR of JDK-8202385: Annotation to mark serial-related fields and methods

Thu Aug 8 14:13:21 UTC 2019

Hi Joe,

Looks good, thanks for the updates.

Roger

On 8/6/19 4:04 PM, Joe Darcy wrote:
> Hi Roger,
>
> Revised webev
>
>     http://cr.openjdk.java.net/~darcy/8202385.5/
>
> Diff of last two versions below. To ease review, I elected not to 
> reflow the paragraphs at this time to reduce the number of lines of 
> differences between the two versions.
>
> Several platform annotation types start there javadoc with 
> "Indicates..." so I kept that pattern.
>
> It is true the serialization-defined fields and methods are accessed 
> reflectively and may not be otherwise accessed. That seems more 
> relevant to emphasize in the serialization spec itself, but it may be 
> helpful to note here so I added one as requested.
>
> Cheers,
>
> -Joe
>
> 31c31
> <  * Indicates that a field or method is related to the {@linkplain
> ---
> >  * Indicates that an annotated field or method is part of the 
> {@linkplain
> 37c37,40
> <  * validate method overriding.
> ---
> >  * validate method overriding. {@code Serializable} classes are 
> encouraged to
> >  * use <code>@Serial</code> annotations to help a compiler catch
> >  * mis-declared serialization-related fields and methods,
> >  * mis-declarations that my otherwise be difficult to detect.
> 39c42
> <  * <p>Specifically, annotations of this type are intended to be
> ---
> >  * <p>Specifically, annotations of this type should be
> 59c62
> <  * A compiler can validate that a method or field marked with a
> ---
> >  * Compilers are encouraged to validate that a method or field 
> marked with a
> 61c64,65
> <  * methods or fields declared in a meaningful context.
> ---
> >  * methods or fields declared in a meaningful context and issue a 
> warning
> >  * if that is not the case.
> 86a91,94
> >  *
> >  * Note that serialization mechanism accesses its designated fields
> >  * and methods reflectively and those fields and methods may appear
> >  * otherwise unused in a {@code Serializable} class.
>
> On 8/1/2019 2:40 PM, Roger Riggs wrote:
>> Hi Joe,
>>
>> It would be good to more closely define the semantics of the @Serial 
>> annotation.
>>
>> Suggestion for the first sentence:
>>
>> "@Serial annotates each field or method specified by the <cite>Java 
>> Object Serialization Specification</cite> of a {@linkplain 
>> Serializable serializable} class."
>>
>> This annotation type is intended to allow compile-time checking of 
>> serialization-related declarations, analogous to the checking enabled 
>> by the {@link java.lang.Override} annotation type to validate method 
>> overriding.
>>
>> It would be useful to describe that reflection is used to access and 
>> use the fields and methods and they may  otherwise appear to be unused.
>>
>> A recommendation could be added in an @impleNote to apply @Serial to 
>> each serialization defined method or field.
>>
>> $02, Roger
>>
>> On 7/13/19 1:16 PM, Joe Darcy wrote:
>>> PS I've uploaded an updated an iteration of the webrev
>>>
>>> http://cr.openjdk.java.net/~darcy/8202385.4/
>>>
>>> to address. the syntactic concerns previously raised. I added
>>>
>>>     ...defined by the  <cite>Java Object Serialization 
>>> Specification</cite>...
>>>
>>> which is the current name of the document and is similar to the 
>>> style of reference made in java.io.Serializable. Offhand, I didn't 
>>> know of the correct idiom to refer to the document as a working 
>>> hyperlink, but would be switch to that idiom.
>>>
>>> Cheers,
>>>
>>> -Joe
>>>
>>> On 7/12/2019 8:19 PM, Joe Darcy wrote:
>>>> Hi Roger,
>>>>
>>>> On 7/12/2019 1:31 PM, Roger Riggs wrote:
>>>>> Hi Joe,
>>>>>
>>>>> As an annotation on a field or method, this is a use site annotation.
>>>>
>>>>
>>>> It is an annotation intended for the declarations of fields and 
>>>> methods of Serializable types.
>>>>
>>>>
>>>>> From the description, the checks that could be added would only be 
>>>>> done
>>>>> if the annotation was present and the annotation is a tag for 
>>>>> existing
>>>>> fields and methods that are part of the serialization spec.
>>>>
>>>>
>>>> Right; the annotation is semantically only applicable to the fields 
>>>> and methods associated with the serialization system.
>>>>
>>>>
>>>>>
>>>>> The signatures of the fields and methods can be known to the 
>>>>> compiler independent
>>>>> of the annotation and produce the same warnings.
>>>>> So this looks like a case of trying to have belt and suspenders.
>>>>>
>>>>> If the checks are not done when the annotation is not present, 
>>>>> then it will still be
>>>>> the case that incorrect or misused fields and methods will still 
>>>>> escape detection.
>>>>>
>>>>> Though the details of the compiler check are outside of the scope 
>>>>> of this annotation,
>>>>> it seems unclear whether the annotation is necessary.
>>>>
>>>> I have a prototype annotation processor to implement checks for
>>>>
>>>>     JDK-8202056: Expand serial warning to check for bad overloads 
>>>> of serial-related methods and ineffectual fields
>>>>
>>>> The current version of the processor does not assume the presence 
>>>> of java.io.Serial. The summarize the existing checking methodology:
>>>>
>>>>     If a type is Serialiazable and a field or method has a name 
>>>> matching the names of one of the special fields or methods to 
>>>> serialization, check that the field or method has the required 
>>>> modifiers, type, and, the the case of methods, parameter types and 
>>>> exception types.
>>>>
>>>> That is all well and good and represents a large fraction of the 
>>>> checking of interest. However, it does not catch a mis-declaration 
>>>> like "readobject" instead of "readObject". One could argue that 
>>>> sufficiently thorough testing should catch that kind of error; 
>>>> however, my impression is that thoroughness of testing is rare in 
>>>> practice. I don't think it would be reasonable for javac to have 
>>>> some kind of Hamming distance 
>>>> (https://en.wikipedia.org/wiki/Hamming_distance) check between the 
>>>> name of fields/methods and the name of the serialization related 
>>>> fields methods to try to catch such mis-declarations. An annotation 
>>>> like java.io.Serial is intended to allow the programmer to indicate 
>>>> "yes, this is supposed to be one of the serialization related 
>>>> fields or methods" and enable the compile to perform checks against 
>>>> that category of error.
>>>>
>>>>
>>>>>
>>>>> Can the name of the annotation be more descriptive?
>>>>> Just "Serial" seems a bit too simple and does not have a strong 
>>>>> binding to the Serialization classes and specification.
>>>>> Alternatives:
>>>>>    SerialMetadata
>>>>>    SerialControl
>>>>>    SerialFunction
>>>>
>>>> From the earlier design iterations "Serial" was chosen to be 
>>>> evocative of the "@serial" javadoc tag.
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>>>
>>>>>
>>>>> 39:  There should be a reference to the serialization 
>>>>> specification for the definition
>>>>> of the fields and methods to make it clear where the authoritative 
>>>>> identification is spec'd.
>>>>>
>>>>> 73-75:  Please align the <ul> and </ul> tags on separate lines 
>>>>> with matching indentation.
>>>>>
>>>>> 77: Extra leading space.
>>>>>
>>>>> Regards, Roger
>>>>>
>>>>> On 7/9/19 7:14 PM, Joe Darcy wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Returning to some old work [1], please review the addition of a 
>>>>>> java.io.Serial annotation type for JDK 14 to mark serial-related 
>>>>>> fields and methods:
>>>>>>
>>>>>>     webrev: http://cr.openjdk.java.net/~darcy/8202385.3/
>>>>>>     CSR: https://bugs.openjdk.java.net/browse/JDK-8217698
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>> [1] Previous review threads:
>>>>>>
>>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-May/053055.html 
>>>>>>
>>>>>>
>>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-August/054801.html 
>>>>>>
>>>>>>
>>>>>
>>