Thanks, Jon. I uploaded a new webrev with additional tests: only package.html, and both package-info.java and package.html.
Like in the first test, the package-info.java in the latter test only contains an annotation but no doc comment, since this is the condition that triggered the bug. http://cr.openjdk.java.net/~hannesw/8222091/webrev.01/ Hannes > Am 23.05.2019 um 23:59 schrieb Jonathan Gibbons <[email protected]>: > > 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
