On Thu, 7 Jan 2021 20:23:16 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

>> This is an update to javac and javadoc, to introduce support for Preview 
>> APIs, and generally improve javac and javadoc behavior to more closely 
>> adhere to JEP 12.
>> 
>> The notable changes are:
>> 
>>  * adding support for Preview APIs (javac until now supported primarily only 
>> preview language features, and APIs associated with preview language 
>> features). This includes:
>>      * the @PreviewFeature annotation has boolean attribute "reflective", 
>> which should be true for reflective Preview APIs, false otherwise. This 
>> replaces the existing "essentialAPI" attribute with roughly inverted meaning.
>>      * the preview warnings for preview APIs are auto-suppressed as 
>> described in the JEP 12. E.g. the module that declares the preview API is 
>> free to use it without warnings
>>  * improving error/warning messages. Please see [1] for a list of 
>> cases/examples.
>>  * class files are only marked as preview if they are using a preview 
>> feature. [1] also shows if a class file is marked as preview or not.
>>  * the PreviewFeature annotation has been moved to jdk.internal.javac 
>> package (originally was in the jdk.internal package).
>>  * Preview API in JDK's javadoc no longer needs to specify @preview tag in 
>> the source files. javadoc will auto-generate a note for @PreviewFeature 
>> elements, see e.g. [2] and [3] (non-reflective and reflective API, 
>> respectively). A summary of preview elements is also provided [4]. Existing 
>> uses of @preview have been updated/removed.
>>  * non-JDK javadoc is also enhanced to auto-generate preview notes for uses 
>> of Preview elements, and for declaring elements using preview language 
>> features [5].
>>  
>>  Please also see the CSR [6] for more information.
>>  
>>  [1] 
>> http://cr.openjdk.java.net/~jlahoda/8250768/JEP12-errors-warnings-v6.html
>>  [2] 
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.base/java/lang/Record.html
>>  [3] 
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/java.compiler/javax/lang/model/element/RecordComponentElement.html
>>  [4] 
>> http://cr.openjdk.java.net/~jlahoda/8250768/jdk.javadoc.00/api/preview-list.html
>>  [5] http://cr.openjdk.java.net/~jlahoda/8250768/test.javadoc.00/
>>  [6] https://bugs.openjdk.java.net/browse/JDK-8250769
>
> Jan Lahoda has updated the pull request with a new target base due to a merge 
> or a rebase. The pull request now contains 57 commits:
> 
>  - Fixing tests after a merge.
>  - Merging master into JDK-8250768
>  - Merging recent master changes into JDK-8250768
>  - Fixing navigator for the PREVIEW page.
>  - Fixing typo.
>  - Removing obsolette @PreviewFeature.
>  - Merging master into JDK-8250768
>  - Removing unnecessary property keys.
>  - Cleanup - removing unnecessary code.
>  - Merging master into JDK-8250768-dev4
>  - ... and 47 more: 
> https://git.openjdk.java.net/jdk/compare/81c06242...a8046dde

I've looked at all the files that were marked as changed since I last looked at 
them.

There's one suggested enhancement to reduce string bashing between `Utils` and 
`ClassWriterImpl` that could be done now or later.

There's a pending conflict with a PR of mine to change to use a new type 
`HtmlId` for HTML ids. This JEP12 work has been in progress for a while, and so 
it would be good to get it in before the `HtmlId` work, and I'll deal with the 
merge conflict in due course.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
 line 194:

> 192: 
> 193:     @Override @SuppressWarnings("preview")
> 194:     public void addClassSignature(String modifiers, Content 
> classInfoTree) {

It seems less than ideal for this method to take a `String` and to then have to 
take it apart and reassemble it. It looks like it should be possible (and 
conceptually better) to change the signature to `List<String>` and to make the 
corresponding change to `utils.modifiersToString`, probably renaming it as 
`utils.modifiersToStrings` or something like that, and dropping the 
always-`true` argument for `trailingSpace`.  

But, the code is OK as is, and the suggestion is just for an enhancement, so is 
OK to defer it, if you would prefer.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
 line 2:

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

(minor) now 2021

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
 line 61:

> 59: public abstract class SummaryListWriter<L extends SummaryAPIListBuilder> 
> extends SubWriterHolderWriter {
> 60: 
> 61:     private String getAnchorName(SummaryElementKind kind) {

Heads-up: at some point this will conflict with another change in 
progress/review, to introduce a new type `HtmlId`  to use instead of `String` 
for ids.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SummaryListWriter.java
 line 227:

> 225:      * @param contentTree the content tree to which the summary table 
> will be added
> 226:      */
> 227:     protected void addSummaryAPI(SortedSet<Element> apiList, String id,

Heads-up, here's another occurrence of where there will be a conflict for ids.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/resources/doclets.properties
 line 154:

> 152: doclet.Errors=Errors
> 153: doclet.Classes=Classes
> 154: doclet.Records=Records

I guess I'm mildly surprised to see all these items being removed...

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/SummaryAPIListBuilder.java
 line 2:

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

Year

test/langtools/jdk/javadoc/doclet/testPreview/TestPreview.java line 2:

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

2021. I assume you will do a bulk update for affected files.

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

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/703

Reply via email to