pinterest / ktlint

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

`ktlint_function_signature_rule_allow_multiline_when_parameter_count_greater_or_equal_than` #2483

Closed terminalnode closed 6 months ago

terminalnode commented 6 months ago

Expected Rule behavior

This is sort of in response to a change in the function signature rule. This rule has a companion rule called ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than.

If this is set to 2, then you have to use multiple lines when having 2 or more parameters in your function signature. Before ktlint 1.0, it didn't care how you formatted it when you had 1 parameter, but after ktlint 1.0 it insists that you have them on a single line.

I propose adding another optional rule ktlint_function_signature_rule_allow_multiline_when_parameter_count_greater_or_equal_than, which sets the lower limit for when you may use multiple lines. I could also see a case for simplifying it with a ktlint_function_signature_rule_force_single_line = [enabled|disabled].

Motivation

The default is reasonable because it's consistent, places where multi-line and single-line params are allowed have no overlaps (I think). So why would I want to allow this overlap?

If we consider a very simple function: fun f(x: String) = println(x), breaking this up to multiple lines is a bit silly. The visual and cognitive load on this line is very low, and if anything breaking it up to multiple lines will only make it more complicated.

Then consider a more complex function, that nontheless is less than 80 chars:

fun <T1 : Generic1, T2: Generic2> String.mySillyFunction(x: String -> T1): T2

This is much easier to read if split up on multiple lines:

fun <T1 : Generic1, T2: Generic2> String.mySillyFunction(
  x: (String) -> T1
): T2

It's still not great, but I'd argue that it's much better. Even if we made it a bit less silly:

fun <T> String.lessSillyFunction(block: (String) -> T): T

fun <T> String.lessSillyFunction(
  block: (String) -> T,
): T

This is in my opinion an improvement over a single line because of the lambda parameter, which is basically a function signature in my function signature.

Example

ktlint_function_signature_rule_allow_multiline_when_parameter_count_greater_or_equal_than = 1
ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 2

Allowed:

// Single line is allowed because params < 2
fun f(x: Int) = TODO()

// Multi-line is allowed because params >= 1
fun f(
  x: Int,
) = TODO()

// Multi-line is allowed because params >= 1, and forced because params >= 2
fun f(
  x: Int,
  y: Int,
) = TODO()

Disallowed:

// Multi-line is forced because params >= 2
fun f(x: Int, y: Int) = TODO()

Additional information

paul-dingemans commented 6 months ago

This is sort of in response to a change in the function signature rule. This rule has a companion rule called ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than.

If this is set to 2, then you have to use multiple lines when having 2 or more parameters in your function signature. Before ktlint 1.0, it didn't care how you formatted it when you had 1 parameter, but after ktlint 1.0 it insists that you have them on a single line.

Just to be sure, do you mean with Before ktlint 1.0 the 0.x version of ktlint? If so, did you notice that the default code style of ktlint has changed from intellij_idea to ktlint_official? As a result of this change, the default value for parameter ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than changed from unset to 2. So if you want to stick to "old" behavior of 0.x version combined with ktlint_official code style, you might want to unset this property in your .editorconfig.

I do not like the idea of having yet another configuration setting (reason) as it further complicates this already complex rule. I am not aware of any change in the function signature rule related to the ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than in recent versions of ktlint. So if it worked in the past with 0.50.0 version, you should be able to get it working by configuring the as mentioned before.

terminalnode commented 6 months ago

Yes, 0.x version.

While I did not notice the default code style change, I already had this parameter set to 2. And it worked as it would've worked with my proposed ...allow_multiline... rule set to 1. That is, we were allowed to use multiline or single line for functions with a single parameter, but when we had two or more we were forced to use multiline.

I can see the reasoning behind not adding more configurations to the rule, and as mentioned in the motivation section the current default is reasonable if ones priority is to only ever allow one way of formatting. But if that's not ones priority, the limitation can feel very strange. I've always viewed it as generally recommended to use multiple lines, but only enforced after reaching X params.

Could be that changing code style to intellij_idea will fix the issue, but isn't the code style just a lot of defaults? Shouldn't this behavior still be possible to disable?

terminalnode commented 6 months ago

Changing code style to intellij_idea fixed the issue for me, so I'm closing this issue. 🙂

paul-dingemans commented 6 months ago

Yes, 0.x version.

The rule has another configuration setting for which the default value in the ktlint_official code style is different from intellij_idea code style. See https://pinterest.github.io/ktlint/latest/rules/configuration-ktlint/#wrapping-the-expression-body-of-a-function. Setting that property would have solved the problem as well.