Long expression inside parentheses causes only trailing ) to be on new line

[att14]

If a conditional line is just over the max-line length, the ) { will appear on the next line.

        if (someReallyLongVariableName == null && someOtherReallyLongVariableName == null && fooBar) {
            TODO()
        }

Will be reformatted as:

        if (someReallyLongVariableName == null && someOtherReallyLongVariableName == null && fooBar
        ) {
            TODO()
        }

If you add some more to the expression, the line gets broken up.

        if (someReallyLongVariableName == null &&
                someOtherReallyLongVariableName == null &&
                fooBar == null
        ) {
            TODO()
        }

It seems like in the first example, it should consider the entire line too long and break up the expression, regardless of if moving the parenthesis to the next line will be short enough.

        if (someReallyLongVariableName == null &&
                someOtherReallyLongVariableName == null &&
                fooBar
        ) {
            TODO()
        }

Maybe this is the expected behavior, but I looked for other issues and couldn't find anything related, so I thought I'd bring it up.

Personally, I'd always put the expression on a separate line when splitting.

        if (
            someReallyLongVariableName == null && someOtherReallyLongVariableName == null && fooBar
        ) {
            TODO()
        }

        if (
            someReallyLongVariableName == null &&
                someOtherReallyLongVariableName == null &&
                fooBar == null
        ) {
            TODO()
        }

But, I know this isn't about my personal style, and I still feel like only the ) { on the next line is a little strange.

[nreid260]

I suspect this is happening because the condition expressions are in their own level, within the paren-level. The layout engine determines that condition-level can fit on one line, but is then forced to take the next break, before the closing paren.

One easy solution would be to unify the breaks after the opening paren and before the closing paren.

[strulovich]

This looks like an issue with Google style only. @nreid260, do you want to deal with that?

Looks like this is the place to add another break:
https://github.com/facebookincubator/ktfmt/blob/bd2e5c73c94673991aa5a34090337c92e650197d/core/src/main/java/com/facebook/ktfmt/format/KotlinInputAstVisitor.kt#L1779-L1785

You might want to check it against your code to see it doesn't create anything funny looking.

[nreid260]

I don't think this is --google-style related. I can see the reported behaviour in https://facebookincubator.github.io/ktfmt/ using the following code.

if (someReallyLongVariableName == null && someOtherReallyLongVariableName == null && fooBaaaaaaaaaar) {
    TODO()
}

That said, it may help to make the unified break guarded by --google-style into a default behaviour.

[strulovich]

We use the "Java-like" option on that page, which makes it break as:

    if (someReallyLongVariableName == null &&
        someOtherReallyLongVariableName == null &&
        fooaaaaar) {
      TODO()
    }

[nreid260]

Ahh ok yeah I can see it now. Without the unified break introduced by --google-style, Java-like is forced to backtrack and find a break in the condition expression. That's how you get the multiline formatting.

I think we'd support either adding a break inside the opening paren, or removing the one before the closing paren. I'll have to confirm.

[att14]

I see this for Kotlin, as well as Google, style.

[strulovich]

@att14, what do you mean by "Kotlin style"?

[att14]

[JavierSegoviaCordoba]

@att14, what do you mean by "Kotlin style"?

kotlinlang, I think I have seen it too.

[JavierSegoviaCordoba]

If the fix is only for Google one, this should be reopened because it is happening in Kotlinlang too

[att14]

@JavierSegoviaCordoba this appears to be fixed for me with Kotlinlang style.

[JavierSegoviaCordoba]

yep, it is working for me too