RFR: 8222091: Javadoc does not handle package annotations correctly on package-info.java

[Jonathan Gibbons]

OK, but I suggest adding additional test cases for the legacy behavior 
of a package with a package.html file, and a package with both a doc 
comment and package.html file.

-- Jon

On 5/13/19 7:30 AM, Hannes Wallnöfer wrote:
> Please review:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222091
> Webrev: http://cr.openjdk.java.net/~hannesw/8222091/webrev.00/jdk.patch
>
> The DocCommentDuo class is a simple pair container for a TreePath and a DocCommentTree. Utils#isValidDuo(DocCommentDuo) checks whether the DocCommentDuo reference is not null and has a non-null DocCommentTree reference. This method is used when retrieving the DocCommentTree/TreePath in getDocCommentTree0(Element) and getTreePath(Element) to check whether a pair could be retrieved  using a given method (synthetic entries map, javadoc source comments, HTML files).
>
> My patch changes all these checks to a simple null check on the DocCommentDuo itself. The rationale is that a comment/path duo may be valid even if the comment is null, for example if there is no javadoc comment associated with a Java element.
>
> This is the case in the current bug when a package-info.java file does not contain a javadoc comment (usually it just contains a java annotation for the package instead). In this case, Utils#isValidDuo returns false because the DocCommentTree is null, and the getDocCommentTree0 method goes on to parse the package-info.java file as if it were a package.html file, thereby interpreting the annotations as javadoc tags.
>
> Although using isValidDuo does not cause bugs in other places of the retrieval code, it can cause cached elements to be refetched unnecessarily. I therefore replaced all instances of isValidDuo with simple null checks in the retrieval code. The isValidDuo method is still used in the code processing the retrieved comment/path.
>
> I have looked for a better way to determine the way to retrieve the comment/path pair for an element, but that would require additional / duplicate work as well as casting the Element to some specific subtype such as PackageSymbol. The simple null check seems like the simplest thing to do.
>
> I tested the patch with Mach5 tier 1.
>
> Hannes