From a01b3fb8e912eadd309e7036995656dd609629b2 Mon Sep 17 00:00:00 2001 From: Pavel Rappo Date: Wed, 6 Sep 2023 07:51:14 +0000 Subject: [PATCH] 8288660: JavaDoc should be more helpful if it doesn't recognize a tag Reviewed-by: jjg --- .../com/sun/tools/javac/api/JavacTrees.java | 8 +- .../formats/html/taglets/TagletManager.java | 19 +-- .../toolkit/resources/doclets.properties | 4 +- .../jdk/javadoc/internal/doclint/Checker.java | 11 +- .../jdk/javadoc/internal/doclint/DocLint.java | 28 +++- .../javadoc/internal/doclint/Messages.java | 19 ++- .../doclint/resources/doclint.properties | 4 +- .../doclet/testSnippetTag/TestSnippetTag.java | 4 +- .../testUknownTags/TestUnknownTags.java | 138 +++++++++++++++++- .../tools/doclint/CustomTagTest.java | 2 +- .../langtools/tools/doclint/CustomTagTest.out | 6 +- .../tools/doclint/CustomTagTestWithOption.out | 2 +- 12 files changed, 213 insertions(+), 32 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java index f93119d16b7..c0dce830d9e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/api/JavacTrees.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -1174,13 +1174,17 @@ public class JavacTrees extends DocTrees { printMessage(kind, msg, ((DCTree) t).pos((DCDocComment) c), root); } + public void printMessage(Diagnostic.Kind kind, CharSequence msg) { + printMessage(kind, msg, (JCDiagnostic.DiagnosticPosition) null, null); + } + private void printMessage(Diagnostic.Kind kind, CharSequence msg, JCDiagnostic.DiagnosticPosition pos, com.sun.source.tree.CompilationUnitTree root) { JavaFileObject oldSource = null; JavaFileObject newSource = null; - newSource = root.getSourceFile(); + newSource = root == null ? null : root.getSourceFile(); if (newSource == null) { pos = null; } else { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/TagletManager.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/TagletManager.java index ed3727fd5bb..804704528ce 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/TagletManager.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/TagletManager.java @@ -64,6 +64,7 @@ import jdk.javadoc.internal.doclets.toolkit.Messages; import jdk.javadoc.internal.doclets.toolkit.Resources; import jdk.javadoc.internal.doclets.toolkit.util.CommentHelper; import jdk.javadoc.internal.doclets.toolkit.util.Utils; +import jdk.javadoc.internal.doclint.DocLint; import static com.sun.source.doctree.DocTree.Kind.AUTHOR; import static com.sun.source.doctree.DocTree.Kind.EXCEPTION; @@ -123,13 +124,6 @@ public class TagletManager { */ private final Set standardTags; - /** - * Keep track of standard tags in lowercase to compare for better - * error messages when a tag like {@code @docRoot} is mistakenly spelled - * lowercase {@code @docroot}. - */ - private final Set standardTagsLowercase; - /** * Keep track of overridden standard tags. */ @@ -185,7 +179,6 @@ public class TagletManager { overriddenStandardTags = new HashSet<>(); potentiallyConflictingTags = new HashSet<>(); standardTags = new HashSet<>(); - standardTagsLowercase = new HashSet<>(); unseenCustomTags = new HashSet<>(); allTaglets = new LinkedHashMap<>(); this.config = config; @@ -363,10 +356,12 @@ public class TagletManager { if (!allTaglets.containsKey(name)) { if (!config.isDocLintSyntaxGroupEnabled()) { var ch = utils.getCommentHelper(element); - if (standardTagsLowercase.contains(Utils.toLowerCase(name))) { - messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTagLowercase", ch.getTagName(tag)); + List suggestions = DocLint.suggestSimilar(allTaglets.keySet(), name); + if (!suggestions.isEmpty()) { + messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTagWithHint", + String.join(", ", suggestions)); // TODO: revisit after 8041488 } else { - messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTag", ch.getTagName(tag)); + messages.warning(ch.getDocTreePath(tag), "doclet.UnknownTag"); } } continue; // unknown tag @@ -660,7 +655,6 @@ public class TagletManager { String name = taglet.getName(); allTaglets.put(name, taglet); standardTags.add(name); - standardTagsLowercase.add(Utils.toLowerCase(name)); } private void addStandardTaglet(Taglet taglet, DocTree.Kind alias) { @@ -668,7 +662,6 @@ public class TagletManager { String name = alias.tagName; allTaglets.put(name, taglet); standardTags.add(name); - standardTagsLowercase.add(Utils.toLowerCase(name)); } public boolean isKnownCustomTag(String tagName) { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties index 41a0394579a..ddc639133ff 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties @@ -116,8 +116,8 @@ doclet.Since=Since: doclet.Throws=Throws: doclet.Version=Version: doclet.Factory=Factory: -doclet.UnknownTag={0} is an unknown tag. -doclet.UnknownTagLowercase={0} is an unknown tag -- same as a known tag except for case. +doclet.UnknownTag=unknown tag. Unregistered custom tag? +doclet.UnknownTagWithHint=unknown tag. Mistyped @{0} or an unregistered custom tag? doclet.inheritDocBadSupertype=cannot find the overridden method doclet.inheritDocWithinInappropriateTag=@inheritDoc cannot be used within this tag doclet.inheritDocNoDoc=overridden methods do not document exception type {0} diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java index a0fad5a0e7d..b097866a2d8 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java @@ -1147,8 +1147,15 @@ public class Checker extends DocTreePathScanner { || 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)) - env.messages.error(SYNTAX, tree, "dc.tag.unknown", tagName); + if (env.customTags != null && !env.customTags.contains(tagName)) { + var suggestions = DocLint.suggestSimilar(env.customTags, tagName); + if (suggestions.isEmpty()) { + env.messages.error(SYNTAX, tree, "dc.unknown.javadoc.tag"); + } else { + env.messages.error(SYNTAX, tree, "dc.unknown.javadoc.tag.with.hint", + String.join(", ", suggestions)); // TODO: revisit after 8041488 + } + } } private Set getStandardTags() { 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 f6c6df22c63..0139f95b74b 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -29,6 +29,8 @@ import java.io.File; import java.io.IOException; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.Collection; +import java.util.Comparator; import java.util.LinkedList; import java.util.List; import java.util.Queue; @@ -61,6 +63,7 @@ import com.sun.tools.javac.main.JavaCompiler; import com.sun.tools.javac.util.Context; import com.sun.tools.javac.util.DefinedBy; import com.sun.tools.javac.util.DefinedBy.Api; +import com.sun.tools.javac.util.StringUtils.DamerauLevenshteinDistance; /** * Multi-function entry point for the doc check utility. @@ -373,6 +376,29 @@ public class DocLint extends com.sun.tools.doclint.DocLint { Env env; Checker checker; + public static List suggestSimilar(Collection knownTags, String unknownTag) { + final double MIN_SIMILARITY = 2.0 / 3; + record Pair(String tag, double similarity) { } + return knownTags.stream() + .distinct() // filter duplicates in known, otherwise they will result in duplicates in suggested + .map(t -> new Pair(t, similarity(t, unknownTag))) + .sorted(Comparator.comparingDouble(Pair::similarity).reversed() /* more similar first */) + // .peek(p -> System.out.printf("%.3f, (%s ~ %s)%n", p.similarity, p.tag, unknownTag)) // debug + .takeWhile(p -> Double.compare(p.similarity, MIN_SIMILARITY) >= 0) + .map(Pair::tag) + .toList(); + } + + // a value in [0, 1] range: the closer the value is to 1, the more similar + // the strings are + private static double similarity(String a, String b) { + // Normalize the distance so that similarity between "x" and "y" is + // less than that of "ax" and "ay". Use the greater of two lengths + // as normalizer, as it's an upper bound for the distance. + return 1.0 - ((double) DamerauLevenshteinDistance.of(a, b)) + / Math.max(a.length(), b.length()); + } + public boolean isValidOption(String opt) { if (opt.equals(XMSGS_OPTION)) return true; 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 2d3be96d7f2..7d38e511d03 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 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 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 @@ -41,6 +41,7 @@ import javax.tools.Diagnostic; import com.sun.source.doctree.DocTree; import com.sun.source.tree.Tree; +import com.sun.tools.javac.api.JavacTrees; import com.sun.tools.javac.util.StringUtils; import jdk.javadoc.internal.doclint.Env.AccessKind; @@ -96,6 +97,10 @@ public class Messages { report(group, Diagnostic.Kind.WARNING, tree, code, args); } + void note(Group group, String code, Object... args) { + report(group, Diagnostic.Kind.NOTE, code, args); + } + void setOptions(String opts) { options.setOptions(opts); } @@ -112,6 +117,18 @@ public class Messages { stats.report(out); } + protected void report(Group group, Diagnostic.Kind dkind, String code, Object... args) { + if (options.isEnabled(group, env.currAccess)) { + if (dkind == Diagnostic.Kind.WARNING && env.suppressWarnings(group)) { + return; + } + String msg = (code == null) ? (String) args[0] : localize(code, args); + ((JavacTrees) env.trees).printMessage(dkind, msg); + + stats.record(group, dkind, code); + } + } + protected void report(Group group, Diagnostic.Kind dkind, DocTree tree, String code, Object... args) { if (options.isEnabled(group, env.currAccess)) { if (dkind == Diagnostic.Kind.WARNING && env.suppressWarnings(group)) { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties index f199b181c37..5b70ddb03ae 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/resources/doclint.properties @@ -1,5 +1,5 @@ # -# Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights reserved. +# Copyright (c) 2012, 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 @@ -84,6 +84,8 @@ dc.tag.p.in.pre= unexpected use of

inside

 element
 dc.tag.requires.heading = heading not found for 
 dc.tag.self.closing = self-closing element not allowed
 dc.tag.start.unmatched = end tag missing: 
+dc.unknown.javadoc.tag = unknown tag. Unregistered custom tag?
+dc.unknown.javadoc.tag.with.hint = unknown tag. Mistyped @{0} or an unregistered custom tag?
 dc.tag.unknown = unknown tag: {0}
 dc.tag.not.supported.html5 = tag not supported in HTML5: {0}
 dc.text.not.allowed = text not allowed in <{0}> element
diff --git a/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java b/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java
index 70b0397c121..b7b54dfdb26 100644
--- a/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java
+++ b/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 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
@@ -664,7 +664,7 @@ public class TestSnippetTag extends SnippetTester {
                 "-sourcepath", srcDir.toString(),
                 "pkg");
         checkExit(Exit.ERROR);
-        long actual = Pattern.compile("error: unknown tag: snippet:")
+        long actual = Pattern.compile("error: unknown tag. Mistyped @snippet")
                 .matcher(getOutput(Output.OUT)).results().count();
         checking("Number of errors");
         int expected = unknownTags.size();
diff --git a/test/langtools/jdk/javadoc/doclet/testUknownTags/TestUnknownTags.java b/test/langtools/jdk/javadoc/doclet/testUknownTags/TestUnknownTags.java
index d036e605d56..ea5cb9b74b3 100644
--- a/test/langtools/jdk/javadoc/doclet/testUknownTags/TestUnknownTags.java
+++ b/test/langtools/jdk/javadoc/doclet/testUknownTags/TestUnknownTags.java
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8314448
+ * @bug 8314448 8288660
  * @library /tools/lib ../../lib
  * @modules jdk.compiler/com.sun.tools.javac.api
  *          jdk.compiler/com.sun.tools.javac.main
@@ -33,6 +33,9 @@
  */
 
 import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.regex.Pattern;
 
 import javadoc.tester.JavadocTester;
 import toolbox.ToolBox;
@@ -68,7 +71,7 @@ public class TestUnknownTags extends JavadocTester {
                     "x");
             new OutputChecker(Output.OUT)
                     .setExpectFound(true)
-                    .checkUnique("unknown tag");
+                    .checkUnique(Pattern.compile("unknown tag."));
         }
         // DocLint is default
         javadoc("-d", base.resolve("out").toString(),
@@ -76,7 +79,7 @@ public class TestUnknownTags extends JavadocTester {
                 "x");
         new OutputChecker(Output.OUT)
                 .setExpectFound(true)
-                .checkUnique("unknown tag");
+                .checkUnique(Pattern.compile("unknown tag."));
     }
 
     // Disabled simple tags are treated as known tags, but aren't checked
@@ -103,4 +106,133 @@ public class TestUnknownTags extends JavadocTester {
         checkOutput(Output.OUT, false, "Tag @myDisabledTag cannot be used in class documentation");
         checkOutput(Output.OUT, true, "Tag @myEnabledTag cannot be used in class documentation");
     }
+
+    // This tests two assertions:
+    //
+    //   - the "helpful note" is output exactly once,
+    //   - some typos have fix suggestions, and
+    //   - there's no difference between inline and block tags as
+    //     far as the diagnostic output is concerned
+    @Test
+    public void testSimilarTags(Path base) throws Exception {
+        var src = base.resolve("src");
+        // put some tags as inline in the main description, so that they are
+        // not parsed as contents of the immediately preceding unknown
+        // block tags
+        tb.writeJavaFiles(src, """
+                package x;
+
+                /**
+                 * {@cod}
+                 * {@codejdk.net.hosts.file}
+                 * {@coe}
+                 * {@cpde}
+                 * {@ocde}
+                 * {@ode}
+                 *
+                 * @auther
+                 *
+                 * @Depricated
+                 * @deprecation
+                 *
+                 * @DocRoot
+                 * @dccRoot
+                 * @docroot
+                 *
+                 * @ecception
+                 * @excception
+                 * @exceptbion
+                 * @exceptino
+                 * @exceptions
+                 * @exceptoin
+                 * @execption
+                 *
+                 * @implnote
+                 *
+                 * @inheritdoc
+                 * @inherotDoc
+                 * @inheretdoc
+                 * @inhertitDoc
+                 *
+                 * @jvm
+                 * @jmvs
+                 *
+                 * @Link
+                 * @linK
+                 * @linbk
+                 * @lini
+                 * @linke
+                 * @linked
+                 *
+                 * @linkplan
+                 *
+                 * @params
+                 * @pararm
+                 * @parasm
+                 * @parem
+                 * @parm
+                 * @parma
+                 * @praam
+                 * @prarm
+                 *
+                 * @Return
+                 * @eturn
+                 * @result
+                 * @retrun
+                 * @retuen
+                 * @retun
+                 * @retunr
+                 * @retur
+                 * @returns
+                 * @returnss
+                 * @retursn
+                 * @rturn
+                 *
+                 * @See
+                 * @gsee
+                 *
+                 * @serialdata
+                 *
+                 * @sinc
+                 * @sine
+                 *
+                 * @systemproperty
+                 *
+                 * @thows
+                 * @thrown
+                 * @throwss
+                 */
+                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", "", null}) {
+            var outputDir = "out-DocLint-" + i++; // use separate output directories
+
+            var args = new ArrayList();
+            if (check != null) // check == null means DocLint is default
+                args.add("-Xdoclint" + check);
+            args.addAll(Arrays.asList(
+                    "-d", base.resolve(outputDir).toString(),
+                    "-tag", "apiNote:a:API Note:",
+                    "-tag", "implSpec:a:Implementation Requirements:",
+                    "-tag", "implNote:a:Implementation Note:",
+                    "-tag", "jls:a:JLS", // this tag isn't exactly that of JDK, for simplicity reasons
+                    "-tag", "jvms:a:JVMS", // this tag isn't exactly that of JDK, for simplicity reasons
+                    "--source-path", src.toString(),
+                    "x"));
+
+            javadoc(args.toArray(new String[]{}));
+
+            new OutputChecker(Output.OUT)
+                    .setExpectFound(true)
+                    .setExpectOrdered(false)
+                    .check("author", "code", "deprecated", "docRoot",
+                            "exception", "implNote", "inheritDoc", "jvms",
+                            "link", "linkplain", "param", "return", "see",
+                            "serialData", "since", "systemProperty", "throws");
+        }
+    }
 }
diff --git a/test/langtools/tools/doclint/CustomTagTest.java b/test/langtools/tools/doclint/CustomTagTest.java
index 416839807c6..7ced9d71568 100644
--- a/test/langtools/tools/doclint/CustomTagTest.java
+++ b/test/langtools/tools/doclint/CustomTagTest.java
@@ -1,6 +1,6 @@
 /*
  * @test /nodynamiccopyright/
- * @bug 8006248 8028318
+ * @bug 8006248 8028318 8288660
  * @summary DocLint should report unknown tags
  * @modules jdk.javadoc/jdk.javadoc.internal.doclint
  * @build DocLintTester
diff --git a/test/langtools/tools/doclint/CustomTagTest.out b/test/langtools/tools/doclint/CustomTagTest.out
index 729a550e426..6ce0367cccc 100644
--- a/test/langtools/tools/doclint/CustomTagTest.out
+++ b/test/langtools/tools/doclint/CustomTagTest.out
@@ -1,10 +1,10 @@
-CustomTagTest.java:15: error: unknown tag: customTag
+CustomTagTest.java:15: error: unknown tag. Unregistered custom tag?
  * @customTag Text for a custom tag.
    ^
-CustomTagTest.java:16: error: unknown tag: custom.tag
+CustomTagTest.java:16: error: unknown tag. Unregistered custom tag?
  * @custom.tag Text for another custom tag.
    ^
-CustomTagTest.java:17: error: unknown tag: unknownTag
+CustomTagTest.java:17: error: unknown tag. Unregistered custom tag?
  * @unknownTag Text for an unknown tag.
    ^
 3 errors
diff --git a/test/langtools/tools/doclint/CustomTagTestWithOption.out b/test/langtools/tools/doclint/CustomTagTestWithOption.out
index 340f5a8a440..4b5a1706d31 100644
--- a/test/langtools/tools/doclint/CustomTagTestWithOption.out
+++ b/test/langtools/tools/doclint/CustomTagTestWithOption.out
@@ -1,4 +1,4 @@
-CustomTagTest.java:17: error: unknown tag: unknownTag
+CustomTagTest.java:17: error: unknown tag. Unregistered custom tag?
  * @unknownTag Text for an unknown tag.
    ^
 1 error