[OpenJDK 2D-Dev] <Beans Dev> JDK 14 RFR of JDK-8231334: Suppress warnings on non-serializable instance fields in client libs serializable classes

[Philip Race]

+        @SuppressWarnings("serial") // Not statically typed as Serializable

So is the comment being used to distinguish this overloading of what "serial" means ?
Why not introduce a new warning category ?

Randomly looking at
====
src/java.desktop/share/classes/java/awt/Container.java

@@ -3849,10 +3849,11 @@

          /**
           * The handler to fire {@code PropertyChange}
           * when children are added or removed
           */
+        @SuppressWarnings("serial") // Not statically typed as Serializable
          protected ContainerListener accessibleContainerHandler = null;
===

I see that Container has a writeObject method which doesn't write this 
field, so it is effectively transient.

I recognise that it is difficult for the compiler to figure this out, so 
perhaps there should just be a policy
not to check classes that have writeObject methods ?

Also in such a case, would it be an effectively compatible change to add 
transient to the field, so that
it presumably would no longer need this warning.

I haven't looked but presumably there could be other such cases.

Will you be filing bugs for all the fixable cases ?

-phil

On 9/21/19, 12:48 PM, Joe Darcy wrote:
>
> Hello,
>
> Quick background, I'm working on expanding the compile-time 
> serialization checks of javac's -Xlint:serial option. Ahead of that 
> work going back, I'm analyzing the JDK sources and plan to 
> pre-suppress the coming-soon new warnings, fixing or at least filing 
> follow-up bugs for any problems that are found. Corresponding 
> suppression bugs are already out for review against core libs 
> (JDK-8231202) and security libs (JDK-8231262).
>
> The new check in development is if a serializable class has an 
> instance field that is not declared to be a serializable type. This 
> might actually be fine in practice, such as if the field in question 
> always points to a serializable object at runtime, but it is arguably 
> worth noting as an item of potential concern. This check is skipped if 
> the class using the serialPersistentFields mechanism.
>
> For the client libs, the webrev with the new @SuppressedWarnings 
> annotations is:
>
>     JDK-8231334: Suppress warnings on non-serializable instance fields 
> in client libs serializable classes
> http://cr.openjdk.java.net/~darcy/8231334.0/
>
> The changes are mostly in awt, but also some in beans, a few in 
> printing, and one in sound.
>
> As discussed with Phil off-line, the new checks also found an existing 
> known issue, the auxiliary class java.awt.ImageMediaEntry declared in 
> MediaTracker.java is not serializable/deserializable in practice 
> (JDK-4397681).
>
> Thanks,
>
> -Joe
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20190922/59c7823a/attachment.html>