8314448: Coordinate DocLint and JavaDoc to report on unknown tags

Reviewed-by: jjg

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/TagletManager.java

@@ -50,8 +50,10 @@ import javax.lang.model.util.SimpleElementVisitor14;
 import javax.tools.JavaFileManager;
 import javax.tools.StandardJavaFileManager;
 
+import com.sun.source.doctree.BlockTagTree;
 import com.sun.source.doctree.DocTree;
 
+import com.sun.source.doctree.InlineTagTree;
 import jdk.javadoc.doclet.Doclet;
 import jdk.javadoc.doclet.DocletEnvironment;
 import jdk.javadoc.doclet.Taglet.Location;
@@ -349,89 +351,88 @@ public class TagletManager {
      * @param trees the trees containing the comments
      */
     public void checkTags(Element element, Iterable<? extends DocTree> trees) {
-        CommentHelper ch = utils.getCommentHelper(element);
         for (DocTree tag : trees) {
-            String name = tag.getKind().tagName;
+            String name = switch (tag.getKind()) {
+                case UNKNOWN_INLINE_TAG -> ((InlineTagTree) tag).getTagName();
+                case UNKNOWN_BLOCK_TAG -> ((BlockTagTree) tag).getTagName();
+                default -> tag.getKind().tagName;
+            };
             if (name == null) {
-                continue;
+                continue; // not a tag
             }
-            if (!name.isEmpty() && name.charAt(0) == '@') {
+            if (!allTaglets.containsKey(name)) {
-                name = name.substring(1);
+                if (!config.isDocLintSyntaxGroupEnabled()) {
-            }
+                    var ch = utils.getCommentHelper(element);
-            if (! (standardTags.contains(name) || allTaglets.containsKey(name))) { // defunct, see 8314213
+                    if (standardTagsLowercase.contains(Utils.toLowerCase(name))) {
-                if (standardTagsLowercase.contains(Utils.toLowerCase(name))) {
+                        messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTagLowercase", ch.getTagName(tag));
-                    messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTagLowercase", ch.getTagName(tag));
+                    } else {
-                    continue;
+                        messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTag", ch.getTagName(tag));
-                } else {
+                    }
-                    messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTag", ch.getTagName(tag));
-                    continue;
                 }
+                continue; // unknown tag
             }
             final Taglet taglet = allTaglets.get(name);
+            if (taglet instanceof SimpleTaglet st && !st.isEnabled()) {
+                continue; // taglet has been disabled
+            }
+
             // Check and verify tag usage
-            if (taglet != null) {
+            new SimpleElementVisitor14<Void, Void>() {
-                if (taglet instanceof SimpleTaglet st && !st.isEnabled()) {
+                @Override
-                    // taglet has been disabled
+                public Void visitModule(ModuleElement e, Void p) {
-                    return;
+                    if (!taglet.inModule()) {
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "module");
+                    }
+                    return null;
                 }
 
-                new SimpleElementVisitor14<Void, Void>() {
+                @Override
-                    @Override
+                public Void visitPackage(PackageElement e, Void p) {
-                    public Void visitModule(ModuleElement e, Void p) {
+                    if (!taglet.inPackage()) {
-                        if (!taglet.inModule()) {
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "package");
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "module");
-                        }
-                        return null;
                     }
+                    return null;
+                }
 
-                    @Override
+                @Override
-                    public Void visitPackage(PackageElement e, Void p) {
+                public Void visitType(TypeElement e, Void p) {
-                        if (!taglet.inPackage()) {
+                    if (!taglet.inType()) {
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "package");
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "class");
-                        }
-                        return null;
                     }
+                    return null;
+                }
 
-                    @Override
+                @Override
-                    public Void visitType(TypeElement e, Void p) {
+                public Void visitExecutable(ExecutableElement e, Void p) {
-                        if (!taglet.inType()) {
+                    if (utils.isConstructor(e) && !taglet.inConstructor()) {
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "class");
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "constructor");
-                        }
+                    } else if (!taglet.inMethod()) {
-                        return null;
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "method");
                     }
+                    return null;
+                }
 
-                    @Override
+                @Override
-                    public Void visitExecutable(ExecutableElement e, Void p) {
+                public Void visitVariable(VariableElement e, Void p) {
-                        if (utils.isConstructor(e) && !taglet.inConstructor()) {
+                    if (utils.isField(e) && !taglet.inField()) {
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "constructor");
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "field");
-                        } else if (!taglet.inMethod()) {
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "method");
-                        }
-                        return null;
                     }
+                    return null;
+                }
 
-                    @Override
+                @Override
-                    public Void visitVariable(VariableElement e, Void p) {
+                public Void visitUnknown(Element e, Void p) {
-                        if (utils.isField(e) && !taglet.inField()) {
+                    if (utils.isOverviewElement(e) && !taglet.inOverview()) {
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "field");
+                        printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "overview");
-                        }
-                        return null;
                     }
+                    return null;
+                }
 
-                    @Override
+                @Override
-                    public Void visitUnknown(Element e, Void p) {
+                protected Void defaultAction(Element e, Void p) {
-                        if (utils.isOverviewElement(e) && !taglet.inOverview()) {
+                    return null;
-                            printTagMisuseWarn(utils.getCommentHelper(e), taglet, tag, "overview");
+                }
-                        }
+            }.visit(element);
-                        return null;
-                    }
-
-                    @Override
-                    protected Void defaultAction(Element e, Void p) {
-                        return null;
-                    }
-                }.visit(element);
-            }
         }
     }
 

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java

