RFR: 8274405: Suppress warnings on non-serializable non-transient instance fields in javac and javadoc [v3]

[Joe Darcy]

On Wed, 29 Sep 2021 22:52:08 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> 
> 
> There are 3, maybe just 2, groups of files in this review.
> 
> `sjavac` is an internal utility that ought not to be in the `src/` tree, so the changes there don't matter.
> 
> The various `Result` classes and the javadoc exceptions are all "internal" exceptions used for internal control flow, and not intended to be seen by API clients. As @pavelrappo notes, we have plans to make the `Result` classes go away by rewriting the relevant scanners to make them unnecessary. That leaves the javadoc exceptions. The commented annotations have a slight code-smell about them, in the sense of brushing the warning under the carpet, so to speak. It would be better if there was a better way to remove the cause of the warning, rather than just hiding the warning itself. But, that's not easy, and the original sin was making `Throwable` (and hence all subtypes) implement `Serializable` in the first place. But, that's the serialization we have and we have to deal with it.
> 
> As a final note, I never did like the `Runnable` in the last exception, and seeing it again may be a good reason to try and get rid of it, like those `Result` classes mentioned earlier. I also note that `DocPath` _could_ be made `Serializable` but `DocFile` cannot reasonably be made serializable (incompatible API change to `Location`) and even thinking about it seems like a case of the tail wagging the elephant!
> 
> So, with some disappointment, I accept that the changes you propose are the least bad of the possible solutions, at least for now, and so I approve the review.

If I were to see

@SuppressWarning("serial") // Type of field not Serializable
Foo foo;

in new code I would question the serial-design of the class; that is one of my goals with augmenting the Xlint:serial checks, notice possibly questionable serial coding patterns sooner.

For types where maintaining cross-release serial compatibility is not strictly needed, one approach would be to mark the fields as transient and give the class a different serialVersionUID. If serial compatibility is needed the conceptually transient field, i.e. one that cannot meaningful be serialized, could be nulled-out in a writeObject method and ignored in a readObject method, at the cost of maintaining those additional methods.

Fortunately, the the javax.lang.model API, the issues with exception classes having non-Serializable fields was recognized during the design and we marked all such fields and transient and documented the corresponding getters to possibly return null.

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

PR: https://git.openjdk.java.net/jdk/pull/5728