Handling of expression bodies for function definitions

[binkley]

Expected Behavior

I'd like ktlint to preserve line breaks in a function definition expression body. That is, allow fun foo... = to preserve the line break for the following function definition. This improves readability, and helps tools like grep.

I find this language in "Kotlin Coding Conventions" unclear:

If the function has an expression body whose first line doesn't fit on the same line as the declaration, put the = sign on the first line and indent the expression body by four spaces.

It is unclear that "doesn't fit on the same line" to mean "if it fits, join". I take the language to mean, "if it does not fit, break to a new line". Given the ambiguity of the language in the positive or negative sense, I understand that Ktlint chose to join when an expression body fits a function, however, I believe Ktlint should not join, but only break lines that exceed limits. I want reformatting to focus on readability and standards.

Note that with IntelliJ, if I add a break after the "=" and reformat, it preserves that break.

Observed Behavior

In this repo, https://github.com/binkley/kotlin-application-transactions. Last commit before updating to 3.0.0 from 2.0.0 of ktlint: https://github.com/binkley/kotlin-application-transactions/commit/715ca4fb53144e1008a1b70ede2638a460a0087a.

I'll suppress warnings in and push a new, poorly-formatted version after upgrading to ktlint 3.0.0 from 2.0.0, and addressing other concerns in upgrading dependencies. I'll try to be clear what commits are ktlint related.

Steps to Reproduce

The following original code:

            override fun hasNext(): Boolean =
                if (lookingAtSpillover) {
                val hasNext = spilloverItr.hasNext()
                if (hasNext) {
                    true
                } else {
                    lookingAtSpillover = false
                    hasNext()
                }
            } else {
                sharedItr.hasNext()
            }

was left alone by IntelliJ, but reformatted by Ktlint to:

            override fun hasNext(): Boolean = if (lookingAtSpillover) {
                val hasNext = spilloverItr.hasNext()
                if (hasNext) {
                    true
                } else {
                    lookingAtSpillover = false
                    hasNext()
                }
            } else {
                sharedItr.hasNext()
            }

Notice in particular the ugly alignment of the "else" block.

Your Environment

• Version of ktlint used: 3.0.0
• Relevant parts of the .editorconfig settings

  
  🌷 cat .editorconfig
  root = true

[**] end_of_line = lf max_line_length = 80

[*.{kt,kts}]

TODO: Decide if an Android mix makes more sense

ktlint_code_style = intellij_idea ktlint_standard_discouraged-comment-location = disabled

Oh, the irony. IntelliJ insists on sorting java* imports last

ktlint_standard_import-ordering = disabled ktlint_standard_trailing-comma-on-call-site = disabled ktlint_standard_trailing-comma-on-declaration-site = disabled

* Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task):
Maven:
```xml
🌷 grep ktlint pom.xml
        <ktlint-maven-plugin.version>3.0.0</ktlint-maven-plugin.version>
                <artifactId>ktlint-maven-plugin</artifactId>
                <version>${ktlint-maven-plugin.version}</version>

• Version of Gradle used (if applicable): n/a
• Operating System and version: I'm running Ubuntu under WSL:

  🌷 uname -a
  Linux Hobbiton 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

[paul-dingemans]

I find this language in "Kotlin Coding Conventions" unclear:

If the function has an expression body whose first line doesn't fit on the same line as the declaration, put the = sign on the first line and indent the expression body by four spaces.

It is unclear that "doesn't fit on the same line" to mean "if it fits, join". I take the language to mean, "if it does not fit, break to a new line". Given the ambiguity of the language in the positive or negative sense, I understand that Ktlint chose to join when an expression body fits a function, however, I believe Ktlint should not join, but only break lines that exceed limits. I want reformatting to focus on readability and standards.

Please see documentation function expression bodies. Maybe one of the other formats is more to your liking.

[binkley]

@paul-dingemans Fantastic beard ... but another conversation.

I would prefer default to be:

• If user put the start of expression body on the same line as function declaration, then wrap if it does not fit
• If the user put the start of expression body on a new line, then keep the line break

Based on my reading of "Kotlin Coding Conventions" (KCC), it seems the less pro-active position is to keep user newlines, and only wrap when the line exceeds limits.

Reading the documentation for Ktlint configuration on this:

• When set to default, the first line of a body expression is appended to the function signature as long as the max line length is not exceeded.
• When set to multiline, the body expression starts on a separate line in case it is a multiline expression. A single line body expression is wrapped only when it does not fit on the same line as the function signature.
• When set to always the body expression is always wrapped to a separate line.

The KCC documentation is unclear on handling all cases, so I respect that Ktlint had to implement something in face of that.

What I suggest is either change default, or add a new more minimalist policy: If user broke the line, respect that unless it violates other KCC guidelines.

For my code, I prefer:

• If the body expression and function declaration fits on one line, Ktlint joins it
• If the body expression and function declaration do not fit on one line, Ktlint uses the line breaks provided in the source

My preference does not precisely match my suggestion. I find IntelliJ does what I'm looking for, but then when I use Ktlint in my build, it reformats.

[paul-dingemans]

Changing default values is merely impossible as it will lead to other issues in which other developers will ask for a different format.

I would prefer default to be:

If user put the start of expression body on the same line as function declaration, then wrap if it does not fit If the user put the start of expression body on a new line, then keep the line break

I think this exactly matches with the default (multiline) behavior in ktlint_official code style. This code has set this as default because it is consistent with rule multiline-expression-wrapping which does the same for other situation like variable declarations.

Given unformatted code below:

fun foo1() = "short single line"
fun foo2(a: String) = "short single line"
fun foo3(a: String, b: String) = "short single line"
fun foox4() = """
    raw string (multiline)
""".trimIndent()

This is formatted by ktlint_official to:

fun foo1() = "short single line"

fun foo2(a: String) = "short single line"

fun foo3(
    a: String,
    b: String,
) = "short single line"

fun foox4() =
    """
    raw string (multiline)
    """.trimIndent()

The other (legacy) code styles have the default set to default which is more in line with the behavior of the default IntelliJ IDEA formatter. In my opinion that style is really flawed in case you have a long signature and short first line of the expression body. If my editor window is smaller than the max line length, I could see something like:

fun someVeryLongggggggggggggggggggggggggggggggggggggFunctionName(foo: String | <- End of editor screen
    bar(foo)
} else {
   doSomething()
}

Without scrolling to the right, I don't know what the condition is of this if statement.

add a new more minimalist policy: If user broke the line, respect that unless it violates other KCC guidelines.

The problem with this, is that it is not consistent. It will leave room for debate between developers (considering a multi person project) whether a line break is needed or not.

[binkley]

@paul-dingemans Thank you for explaining this in more depth. I'll try out more options from ktlint.