On Wed, 2 Mar 2022 21:58:10 GMT, Jonathan Gibbons <[email protected]> wrote:
>> Instances of TypeMirror that are equal (TypeMirror::equals), aren't
>> necessarily the same (Types::isSameType). If care is not taken when putting
>> instances of TypeMirror into a set, that set might end up containing the
>> same instances.
>>
>> If Utils.getAllInterfaces is called on a type that extends or implements a
>> particular interface multiple times (on different levels of that type's
>> hierarchy), the returned set might contain multiple representations of that
>> interface. For example, I've seen a case where getAllInterfaces that was
>> passed a TypeElement corresponding to java.util.ArrayList returned a set
>> containing 3 instances of TypeMirror corresponding to
>> `java.util.Collection<E>`.
>>
>> A bit of archaeology. The old standard doclet (removed in JDK-8177511,
>> commit 33ab1995) collected instances of com.sun.javadoc.Type in a TreeMap,
>> keying them by instances of com.sun.tools.javadoc.main.ClassDocImpl (removed
>> in JDK-8215584, commit 151e628) which implemented Comparable.
>> ClassDocImpl.compareTo worked by comparing instances of CollationKey derived
>> from FQNs of types represented by respective instances of ClassDocImpl.
>>
>> I'm not sure why `TreeMap<ClassDocImpl, Type>` was changed to
>> `LinkedHashSet<TypeMirror>`.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
> line 919:
>
>> 917: TypeElement e = asTypeElement(t);
>> 918: if (isInterface(e)) {
>> 919: if ((isPublic(e) || isLinkable(e)) &&
>> visited.add(typeUtils.asElement(t))) {
>
> Don't you always want to record you've visited the item, whether or not it is
> public or linkable? Would it be better to put the new `&& expr` in the
> previous `if` expression?
>
> or even `if (isInterface(e) && visited.add(e)) ...`
It would achieve the goal either way. However, your suggestion might look
cleaner; I'll try it shortly.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7659
