Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter wrapping should take priority over expression wrapping #2454

Closed
mklnwt opened this issue Dec 21, 2023 · 16 comments
Closed

Parameter wrapping should take priority over expression wrapping #2454

mklnwt opened this issue Dec 21, 2023 · 16 comments

Comments

@mklnwt
Copy link

mklnwt commented Dec 21, 2023

function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than should take priority over function_signature_body_expression_wrapping.

This can save horizontal space at the cost of 1 line vertical space.

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo = Foo.baz(a = "looooooooooooooooooooooooooooooooooooong", b = "looooooooooooooooooooooooooooooooooooong")

Expected Behavior

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(
    bar: String = "loooooooooong",
): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

Current Behavior

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo =
    Foo.baz(
        a = "looooooooooooooooooooooooooooooooooooong",
        b = "looooooooooooooooooooooooooooooooooooong",
    )

Additional error:

Exceeded max line length (?) (cannot be auto-corrected) (standard:max-line-length)

Additional information

  • Current version of ktlint: 1.1.0
@paul-dingemans
Copy link
Collaborator

This can be achieved by setting ktlint_function_signature_body_expression_wrapping. See https://pinterest.github.io/ktlint/latest/rules/configuration-ktlint/#wrapping-the-expression-body-of-a-function

@mklnwt
Copy link
Author

mklnwt commented Dec 30, 2023

No setting of ktlint_function_signature_body_expression_wrapping achieves the expected result.

To clarify. For always it would make sense to first wrap the expression body.

For default (and probably multiline as well) it would make sense to let function_signature (including function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than) check the line length with expression body and wrap it if necessary.

Given this input:

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(
    bar: String = "loooooooooong",
): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

The parameter is joined first.

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

Then the expression is wrapped since it doesn't fit.

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo =
    Foo.baz(
        a = "looooooooooooooooooooooooooooooooooooong",
        b = "looooooooooooooooooooooooooooooooooooong",
    )

Expected would be that the given should stay the same since joining it would make the line too long.

The same way the following should wrap the parameter before wrapping the expression body.

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo = Foo.baz(a = "looooooooooooooooooooooooooooooooooooong", b = "looooooooooooooooooooooooooooooooooooong")

@paul-dingemans
Copy link
Collaborator

Let me investigate this once more.

@paul-dingemans
Copy link
Collaborator

Can you double check that you are using the configuration settings correctly? I see in this post several references to settings which are missing the ktlint_ prefix.

Given .editorconfig:

root = true

[*.{kt,kts}]
ktlint_code_style = ktlint_official
ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 1
ktlint_function_signature_body_expression_wrapping = default
max_line_length = 53
ktlint_standard = enabled
ktlint_experimental = enabled

the code below:

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "loooooooooong"): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

is formatted as:

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(
    bar: String = "loooooooooong",
): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

@mklnwt
Copy link
Author

mklnwt commented Jan 7, 2024

I have not set ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 1 and left it with the default of 2.

Imo it should still prefer to break the argument over breaking the expression since the line is too long.

@paul-dingemans
Copy link
Collaborator

On response to my suggestion:

This can be achieved by setting ktlint_function_signature_body_expression_wrapping. See https://pinterest.github.io/ktlint/latest/rules/configuration-ktlint/#wrapping-the-expression-body-of-a-function

you said that:

No setting of ktlint_function_signature_body_expression_wrapping achieves the expected result.

You can achieve what you want, by setting it to 1.

@mklnwt
Copy link
Author

mklnwt commented Jan 7, 2024

I'm trying to say that it should be wrapped based on line length, not because of the parameter number.

With the default setting (2) both of the following should be correctly formatted.

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(
    bar: String = "loooooooooong",
): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)
// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(bar: String = "short"): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

@paul-dingemans
Copy link
Collaborator

Please provide your .editorconfig settings.

When I use:

root = true

[*.{kt,kts}]
ktlint_code_style = intellij_idea
max_line_length = 53
ktlint_standard = enabled
ktlint_experimental = enabled

code is formatted as you requested. Note that max_line_length must be set explicitly when using code style intellij_idea.

@mklnwt
Copy link
Author

mklnwt commented Jan 9, 2024

Minimal example .editorconfig:

root = true

[*.{kt,kts}]
max_line_length = 53
ktlint_code_style = ktlint_official
ktlint_standard = enabled
ktlint_experimental = enabled

