On Thu, 22 Apr 2021 02:21:14 GMT, Vicente Romero <[email protected]> wrote:
> Please review the javadoc related changes to make sealed classes a final
> feature in Java. The patch is mostly removing preview features related code
> that is not necessary anymore in javadoc.
>
> TIA
Generally, I think you have have deleted too much.
Although the code will work as you propose, at some point there will be new
preview language features, and I think it would be better to leave a skeleton
of the code in place, even if the set of preview language features is empty for
now.
I think it would be better to limit the deletions to those lines of code that
are specific to sealed classes and permits.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 87:
> 85: private static final Set<String> previewModifiers
> 86: = Set.of("sealed", "non-sealed");
> 87:
Rather than delete those lines, for future support it might be better just to
set previewModifiers to `Collections.emptySet()`
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java
line 193:
> 191: }
> 192:
> 193: mods.add(modifiersPart);
See previous comment, to leave previewModifiers in place as an empty set
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 2242:
> 2240: Set<TypeElement> reflectivePreviewAPI = new
> HashSet<>(previewAPITypes.reflectivePreviewAPI);
> 2241: Set<TypeElement> declaredUsingPreviewFeature = new
> HashSet<>(previewAPITypes.declaredUsingPreviewFeature);
> 2242: Set<DeclarationPreviewLanguageFeatures> previewLanguageFeatures
> = new HashSet<>();
consider not deleting this; just leave it empty
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 2261:
> 2259: if
> (previewLanguageFeatures.contains(DeclarationPreviewLanguageFeatures.SEALED_PERMITS))
> {
> 2260:
> previewLanguageFeatures.remove(DeclarationPreviewLanguageFeatures.SEALED);
> 2261: }
These lines should go, because they specifically refer to SEALED stuff, but the
rest of the block could stay
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 2295:
> 2293: featureCodes);
> 2294: }
> 2295:
Nothing here seems specific to SEALED, so leave it
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Signatures.java
line 191:
> 189:
> classWriter.links.createLink(classWriter.htmlIds.forPreviewSection(typeElement),
> 190:
> classWriter.contents.previewMark);
> 191: permitsSpan.add(HtmlTree.SUP(link));
OK, yes, this is specific to permits, part of the sealed feature
-------------
PR: https://git.openjdk.java.net/jdk/pull/3613
