On Thu, 5 May 2022 00:18:56 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
> Please review some cleanup updates to address issues reported by an IDE.
>
> The seeds for the change were a series of redundant casts, that have now all
> been removed. Various other warnings and suggestions were made by the IDE
> for the affected files. There were a number of places with redundant type
> arguments, for which the general fix was in favor of using `var` instead of
> `<>`.
>
> Some `switch` statements were converted to the enhanced `switch` form, which
> also revealed a couple of places where `RECORD` should have been added
> alongside `ENUM`.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java
line 297:
> 295: VisibleMemberTable vmt =
> configuration.getVisibleMemberTable(enclosing);
> 296: // Check whether there is any implementation or overridden info
> to be
> 297: // printed. If not overridden or implementation info needs to be
Although awkward, it was correct before the change. Consider rewriting for
clarity or deleting it. The code is pretty self-descriptive, if you ask me.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/MetaKeywords.java
line 81:
> 79:
> results.addAll(getMemberKeywords(utils.getMethods(typeElement)));
> 80: }
> 81: results.trimToSize();
I wonder if this method will look better this way:
public List<String> getMetaKeywords(TypeElement typeElement) {
var results = new ArrayList<String>();
...
results.trimToSize();
return results;
}
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/MetaKeywords.java
line 125:
> 123: */
> 124: public List<String> getOverviewMetaKeywords(String title, String
> docTitle) {
> 125: List<String> result = new ArrayList<>(1);
Consider using `var` similarly to my suggestion above. Alternatively, we can
use two calls to `List.of(e)` and one call to `List.of()`.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/MetaKeywords.java
line 156:
> 154: }
> 155: }
> 156: results.trimToSize();
Same suggestion for `var` as above.
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java line
273:
> 271: * - is specified on the command line --module
> 272: * - is derived from the module graph, that is, by expanding the
> 273: * 'requires' directive, based on --expand-requires
Thanks for using these clarifying quotes!
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java line
1063:
> 1061: if (enclosing != null) {
> 1062: return switch (enclosing.getKind()) {
> 1063: case CLASS, ENUM, RECORD, INTERFACE,
> ANNOTATION_TYPE -> visit(enclosing);
Whoa! `RECORD` was missing. Does it make sense to accompany this PR with a
small test that crashes javadoc with a type nested in a non-included record?
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/ElementsTable.java line
1206:
> 1204: AccessKind accessValue = null;
> 1205: for (ElementKind kind : ALLOWED_KINDS) {
> 1206: accessValue = switch (kind) {
It feels awkward when adjacent `switch` statements use different formatting.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8543
