[jdk17] RFR: JDK-8270866: NPE in DocTreePath.getTreePath() [v2]

[Hannes Wallnöfer]

On Wed, 21 Jul 2021 19:59:37 GMT, Hannes Wallnöfer <hannesw at openjdk.org> 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