From 59e9700c4e0ae892f15607bcaa267e5868eb0512 Mon Sep 17 00:00:00 2001 From: Jonathan Gibbons Date: Fri, 3 Jun 2022 16:17:23 +0000 Subject: [PATCH] 8252717: Integrate/merge legacy standard doclet diagnostics and doclint Reviewed-by: erikj, prappo --- make/Docs.gmk | 4 +- .../formats/html/HtmlConfiguration.java | 10 - .../formats/html/HtmlDocletWriter.java | 22 +- .../doclets/toolkit/BaseConfiguration.java | 24 ++- .../internal/doclets/toolkit/Messages.java | 8 +- .../doclets/toolkit/taglets/ParamTaglet.java | 8 +- .../doclets/toolkit/taglets/ReturnTaglet.java | 2 +- .../jdk/javadoc/internal/doclint/DocLint.java | 4 + .../javadoc/internal/doclint/Messages.java | 4 + .../TestDocLintDocletMessages.java | 197 ++++++++++++++++++ .../jdk/javadoc/tool/doclint/DocLintTest.java | 2 +- 11 files changed, 246 insertions(+), 39 deletions(-) create mode 100644 test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java diff --git a/make/Docs.gmk b/make/Docs.gmk index 6ea5c7fb638..1e99f840c28 100644 --- a/make/Docs.gmk +++ b/make/Docs.gmk @@ -104,14 +104,14 @@ JAVADOC_DISABLED_DOCLINT_PACKAGES := org.w3c.* javax.smartcardio # The initial set of options for javadoc JAVADOC_OPTIONS := -use -keywords -notimestamp \ - -serialwarn -encoding ISO-8859-1 -docencoding UTF-8 -breakiterator \ + -encoding ISO-8859-1 -docencoding UTF-8 -breakiterator \ -splitIndex --system none -javafx --expand-requires transitive \ --override-methods=summary # The reference options must stay stable to allow for comparisons across the # development cycle. REFERENCE_OPTIONS := -XDignore.symbol.file=true -use -keywords -notimestamp \ - -serialwarn -encoding ISO-8859-1 -breakiterator -splitIndex --system none \ + -encoding ISO-8859-1 -breakiterator -splitIndex --system none \ -html5 -javafx --expand-requires transitive # Should we add DRAFT stamps to the generated javadoc? diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java index d09f6c7d5a6..3705b2b1eb7 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java @@ -403,16 +403,6 @@ public class HtmlConfiguration extends BaseConfiguration { return docEnv.getJavaFileManager(); } - @Override - public boolean showMessage(DocTreePath path, String key) { - return (path == null || !haveDocLint()); - } - - @Override - public boolean showMessage(Element e, String key) { - return (e == null || !haveDocLint()); - } - @Override protected boolean finishOptionSettings0() throws DocletException { if (options.docEncoding() == null) { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java index ac1ddbfbc22..7ca8824a818 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java @@ -1054,10 +1054,12 @@ public class HtmlDocletWriter { (labelContent.isEmpty() ? text : labelContent)); } else { // No cross link found so print warning - messages.warning(ch.getDocTreePath(see), - "doclet.see.class_or_package_not_found", - "@" + tagName, - seeText); + if (!configuration.isDocLintReferenceGroupEnabled()) { + messages.warning(ch.getDocTreePath(see), + "doclet.see.class_or_package_not_found", + "@" + tagName, + seeText); + } return invalidTagOutput(resources.getText("doclet.tag.invalid", tagName), Optional.of(labelContent.isEmpty() ? text: labelContent)); } @@ -1107,9 +1109,11 @@ public class HtmlDocletWriter { ch.getDocTreePath(see), "doclet.see.class_or_package_not_accessible", tagName, utils.getFullyQualifiedName(containing)); } else { - messages.warning( - ch.getDocTreePath(see), "doclet.see.class_or_package_not_found", - tagName, seeText); + if (!configuration.isDocLintReferenceGroupEnabled()) { + messages.warning( + ch.getDocTreePath(see), "doclet.see.class_or_package_not_found", + tagName, seeText); + } } } if (configuration.currentTypeElement != containing) { @@ -1599,7 +1603,9 @@ public class HtmlDocletWriter { Matcher m = Pattern.compile("(?i)\\{@([a-z]+).*").matcher(body); String tagName = m.matches() ? m.group(1) : null; if (tagName == null) { - messages.warning(dtp, "doclet.tag.invalid_input", body); + if (!configuration.isDocLintSyntaxGroupEnabled()) { + messages.warning(dtp, "doclet.tag.invalid_input", body); + } result.add(invalidTagOutput(resources.getText("doclet.tag.invalid_input", body), Optional.empty())); } else { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseConfiguration.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseConfiguration.java index b0b99481426..a3115dd8dfd 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseConfiguration.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/BaseConfiguration.java @@ -28,7 +28,6 @@ package jdk.javadoc.internal.doclets.toolkit; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.util.ArrayList; @@ -60,7 +59,6 @@ import javax.tools.JavaFileObject; import javax.tools.StandardJavaFileManager; import com.sun.source.tree.CompilationUnitTree; -import com.sun.source.util.DocTreePath; import com.sun.source.util.TreePath; import com.sun.tools.javac.util.DefinedBy; import com.sun.tools.javac.util.DefinedBy.Api; @@ -74,7 +72,6 @@ import jdk.javadoc.internal.doclets.toolkit.taglets.TagletManager; import jdk.javadoc.internal.doclets.toolkit.util.Comparators; import jdk.javadoc.internal.doclets.toolkit.util.DocFile; import jdk.javadoc.internal.doclets.toolkit.util.DocFileFactory; -import jdk.javadoc.internal.doclets.toolkit.util.DocFileIOException; import jdk.javadoc.internal.doclets.toolkit.util.Extern; import jdk.javadoc.internal.doclets.toolkit.util.Group; import jdk.javadoc.internal.doclets.toolkit.util.MetaKeywords; @@ -85,6 +82,7 @@ import jdk.javadoc.internal.doclets.toolkit.util.Utils.Pair; import jdk.javadoc.internal.doclets.toolkit.util.VisibleMemberCache; import jdk.javadoc.internal.doclets.toolkit.util.VisibleMemberTable; import jdk.javadoc.internal.doclint.DocLint; +import jdk.javadoc.internal.doclint.Env; /** * Configure the output based on the options. Doclets should subclass @@ -640,10 +638,6 @@ public abstract class BaseConfiguration { */ public abstract JavaFileManager getFileManager(); - public abstract boolean showMessage(DocTreePath path, String key); - - public abstract boolean showMessage(Element e, String key); - /* * Splits the elements in a collection to its individual * collection. @@ -803,8 +797,20 @@ public abstract class BaseConfiguration { doclintOpts.toArray(new String[0])); } - public boolean haveDocLint() { - return (doclint != null); + public boolean isDocLintReferenceGroupEnabled() { + return isDocLintGroupEnabled(jdk.javadoc.internal.doclint.Messages.Group.REFERENCE); + } + + public boolean isDocLintSyntaxGroupEnabled() { + return isDocLintGroupEnabled(jdk.javadoc.internal.doclint.Messages.Group.SYNTAX); + } + + private boolean isDocLintGroupEnabled(jdk.javadoc.internal.doclint.Messages.Group group) { + // Use AccessKind.PUBLIC as a stand-in, since it is not common to + // set DocLint options per access kind (as is common with javac.) + // A more sophisticated solution might be to derive the access kind from the + // element owning the comment, and its enclosing elements. + return doclint != null && doclint.isGroupEnabled(group, Env.AccessKind.PUBLIC); } // } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java index 9dcd8626ba3..1a1654cd2e2 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/Messages.java @@ -143,9 +143,7 @@ public class Messages { * @param args optional arguments to be replaced in the message */ public void warning(DocTreePath path, String key, Object... args) { - if (configuration.showMessage(path, key)) { - report(WARNING, path, resources.getText(key, args)); - } + report(WARNING, path, resources.getText(key, args)); } /** @@ -170,9 +168,7 @@ public class Messages { * @param args optional arguments to be replaced in the message */ public void warning(Element e, String key, Object... args) { - if (configuration.showMessage(e, key)) { - report(WARNING, e, resources.getText(key, args)); - } + report(WARNING, e, resources.getText(key, args)); } /** diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java index f59461a67e0..62fa7b4544b 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ParamTaglet.java @@ -222,7 +222,9 @@ public class ParamTaglet extends BaseTaglet implements InheritableTaglet { case TYPE_PARAMETER -> "doclet.TypeParameters_warn"; case RECORD_COMPONENT -> "doclet.RecordComponents_warn"; }; - messages.warning(ch.getDocTreePath(dt), key, paramName); + if (!writer.configuration().isDocLintReferenceGroupEnabled()) { + messages.warning(ch.getDocTreePath(dt), key, paramName); + } } String rank = rankMap.get(name); if (rank != null) { @@ -232,7 +234,9 @@ public class ParamTaglet extends BaseTaglet implements InheritableTaglet { case TYPE_PARAMETER -> "doclet.TypeParameters_dup_warn"; case RECORD_COMPONENT -> "doclet.RecordComponents_dup_warn"; }; - messages.warning(ch.getDocTreePath(dt), key, paramName); + if (!writer.configuration().isDocLintReferenceGroupEnabled()) { + messages.warning(ch.getDocTreePath(dt), key, paramName); + } } else { documented.put(rank, dt); } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ReturnTaglet.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ReturnTaglet.java index 350b247a8bd..1a7e0028a06 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ReturnTaglet.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/ReturnTaglet.java @@ -96,7 +96,7 @@ public class ReturnTaglet extends BaseTaglet implements InheritableTaglet { // Make sure we are not using @return tag on method with void return type. TypeMirror returnType = utils.getReturnType(writer.getCurrentPageElement(), (ExecutableElement) holder); if (returnType != null && utils.isVoid(returnType)) { - if (!tags.isEmpty()) { + if (!tags.isEmpty() && !writer.configuration().isDocLintReferenceGroupEnabled()) { messages.warning(holder, "doclet.Return_tag_on_void_method"); } return null; diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/DocLint.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/DocLint.java index 929535bb450..f6c6df22c63 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/DocLint.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/DocLint.java @@ -384,6 +384,10 @@ public class DocLint extends com.sun.tools.doclint.DocLint { return false; } + public boolean isGroupEnabled(Messages.Group group, Env.AccessKind accessKind) { + return env.messages.isEnabled(group, accessKind); + } + private String localize(String code, Object... args) { Messages m = (env != null) ? env.messages : new Messages(null); return m.localize(code, args); diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Messages.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Messages.java index cfd7b8869cc..2d3be96d7f2 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Messages.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Messages.java @@ -104,6 +104,10 @@ public class Messages { stats.setEnabled(b); } + boolean isEnabled(Group group, Env.AccessKind ak) { + return options.isEnabled(group, ak); + } + void reportStats(PrintWriter out) { stats.report(out); } diff --git a/test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java b/test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java new file mode 100644 index 00000000000..049688e7fe8 --- /dev/null +++ b/test/langtools/jdk/javadoc/doclet/testDoclintDocletMessages/TestDocLintDocletMessages.java @@ -0,0 +1,197 @@ +/* + * Copyright (c) 2022, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8252717 + * @summary Integrate/merge legacy standard doclet diagnostics and doclint + * @library ../../lib /tools/lib + * @modules jdk.javadoc/jdk.javadoc.internal.tool + * @build toolbox.ToolBox javadoc.tester.* + * @run main TestDocLintDocletMessages + */ + + +import javadoc.tester.JavadocTester; +import toolbox.ToolBox; + +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +/** + * Some conditions may be detected by both DocLint and the Standard Doclet. + * This test is to verify that in such cases, only one of those generates + * a message. + */ +public class TestDocLintDocletMessages extends JavadocTester { + + public static void main(String... args) throws Exception { + TestDocLintDocletMessages tester = new TestDocLintDocletMessages(); + tester.runTests(); + } + + final ToolBox tb = new ToolBox(); + + @Test + public void testSyntaxError(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + /** + * Comment. + * Bad < HTML. + * End of comment. + */ + public class C { + private C() { } + } + """); + + var doclintResult = new Result(Exit.ERROR,"C.java:3: error: malformed HTML"); + var docletResult = new Result(Exit.OK, "C.java:3: warning: invalid input: '<'"); + + testSingle(base, "syntax", doclintResult, docletResult); + } + + @Test + public void testReferenceNotFoundError(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + /** + * Comment. + * @see DoesNotExist + */ + public class C { + private C() { } + } + """); + + var doclintResult = new Result(Exit.ERROR, "C.java:3: error: reference not found"); + var docletResult = new Result(Exit.OK, "C.java:3: warning: Tag @see: reference not found: DoesNotExist"); + + testSingle(base, "reference", doclintResult, docletResult); + } + + @Test + public void testParamNotFoundError(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + /** + * Comment. + */ + public class C { + /** + * Comment. + * @param y wrong name + */ + public C(int x) { } + } + """); + + var doclintResult = new Result(Exit.ERROR, "C.java:7: error: @param name not found"); + var docletResult = new Result(Exit.OK, "C.java:7: warning: @param argument \"y\" is not a parameter name."); + + testSingle(base, "reference", doclintResult, docletResult); + } + + @Test + public void testParamDuplicateError(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + /** + * Comment. + */ + public class C { + /** + * Comment. + * @param x first + * @param x second + */ + public C(int x) { } + } + """); + + var doclintResult = new Result(Exit.OK, "C.java:8: warning: @param \"x\" has already been specified"); + var docletResult = new Result(Exit.OK, "C.java:8: warning: Parameter \"x\" is documented more than once."); + + testSingle(base, "reference", doclintResult, docletResult); + } + + @Test + public void testReturnOnVoid(Path base) throws Exception { + Path src = base.resolve("src"); + tb.writeJavaFiles(src, """ + /** + * Comment. + */ + public class C { + private C() { } + /** + * Comment. + * @return nothing + */ + public void m() { } + } + """); + + var doclintResult = new Result(Exit.ERROR, "C.java:8: error: invalid use of @return"); + var docletResult = new Result(Exit.OK, "C.java:10: warning: @return tag cannot be used in method with void return type."); + + testSingle(base, "reference", doclintResult, docletResult); + } + + /** Captures an expected exit code and diagnostic message. */ + record Result(Exit exit, String message) { } + + void testSingle(Path base, String group, Result doclintResult, Result docletResult) { + int index = 1; + + // test options that should trigger the doclint message + for (String o : List.of("", "-Xdoclint", "-Xdoclint:" + group)) { + testSingle(base, index++, o.isEmpty() ? List.of() : List.of(o), doclintResult, docletResult); + } + + // test options that should trigger the doclet message + for (String o : List.of("-Xdoclint:none", "-Xdoclint:all,-" + group)) { + testSingle(base, index++, List.of(o), docletResult, doclintResult); + } + } + + void testSingle(Path base, int index, List options, Result expect, Result doNotExpect) { + var allOptions = new ArrayList(); + allOptions.addAll(List.of("-d", base.resolve("out-" + index).toString())); + allOptions.addAll(options); + allOptions.addAll(List.of("-noindex", "-nohelp")); // omit unnecessary files + allOptions.add(base.resolve("src").resolve("C.java").toString()); + + javadoc(allOptions.toArray(String[]::new)); + checkExit(expect.exit); + + checkOutput(Output.OUT, true, expect.message); + + // allow that the "other" result might be the same as the main result + if (!doNotExpect.message.equals(expect.message)) { + checkOutput(Output.OUT, false, doNotExpect.message); + } + } +} \ No newline at end of file diff --git a/test/langtools/jdk/javadoc/tool/doclint/DocLintTest.java b/test/langtools/jdk/javadoc/tool/doclint/DocLintTest.java index 0d46d92228b..4af51070468 100644 --- a/test/langtools/jdk/javadoc/tool/doclint/DocLintTest.java +++ b/test/langtools/jdk/javadoc/tool/doclint/DocLintTest.java @@ -256,7 +256,7 @@ public class DocLintTest { Set found = EnumSet.noneOf(Message.class); int e = 0, w = 0; for (String line: out.split("[\r\n]+")) { - if (ignore.matcher(line).matches()) + if (ignore.matcher(line).matches() || line.contains("javadoc.warn.message")) continue; Matcher s = stats.matcher(line);