RFR: JEP 359-Records: javadoc code

[Kumar Srinivasan]

Hi Jon,

Firstly I must commend the javadoc.next project for updating javadoc/doclet
to use jx.l.m and
other improvements to the doc comments management, this has made it
relatively easy for javadoc
to implement new language features, such as this. Having said that, the
changes are looking
great, and the langtools stalwarts have done due diligence.

Here are some minor review comments, I don't need to see another revision.

Default webrev:
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/ClassBuilder.java
Likely search/replace

-     * Keep track of whether or not this typeElement is an record.

+     * Keep track of whether or not this typeElement is a record.


- // only one canonical constructor; no longer need to keep looking

+ // only one canonical constructor; no need to keep looking

-        TypeMirror objectType =
utils.elementUtils.getTypeElement("java.lang.Object").asType();

+        TypeMirror objectType = utils.getObjectType();

There is a symbol table cache maintained in Utils, for performance improvements.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java

Nice simplifications to modifiersToString. <blush>

With the Element ordering changes (ElementComparator) I thought this might
cause failures
in TestOrdering, I guess the previous ordering are being preserved, but
there are no  Records
elements in this test ?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/CommentUtils.java

+    private void add(List<DocTree> contents, String resourceKey) {+
     // Special case to allow '{@link ...}' to appear in the string.+
      // A less general case would be to detect literal use of
Object.equals+        // A more general case would be to allow access
to DocCommentParser somehow

Yes the church vs. state rule, I don't quite understand the problem we
are trying solve with this code.


+     * name is to be inserted. T+     * @param key the resource key
for the description

Looks like we have a hanging "T".

One last thing, was the doc comparator run ? The reason I ask,  there seems
to be unrelated
improvements ex: Utils::modifiersToString etc, not sure if regressions are
lurking in the output.

Best,

Kumar Srinivasan

PS: I will have to go into radio silence for some time.

On Wed, Oct 30, 2019 at 4:54 PM Jonathan Gibbons <
jonathan.gibbons at oracle.com> wrote:

> Please review a moderately small update to the proposed support for
> records in javadoc.
>
> The primary change is to include record components in the signature of a
> record displayed near the top of the page.
>
> In addition, a "combo test" is added into TestRecordTypes.java to verify
> the presence or absence of annotations in various places in the
> generated page for a record, depending on the `@Target` of the annotations.
>
> Finally, there are some small cosmetic changes, and the supporting files
> for some previously published examples.
>
> Two webrevs are provided.
>
> The first is a cumulative webrev of the modified javadoc source and test
> files, compared against the default branch of the amber repo (i.e. the
> state of the jdk/jdk repo)
> http://cr.openjdk.java.net/~jjg/amber-records/webrev.default/
>
> The second is a "delta webrev" of the recently modified javadoc source
> and test files, compared against the tip of the records branch of the
> amber repo.
> http://cr.openjdk.java.net/~jjg/amber-records/webrev.tip/
>
> Also, the sets of examples are updated, showing examples linked and not
> linked to JDK API docs
> http://cr.openjdk.java.net/~jjg/amber-records/examples/api-with-link/
> http://cr.openjdk.java.net/~jjg/amber-records/examples/api-no-link/
>
> Finally, I note a curiosity, arising from the proposed spec.  This is
> the first occurrence that I can think of in which an item that is
> syntactically necessary in the source code does /not/ show up in the
> same place in the generated documentation.  In general, in previous
> instances where the documented signatures differ from the source code,
> the difference has been the addition of default or mandated elements.
> Here, the presence of an annotation on the declaration of a record
> component in source code may not show up in the corresponding place in
> the documented signature, depending on the specified @Target for the
> annotation. I'm not saying that's wrong, but it is curious, and may need
> explaining to some.
>
> -- Jon
>
> JEP 359: https://openjdk.java.net/jeps/359
>
>