Hannes,
The test is weird, and it took me a while to understand what I think is
going on. This predates your changes, but is still very confusing.
If I understand the code correctly, initModulesAndPackages creates both
packages and modules (which contain packages) in the same "src"
directory. The same "src" directory is then used as an argument for both
the module source path and source path, in different runs of javadoc.
This is confusing, at least to me, because you can put a module on the
source path and have javadoc recognize it as a module. But, it seems to
work in the test because the test is relying on the naming conventions
for qualified module names, and so it doesn't get confused when looking
for packages or modules. But, I'm mildly surprised javac/javadoc doesn't
complain at the "packages" found on the module source path.
The test would be clearer if initModulesAndPackages wrote into two
separate directories, one for module source code and the other for
package source code. This would then affect/improve all the existing
@Test methods by making it clear which directory structure they are
reading source code from.
If this simple refactoring works, the review is Approved. Otherwise, I'd
like to understand the issues in more detail.
-- Jon
On 4/3/20 1:38 AM, Hannes Wallnoefer wrote:
I realised there’s a simpler solution to the problem. Instead of using
a Set to track packages with non-modular documentation we can just
return the module name that matches our internal model in
#checkLinkCompatibility.
I also changed the JBS summary to „javadoc fails to link to docs with
non-matching modularity“ so it covers both cases.
New Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.01/
Hannes
Am 02.04.2020 um 16:06 schrieb Hannes Wallnoefer
<hannes.wallnoe...@oracle.com <mailto:hannes.wallnoe...@oracle.com>>:
Please review:
JBS: https://bugs.openjdk.java.net/browse/JDK-8240169
Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.00/
This patch allows using external documentation even if it doesn’t
match the external library in terms of modularity, i.e.
non-modular documentation can be used for an modular library and vice
versa. Instead of showing an error a warning is issued. There is
still a warning if code and documentation do not match, but we use
the check to tweak reference lookup so that we are still able to link
to the appropriate documentation.
I think that all relevant cases for combinations of modular and
non-modular code are covered by existing tests (some of which change
with this patch obviouly).
TestLinkOptionWithAutomaticModule.java covers using a jar file as
automatic or unnamed module, both of which cases were
already supported as of JDK-8212233, so no changes there.
TestLinkOptionWithModule.java covers all combinations of modular and
non-modular code and documentation. For the two tests that cover
non-matching combinations, I changed the expected return code to OK
and added expected output with the link HTML. The message about the
mismatching documentation is still there, but it is a warning instead
of an error.
I also added the JBS id to the @bug tag in
TestLinkOptionWithModule.java as well as in
TestClassCrossReferences.java which is another test that contains
significant changes.
Thanks,
Hannes