RFR: 8284447: Remove the unused NestedClassWriter interface [v2]

[Pavel Rappo]

On Thu, 7 Apr 2022 01:15:08 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> Sigh, I "reviewed" this earlier, but apparently did not complete the review. So I'll try and reconstruct my earlier comments.
> 
> Approving this change is not as simple as it may seem. Yes, `NestedClassWriter` is unused, and while removing it may seem to be the right thing to do, renaming the `Impl` class just swaps one weirdness for another ... In particular, what was the `Impl` class is now stylistically inconsistent with other related/similar subtypes of `AbstractMemberWriter`, and that shows up in the first few lines of the review (in `ClassUseWriter`). As curious/weird as the world was before this PR, it is still somewhat weird after this PR, but just in a new and different way.
> 
> The underlying problem is that we have no single good naming methodology for interfaces and implementations across the `toolkit` world and the `formats.html` world. In particular, in `formats.html` we inconsistently have interfaces and implementations, `Xyz` interfaces and classes, and `XyzImpl` classes and `HtmlXyz` classes.
> 
> For what it's worth, I think a better solution from a consistency point of view would be to not delete `NestedClassWriter`, and have `NestedClassWriterImpl` implement `NestedClassWriter`, even though it is (currently) an empty interface, and even though it may be likely to remain so.
> 
> That being said, I'll (reluctantly?) approve this as-is, since I think there is a bigger problem to be addressed, which is the overall general "Writer" architecture in terms of the inheritance hierarchy and packages used to model writers (and that is even before we look at the similar but different "Builder" architecture.) Overall, I don't think this change makes the world any worse, just annoyingly different, and it is arguably a tiny bit better for removing an unused empty interface.

Initially, I thought about fixing this issue by making `NestedClassWriterImpl` implement `NestedClassWriter`. However, since `NestedClassWriter` has no methods, in order for it to be useful it would need to declare those methods of the `NestedClassWriterImpl` that are currently used. If it doesn't, this whole situation would look even weirder. After all, `NestedClassWriter` is not a marker interface, is it?

On the other hand, a quick IDE analysis shows that along with many unused interface methods, we have some interface methods that are only called on their implementations and not on interfaces. This does not look right, as it means that we use classes where we should use interfaces. Here's an (incomplete) list of such interface methods:

  * jdk.javadoc.internal.doclets.toolkit.MemberSummaryWriter#getMember
  * jdk.javadoc.internal.doclets.toolkit.WriterFactory#getAnnotationTypeOptionalMemberWriter
  * jdk.javadoc.internal.doclets.toolkit.WriterFactory#getAnnotationTypeRequiredMemberWriter
  * jdk.javadoc.internal.doclets.toolkit.builders.AbstractMemberBuilder#hasMembersToDocument
 
I think that this PR should be withdrawn, and the issue it tries to address should be considered in JDK-8283576, which I filed earlier.

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

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