pinterest / ktlint

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

Wrong indentation for lambdas with many parameters #2816

Closed ln-12 closed 1 month ago

ln-12 commented 1 month ago

The parameter list of a multiline lambda does not have the correct indentation. The corresponding unit test seems to do it wrong on purpose: https://github.com/pinterest/ktlint/blob/master/ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/rules/IndentationRuleTest.kt#L2607

Expected Behavior

Consider the last code block from https://kotlinlang.org/docs/coding-conventions.html#lambdas:

foo {
   context: Context,
   environment: Env
   ->
   context.configureEnv(environment)
}

The lambda parameters have the same indentation as the arrow and the following statement.

Observed Behavior

When turning on the Wrapping rule with autoCorrect enabled via detekt, the code is formatted like this:

foo {
        context: Context,
        environment: Env,
    ->
    context.configureEnv(environment)
}

Steps to Reproduce

Enable the Wrapping rule and let the check run for the following code:

class Env
class Context {
    fun configureEnv(env: Env) {}
}
fun foo(action: (context: Context, environment: Env) -> Unit) {}

fun bar() {
    foo {
        context: Context,
        environment: Env,
        ->
        context.configureEnv(environment)
    }
}

Your Environment

paul-dingemans commented 1 month ago

You're right that this contradicts the Kotlin style guide.

Reasoning is that when the parameters and arrows are on the same line, the distinction between parameters and body becomes blurry.

fun bar() {
    foo {
        context: Context, environment: Env, ->
        context.configureEnv(environment)
    }
}
ln-12 commented 1 month ago

I see, thanks for the fast reply!

Do you think that you could add a flag to toggle that behavior? I strongly prefer the official way and always add a blank line after the parameter list.

paul-dingemans commented 1 month ago

My default reply to toggles is: https://pinterest.github.io/ktlint/latest/faq/#can-a-new-toggle-be-added-to-optionally-enabledisable-format-code-in-a-particular-way

This case however is non-standard as the default behavior is not compliant with code style. But, if implemented, the default for ktlint_official code style is not likely to be changed. For code style intellij-idea it seems logical to change the default behavior to be compliant so that it does not contradict default IDEA formatting.

ln-12 commented 1 month ago

Ok, I see. That sounds like a reasonable way to go as far as I understand it. With that I could still switch it like with other flags to both options, independent of the selected code style, right? Then I would definitely support that idea.

paul-dingemans commented 1 month ago

This case however is non-standard as the default behavior is not compliant with code style. But, if implemented, the default for ktlint_official code style is not likely to be changed. For code style intellij-idea it seems logical to change the default behavior to be compliant so that it does not contradict default IDEA formatting.

It should just be the other way around. The current formatting complies with the default formatting of Intellij IDEA. For code styles android_studio and intellij_idea it has always been an important requirement that the code formatted by ktlint should not conflict with the default formatting of the IDEA (see #1247). In code style ktlint_official this requirement has been dropped. So this code style should actually enforce the formatting below:

fun bar() {
    foo {
        context: Context,
        environment: Env,
        ->
        context.configureEnv(environment)
    }
}

I will not introduce a new configurable toggle for this, but enforce it based on the code style.