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
