RFR: JDK-8286153: Remove redundant casts and other cleanup

Pavel Rappo prappo at openjdk.java.net
Thu May 5 10:35:31 UTC 2022


On Thu, 5 May 2022 00:18:56 GMT, Jonathan Gibbons <jjg at 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


More information about the javadoc-dev mailing list