RFR: 8280713: Related to comment inheritance jdk.javadoc cleanup and refactoring

Jonathan Gibbons jjg at openjdk.java.net
Fri Feb 25 00:56:05 UTC 2022


On Wed, 26 Jan 2022 16:20:30 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

> Explorative refactoring performed while looking into multiple `@inheritDoc` issues. The easiest way to review it is to, probably, go commit by commit; they are quite focused and commented. Not only the branch as a whole, but all the constituent commits should pass tests and leave JDK API Documentation unchanged.

Second commit. 83040e5  Utils.

It's OK as far as it goes, but ...  the following may be out-of-scope for what you are doing, but I'll make the comments anyway, perhaps for later attention. Generally, the code from 162:206 reflects the historical precedent of using `Utils` as a dumping ground for stuff that didn't have a better place to go.   In the old world, I'm guessing the old tool/doclet used direct access to the javac internal `Symtab` class, and the code here reflects the desire not to do that. (Which is good.) A different presentation of this code would be to have a new class, possibly called `Symtab` which contains fields and access methods for the different types ... as compared to the `HashMap` here. That being said, we'd then have to find a way to get access to the `Symtab` class, perhaps still via `Utils`, so it may be a toss-up whether it is worth it or not.

3rd commit. Fix equality check in Utils.getAllInterfaces

I'm guessing the old code was OK, but the new code is more robust.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 168:

> 166:             var typeElement = elementUtils.getTypeElement(s);
> 167:             return typeElement == null ? null : typeElement.asType();
> 168:         });

had to think about this one, but I guess OK. The difference is that the old code never put `null` in the symtab; the new code does.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/VisibleMemberTable.java line 529:

> 527:         Map<ExecutableElement, List<ExecutableElement>> overriddenByTable = new HashMap<>();
> 528:         for (VisibleMemberTable pvmt : parents) {
> 529:             // Merge the lineage overrides into local table

not sure what "lineage" means!

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

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


More information about the javadoc-dev mailing list