@paul-dingemans
Copy link
Collaborator

You need to overwrite ktlint_function_signature_body_expression_wrapping.

root = true

[*.{kt,kts}]
max_line_length = 53
ktlint_code_style = ktlint_official
ktlint_standard = enabled
ktlint_experimental = enabled
ktlint_function_signature_body_expression_wrapping = default

@mklnwt
Copy link
Author

mklnwt commented Jan 9, 2024

Actually while producing a minimal example I might've accidentally omitted that part and also forgot another crucial overwrite,
Here is a complete picture that I'm now running on 1.1.1:

[*.{kt,kts}]
max_line_length = 53
ktlint_standard = enabled
ktlint_experimental = enabled
ktlint_code_style = ktlint_official
ktlint_function_signature_body_expression_wrapping = default
ktlint_standard_multiline-expression-wrapping = disabled
ktlint_standard_string-template-indent = disabled # depends on multiline-expression-wrapping

Still, my examples show violations:

// Assume that the last allowed character is
// at the X character on the right                  X
fun foo(
    bar: String = "loooooooooong",
): Foo = Foo.baz(
    a = "looooooooooooooooooooooooooooooooooooong",
    b = "looooooooooooooooooooooooooooooooooooong",
)

4:5 No whitespace expected between opening parenthesis and first parameter name (standard:function-signature)
4:35 No whitespace expected between last parameter and closing parenthesis (standard:function-signature)
5:10 Newline expected before expression body (standard:function-signature)

@paul-dingemans
Copy link
Collaborator

I still can not reproduce this. With .editorconfig and code sample above, I get following:

17:56:26.944 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
src/main/kotlin/Foo.kt:5:54: Exceeded max line length (53) (standard:max-line-length)
src/main/kotlin/Foo.kt:6:54: Exceeded max line length (53) (standard:max-line-length)

Summary error count (descending) by rule:
  standard:max-line-length: 2

But, I do note that your .editorconfig is missing line below which should be at the top of the file:

root = true

Without this property, you might actually pick-up an .editorconfig file at a higher level in your directory structure.

@mklnwt
Copy link
Author

mklnwt commented Jan 10, 2024

I tried it with and without root.

Did you run check or format?
With format I get the same error as you, but the code is reformatted as stated above.

fun foo(bar: String = "loooooooooong"): Foo =
    Foo.baz(
        a = "looooooooooooooooooooooooooooooooooooong",
        b = "looooooooooooooooooooooooooooooooooooong",
    )

@paul-dingemans
Copy link
Collaborator

With format I get the same error as you, but the code is reformatted as stated above.

Did you mean "below" instead of "above"?

Please try to be more explicit, because I getting confused and need to guess. So please, specify:

  • (full) editorconfig
  • Input
  • Output
  • Expected output

@mklnwt
Copy link
Author

mklnwt commented Jan 10, 2024

Sorry for the confusion. I wrote tests instead.

If ktlint_function_signature_body_expression_wrapping is set to multiline or always, everything works as expected.
If it is set to default, argument-list-wrapping should wrap regardless of the value of ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than, because the remainder after = doesn't fit into the line.

After trying a few more examples, though, I think my expected cases would cause too much complexity. For this to work argument-list-wrapping & function_signature would have to interact with each other.

I think it's better to leave the issue closed, but for completeness sake and future reference I'll leave the expectation and test cases here.

Ideally both of the following would be allowed:

fun foo(
    bar: String
): Foo = Foo.baz(
    a = "looooooooooooooooooooong",
    b = "looooooooooooooooooooong"
)

fun foo(bar: String): String =
    Foo.baz(a = "a", b = "b")

.editorconfig

root = true

[*.{kt,kts}]
max_line_length = 35
ktlint_standard = enabled
ktlint_function_signature_body_expression_wrapping = default
ktlint_standard_multiline-expression-wrapping = disabled
ktlint_standard_string-template-indent = disabled # depends on multiline-expression-wrapping

Test cases

class ExampleTest {
    private val argumentListWrappingRuleAssertThat =
        assertThatRuleBuilder { ArgumentListWrappingRule() }
            .addRequiredRuleProviderDependenciesFrom(StandardRuleSetProvider())
            .assertThat()

