8299080: Wrong default value of snippet lang attribute

Reviewed-by: jjg

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

@@ -26,6 +26,8 @@
 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;
@@ -65,41 +67,16 @@ import jdk.javadoc.internal.doclets.toolkit.Resources;
 import jdk.javadoc.internal.doclets.toolkit.util.DocPaths;
 import jdk.javadoc.internal.doclets.toolkit.util.Utils;
 
+import static jdk.javadoc.internal.doclets.formats.html.taglets.SnippetTaglet.Language.*;
+
 /**
  * A taglet that represents the {@code @snippet} tag.
  */
 public class SnippetTaglet extends BaseTaglet {
 
     public enum Language {
-
+        JAVA,
-        JAVA("java"),
+        PROPERTIES;
-        PROPERTIES("properties");
-
-        private static final Map<String, Language> languages;
-
-        static {
-            Map<String, Language> tmp = new HashMap<>();
-            for (var language : values()) {
-                String id = Objects.requireNonNull(language.identifier);
-                if (tmp.put(id, language) != null)
-                    throw new IllegalStateException(); // 1-1 correspondence
-            }
-            languages = Map.copyOf(tmp);
-        }
-
-        Language(String id) {
-            identifier = id;
-        }
-
-        private final String identifier;
-
-        public static Optional<Language> of(String identifier) {
-            if (identifier == null)
-                return Optional.empty();
-            return Optional.ofNullable(languages.get(identifier));
-        }
-
-        public String getIdentifier() {return identifier;}
     }
 
     SnippetTaglet(HtmlConfiguration config) {
@@ -361,18 +338,30 @@ public class SnippetTaglet extends BaseTaglet {
             }
         }
 
-        String lang = null;
+        String lang;
         AttributeTree langAttr = attributes.get("lang");
-        if (langAttr != null) {
+
+        if (langAttr != null) { // the lang attribute overrides everything else
             lang = stringValueOf(langAttr);
-        } else if (containsClass) {
+        } else if (inlineContent != null && externalContent == null) { // an inline snippet
             lang = "java";
-        } else if (containsFile) {
+        } else if (externalContent != null) { // an external or a hybrid snippet
-            lang = languageFromFileName(fileObject.getName());
+            if (containsClass) { // the class attribute means Java
+                lang = "java";
+            } else {
+                var uri = fileObject.toUri();
+                var path = uri.getPath() != null ? uri.getPath() : "";
+                var fileName = path.substring(path.lastIndexOf('/') + 1);
+                lang = languageFromFileName(fileName);
+            }
+        } else {
+            throw new AssertionError();
         }
 
-        Optional<Language> language = Language.of(lang);
+        var language = switch (lang) {
-
+            case "properties" -> PROPERTIES;
+            case null, default -> JAVA;
+        };
 
         // TODO cache parsed external snippet (WeakHashMap)
 
@@ -482,7 +471,7 @@ public class SnippetTaglet extends BaseTaglet {
                """.formatted(inline, external);
     }
 
-    private StyledText parse(Resources resources, Diags diags, Optional<Language> language, String content) throws ParseException {
+    private StyledText parse(Resources resources, Diags diags, Language language, String content) throws ParseException {
         Parser.Result result = new Parser(resources).parse(diags, language, content);
         result.actions().forEach(Action::perform);
         return result.text();
@@ -505,13 +494,15 @@ public class SnippetTaglet extends BaseTaglet {
     }
 
     private String languageFromFileName(String fileName) {
-        // TODO: find a way to extend/customize the list of recognized file name extensions
+        // The assumption is simple: a file extension is the language
-        if (fileName.endsWith(".java")) {
+        // identifier.
-            return "java";
+        // Was about to use Path.getExtension introduced in 8057113, but then
-        } else if (fileName.endsWith(".properties")) {
+        // learned that it was removed in 8298303.
-            return "properties";
+        int lastPeriod = fileName.lastIndexOf('.');
+        if (lastPeriod <= 0) {
+            return null;
         }
-        return null;
+        return (lastPeriod == fileName.length() - 1) ? null : fileName.substring(lastPeriod + 1);
     }
 
     private void error(TagletWriter writer, Element holder, DocTree tag, String key, Object... args) {

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2024, 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
@@ -89,9 +89,8 @@ public final class Parser {
         this.markupParser = new MarkupParser(resources);
     }
 
-    public Result parse(SnippetTaglet.Diags diags, Optional<SnippetTaglet.Language> language, String source) throws ParseException {
+    public Result parse(SnippetTaglet.Diags diags, SnippetTaglet.Language language, String source) throws ParseException {
-        SnippetTaglet.Language lang = language.orElse(SnippetTaglet.Language.JAVA);
+        var p = switch (language) {
-        var p = switch (lang) {
             case JAVA -> JAVA_COMMENT;
             case PROPERTIES -> PROPERTIES_COMMENT;
         };

test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdownTaglets.java

@@ -121,7 +121,7 @@ public class TestMarkdownTaglets extends JavadocTester {
                     <div class="snippet-container"><button class="copy snippet-copy" aria-label="Copy snippet" \
                     onclick="copySnippet(this)"><span data-copied="Copied!">Copy</span>\
                     <img src="../resource-files/copy.svg" alt="Copy snippet"></button>
-                    <pre class="snippet" id="snippet-snippet_standalone()1"><code>   this is snippet_standalone
+                    <pre class="snippet" id="snippet-snippet_standalone()1"><code class="language-java">   this is snippet_standalone
                     </code></pre>
                     </div>
 
@@ -131,7 +131,7 @@ public class TestMarkdownTaglets extends JavadocTester {
                     <div class="block"><p>First sentence.</p>
                     <p>Before.</p>
                     <div class="snippet-container"><button class="copy snippet-copy" aria-label="Copy snippet" onclick="copySnippet(this)"><span data-copied="Copied!">Copy</span><img src="../resource-files/copy.svg" alt="Copy snippet"></button>
-                    <pre class="snippet" id="snippet-snippet_wrapped()1"><code>   this is a snippet_wrapped
+                    <pre class="snippet" id="snippet-snippet_wrapped()1"><code class="language-java">   this is a snippet_wrapped
                     </code></pre>
                     </div>
 

test/langtools/jdk/javadoc/doclet/testSnippetTag/SnippetTester.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, 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
@@ -100,7 +100,7 @@ public class SnippetTester extends JavadocTester {
 
     protected String getSnippetHtmlRepresentation(String pathToHtmlFile,
                                                   String content) {
-        return getSnippetHtmlRepresentation(pathToHtmlFile, content, Optional.empty(), Optional.empty());
+        return getSnippetHtmlRepresentation(pathToHtmlFile, content, Optional.of("java"), Optional.empty());
     }
 
     protected String getSnippetHtmlRepresentation(String pathToHtmlFile,

test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetMarkup.java

@@ -366,7 +366,7 @@ First line // @highlight :
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
                         %s
-                        </div>""".formatted(index, getSnippetHtmlRepresentation("A.html", t.expectedOutput(), Optional.empty(), Optional.of("snippet-case" + index + "()2")));
+                        </div>""".formatted(index, getSnippetHtmlRepresentation("A.html", t.expectedOutput(), Optional.of("java"), Optional.of("snippet-case" + index + "()2")));
             checkOutput("A.html", true, html);
         });
     }

test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8266666 8275788 8276964
+ * @bug 8266666 8275788 8276964 8299080
  * @summary Implementation for snippets
  * @library /tools/lib ../../lib
  * @modules jdk.compiler/com.sun.tools.javac.api
@@ -108,27 +108,27 @@ public class TestSnippetTag extends SnippetTester {
                     {@snippet id="foo1" :
                         Hello, Snippet!
                     }
-                    """, "foo1", null),
+                    """, "foo1", "java"),
                 new SnippetAttributes("""
                     {@snippet id="foo2":
                         Hello, Snippet!
                     }
-                    """, "foo2", null),
+                    """, "foo2", "java"),
                 new SnippetAttributes("""
                     {@snippet id='foo3' :
                         Hello, Snippet!
                     }
-                    """, "foo3", null),
+                    """, "foo3", "java"),
                 new SnippetAttributes("""
                     {@snippet id='foo4':
                         Hello, Snippet!
                     }
-                    """, "foo4", null),
+                    """, "foo4", "java"),
                 new SnippetAttributes("""
                     {@snippet id=foo5 :
                         Hello, Snippet!
                     }
-                    """, "foo5", null),
+                    """, "foo5", "java"),
 // (1) Haven't yet decided on this one. It's a consistency issue. On the one
 // hand, `:` is considered a part of a javadoc tag's name (e.g. JDK-4750173);
 // on the other hand, snippet markup treats `:` (next-line modifier) as a value
@@ -137,28 +137,28 @@ public class TestSnippetTag extends SnippetTester {
 //                    {@snippet id=foo6:
 //                        Hello, Snippet!
 //                    }
-//                    """, "foo6", null),
+//                    """, "foo6", "java"),
 
                 new SnippetAttributes("""
                     {@snippet id="" :
                         Hello, Snippet!
                     }
-                    """, null, null),
+                    """, null, "java"),
                 new SnippetAttributes("""
                     {@snippet id="":
                         Hello, Snippet!
                     }
