RFR: 8325088: Overloads that differ in type parameters may be lost [v4]
Pavel Rappo
prappo at openjdk.org
Thu Apr 4 21:25:10 UTC 2024
On Thu, 4 Apr 2024 18:52:42 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:
>> Pavel Rappo has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Respond to feedback
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java line 200:
>
>> 198: content.add(heading);
>> 199: return HtmlTree.SECTION(HtmlStyle.detail, content)
>> 200: .setId(memberAnchor);
>
> It's a bit disappointing that more of this isn't in `HtmlIds`.
> It feels like it perpetuates the original ugly code.
>
> Would it make sense for `htmlIds` to return a record/pair containing both the `memberId` and `erasureId` for `ExecutableElement` ?
I'll see how it pans out.
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlIds.java line 567:
>
>> 565: var methods = vmt.getVisibleMembers(VisibleMemberTable.Kind.METHODS);
>> 566: // for whatever reason annotation methods are not of Kind.METHODS
>> 567: var otherMethods = vmt.getVisibleMembers(VisibleMemberTable.Kind.ANNOTATION_TYPE_MEMBER);
>
> I'm surprised you need to worry about annotation type members here -- annotation types cannot have type arguments, and so the "simple" id should always be sufficient.
True, type parameters are not an issue for annotation interface methods, which [are not allowed to have any parameters][], type or otherwise. However, the code that prints annotations for method signatures does not know that and uses `forMember`, which is applicable to any executable member, of annotation or otherwise.
<img width="356" alt="a screenshot from the generated API Documentation for testNewAndDeprecated" src="https://github.com/openjdk/jdk/assets/32523691/fc808159-7f3f-4a2a-bb25-41474c3b5833">
In principle, I could remove that `vmt.getVisibleMembers(VisibleMemberTable.Kind.ANNOTATION_TYPE_MEMBER)` and the annotation member will be caught by the ["safety net"](https://github.com/openjdk/jdk/pull/18519/files#diff-22d9182196ae739a6de9c29801bb3ca788992b0cbf44564b2aeda2018a7b78e1R611-R621).
Since we are here, there's a `forMember` overload used by `AnnotationTypeMemberWriter`, I probably should remove it for consistency. Thoughts?
[are not allowed to have any parameters]: https://docs.oracle.com/javase/specs/jls/se22/html/jls-9.html#jls-9.6.1
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1552464493
PR Review Comment: https://git.openjdk.org/jdk/pull/18519#discussion_r1552462722
More information about the javadoc-dev
mailing list