8346659: SnippetTaglet should report an error if provided ambiguous links

nizarbenalla · nizarbenalla · commit 8323ddfe189e · 2025-02-27T14:52:18.000Z
Reviewed-by: hannesw, liach
diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2025, 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
@@ -26,8 +26,6 @@
 package jdk.javadoc.internal.doclets.formats.html.taglets;
 
 import java.io.IOException;
-import java.net.URI;
-import java.nio.file.Path;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -133,51 +131,47 @@ private Content snippetTagOutput(Element element, SnippetTree tag, StyledText co
         if (lang != null && !lang.isBlank()) {
             code.addStyle("language-" + lang);
         }
-
         content.consumeBy((styles, sequence) -> {
             CharSequence text = Text.normalizeNewlines(sequence);
             if (styles.isEmpty()) {
                 code.add(text);
             } else {
                 Element e = null;
-                String t = null;
-                boolean linkEncountered = false;
+                String linkTarget = null;
                 boolean markupEncountered = false;
                 Set<String> classes = new HashSet<>();
                 for (Style s : styles) {
-                    if (s instanceof Style.Name n) {
-                        classes.add(n.name());
-                    } else if (s instanceof Style.Link l) {
-                        assert !linkEncountered; // TODO: do not assert; pick the first link report on subsequent
-                        linkEncountered = true;
-                        t = l.target();
-                        e = getLinkedElement(element, t);
-                        if (e == null) {
-                            // TODO: diagnostic output
+                    switch (s) {
+                        case Style.Name n -> classes.add(n.name());
+                        case Style.Link l -> {
+                            if (linkTarget != null) {
+                                messages.error(utils.getCommentHelper(element).getDocTreePath(tag),
+                                        "doclet.error.snippet.ambiguous.link",
+                                        linkTarget,
+                                        l.target(),
+                                        content.asCharSequence().toString().trim());
+                            }
+                            linkTarget = l.target();
+                            e = getLinkedElement(element, linkTarget);
+                            if (e == null) {
+                                // TODO: diagnostic output
+                            }
                         }
-                    } else if (s instanceof Style.Markup) {
-                        markupEncountered = true;
-                        break;
-                    } else {
-                        // TODO: transform this if...else into an exhaustive
-                        // switch over the sealed Style hierarchy when "Pattern
-                        // Matching for switch" has been implemented (JEP 406
-                        // and friends)
-                        throw new AssertionError(styles);
+                        case Style.Markup m -> markupEncountered = true;
                     }
                 }
                 Content c;
                 if (markupEncountered) {
                     return;
-                } else if (linkEncountered) {
+                } else if (linkTarget != null) {
                     assert e != null;
                     //disable preview tagging inside the snippets:
                     Utils.PreviewFlagProvider prevPreviewProvider = utils.setPreviewFlagProvider(el -> false);
                     try {
                         var lt = (LinkTaglet) config.tagletManager.getTaglet(DocTree.Kind.LINK);
                         c = lt.linkSeeReferenceOutput(element,
                                 null,
-                                t,
+                                linkTarget,
                                 e,
                                 false, // TODO: for now
                                 Text.of(sequence.toString()),
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
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2010, 2025, 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
@@ -432,3 +432,7 @@ doclet.cannot_use_snippet_path=\
 # 0: path; 1: exception
 doclet.error_setting_snippet_path=\
   Error setting snippet path {0}: {1}
+
+# 0: location
+doclet.error.snippet.ambiguous.link=\
+  snippet link tags: {0} and {1} overlap in {2}
diff --git a/test/langtools/jdk/javadoc/doclet/ReproducibleSnippet/ReproducibleSnippetTest.java b/test/langtools/jdk/javadoc/doclet/ReproducibleSnippet/ReproducibleSnippetTest.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) 2025, 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      8346128 8346659
+ * @summary  Check that snippet generation is reproducible
+ * @library  /tools/lib ../../lib
+ * @modules  jdk.javadoc/jdk.javadoc.internal.tool
+ * @build    toolbox.ToolBox javadoc.tester.*
+ * @run main ReproducibleSnippetTest
+ */
+
+import javadoc.tester.JavadocTester;
+import toolbox.ToolBox;
+
+import java.nio.file.Path;
+
+public class ReproducibleSnippetTest extends JavadocTester {
+    ToolBox tb = new ToolBox();
+
+    public static void main(String... args) throws Exception {
+        var tester = new ReproducibleSnippetTest();
+        tester.runTests();
+    }
+
+    @Test
+    public void test(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                """
+                        package p;
+                        public interface One {
+                            /**
+                             * {@code One obj1}
+                             * {@snippet lang = java:
+                             * // @link substring="ab" target="One#ab" :
+                             * obj1.ab(a()); // @link substring="a" target="#a"
+                             *} class comment
+                             */
+                            int a();
+                            void ab(int i);
+                        }
+                        """);
+        javadoc("-d",
+                "out",
+                "-sourcepath",
+                src.toString(),
+                "p");
+        checkExit(Exit.ERROR);
+
+        checkOutput(Output.OUT, true,
+                "One.java:5: error: snippet link tags:",
+                "#a",
+                "One#ab",
+                "overlap in obj1.ab(a());\n     * {@snippet lang = java:\n       ^");
+    }
+}
diff --git a/test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java b/test/langtools/jdk/javadoc/doclet/TestGlobalHtml/TestGlobalHtml.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024, 2025, 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
@@ -26,7 +26,7 @@
  * @bug      8322708
  * @summary  Test to make sure global tags work properly
  * @library  /tools/lib ../../lib
- * @modules jdk.javadoc/jdk.javadoc.internal.tool
+ * @modules  jdk.javadoc/jdk.javadoc.internal.tool
  * @build    toolbox.ToolBox javadoc.tester.*
  * @run main TestGlobalHtml
  */

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`#`
`2`		`-# Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+# Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`#`
`5`	`5`	`# This code is free software; you can redistribute it and/or modify it`
`@@ -432,3 +432,7 @@ doclet.cannot_use_snippet_path=\`
`432`	`432`	`# 0: path; 1: exception`
`433`	`433`	`doclet.error_setting_snippet_path=\`
`434`	`434`	`Error setting snippet path {0}: {1}`
	`435`	`+`
	`436`	`+# 0: location`
	`437`	`+doclet.error.snippet.ambiguous.link=\`
	`438`	`+ snippet link tags: {0} and {1} overlap in {2}`