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