-                    """, null, null),
+                    """, null, "java"),
                 new SnippetAttributes("""
                     {@snippet id='':
                         Hello, Snippet!
                     }
-                    """, null, null),
+                    """, null, "java"),
                 new SnippetAttributes("""
                     {@snippet id=:
                         Hello, Snippet!
                     }
-                    """, null, null),
+                    """, null, "java"),
                 new SnippetAttributes("""
                     {@snippet lang="java" :
                         Hello, Snippet!
@@ -880,7 +880,7 @@ public class TestSnippetTag extends SnippetTester {
                         """
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
-                        %s""".formatted(id, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.empty(),
+                        %s""".formatted(id, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.of("java"),
                                 Optional.of("snippet-case" + id + "()2"))));
         });
     }
@@ -973,7 +973,7 @@ public class TestSnippetTag extends SnippetTester {
                         """
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
-                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", expectedOutput, Optional.empty(),
+                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", expectedOutput, Optional.of("txt"),
                                 Optional.of("snippet-case" + index + "()2"))));
         });
     }
@@ -1552,7 +1552,7 @@ public class TestSnippetTag extends SnippetTester {
                         """
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
-                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.empty(),
+                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.of("java"),
                                 Optional.of("snippet-case" + index + "()2"))));
         });
     }
@@ -1666,13 +1666,13 @@ public class TestSnippetTag extends SnippetTester {
                     """
                     <span class="element-name">case0</span>()</div>
                     <div class="block">
-                    """ + getSnippetHtmlRepresentation("pkg/A.html", "", Optional.empty(),
+                    """ + getSnippetHtmlRepresentation("pkg/A.html", "", Optional.of("txt"),
                             Optional.of("snippet-case0()2")));
         checkOutput("pkg/A.html", true,
                     """
                     <span class="element-name">case1</span>()</div>
                     <div class="block">
-                    """ + getSnippetHtmlRepresentation("pkg/A.html", "", Optional.empty(),
+                    """ + getSnippetHtmlRepresentation("pkg/A.html", "", Optional.of("txt"),
                             Optional.of("snippet-case1()2")));
     }
 