@@ -1142,8 +1142,9 @@ public class Checker extends DocTreePathScanner<Void, Void> {
     private void checkUnknownTag(DocTree tree, String tagName) {
         // if it were a standard tag, this method wouldn't be called:
         // a standard tag is never represented by Unknown{Block,Inline}TagTree
-        assert tree instanceof UnknownBlockTagTree
+        var k = tree.getKind();
-                || tree instanceof UnknownInlineTagTree;
+        assert k == DocTree.Kind.UNKNOWN_BLOCK_TAG
+                || k == DocTree.Kind.UNKNOWN_INLINE_TAG;
         assert !getStandardTags().contains(tagName);
         // report an unknown tag only if custom tags are set, see 8314213
         if (env.customTags != null && !env.customTags.contains(tagName))

test/langtools/jdk/javadoc/doclet/testAutoLoadTaglets/TestAutoLoadTaglets.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2023, 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
@@ -110,7 +110,7 @@ public class TestAutoLoadTaglets extends JavadocTester {
                         \simplements Taglet {
                             @Override
                             public Set<Location> getAllowedLocations() {
-                                return null;
+                                return Set.of();
                             }
                             @Override
                             public boolean isInlineTag() {

test/langtools/jdk/javadoc/doclet/testUknownTags/TestUnknownTags.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2023, 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 8314448
+ * @library /tools/lib ../../lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ *          jdk.javadoc/jdk.javadoc.internal.tool
+ * @build toolbox.ToolBox javadoc.tester.*
+ * @run main TestUnknownTags
+ */
+
+import java.nio.file.Path;
+
+import javadoc.tester.JavadocTester;
+import toolbox.ToolBox;
+
+public class TestUnknownTags extends JavadocTester {
+
+    public static void main(String... args) throws Exception {
+        new TestUnknownTags().runTests();
+    }
+
+    private final ToolBox tb = new ToolBox();
+
+    // DocLint or not, there should be exactly one diagnostic message about
+    // an unknown tag. No doubled, no "swallowed" messages, just one.
+    @Test
+    public void testExactlyOneMessage(Path base) throws Exception {
+        var src = base.resolve("src");
+        tb.writeJavaFiles(src, """
+                package x;
+
+                /** @mytag */
+                public class MyClass { }
+                """);
+        // don't check exit status: we don't care if it's an error or warning
+
+        // DocLint is explicit
+        int i = 0;
+        for (var check : new String[]{":all", ":none", ""}) {
+            var outputDir = "out-DocLint-" + i++; // use separate output directories
+            javadoc("-Xdoclint" + check,
+                    "-d", base.resolve(outputDir).toString(),
+                    "--source-path", src.toString(),
+                    "x");
+            new OutputChecker(Output.OUT)
+                    .setExpectFound(true)
+                    .checkUnique("unknown tag");
+        }
+        // DocLint is default
+        javadoc("-d", base.resolve("out").toString(),
+                "--source-path", src.toString(),
+                "x");
+        new OutputChecker(Output.OUT)
+                .setExpectFound(true)
+                .checkUnique("unknown tag");
+    }
+
+    // Disabled simple tags are treated as known tags, but aren't checked
+    // for misuse. Additionally, they don't prevent other tags from being
+    // checked.
+    @Test
+    public void testDisabledSimpleTags(Path base) throws Exception {
+        var src = base.resolve("src");
+        tb.writeJavaFiles(src, """
+                package x;
+
+                /**
+                 * @myDisabledTag foo
+                 * @myEnabledTag bar
+                 */
+                public class MyClass extends RuntimeException { }
+                """);
+        javadoc("-d", base.resolve("out").toString(),
+                "-sourcepath", src.toString(),
+                "-tag", "myDisabledTag:mX:Disabled Tag", // may appear in methods; disabled
+                "-tag", "myEnabledTag:mf:Enabled Tag", // may appear in method and fields; enabled
+                "x");
+        checkOutput(Output.OUT, false, "unknown tag");
+        checkOutput(Output.OUT, false, "Tag @myDisabledTag cannot be used in class documentation");
+        checkOutput(Output.OUT, true, "Tag @myEnabledTag cannot be used in class documentation");
+    }
+}