    @Test
    fun `Expected signature_body_expression_wrapping=default (unformatted)`() {
        val code =
            """
            fun foo(bar: String): Foo = Foo.baz(a = "looooooooooooooooooooong", b = "looooooooooooooooooooong")
            """.trimIndent()
        val formattedCode =
            """
            fun foo(
                bar: String
            ): Foo = Foo.baz(
                a = "looooooooooooooooooooong",
                b = "looooooooooooooooooooong"
            )
            """.trimIndent()
        argumentListWrappingRuleAssertThat(code)
            .addAdditionalRuleProviders(
                { FunctionSignatureRule() },
                { IndentationRule() },
            ).withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 35)
            .withEditorConfigOverride(FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY to "default")
            .isFormattedAs(formattedCode)
    }

    @Test
    fun `Expected signature_body_expression_wrapping=default (pre-formatted)`() {
        val code =
            """
            fun foo(
                bar: String
            ): Foo = Foo.baz(
                a = "looooooooooooooooooooong",
                b = "looooooooooooooooooooong"
            )
            """.trimIndent()
        argumentListWrappingRuleAssertThat(code)
            .addAdditionalRuleProviders(
                { FunctionSignatureRule() },
                { IndentationRule() },
            ).withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 35)
            .withEditorConfigOverride(FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY to "default")
            .hasNoLintViolations()
    }

    @Test
    fun `Current signature_body_expression_wrapping=default (pre-formatted)`() {
        val code =
            """
            fun foo(
                bar: String
            ): Foo = Foo.baz(
                a = "looooooooooooooooooooong",
                b = "looooooooooooooooooooong"
            )
            """.trimIndent()
        val formattedCode =
            """
            fun foo(bar: String): Foo =
                Foo.baz(
                    a = "looooooooooooooooooooong",
                    b = "looooooooooooooooooooong"
                )
            """.trimIndent()
        argumentListWrappingRuleAssertThat(code)
            .addAdditionalRuleProviders(
                { FunctionSignatureRule() },
                { IndentationRule() },
            ).withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 35)
            .withEditorConfigOverride(FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY to "default")
            .hasLintViolationsForAdditionalRule(
                LintViolation(2, 5, "No whitespace expected between opening parenthesis and first parameter name", true),
                LintViolation(2, 16, "No whitespace expected between last parameter and closing parenthesis", true),
                LintViolation(3, 10, "Newline expected before expression body", true),
            ).isFormattedAs(formattedCode)
    }

    @Test
    fun `Current & Expected signature_body_expression_wrapping=multiline`() {
        val code =
            """
            fun foo(
                bar: String
            ): Foo = Foo.baz(
                a = "looooooooooooooooooooong",
                b = "looooooooooooooooooooong"
            )
            """.trimIndent()
        val formattedCode =
            """
            fun foo(bar: String): Foo =
                Foo.baz(
                    a = "looooooooooooooooooooong",
                    b = "looooooooooooooooooooong"
                )
            """.trimIndent()
        argumentListWrappingRuleAssertThat(code)
            .addAdditionalRuleProviders(
                { FunctionSignatureRule() },
                { IndentationRule() },
            ).withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 35)
            .withEditorConfigOverride(FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY to "multiline")
            .hasLintViolationsForAdditionalRule(
                LintViolation(2, 5, "No whitespace expected between opening parenthesis and first parameter name", true),
                LintViolation(2, 16, "No whitespace expected between last parameter and closing parenthesis", true),
                LintViolation(3, 10, "Newline expected before expression body", true),
            ).isFormattedAs(formattedCode)
    }

    @Test
    fun `Current & Expected where argument-list-wrapping would lead to additional breaks signature_body_expression_wrapping=default (pre-formatted)`() {
        val code =
            """
            fun foo(bar: String): String =
                Foo.baz(a = "a", b = "b")
            """.trimIndent()
        argumentListWrappingRuleAssertThat(code)
            .addAdditionalRuleProviders(
                { FunctionSignatureRule() },
                { IndentationRule() },
            ).withEditorConfigOverride(MAX_LINE_LENGTH_PROPERTY to 35)
            .withEditorConfigOverride(FUNCTION_BODY_EXPRESSION_WRAPPING_PROPERTY to "default")
            .hasNoLintViolations()
    }
}

@paul-dingemans
Copy link
Collaborator

It took a while before I had a chance to look at this again. It is not that difficult to achieve now you already provided as set of tests that would statisfy you. The argument-list-wrapping rule already had a dependency on the class-signature rule for more or less the same reason. This has now been added for the function-signature rule as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants