Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JDK-8274639: Provide a way to disable warnings for cross-modular links #5900

Closed
wants to merge 5 commits into from
Closed
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -453,6 +453,14 @@ doclet.usage.linkoffline.parameters=\
doclet.usage.linkoffline.description=\
Link to docs at <url1> using package list at <url2>

# L10N: do not localize these words: warn info
doclet.usage.link-modularity-mismatch.parameters=\
(warn|info)
hns marked this conversation as resolved.
Show resolved Hide resolved
doclet.usage.link-modularity-mismatch.description=\
Report external documentation with wrong modularity as either\n\
warning or informational message. The default behaviour is to\n\
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggest:

Report external documentation with wrong modularity with either\n
a warning or informational message.

(change "as" to "with"; add "a" before "warning")

report a warning.

doclet.usage.link-platform-properties.parameters=\
<url>
doclet.usage.link-platform-properties.description=\
@@ -165,6 +165,20 @@ public abstract class BaseOptions {
// A list of pairs containing urls and package list
private final List<Utils.Pair<String, String>> linkOfflineList = new ArrayList<>();

/**
* An enum of policies for handling modularity mismatches in external documentation.
*/
public enum ModularityMismatchPolicy {
info,
warn
}
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum name is good; constants could be upper case?


/**
* Argument for command-line option {@code --link-modularity-mismatch}.
* Describes how to handle external documentation with non-matching modularity.
*/
private ModularityMismatchPolicy linkModularityMismatch = ModularityMismatchPolicy.warn;

/**
* Location of alternative platform link properties file.
*/
@@ -403,6 +417,22 @@ public boolean process(String opt, List<String> args) {
}
},

new Option(resources, "--link-modularity-mismatch", 1) {
@Override
public boolean process(String opt, List<String> args) {
String s = args.get(0);
switch (s) {
case "warn", "info" -> linkModularityMismatch = ModularityMismatchPolicy.valueOf(s);
Copy link
Contributor

@jonathan-gibbons jonathan-gibbons Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the constants were upper case, you could use s.toUpperCase(Locale.ROOT)

default -> {
reporter.print(ERROR, resources.getText(
"doclet.Option_invalid", s, "--link-modularity-mismatch"));
return false;
}
}
return true;
}
},

