On Thu, 28 Apr 2022 02:05:47 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> Please review the code and tests to add support for a new `@spec url title`
> tag to javadoc. Note, this does not include any changes to API doc comments
> to use the new tag in JDK API documentation; that will come later.
This is nice work. I haven't seen it in detail, but I'm plannig to do so later.
What's the logistics here? There are two interrelated bugs: 6251738 and
8226279. Are you going to add an 8226279 as an issue to this PR (`/issue add
8226279`)? What about @bug tags in tests? Are they going to mention both issues?
src/jdk.compiler/share/classes/com/sun/source/doctree/DocTreeVisitor.java line
290:
> 288: R visitSince(SinceTree node, P p);
> 289:
> 290: /**
This file uses a consistent style for doc comments for `visit` methods.
Consider adhering to that style for the time being. If you think that the style
could be improved, it should be improved in a separate PR.
src/jdk.compiler/share/classes/com/sun/source/util/DocTreeFactory.java line 340:
> 338: SnippetTree newSnippetTree(List<? extends DocTree> attributes,
> TextTree text);
> 339:
> 340: /**
Similar comment to that of DocTreeVisitor: consider adhering to the file style.
src/jdk.compiler/share/classes/com/sun/source/util/DocTreeScanner.java line 512:
> 510: }
> 511:
> 512: /**
Similar comment on the file style.
src/jdk.compiler/share/classes/com/sun/source/util/DocTreeScanner.java line 532:
> 530:
> 531: /**
> 532: * {@inheritDoc} This implementation scans the children in left to
> right order.
Same as above: surprising.
src/jdk.compiler/share/classes/com/sun/source/util/DocTreeScanner.java line 544:
> 542:
> 543: /**
> 544: * {@inheritDoc} This implementation scans the children in left to
> right order.
This change is surprising; firstly, it's unrelated to this PR, secondly... why?
src/jdk.compiler/share/classes/com/sun/source/util/SimpleDocTreeVisitor.java
line 466:
> 464: }
> 465:
> 466: /**
Again: the file style.
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DCTree.java line 1217:
> 1215: @Override
> 1216: public boolean isBlank() {
> 1217: return text.isBlank();
Can text be null?
src/jdk.compiler/share/classes/com/sun/tools/javac/tree/DocTreeMaker.java line
419:
> 417:
> 418: @Override @DefinedBy(Api.COMPILER_TREE)
> 419: public DCSpec newSpecTree(TextTree url, List<? extends DocTree>
> title) {
In DocTreeFactory the first parameter is called uri.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ExternalSpecsWriter.java
line 68:
> 66: * Generates the file with the summary of all the references to external
> specifications.
> 67: *
> 68: * <p><b>This is NOT part of any supported API.
We abolished such notes in JDK-8284362.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDoclet.java
line 33:
> 31: import java.nio.file.InvalidPathException;
> 32: import java.nio.file.Path;
> 33: import java.util.ArrayList;
This does not seem necessary for this PR and looks like an artifact of editing
in an IDE.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SpecTaglet.java
line 44:
> 42: * A taglet that represents the {@code @spec} tag.
> 43: *
> 44: * <p><b>This is NOT part of any supported API.
See my comment on such notes above.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SpecTaglet.java
line 49:
> 47: * deletion without notice.</b>
> 48: */
> 49: public class SpecTaglet extends BaseTaglet implements InheritableTaglet {
Ooh! This taglet is inheritable. Is it the right thing to do? If the
inheritance is not needed down the hierarchy, how can the author avoid it?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/TagletWriter.java
line 192:
> 190: *
> 191: * @param element the element that owns the doc comment
> 192: * @param specTags the array of @spec tags.
Naked tags that aren't meant to be interpreted by javadoc make me uncomfortable.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/DocFinder.java
line 277:
> 275:
> 276: private static boolean isNonEmpty(List<? extends DocTree> list) {
> 277: return list != null && !list.isEmpty();
`output.inlineTags` should never be null. Separately, can this readability
change be done in a separate PR? I do have one suitable PR in the works, which
can include it.
test/langtools/jdk/javadoc/doclet/testConditionalPages/TestConditionalPages.java
line 2:
> 1: /*
> 2: * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights
> reserved.
Consider adding 6251738 to `@bug`.
test/langtools/jdk/javadoc/doclet/testMetadata/TestMetadata.java line 2:
> 1: /*
> 2: * Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights
> reserved.
Same as above.
test/langtools/tools/javac/diags/examples/NoTitle.java line 2:
> 1: /*
> 2: * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights
> reserved.
It's surprising to see 2012 here.
test/langtools/tools/javac/diags/examples/NoURI.java line 2:
> 1: /*
> 2: * Copyright (c) 2012, 2022, Oracle and/or its affiliates. All rights
> reserved.
Same as above.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8439
