pinterest / ktlint

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

ktlint sometimes breaks return statements in high order functions #2862

Open buchner opened 2 weeks ago

buchner commented 2 weeks ago

Expected Behavior

ktlint

Observed Behavior

redacted/MergeKt.kt:1:2: Function name should start with a lowercase letter (except factory methods) and use camel case (standard:function-naming)
redactedy/MergeKt.kt:4:12: Expected a blank line for this declaration (standard:blank-line-before-declaration)

Summary error count (descending) by rule:
  standard:blank-line-before-declaration: 1
  standard:function-naming: 1

Steps to Reproduce

minimal non working example file MergeKt.kt

package test.example

fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit {
    return fun T.() {
        this@merge(this)
        block(this)
    }
}

results in

package test.example

fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit {
    return

    fun T.() {
        this@merge(this)
        block(this)
    }
}

Your Environment

paul-dingemans commented 2 weeks ago

Thanks for reporting. This is clearly a bug. You can work around it as follows:

fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit =
    fun T.() {
        this@merge(this)
        block(this)
    }

or

@Suppress("ktlint:standard:function-expression-body")
fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit {
    return fun T.() {
        this@merge(this)
        block(this)
    }
}
buchner commented 2 weeks ago

Thank you for the fast response. The first workaround solves the problem for me that after formatting the code is not valid Kotlin anymore. It still results into reporting a format rule violation - the function name rule, as above.

The second workaround results for me in the same ktlint behavior as the minimal not working example.

paul-dingemans commented 2 weeks ago

Thank you for the fast response. The first workaround solves the problem for me that after formatting the code is not valid Kotlin anymore. It still results into reporting a format rule violation - the function name rule, as above.

The sample you have provided uses a valid function name. The message Function name should start with a lowercase letter (except factory methods) and use camel case can not be reproduced with this sample. But you can suppress it in a similar way as the rule that causes the compilation error.

In code sample below, I have modified the function name to be invalid:

$ ktlint-1.4.1 --stdin
17:25:22.591 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
fun <T> (T.() -> Unit).Merge(block: T.() -> Unit): T.() -> Unit =
    fun T.() {
        this@Merge(this)
        block(this)
    }
<stdin>:1:24: Function name should start with a lowercase letter (except factory methods) and use camel case (standard:function-naming)

Summary error count (descending) by rule:
  standard:function-naming: 1

With suppression, the violation is not reported:

$ ktlint-1.4.1 --stdin
17:28:45.023 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
@Suppress("ktlint:standard:function-naming")
fun <T> (T.() -> Unit).Merge(block: T.() -> Unit): T.() -> Unit =
    fun T.() {
        this@Merge(this)
        block(this)
    }
buchner commented 2 weeks ago

The sample you have provided uses a valid function name. That is what puzzles me. Ktlint reported in a file an invalid function name but I could not spot any while reading it thoroughly. I had to fall back to bisection to find which function was triggering it.

package test.example

fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit =
    fun T.() {
        this@merge(this)
        block(this)
    }

results for me in

/redacted/MergeKt.kt:1:2: Function name should start with a lowercase letter (except factory methods) and use camel case (standard:function-naming)

Summary error count (descending) by rule:
  standard:function-naming: 1

Adding the function naming suppression resolves it, despite the name - to my understanding - is already valid.

package test.example

@Suppress("ktlint:standard:function-naming")
fun <T> (T.() -> Unit).merge(block: T.() -> Unit): T.() -> Unit =
    fun T.() {
        this@merge(this)
        block(this)
    }

[...] Function name should start with a lowercase letter (except factory methods) and use camel case can not be reproduced with this sample.

Is there something environment related that could trigger the different behavior in my test environment? Such as that I have the function in a file?

paul-dingemans commented 2 weeks ago

Is there something environment related that could trigger the different behavior in my test environment? Such as that I have the function in a file?

Only possible suspects that I can think of is your .editorconfig, and/or your config of the ktlint-gradle plugin. But I can not think of any reason that those configs can result in this problem.