RFR: JDK-8298405: Markdown support in the standard doclet

[Pavel Rappo]

On Thu, 15 Dec 2022 23:47:45 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

> Support for Markdown comments in the standard doclet.
> 
> To enable Markdown in a comment, start the comment with `/**md` followed by whitespace.  The syntax is as defined for CommonMark.
> 
> The work is in 3 parts:
> 
> 1. Update the Compiler Tree API to support Markdown tree nodes, containing strings of (uninterpreted) Markdown source code.
> 2. Import commonmark-java into the `jdk.javadoc` module, to be able to convert Markdown strings to HTML.
> 3. Update the standard doclet, to leverage the preceding two parts, to translate Markdown in documentation comments to `Content` nodes.
> 
> There are new tests both for the low level work in the Compiler Tree API, and for the overall high-level work in the doclet.

This is a welcome change and impressive work! My initial comments are inline.

src/jdk.compiler/share/classes/com/sun/source/doctree/DocTree.java line 158:

> 156:         /**
> 157:          * Used for instances of {@link MarkdownTree}
> 158:          * representing a fragment of Markdown code.

_Markdown code_: is there a better term for a run/span/block of Markdown? Here and elsewhere in this PR, _Markdown code_ reads slightly weird and misleading.

src/jdk.compiler/share/classes/com/sun/source/doctree/InheritDocTree.java line 31:

> 29:  * A tree node for an {@code @inheritDoc} inline tag.
> 30:  *
> 31:  * @apiNote

1. Consider wrapping that long line.
2. While it's good to have that fact noted here, it's more important to note it in "Documentation Comment Specification for the Standard Doclet".

src/jdk.compiler/share/classes/com/sun/source/doctree/MarkdownTree.java line 2:

> 1: /*
> 2:  * Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.

2011?

src/jdk.compiler/share/classes/com/sun/source/doctree/MarkdownTree.java line 34:

> 32:  * The code may contain plain text, entities and HTML elements,
> 33:  * all represented directly in the text of the code,
> 34:  * but not {@linkplain InlineTagTree inline tags}.

IIUC, constructs represented by `BlockTagTree` are also NOT contained by this kind of node, right?

src/jdk.compiler/share/classes/com/sun/source/doctree/package-info.java line 30:

> 28:  * trees (AST).
> 29:  *
> 30:  * <h2>Markdown</h2>

Consider updating the copyright years in this file.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocPretty.java line 214:

> 212:                 for (DocTree node : nodes) {
> 213:                     Boolean b = scan(node, ignore);
> 214:                     if (b != null && b.equals(Boolean.TRUE)) {

Could it be `b == Boolean.TRUE`?

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocPretty.java line 668:

> 666:                 print(" ");
> 667:                 print(content);
> 668:             }

This seems unrelated to this PR.

src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line 550:

> 548:                 switch (dt.getKind()) {
> 549:                     case RETURN,
> 550:                         SUMMARY ->

Indent.

src/jdk.javadoc/share/classes/jdk/internal/org/commonmark/internal/inline/AsteriskDelimiterProcessor.java line 1:

> 1: package jdk.internal.org.commonmark.internal.inline;

1. Does this and the other CommonMark files in this PR require any copyright header? 
2. Does it make sense to add a link to the place from where the (CommonMark) snapshot was taken? If it's a VCS, maybe include a revision/hash for future maintainers and archaeologists.

src/jdk.javadoc/share/classes/jdk/internal/org/commonmark/internal/util/Html5Entities.java line 15:

> 13: 
> 14:     private static final Map<String, String> NAMED_CHARACTER_REFERENCES = readEntities();
> 15:     private static final String ENTITY_PATH = "/org/commonmark/internal/util/entities.properties";

This properties file is missing from this PR. If you add a test with an md- doc comment that has entities (e.g. `&amp`), the test will crash.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1248:

> 1246:     }
> 1247: 
> 1248:     private class MarkdownHandler {

Changes in this file seem to be the meat of the mechanism whereby Markdown-JavaDoc-HTML soup is handled. The mechanics looks good.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1249:

> 1247: 
> 1248:     private class MarkdownHandler {
> 1249:         private static final char FFFC = '\uFFFC'; // Unicode Object Replacement Character

Can we use a better name for the FFFC constant? PLACEHOLDER or some such.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1281:

> 1279:             Node document = parser.parse(markdownInput.toString());
> 1280:             HtmlRenderer renderer = HtmlRenderer.builder().build();
> 1281:             String markdownOutput = renderer.render(document);

Curious how heavyweight the parser and renderer are; should we cache them for reuse?

test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java line 42:

> 40: 
> 41:     public static void main(String... args) throws Exception {
> 42:         TestMarkdown tester = new TestMarkdown();

Consider `var tester` in the spirit of your recent cleanup PR (#11746).

test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java line 211:

> 209:                         /**md
> 210:                          * Markdown comment.
> 211:                          * @throws {@inheritDoc}

That `{@inheritDoc}` is ineffective because the enclosing `@throws` is invalid: `@throws` must provide an exception name. There's probably a warning or an error masked by `-Xdoclint:none` (BTW, it might be a good idea to turn on DocLint and assert exit code in each of these tests).

That exception description is inherited only because the exception is mentioned in the `throws` clause.

test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java line 248:

> 246:                         /**
> 247:                          * Plain comment.
> 248:                          * @throws {@inheritDoc}

Similar comment to that one above.

test/langtools/jdk/javadoc/doclet/testMarkdown/TestMarkdown.java line 285:

> 283:                         /**md
> 284:                          * Markdown comment.
> 285:                          * @throws {@inheritDoc}

Ditto.

test/langtools/tools/javac/doctree/MarkdownTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.

Copyright years seem strange.

-------------

PR: https://git.openjdk.org/jdk/pull/11701