pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.06k stars 504 forks source link

Split `multiline-if-else` into two rules #2648

Closed FloEdelmann closed 2 months ago

FloEdelmann commented 2 months ago

The multiline-if-else rule (https://pinterest.github.io/ktlint/latest/rules/standard/#multiline-if-else) currently disallows both styles:

if (foo)
    bar
else
    baz
if (foo) bar
else baz

and suggests adding braces:

if (foo) {
    bar
} else {
    baz
}

While the first one is properly documented, the second one is more debatable (see also https://github.com/streetcomplete/StreetComplete/commit/7f232f174fbd560ece7965f447748b7a64994bfc). The Kotlin coding conventions are not clear about what "multiline" really means.

I suggest splitting the rule into two (but I can't think of good names right now), so that the second style can be allowed separately in a project.

paul-dingemans commented 2 months ago

The Kotlin coding conventions are not clear about what "multiline" really means.

That is true. Ktlint has a rather simplistic definition for it. If a construct fits on one single line than it is a single line construct. Otherwise it is a multiline construct. The multiline-if-else handles the if-else construct. This means that the construct below is a multiline construct:

if (foo) bar
else baz

Consistent formatting is one the main characteristics of ktlint.

Splitting the rule most likely will lead to less consistent code, or to follow-up issues unless the boundaries can be made 100% clear. I see at least two possible problems for now.

Your proposal does not make clear whether if (foo) bar and else baz are single line constructs or that it applies to body containing only one statement, which possibly could be a multiline statement:

if (foo) bar
    .filterNotNull()
else baz

But assuming that if (foo) bar and else baz are single line constructs, then the next problem will be the code below:

if (foo) bar
else if (foo2) baz2
else baz3

From a developer perspective those are three single line constructs. But from the Kotlin parser perspective the body of the else statement is a multiline construct:

if (foo2) baz2
else baz3

Summarizing, from the Ktlint perspective the rule multiline-if-else is doing a single thing: "formatting a multiline if-else construct". As you already remarked in the originating issue, the rule can be disabled if you strongely disagree with it.

FloEdelmann commented 2 months ago

Thanks for your explanation and consideration! I agree that the simple definition is probably the easiest and least controversial, and thus the best one for Ktlint. I'll keep the rule disabled in the linked project then.