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

Indentation of inheritance with parameters #2661

Closed
mklnwt opened this issue May 18, 2024 · 7 comments
Closed

Indentation of inheritance with parameters #2661

mklnwt opened this issue May 18, 2024 · 7 comments

Comments

@mklnwt
Copy link

mklnwt commented May 18, 2024

Indentation for inheritance implementation does not look correct.

For example

class Bar(
    baz: String,
) : Foo(
        baz = baz,
    ) {
    fun qux() { /* ... */ }
}

Expected Behavior

@Test
fun `Given a class with inheritance`() {
    val code =
        """
        class Bar(
            baz: String,
        ) : Foo(
            baz = baz,
        )
        """.trimIndent()
    indentationRuleAssertThat(code).hasNoLintViolations()
}

Current Behavior

@Test
fun `Given a class with inheritance`() {
    val code =
        """
        class Bar(
            baz: String,
        ) : Foo(
                baz = baz,
            )
        """.trimIndent()
    indentationRuleAssertThat(code).hasNoLintViolations()
}

Additional information

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

Code style ktlint_official takes a different perspective than you might be used. From a consistency viewpoint the new format allows better formatting in case the class extends a super class having arguments as well as some additional interfaces. Code below is consistently formatted:

class Foo(
    foo1: Foo1,
    foo2: Foo2,
) : Foobar1(
        "foobar1",
        "foobar2",
    ),
    Foobar2,
    Foobar3 {
    fun foo() = "foo"
}

The super class Foobar1 and interfaces Foobar2 and Foobar3 are all aligned without the closing parenthesis and comma of Foobar1 disturbing the formatting.

With the other code styles, the old way of formatting is still available:

class Foo(
    foo1: Foo1,
    foo2: Foo2
) : Foobar1(
    "foobar1",
    "foobar2"
),
    FooBar2,
    FooBar3 {
    fun foo() = "foo"
}

Code style ktlint_official will always choose the most consistent formatting possible. If might take some time to get used to.

@mklnwt
Copy link
Author

mklnwt commented May 20, 2024

Considering that function calls look like this:

foos.map {
    it + 1
}.bar()
    .baz()

or

foo(
    bar = "bar",
).bar()
    .baz()

I would say that the inheritance example is not fully consistent.

It becomes even weirder. This is the formatted behavior when breaking the first inheritance:

class Foo(
    foo1: Int,
    foo2: Int,
) :
    Foobar1(
            a = 1,
            b = 2,
        ),
        Foobar2,
        Foobar3 {
    fun foo() = "foo"
}

Otherwise is it possible to use ktlint_official, but switch this specific rule part to the old style?

@paul-dingemans
Copy link
Collaborator

I think we have had discussions like this before. You have to disclose your .editorconfig settings. Above is most likely the result of disabling certains rules or use non-default settings.

With .editorconfig below, the code above is not accepted, and it is formatted consistently.

root = true

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

Otherwise is it possible to use ktlint_official, but switch this specific rule part to the old style?

No that is not supported, and will not be supported.

@mklnwt
Copy link
Author

mklnwt commented May 24, 2024

@paul-dingemans I have mostly worked with the unit test provided above.

I experimented a bit locally and it seems that a missing experimental rule is the reason the join to the previous line is not enforced.

I still believe that the indentation rule by itself (deactivated Expected single space before the first super type (standard:class-signature)) should not produce the example above.

This config for 1.2.1:

root = true

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

@paul-dingemans
Copy link
Collaborator

I experimented a bit locally and it seems that a missing experimental rule is the reason the join to the previous line is not enforced.

Ah good remark. Indeed the chain-method-continuation rule is crucial for the "correct" formatting. This can be easily found, using Ktlint CLI:

$ ktlint-1.2.1 --stdin
16:19:58.278 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
val foos =
    foos.map {
        it + 1
    }.bar()
    .baz()
