changeset 58840:bb83f1dff441

8236949: javadoc -Xdoclint does not accumulate options correctly Reviewed-by: prappo
author jjg
date Thu, 30 Jan 2020 11:20:09 -0800
parents 84d6423a759a
children 8d8bd676484d
files src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlOptions.java src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java test/langtools/jdk/javadoc/doclet/testDocLintOption/TestDocLintOption.java test/langtools/jdk/javadoc/tool/treeapi/TestDocTrees.java
diffstat 5 files changed, 291 insertions(+), 64 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java	Thu Jan 30 10:27:08 2020 -0800
+++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlConfiguration.java	Thu Jan 30 11:20:09 2020 -0800
@@ -188,7 +188,7 @@
         docPaths = new DocPaths(utils);
         setCreateOverview();
         setTopFile(docEnv);
-        workArounds.initDocLint(options.doclintOpts().values(), tagletManager.getAllTagletNames());
+        workArounds.initDocLint(options.doclintOpts(), tagletManager.getAllTagletNames());
         return true;
     }
 
--- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlOptions.java	Thu Jan 30 10:27:08 2020 -0800
+++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlOptions.java	Thu Jan 30 11:20:09 2020 -0800
@@ -28,14 +28,11 @@
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
 import com.sun.tools.doclint.DocLint;
-import jdk.javadoc.doclet.Doclet;
 import jdk.javadoc.doclet.Reporter;
 import jdk.javadoc.internal.doclets.toolkit.BaseOptions;
 import jdk.javadoc.internal.doclets.toolkit.Resources;
@@ -105,7 +102,7 @@
      * Arguments for command-line option {@code -Xdoclint} and friends.
      * Collected set of doclint options.
      */
-    private Map<Doclet.Option, String> doclintOpts = new LinkedHashMap<>();
+    private List<String> doclintOpts = new ArrayList<>();
 
     /**
      * Argument for command-line option {@code -Xdocrootparent}.
@@ -411,7 +408,37 @@
                 new XOption(resources, "-Xdoclint") {
                     @Override
                     public boolean process(String opt,  List<String> args) {
-                        doclintOpts.put(this, DocLint.XMSGS_OPTION);
+                        doclintOpts.add(DocLint.XMSGS_OPTION);
+                        return true;
+                    }
+                },
+
+                new XOption(resources, "doclet.usage.xdoclint-extended", "-Xdoclint:", 0) {
+                    @Override
+                    public boolean process(String opt,  List<String> args) {
+                        String dopt = opt.replace("-Xdoclint:", DocLint.XMSGS_CUSTOM_PREFIX);
+                        if (dopt.contains("/")) {
+                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_no_qualifiers"));
+                            return false;
+                        }
+                        if (!DocLint.isValidOption(dopt)) {
+                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_invalid_arg"));
+                            return false;
+                        }
+                        doclintOpts.add(dopt);
+                        return true;
+                    }
+                },
+
+                new XOption(resources, "doclet.usage.xdoclint-package", "-Xdoclint/package:", 0) {
+                    @Override
+                    public boolean process(String opt,  List<String> args) {
+                        String dopt = opt.replace("-Xdoclint/package:", DocLint.XCHECK_PACKAGE);
+                        if (!DocLint.isValidOption(dopt)) {
+                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_package_invalid_arg"));
+                            return false;
+                        }
+                        doclintOpts.add(dopt);
                         return true;
                     }
                 },
@@ -430,36 +457,6 @@
                     }
                 },
 
-                new XOption(resources, "doclet.usage.xdoclint-extended", "-Xdoclint:", 0) {
-                    @Override
-                    public boolean process(String opt,  List<String> args) {
-                        String dopt = opt.replace("-Xdoclint:", DocLint.XMSGS_CUSTOM_PREFIX);
-                        doclintOpts.put(this, dopt);
-                        if (dopt.contains("/")) {
-                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_no_qualifiers"));
-                            return false;
-                        }
-                        if (!DocLint.isValidOption(dopt)) {
-                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_invalid_arg"));
-                            return false;
-                        }
-                        return true;
-                    }
-                },
-
-                new XOption(resources, "doclet.usage.xdoclint-package", "-Xdoclint/package:", 0) {
-                    @Override
-                    public boolean process(String opt,  List<String> args) {
-                        String dopt = opt.replace("-Xdoclint/package:", DocLint.XCHECK_PACKAGE);
-                        doclintOpts.put(this, dopt);
-                        if (!DocLint.isValidOption(dopt)) {
-                            reporter.print(ERROR, resources.getText("doclet.Option_doclint_package_invalid_arg"));
-                            return false;
-                        }
-                        return true;
-                    }
-                },
-
                 new XOption(resources, "--no-frames") {
                     @Override
                     public boolean process(String opt, List<String> args) {
@@ -589,7 +586,7 @@
      * Arguments for command-line option {@code -Xdoclint} and friends.
      * Collected set of doclint options.
      */
