Wrap mulitline binary expression in condition on || and &&

[paul-dingemans]

Given a multiline binary expression in an condition then wrap the right hand side of each binary expression to a new line:

if (foo || (bar && true) ||
    false
) { ... }

while (foo || (bar && true) ||
    false
    ) { ... }

val foo =
    foo || (bar && true) ||
    false

should be formatted as:

if (foo ||
    (bar && true) ||
    false
) { ... }

while (foo ||
    (bar && true) ||
    false
    ) { ... }

val foo =
    foo ||
    (bar && true) ||
    false

This has to be done recursively per binary expression. So:

if (foo || (bar &&
    true || foobar) || false
) { ... }

is formatted as:

    if (foo ||
        (
            bar &&
                true ||
            foobar
        ) ||
        false
) { ... }

[atulgpt]

Hi @paul-dingemans why not below as the correct format

if (
    foo ||
    (bar && true) ||
    false
) { ... }

while (
    foo ||
    (bar && true) ||
    false
) { ... }

val foo =
    foo ||
    (bar && true) ||
    false

Also in your if and while closing braces are not indented at the same level. Is this rule lie in ktlint_official or android?

[paul-dingemans]

This rule is primarily about wrapping at the && and || operators, so inside the binary expression.

In case of the while and do ... while expressions and a multiline condition your proposal indeed looks better but it should be adressed in a different rule.

For the case of the if expression, I see no (or at least less) value in wrapping the entire multiline expression and leaving if ( orphaned. But, I do admit that with other indent sizes than 4, it is more comparable to the while statement. There are other examples in ktlint where the indent_size of 4 spaces is used to allow special spacing. For example a function with a multiline parameter list and no return type is formatted as:

fun foo(
   ...
) = listOf(1, 2, 3)
    .filter { it > 2 }

| Also in your if and while closing braces are not indented at the same level. Is this rule lie in ktlint_official or android?

No it is not. { ... } is just an abbreviation for a body which is meaningless for the example.

[atulgpt]

No it is not. { ... } is just an abbreviation for a body which is meaningless for the example.

No, I am talking bout ) this at while example

[paul-dingemans]

Also in your if and while closing braces are not indented at the same level. Is this rule lie in ktlint_official or android?

No it is not. { ... } is just an abbreviation for a body which is meaningless for the example.

No, I am talking bout ) this at while example

Ok, if you mean closing parenthesis instead of braces then the answer is different. From Kotlin Coding conventions:

If the condition of an if or when statement is multiline, always use curly braces around the body of the statement. Indent each subsequent line of the condition by four spaces relative to the statement start. Put the closing parentheses of the condition together with the opening curly brace on a separate line:

if (!component.isSyncing &&
    !hasAnyKotlinRuntimeInScope(module)
) {
    return createKotlinNotConfiguredPanel(module)
}

[atulgpt]

Hi, @paul-dingemans if this is available I can pick this up

[eygraber]

A lot of times I'll have multiline expressions where the || is on a newline, but the && is not to aid in understanding the precedence (() is flagged as unnecessary, which is correct since it should only be used in cases of ambiguity).

Could an exception be made to the rule to allow this form:

    require(
      light.isSpecified && dark.isSpecified ||
      light.isUnspecified && dark.isUnspecified
    )

[paul-dingemans]

A lot of times I'll have multiline expressions where the || is on a newline, but the && is not to aid in understanding the precedence (() is flagged as unnecessary, which is correct since it should only be used in cases of ambiguity).

I do not understand this sentence. I am not a native speaker and I have the feeling that I miss some nuance here.

When running the wrapping rule, the result below is less clear than the original:

    require(
        bool1 &&
            bool2 ||
            bool3 &&
            bool4,
    )

Code above could be written as below, which is not ambigue and will be accepted by ktlint as long as each subexpression fits on a single line:

    require(
        (bool1 && bool2) ||
            (bool3 && bool4)
    )

Above way of writing is that it makes explicit which sub-expressions are related. Also, it makes the code more refactor-proof. Suppose that bool1 is refactored to something so long that (bool1 && bool2) no longer fits on a single line, then this will result in code below:

    require(
        (
            boolWhichGotVeryLongDueToSomeRefactoringSomewhereElseboolWhichGotVeryLongDueToSomeRefactoringSomewhereElse &&
                boolWhichGotVeryLongDueToSomeRefactoringSomewhereElse2
        ) ||
            (bool3 && bool4),
    )

[eygraber]

I am using other static checkers (e.g. detekt) which would flag this:

    require(
        (bool1 && bool2) ||
            (bool3 && bool4)
    )

as using unnecessary parentheses. I believe Intellij would do this as well.

detekt and Intellij are correctly flagging that, since there's no need to use parentheses here; && takes precedence over ||

[paul-dingemans]

detekt and Intellij are correctly flagging that, since there's no need to use parentheses here; && takes precedence over ||

IntelliJ does not flag this by default. You are right about the precedence, but I think that the parentheses makes the code clearer and easier to read. You have also added a newline between the pairs of booleans for clarity. When written as single line, it just becomes harder to read when no parenthesis are used:

// Harder to read when on single line
require(
        bool1 && bool2 || bool3 && bool4
)

// Easy to read even when on single line
require(
        (bool1 && bool2) || (bool3 && bool4)
)