Double indentation of class body when superclass is wrapped

TWiStErRob commented 9 months ago

Expected Behavior

Input is valid. Class body is correctly indented. Final brace is aligned with class opener. Maybe the SuperClass(primaryParam) { line indented, but not the whole body. IntelliJ also just does this one line indent.

Observed Behavior

internal abstract class MyClass internal constructor(primaryParam: ResourceFolderRepository) :
    SuperClass(primaryParam) {
        private val privateProperty: File = primaryProperty.resourceDir

        fun doStuff() {
            val file = privateProperty.resolve("foo")
        }
    }

Steps to Reproduce

ktlint "MyClass.kt" --format

internal abstract class MyClass internal constructor(primaryParam: ResourceFolderRepository) :
SuperClass(primaryParam) {
    private val privateProperty: File = primaryProperty.resourceDir

    fun doStuff() {
        val file = privateProperty.resolve("foo")
    }
}

Note: when I unwrap the first line:

internal abstract class MyClass internal constructor(primaryParam: ResourceFolderRepository) : SuperClass(primaryParam) {

the indentation does not change at all, neither with:

internal abstract class MyClass internal constructor(
    primaryParam: ResourceFolderRepository
) : SuperClass(primaryParam) {

But this header also reproduces this issue:

internal abstract class MyClass internal constructor(primaryParam: ResourceFolderRepository)
: SuperClass(primaryParam) {

Your Environment

Version of ktlint used: 1.0.1
Relevant parts of the .editorconfig settings: none
Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): ktlint CLI
Version of Gradle used (if applicable): N/A
Operating System and version: Windows 10

paul-dingemans commented 9 months ago

This is intended behavior for code style ktlint_official which is the default code style since 1.0. See:

TWiStErRob commented 9 months ago

What is the rationale behind re-indenting hundreds of lines based only on a very few lines of class header? I guess that part of the answer is that it matches some level of logical indentation, but which part? (I'm really curious about this!)

Here's a reason for not doing this:

When I'm looking at any random code at line 230, and see 2 levels of indentation it gives implicit context that the code is nested inside something. This rule takes away this crutch and requires anyone reading any code formatted with ktlint_official to actually have to look at the top level definition (which might be at the top of the file, or might be at line 145) to figure out if this is the case. This requires a lot of mental and physical (scrolling) gymnastics for something that's a given in 99.9% of auto-formatted code styles in a plethora of languages not just Kotlin.

Note: I might be actually fine with this, because I always advocate for the clearly structured:

class Name(
    param: Type, // any number of times
) : SuperClass(...) {

style which gives a clean and immediate overview without the need for brain-parsing the code on a single line into disparate pieces. And this style doesn't reproduce this issue. It just feels weird to reformat a whole file based on the placement of :

) : Super -> "normal" indent
)\n: Super -> "double" indent
) :\nSuper -> "double" indent

The class body logically belongs to the class, not the superclass constructor call, which means it should be always single-indented compared to the beginning of the class declaration. Having or not having a superclass or formatting thereof is irrelevant when it comes to the class body.

paul-dingemans commented 9 months ago

What is the rationale behind re-indenting hundreds of lines based only on a very few lines of class header? I guess that part of the answer is that it matches some level of logical indentation, but which part? (I'm really curious about this!)

The idea is to write all class headers in a consistent way. There are a few pain points when doing so:

annotations can be placed at many different elements in the class header. Not only on top of the class, but also on an explicit constructor, on type parameters, on class parameters, etc. From a consistency viewpoint, I want to handle annotions the same way, regardless of the construct on which they are used.
different indenting rules were applies on type parameters (with an indented >), versus class parameters (with de-indented ))

Is it worth to indent the entire class body compared to the size of the class header? That depends on your context and the size of your classes. For small classes it doesn't hurt at all. For big classes (code smell) it might actually hurt.

diasDominik commented 8 months ago

I also agree with @TWiStErRob the content belongs to the class and not the superclass so have the double indent makes it less clear were the code belongs to. Also I don't see a bigger class means that it is automatically a code smell(at which point is it actually a bigger class).

I'm all in for consistency but at the same time we should also not forget that the code should not get more complicated to read for humans.

Coming from python this indentiation would also be wrong so depends from where you come. But having it with a single indent is i believe in all languages the same

itera-brodmo commented 1 month ago

I have to agree with the OP, and even go a step further, I think this makes ktlint basically unusable for common scenarios. With test code like this

class Test(
    val mockService: Service
) : WordSpec({
    val test = 0
})

which you will often have when using Spring, the body is double indented with no way to disable that behavior other than disabling the standard indent rule. Having thousands of lines of test code double intended is just not an option, especially since Kotest already adds another level of indention with most specs.

pinterest / ktlint