@@ -1851,12 +1851,126 @@ public class TestSnippetTag extends SnippetTester {
                         """
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
-                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.empty(),
+                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.of("java"),
                                 Optional.of("snippet-case" + index + "()2"))));
         });
     }
 
     @Test
+    public void testPositiveExternalHybridLangAttribute(Path base) throws Exception {
+
+        Path srcDir = base.resolve("src");
+        Path outDir = base.resolve("out");
+
+        record TestCase(String tag, String expectedContent, String expectedLang) { }
+
+        final var testCases = List.of(
+                // -------------------- external snippets --------------------
+                // if there's no file extension and no lang attribute, then
+                // markup is that of "java" and the class="language-" attribute
+                // is absent
+                new TestCase("""
+                        {@snippet file=".file"}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, null),
+                new TestCase("""
+                        {@snippet file="file"}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, null),
+                // if the file extension differs from the value of the lang
+                // attribute, which is set to "java", "properties" or other,
+                // then the class="language-" attribute is that of lang and
+                // markup is that of "properties" for lang=properties and
+                // markup is that of "java" for anything else
+                new TestCase("""
+                        {@snippet file="File.java" lang=properties}
+                        """, """
+                        <span class="bold">hi</span> there // @<span class="bold">hi</span>ghlight substring=there
+                        """, "properties"),
+                new TestCase("""
+                        {@snippet file="file.properties" lang=java}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, "java"),
+                new TestCase("""
+                        {@snippet file="File.java" lang=txt}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, "txt"),
+                // if there's no file extension, but the lang attribute is set
+                // to "java", "properties", or other, then the class="language-"
+                // attribute is that of lang and markup is that of "properties"
+                // for lang=properties and markup is that of "java" for
+                // anything else
+                new TestCase("""
+                        {@snippet file="file" lang=properties}
+                        """, """
+                        <span class="bold">hi</span> there // @<span class="bold">hi</span>ghlight substring=there
+                        """, "properties"),
+                new TestCase("""
+                        {@snippet file="file" lang=java}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, "java"),
+                new TestCase("""
+                        {@snippet file="file" lang=txt}
+                        """, """
+                        # @highlight substring=hi:
+                        hi <span class="bold">there</span>
+                        """, "txt"),
+                // --------------------- hybrid snippets ---------------------
+                // the lang attribute "overrides" file extension
+                new TestCase("""
+                        {@snippet file="File.java" lang=properties:
+                        # @highlight substring=hi:
+                        hi there // @highlight substring=there
+                        }
+                        """, """
+                        <span class="bold">hi</span> there // @<span class="bold">hi</span>ghlight substring=there
+                        """, "properties"),
+                // if the lang attribute is absent, file extension determines
+                // markup and the the class="language-" attribute
+                new TestCase("""
+                        {@snippet file="file.properties":
+                        # @highlight substring=hi:
+                        hi there // @highlight substring=there
+                        }
+                        """, """
+                        <span class="bold">hi</span> there // @<span class="bold">hi</span>ghlight substring=there
+                        """, "properties")
+        );
+
+        for (var f : List.of(".file", "file", "File.java", "file.properties"))
+            addSnippetFile(srcDir, "pkg", f, """
+                    # @highlight substring=hi:
+                    hi there // @highlight substring=there
+                    """);
+
+        ClassBuilder classBuilder = new ClassBuilder(tb, "pkg.A")
+                .setModifiers("public", "class");
+        forEachNumbered(testCases, (s, i) -> classBuilder.addMembers(
+                MethodBuilder.parse("public void case%s() { }".formatted(i)).setComments(s.tag)));
+        classBuilder.write(srcDir);
+        javadoc("-d", outDir.toString(),
+                "-sourcepath", srcDir.toString(),
+                "pkg");
+        checkExit(Exit.OK);
+        forEachNumbered(testCases, (t, i) -> checkOutput("pkg/A.html", true, """
+                        <span class="element-name">case%s</span>()</div>
+                        <div class="block">
+                        %s
+                        """.formatted(i, getSnippetHtmlRepresentation("pkg/A.html", t.expectedContent, Optional.ofNullable(t.expectedLang),
+                Optional.of("snippet-case" + i + "()2")))));
+    }
+
+    //@Test
     public void testNegativeHybridTag_FileNotFound(Path base) throws Exception {
         Path srcDir = base.resolve("src");
         Path outDir = base.resolve("out");
@@ -2310,7 +2424,7 @@ public class TestSnippetTag extends SnippetTester {
                         """
                         <span class="element-name">case%s</span>()</div>
                         <div class="block">
-                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.empty(),
+                        %s""".formatted(index, getSnippetHtmlRepresentation("pkg/A.html", t.expectedOutput(), Optional.of("txt"),
                                 Optional.of("snippet-case" + index + "()2"))));
         });
     }