-    Map<Doclet.Option, String> doclintOpts() {
+    List<String> doclintOpts() {
         return doclintOpts;
     }
 
--- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java	Thu Jan 30 10:27:08 2020 -0800
+++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/WorkArounds.java	Thu Jan 30 11:20:09 2020 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2020, 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
@@ -27,10 +27,11 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
 
@@ -111,37 +112,72 @@
         }
     }
 
-    // TODO: fix this up correctly
-    public void initDocLint(Collection<String> opts, Collection<String> customTagNames) {
-        ArrayList<String> doclintOpts = new ArrayList<>();
-        boolean msgOptionSeen = false;
+    /**
+     * Initializes doclint, if appropriate, depending on options derived
+     * from the doclet command-line options, and the set of custom tags
+     * that should be ignored by doclint.
+     *
+     * DocLint is not enabled if the option {@code -Xmsgs:none} is given,
+     * and it is not followed by any options to enable any groups.
+     * Note that arguments for {@code -Xmsgs:} can be given individually
+     * in separate {@code -Xmsgs:} options, or in a comma-separated list
+     * for a single option. For example, the following are equivalent:
+     * <ul>
+     *     <li>{@code -Xmsgs:all} {@code -Xmsgs:-html}
+     *     <li>{@code -Xmsgs:all,-html}
+     * </ul>
+     *
+     * @param opts  options for doclint, derived from the corresponding doclet
+     *              command-line options
+     * @param customTagNames the names of custom tags, to be ignored by doclint
+     */
+    public void initDocLint(List<String> opts, Set<String> customTagNames) {
+        List<String> doclintOpts = new ArrayList<>();
 
+        // basic analysis of -Xmsgs and -Xmsgs: options to see if doclint is enabled
+        Set<String> groups = new HashSet<>();
+        boolean seenXmsgs = false;
         for (String opt : opts) {
-            if (opt.startsWith(DocLint.XMSGS_OPTION)) {
-                if (opt.equals(DocLint.XMSGS_CUSTOM_PREFIX + "none"))
-                    return;
-                msgOptionSeen = true;
+            if (opt.equals(DocLint.XMSGS_OPTION)) {
+                groups.add("all");
+                seenXmsgs = true;
+            } else if (opt.startsWith(DocLint.XMSGS_CUSTOM_PREFIX)) {
+                String[] args = opt.substring(DocLint.XMSGS_CUSTOM_PREFIX.length())
+                        .split(DocLint.SEPARATOR);
+                for (String a : args) {
+                    if (a.equals("none")) {
+                        groups.clear();
+                    } else if (a.startsWith("-")) {
+                        groups.remove(a.substring(1));
+                    } else {
+                        groups.add(a);
+                    }
+                }
+                seenXmsgs = true;
             }
             doclintOpts.add(opt);
         }
 
-        if (!msgOptionSeen) {
+        if (seenXmsgs) {
+            if (groups.isEmpty()) {
+                // no groups enabled; do not init doclint
+                return;
+            }
+        } else {
+            // no -Xmsgs options of any kind, use default
             doclintOpts.add(DocLint.XMSGS_OPTION);
         }
 
-        String sep = "";
-        StringBuilder customTags = new StringBuilder();
-        for (String customTag : customTagNames) {
-            customTags.append(sep);
-            customTags.append(customTag);
-            sep = DocLint.SEPARATOR;
+        if (!customTagNames.isEmpty()) {
+            String customTags = String.join(DocLint.SEPARATOR, customTagNames);
+            doclintOpts.add(DocLint.XCUSTOM_TAGS_PREFIX + customTags);
         }
-        doclintOpts.add(DocLint.XCUSTOM_TAGS_PREFIX + customTags.toString());
+
         doclintOpts.add(DocLint.XHTML_VERSION_PREFIX + "html5");
 
         JavacTask t = BasicJavacTask.instance(toolEnv.context);
         doclint = new DocLint();
-        doclint.init(t, doclintOpts.toArray(new String[doclintOpts.size()]), false);
+        doclint.init(t, doclintOpts.toArray(new String[0]), false);
     }
 
     // TODO: fix this up correctly
@@ -422,7 +458,7 @@
                 if (md != null) {
                     methods.add(md);
                 }
-                md = findMethod((ClassSymbol) te, "writeExternal", Arrays.asList(writeExternalParamArr));
+                md = findMethod(te, "writeExternal", Arrays.asList(writeExternalParamArr));
                 if (md != null) {
                     methods.add(md);
                 }
@@ -434,7 +470,7 @@
                      * serialField tag.
                      */
                     definesSerializableFields = true;
-                    fields.add((VariableElement) dsf);
+                    fields.add(dsf);
                 } else {
 
                     /* Calculate default Serializable fields as all
@@ -573,9 +609,8 @@
     public PackageElement getAbbreviatedPackageElement(PackageElement pkg) {
         String parsedPackageName = utils.parsePackageName(pkg);
         ModuleElement encl = (ModuleElement) pkg.getEnclosingElement();
-        PackageElement abbrevPkg = encl == null
+        return encl == null
                 ? utils.elementUtils.getPackageElement(parsedPackageName)
                 : ((JavacElements) utils.elementUtils).getPackageElement(encl, parsedPackageName);
-        return abbrevPkg;
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/jdk/javadoc/doclet/testDocLintOption/TestDocLintOption.java	Thu Jan 30 11:20:09 2020 -0800
@@ -0,0 +1,195 @@
+/*
+ * Copyright (c) 2020, 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     8236949
+ * @summary javadoc -Xdoclint does not accumulate options correctly
+ * @library /tools/lib ../../lib
+ * @modules jdk.compiler/com.sun.tools.doclint
+ *          jdk.javadoc/jdk.javadoc.internal.tool
+ * @build   toolbox.ToolBox javadoc.tester.*
+ * @run main TestDocLintOption
+ */
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import com.sun.tools.doclint.Messages.Group;
+import static com.sun.tools.doclint.Messages.Group.*;
+
+import javadoc.tester.JavadocTester;
+import toolbox.ToolBox;
+
+/**
+ * Runs javadoc with different sets of doclint options, and checks that
+ * only the appropriate set of doclint messages are reported.
+ */
+public class TestDocLintOption extends JavadocTester {
+
+    public static void main(String... args) throws Exception {
+        TestDocLintOption tester = new TestDocLintOption();
+        tester.generateSrc();
+        tester.runTests(m -> new Object[] { Paths.get(m.getName()) });
+    }
+
+    private final ToolBox tb = new ToolBox();
+
+    @Test
+    public void testNone(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:none"), Exit.OK, EnumSet.noneOf(Group.class));
+    }
+
+    @Test
+    public void testAll(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:all"), Exit.ERROR, EnumSet.allOf(Group.class));
+    }
+
+    @Test
+    public void testAccessibility(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:accessibility"), Exit.ERROR, EnumSet.of(ACCESSIBILITY));
+    }
+
+    @Test
+    public void testHtml(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:html"), Exit.ERROR, EnumSet.of(HTML));
+    }
+
+    @Test
+    public void testMissing(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:missing"), Exit.OK, EnumSet.of(MISSING));
+    }
+
+    @Test
+    public void testReference(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:reference"), Exit.ERROR, EnumSet.of(REFERENCE));
+    }
+
+    @Test
+    public void testSyntax(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:syntax"), Exit.ERROR, EnumSet.of(SYNTAX));
+    }
+
+    @Test
+    public void testHtmlSyntax_1(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:html,syntax"), Exit.ERROR, EnumSet.of(HTML, SYNTAX));
+    }
+
+    @Test
+    public void testHtmlSyntax_2(Path base) throws Exception {
+        test(base, List.of("-Xdoclint:html", "-Xdoclint:syntax"), Exit.ERROR, EnumSet.of(HTML, SYNTAX));
+    }
+
+    @Test
+    public void testNoSyntax_1(Path base) throws Exception {
+        Set<Group> set = EnumSet.allOf(Group.class);
+        set.remove(SYNTAX);
+        test(base, List.of("-Xdoclint:all,-syntax"), Exit.ERROR, set);
+    }
+
+    @Test
+    public void testNoSyntax_2(Path base) throws Exception {
+        Set<Group> set = EnumSet.allOf(Group.class);
+        set.remove(SYNTAX);
+        test(base, List.of("-Xdoclint:all", "-Xdoclint:-syntax"), Exit.ERROR, set);
+    }
+
+    @Test
+    public void testNoSyntax_3(Path base) throws Exception {
+        // no positive entries; equivalent to "none"
+        test(base, List.of("-Xdoclint:-syntax"), Exit.OK, EnumSet.noneOf(Group.class));
+    }
+
+    /**
+     * Runs javadoc with a given set of doclint options, and checks that
+     * only the appropriate set of doclint messages are reported.
+     * Note: this does not bother to check the "legacy" messages generated
+     * when doclint is disabled (for example, with {@code -Xdoclint:none}).
+     */
+    void test(Path base, List<String> options, Exit expectExit, Set<Group> expectGroups) {
+        List<String> allOpts = new ArrayList<>();
+        allOpts.addAll(List.of(
+                "-d", base.resolve("out").toString(),
+                "-sourcepath", "src"));
+        allOpts.addAll(options);
+        allOpts.add("p");
+
+        javadoc(allOpts.toArray(new String[0]));
+        checkExit(expectExit);
+
+        checkOutput(Output.OUT, expectGroups.contains(ACCESSIBILITY),
+                "src/p/C.java:4: error: no \"alt\" attribute for image");
+
+        checkOutput(Output.OUT, expectGroups.contains(HTML),
+                "src/p/C.java:8: error: text not allowed in <ul> element");
+
+        checkOutput(Output.OUT, expectGroups.contains(MISSING),
+                "src/p/C.java:13: warning: no @return");
+
+        checkOutput(Output.OUT, expectGroups.contains(REFERENCE),
+                "src/p/C.java:15: error: invalid use of @return");
+
+        checkOutput(Output.OUT, expectGroups.contains(SYNTAX),
+                "src/p/C.java:19: error: bad HTML entity");
+    }
+
+    /**
+     * Generates a source file containing one issue in each group of
+     * issues detected by doclint. The intent is not to detect all issues,
+     * but instead, to detect whether the different groups of issues are
+     * correctly enabled or disabled by the {@code -Xdoclint} options.
+     */
+    private void generateSrc() throws IOException {
+        Path src = Path.of("src");
+        tb.writeJavaFiles(src,
+                  "package p;\n"
+                + "public class C {\n"
+                + "  /** Comment.\n"
+                + "   *  <img src=\"foo.png\">\n"       // "accessibility"" no alt attribute
+                + "   */\n"
+                + "  public void mAccessibility() { }\n"
+                + "  /** Comment.\n"
+                + "   *  <ul>123</ul>\n"                // "HTML": text not allowed
+                + "   */\n"
+                + "  public void mHtml() { }\n"
+                + "  /** Comment.\n"
+                + "   */\n"                             // "missing": no @return
+                + "  public int mMissing() { }\n"
+                + "  /** Comment.\n"
+                + "   *  @return error\n"               // "reference": invalid @return
+                + "   */\n"
+                + "  public void mReference() { }\n"
+                + "  /** Comment.\n"
+                + "   *  a & b\n"                       // "syntax": bad use of &
+                + "   */\n"
+                + "  public void mSyntax() { }\n"
+                + "}");
+    }
+}
+
+
--- a/test/langtools/jdk/javadoc/tool/treeapi/TestDocTrees.java	Thu Jan 30 10:27:08 2020 -0800
+++ b/test/langtools/jdk/javadoc/tool/treeapi/TestDocTrees.java	Thu Jan 30 11:20:09 2020 -0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2020, 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 8157611
+ * @bug 8157611 8236949
  * @summary test DocTrees is working correctly relative to HTML access
  * @modules
  *      jdk.javadoc/jdk.javadoc.internal.api
@@ -76,7 +76,7 @@
         execTask("-d", out.toString(),
                 "--release", "8",
                 "-Xdoclint:all",
-                "-Xdoclint:-missing",
+                "-Xdoclint:-reference",
                 "-sourcepath", testSrc.getAbsolutePath(),
                 testFile.getAbsolutePath(),
                 "-overview", overviewFile.getAbsolutePath());
@@ -86,7 +86,7 @@
     public void testOverviewWithoutRelease(Path out) throws Exception {
         execTask("-d", out.toString(),
                 "-Xdoclint:all",
-                "-Xdoclint:-missing",
+                "-Xdoclint:-reference",
                 "-sourcepath", testSrc.getAbsolutePath(),
                 testFile.getAbsolutePath(),
                 "-overview", overviewFile.getAbsolutePath());