new Option(resources, "--link-platform-properties", 1) {
@Override
public boolean process(String opt, List<String> args) {
@@ -464,16 +494,13 @@ public boolean process(String opt, List<String> args) {
public boolean process(String opt, List<String> args) {
String o = args.get(0);
switch (o) {
case "summary":
summarizeOverriddenMethods = true;
break;
case "detail":
summarizeOverriddenMethods = false;
break;
default:
case "summary" -> summarizeOverriddenMethods = true;
case "detail" -> summarizeOverriddenMethods = false;
default -> {
reporter.print(ERROR,
resources.getText("doclet.Option_invalid",o, "--override-methods"));
return false;
}
}
return true;
}
@@ -827,6 +854,14 @@ List<Utils.Pair<String, String>> linkOfflineList() {
return linkOfflineList;
}

/**
* Argument for command-line option {@code --link-modularity-mismatch}.
* Describes how to handle external documentation with non-matching modularity.
*/
public ModularityMismatchPolicy linkModularityMismatch() {
return linkModularityMismatch;
}

/**
* Argument for command-line option {@code --link-platform-properties}.
*/
@@ -514,7 +514,7 @@ private void readElementList(InputStream input, String path, boolean relative, i
DocPath elempath;
String moduleName = null;
DocPath basePath = DocPath.create(path);
boolean issueWarning = true;
boolean showDiagnostic = true;
while ((elemname = in.readLine()) != null) {
if (elemname.length() > 0) {
elempath = basePath;
@@ -534,14 +534,14 @@ private void readElementList(InputStream input, String path, boolean relative, i
// For user provided libraries we check whether modularity matches the actual library.
// We trust modularity to be correct for platform library element lists.
if (platformVersion == 0) {
actualModuleName = checkLinkCompatibility(elemname, moduleName, path, issueWarning);
actualModuleName = checkLinkCompatibility(elemname, moduleName, path, showDiagnostic);
} else {
actualModuleName = moduleName == null ? DocletConstants.DEFAULT_ELEMENT_NAME : moduleName;
}
Item item = new Item(elemname, elempath, relative);
packageItems.computeIfAbsent(actualModuleName, k -> new TreeMap<>())
.putIfAbsent(elemname, item); // first-one-wins semantics
issueWarning = false;
showDiagnostic = false;
}
}
}
@@ -556,25 +556,23 @@ private void readElementList(InputStream input, String path, boolean relative, i
* @param packageName the package name
* @param moduleName the module name or null
* @param path the documentation path
* @param issueWarning whether to print a warning in case of modularity mismatch
* @param showDiagnostic whether to print a diagnostic message in case of modularity mismatch
* @return the module name to use according to actual modularity of the package
*/
private String checkLinkCompatibility(String packageName, String moduleName, String path, boolean issueWarning) {
private String checkLinkCompatibility(String packageName, String moduleName, String path, boolean showDiagnostic) {
PackageElement pe = utils.elementUtils.getPackageElement(packageName);
if (pe != null) {
ModuleElement me = (ModuleElement)pe.getEnclosingElement();
if (me == null || me.isUnnamed()) {
if (moduleName != null && issueWarning) {
configuration.getReporter().print(Kind.WARNING,
resources.getText("doclet.linkMismatch_PackagedLinkedtoModule", path));
if (moduleName != null && showDiagnostic) {
printModularityMismatchDiagnostic("doclet.linkMismatch_PackagedLinkedtoModule", path);
}
// library is not modular, ignore module name even if documentation is modular
return DocletConstants.DEFAULT_ELEMENT_NAME;
} else if (moduleName == null) {
// suppress the warning message in the case of automatic modules
if (!utils.elementUtils.isAutomaticModule(me) && issueWarning) {
configuration.getReporter().print(Kind.WARNING,
resources.getText("doclet.linkMismatch_ModuleLinkedtoPackage", path));
// suppress the diagnostic message in the case of automatic modules
if (!utils.elementUtils.isAutomaticModule(me) && showDiagnostic) {
printModularityMismatchDiagnostic("doclet.linkMismatch_ModuleLinkedtoPackage", path);
}
// library is modular, use module name for lookup even though documentation is not
return utils.getModuleName(me);
@@ -659,4 +657,11 @@ private InputStream open(URL url) throws IOException {

return in;
}

private void printModularityMismatchDiagnostic(String key, Object arg) {
switch (configuration.getOptions().linkModularityMismatch()) {
case info -> configuration.getMessages().notice(key, arg);
case warn -> configuration.getMessages().warning(key, arg);
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -23,7 +23,7 @@

/*
* @test
* @bug 8205593 8240169
* @bug 8205593 8240169 8274639
* @summary Javadoc -link makes broken links if module name matches package name
* @library /tools/lib ../../lib
* @modules
@@ -71,6 +71,7 @@ public void testModuleLinkedToModule(Path base) throws Exception {
"--module", "com.ex1");

javadoc("-d", out2.toString(),
"-Werror", "-Xdoclint:-missing",
"--module-source-path", moduleSrc.toString(),
"--module", "com.ex2",
"-link", "../" + out1.getFileName());
@@ -109,13 +110,14 @@ public void testModuleLinkedToPackage(Path base) throws Exception {
"-subpackages", "com.ex1");

javadoc("-d", out2.toString(),
"--link-modularity-mismatch", "warn",
"--module-source-path", moduleSrc.toString(),
"--module", "com.ex2",
"-link", "../" + out1.getFileName());

checkExit(Exit.OK);
checkOutput(Output.OUT, true,
"The code being documented uses modules but the packages defined "
"warning: The code being documented uses modules but the packages defined "
+ "in ../out3a/ are in the unnamed module");
checkOutput("com.ex2/com/ex2/B.html", true,
"""
@@ -137,13 +139,62 @@ public void testPackageLinkedToModule(Path base) throws Exception {

checkExit(Exit.OK);
checkOutput(Output.OUT, true,
"The code being documented uses packages in the unnamed module, but the packages defined "
"warning: The code being documented uses packages in the unnamed module, but the packages defined "
+ "in ../out4a/ are in named modules");
checkOutput("com/ex2/B.html", true,
"""
<a href="../../../out4a/com.ex1/com/ex1/A.html" title="class or interface in com.ex1" class="external-link">A</a>""");
}

@Test
public void testModuleLinkedToPackageNoWarning(Path base) throws Exception {
Path out1 = base.resolve("out5a"), out2 = base.resolve("out5b");

javadoc("-d", out1.toString(),
"-sourcepath", packageSrc.toString(),
"-subpackages", "com.ex1");

javadoc("-d", out2.toString(),
"--link-modularity-mismatch", "info",
"-Werror", "-Xdoclint:-missing",
"--module-source-path", moduleSrc.toString(),
"--module", "com.ex2",
"-link", "../" + out1.getFileName());

checkExit(Exit.OK);
checkOutput(Output.OUT, true,
"The code being documented uses modules but the packages defined "
+ "in ../out5a/ are in the unnamed module");
checkOutput("com.ex2/com/ex2/B.html", true,
"""
<a href="../../../../out5a/com/ex1/A.html" title="class or interface in com.ex1" class="external-link">A</a>""");
}

@Test
public void testPackageLinkedToModuleNoWarning(Path base) throws Exception {
Path out1 = base.resolve("out6a"), out2 = base.resolve("out6b");

javadoc("-d", out1.toString(),
"--module-source-path", moduleSrc.toString(),
"--module", "com.ex1");

javadoc("-d", out2.toString(),
"--link-modularity-mismatch", "info",
"-quiet", // should not print modularity mismatch info
"-Werror", "-Xdoclint:-missing",
"-sourcepath", packageSrc.toString(),
"-subpackages", "com.ex2",
"-link", "../" + out1.getFileName());

checkExit(Exit.OK);
// Modularity mismatch diagnostic should not be printed because we're runnning with -quiet option
checkOutput(Output.OUT, false,
"The code being documented uses packages in the unnamed module, but the packages defined "
+ "in ../out6a/ are in named modules");
checkOutput("com/ex2/B.html", true,
"""
<a href="../../../out6a/com.ex1/com/ex1/A.html" title="class or interface in com.ex1" class="external-link">A</a>""");
}

void initModulesAndPackages() throws Exception{
new ModuleBuilder(tb, "com.ex1")