Skip to content

A cast breaks inside its type when the call around it stays on one line #60

Description

@abashev

What

Found while formatting the JDK 21 sources with main (86cdc74e). sun.rmi.transport.tcp.TCPTransport comes out as:

    @SuppressWarnings("removal")
    static final Log tcpLog = Log.getLog(
            "sun.rmi.transport.tcp", "tcp", LogStream.parseLevel(AccessController.doPrivileged((PrivilegedAction<
                            String>)
                    () -> System.getProperty("sun.rmi.transport.tcp.logLevel"))));

The cast (PrivilegedAction<String>) is cut between < and String, and the lambda it applies to lands on a third line. Formatting the result again changes nothing, so this is what the file settles on.

Wanted: the arguments of getLog one per line, and the cast whole, on the line with its lambda:

    @SuppressWarnings("removal")
    static final Log tcpLog = Log.getLog(
            "sun.rmi.transport.tcp",
            "tcp",
            LogStream.parseLevel(AccessController.doPrivileged(
                    (PrivilegedAction<String>) () -> System.getProperty("sun.rmi.transport.tcp.logLevel"))));

One call shallower, the same cast is right today:

    static final Level level = LogStream.parseLevel(AccessController.doPrivileged(
            (PrivilegedAction<String>) () -> System.getProperty("sun.rmi.transport.tcp.logLevel.and.some.more")));

So are a cast lambda as the only argument, a cast of a method reference instead of a lambda, and the same statement in a local variable. The code is palantir-java-format 2.98.0's, so it should format this the same way; google-java-format does not have this layout logic.

Why

The arguments of getLog are laid out with breakOnlyIfInnerLevelsThenFitOnOneLine: keep them on one line when a "sensible prefix" fits, where the prefix is everything up to the first break (CountWidthUntilBreakVisitor). The first break inside a cast is not the one before its expression but the one visitParameterizedType emits right after < in its type. The prefix up to there is 113 columns, so it fits, and the type then has to take that break because String>) does not fit behind it.

visitTypeCast opens its level with preferBreakingLastInnerLevel and LastLevelBreakability.ACCEPT_INLINE_CHAIN, whose javadoc asks for a level whose first non-level doc is a break. A cast starts with (.

Confirmed with a spike that took the break after < out of visitParameterizedType: with no break inside the type, the prefix runs to the cast's own break, is 121 columns, does not fit, the arguments break one per line, and the statement comes out as in "wanted". That spike changes the layout of every long parameterized type (6 goldens), so it is not the fix.

How

Two narrow options, neither tried:

  • In visitTypeCast, lay the type out without a break after <, the way addTypeArguments does for ImmutableList.<Foo>of(). A cast's type is short by nature, and the first break in a cast is then the one before the expression.
  • Or give the cast level a LastLevelBreakability that ends the inline chain when the level does not start with a break (CHECK_INNER), and check what that does to the shallower shape above, which is right today.

Compatibility

It changes the output of that one shape: one statement in the 15,747 files of the JDK 21 sources. The README allows a bug fix to change the output in 2.x, and the Migrate page lists such fixes.

Done when

  • a golden with the getLog statement fails today and passes with the fix, and the shallower shape stays as it is;
  • a JDK 21 corpus run shows only that statement moving, and the release notes and the Migrate page say so.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions