On Fri, 25 Feb 2022 17:46:05 GMT, Pavel Rappo <pra...@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.
>
> Pavel Rappo has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Fix outdated code
>
> Uses a set instead of list for quick method search. In a future commit
> we'll try to figure out why `found` are not unique.
> - Fix outdated code
>
> Fix outdated inline comments and names. Both the assertion and the
> `contains` method will be removed in a future commit.
Commit: Check type mirrors for equality correctly
Reviewed. Generally OK. Some comments/info inline.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ThrowsTaglet.java
line 239:
> 237: * @return map of declared to instantiated thrown types or an empty
> map.
> 238: */
> 239: private Map<String, TypeMirror> getSubstitutedThrownTypes(Types
> types,
ok, but mild uugh, and I don't see a better way to handle it in the current
architecture.
In general, objects like `utils`, `elements, `types` get cached because they
are unique for life of the doclet. But that brings up an initialization issue,
since they currently only come from the writer, which is passed in as a
parameter. And, we can't make the constructor more general (can we?) because of
the possibility of user taglets. Put this down as a separate cleanup later!
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
line 1183:
> 1181: }
> 1182:
> 1183: private boolean checkType(TypeElement te) {
The name isn't great, and seems to be more general than the implementation
might suggest.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
line 1188:
> 1186: }
> 1187:
> 1188: public TypeElement
> getFirstVisibleSuperClassAsTypeElement(TypeElement te) {
If you were to add a comment here, I would note that the first visible
superclass explicitly does NOT include `java.lang.Object`. There's an
underlying policy in javadoc to ignore some types like that which are "too
prolific". IIRC, for a while, this included CORBA's Object as well.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7233