<stdin>:2:9: Expected newline before '.' (standard:chain-method-continuation)
<stdin>:5:1: Unexpected indentation (4) (should be 8) (standard:indent)

Summary error count (descending) by rule:
  standard:chain-method-continuation: 1
  standard:indent: 1

The chain-method-continuation becomes a non-experimental rule in the upcoming release. The code above will then be formatted as:

val foos =
    foos
        .map {
            it + 1
        }.bar()
        .baz()

Your second example is:

foo(
    bar = "bar",
).bar()
    .baz()

I have no idea how this should be written more nicely, given that the arguments of foo are to be wrapped to separate lines.

For shorter chains, the following is accepted by ktlint and might be a workaround:

val foos =
    foo(
        bar = "bar",
    ).bar().baz()

Another, but worse workaround, is to write it like:

val foos =
    foo(
        bar = "bar",
    ).apply {
        barbarbarbarbarbarbarbarbarbarbarbar()
            .bazbazbazbazbazbazbazbazbazbazbaz()
    }

For now this example can not be marked as a 'bug', until a solution can be defined what formatting would work.

Your last example is caused by missing the class-signature rule which also becomes non-experimental in the upcoming release:

$ ktlint-1.2.1 --stdin
16:34:13.568 [main] INFO com.pinterest.ktlint.cli.internal.KtlintCommandLine -- Enable default patterns [**/*.kt, **/*.kts]
class Foo(
    foo1: Int,
    foo2: Int,
) :
    Foobar1(
        a = 1,
        b = 2,
    ),
    Foobar2,
    Foobar3 {
    fun foo() = "foo"
}
<stdin>:5:5: Expected single space before the first super type (standard:class-signature)
<stdin>:6:1: Unexpected indentation (8) (should be 12) (standard:indent)
<stdin>:7:1: Unexpected indentation (8) (should be 12) (standard:indent)
<stdin>:8:1: Unexpected indentation (4) (should be 8) (standard:indent)
<stdin>:9:1: Unexpected indentation (4) (should be 8) (standard:indent)
<stdin>:10:1: Unexpected indentation (4) (should be 8) (standard:indent)

Summary error count (descending) by rule:
  standard:indent: 5
  standard:class-signature: 1

You're right that the behavior of the indent rule without the class-signature can be seen as a bug. But with class-signature becoming a non-experimental rule this will be prevented. Only if the class-signature is disabled, this problem will re-appear again but that is than a side effect that is to be accepted.

@mklnwt
Copy link
Author

mklnwt commented May 28, 2024

I think the last comment was in the wrong ticket or rather a mix between tickets.

In general I reckon we can close these tickets for now.

Sidenote and personal opinion:
Breaking after the assignment AND at the first method call seems very inelegant and wasting a lot of space (both vertically and horizontally).

val foos =
    foos
        .foo()
        .bar()
        .baz()
        .qux()

seems a lot less readable than

val foos = foos.foo()
    .bar()
    .baz()
    .qux()

@mklnwt mklnwt closed this as not planned Won't fix, can't repro, duplicate, stale May 28, 2024
@paul-dingemans
Copy link
Collaborator

Sidenote and personal opinion:
Breaking after the assignment AND at the first method call seems very inelegant and wasting a lot of space (both vertically and horizontally).

With short identifiers it seems indeed a waste of space. In real life, identifiers can also become (very) long.

val foos =
    foosfoosfoosfoosfoosfoosfoosfoosfoosfoosfoosfoos
        .foofoofoo()
        .bar()
        .baz()
        .qux()

which makes foofoofoo harder to discover

val foos = foosfoosfoosfoosfoosfoosfoosfoosfoosfoosfoosfoos.foofoofoo()
    .bar()
    .baz()
    .qux()

As with a lot of decisions regarding formatting it is subjective what reads better. I always try to look at extreme examples when to decide what format to use.

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

No branches or pull requests

2 participants