On Wed, 21 Jul 2021 19:59:37 GMT, Hannes Wallnöfer <[email protected]> wrote:
>> src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/JavadocLog.java line
>> 523:
>>
>>> 521: if (path == null || path.getTreePath() == null) {
>>> 522: return null;
>>> 523: }
>>
>> Please justify this change and the other similar changes in this file.
>> It was a deliberate change to drop these checks; I don't see why they're
>> being undone. These checks have the effect of covering up bugs. Just
>> because the old code was sloppy doesn't mean this code should be as well.
>
> I know the removal was a deliberate change. The decision to propose to
> reintroduce those checks was based on the following considerations:
>
> - reporting/formatting code tolerates missing source/position
> - NPE crashes on the other hand are quite serious
> - this code is called from a lot of places
> - we removed these checks quite recently, and we're very close to release
> JDK 17
>
> That said, I'm not fully convinced we need those checks either. If you feel
> we can do without it I'm fine with that.
I had a bit of a surprise when I removed the null checks in `JavadocLog` as I
saw the same NPE when building JDK API docs (the tests were obviously fine).
Turns out the problem was the code I removed from `CommentHelper` that was
labeled as "Case B":
https://github.com/openjdk/jdk17/pull/264/files#diff-8e5643f82a3b1712561fbeb4ab7a60b0a0cc3e88d7abf9b86b4d15a6bb303d87L183-L187
Although the purpose of this code was supposedly to handle the case of a
comment inherited via `@inheritDoc` (which I confirmed, and which is why I
concluded the code was no longer needed because `InheritDocTaglet` already uses
the "correct" member), there is another case where this code was needed, which
is when an overriding member has a doc comment to only add a block tag but
still inherits the main description from the overridden member (without
`{@inheritDoc}` tag).
I added commit
https://github.com/openjdk/jdk17/pull/264/commits/afc0605f54be7247b4361a399cbf12aea89511ee
that adds code to `CommentHelper.getPath` that is equivalent to the old "Case
B" code. It also adds a test for this case and removes the null checks in
`JavadocLog`.
-------------
PR: https://git.openjdk.java.net/jdk17/pull/264
