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 class body when class has multiline super type list (ktlint_official only) #2115

Closed
paul-dingemans opened this issue Jul 7, 2023 · 6 comments · Fixed by #2116

Comments

@paul-dingemans
Copy link
Collaborator

#1916 fixed the wrapping of an explicit constructor for code style ktlint_official. Code like below:

class Foo
@Bar1 @Bar2
constructor(
    foo1: Foo1,
    foo2: Foo2,
) : Foobar(
    "foobar1",
    "foobar2",
) {
    fun foo() = "foo"
}

is formatted as:

class Foo
    @Bar1 @Bar2
    constructor(
        foo1: Foo1,
        foo2: Foo2,
    ) : Foobar(
        "foobar1",
        "foobar2",
    ) {
        fun foo() = "foo"
    }

The identation of the class body however is unwanted as it also affects classes not using the explicit constructor keyword.

Example:

class Foo(
    val bar1: Bar,
    val bar2: Bar,
) : FooBar(bar1, bar2),
    BarFoo1,
    BarFoo2 {
    // body
}

is formatted with 0.50.0 as:

class Foo(
    val bar1: Bar,
    val bar2: Bar,
) : FooBar(bar1, bar2),
    BarFoo1,
    BarFoo2 {
        // body
    }

while it would be expected that the original format was kept.

Also classes having an explicit constructor and a long super type list are not indented correctly:

class Foo
    constructor(
        val bar1: Bar,
        val bar2: Bar,
    ) : FooBar(bar1, bar2),
        BarFoo1,
        BarFoo2 {
    // body
}

is formatted as:

class Foo
    constructor(
        val bar1: Bar,
        val bar2: Bar,
    ) : FooBar(bar1, bar2),
    BarFoo1,
    BarFoo2 {
        // body
    }
paul-dingemans added a commit that referenced this issue Jul 7, 2023
… code style `ktlint_official` as it is inconsistent compared to other class bodies `indent`

 Closes #2115
paul-dingemans added a commit that referenced this issue Jul 7, 2023
… code style `ktlint_official` as it is inconsistent compared to other class bodies `indent` (#2116)

Closes #2115
paul-dingemans added a commit that referenced this issue Jul 9, 2023
paul-dingemans added a commit that referenced this issue Jul 9, 2023
* Fix indent of explicit constructor (also see #2115)
* Fix error in `hasNewLineInClosedRange` and `noNewLineInClosedRange`
@DennisBauer
Copy link

@paul-dingemans is this fixed for you now? For me Foo is still formatted as you mentioned it in your initial post. I would also prefer the formatting you suggested there.

@paul-dingemans
Copy link
Collaborator Author

@paul-dingemans is this fixed for you now? For me Foo is still formatted as you mentioned it in your initial post. I would also prefer the formatting you suggested there.

Can you be more specific? The original post contains multiple Foo's. Also, see this unit test for tests regarding formatting explicit constructors. If this doesn't answer your question, please raise a new issue.

@PauGuillamon
Copy link

Giving my feedback on this topic, I find it quite strange that classes with annotated constructor causes the class body to have 2 indents.

All of a sudden I find some classes with 2 tabs and others with 1 tab. Instead of all classes looking similar, it seems quite random and does not give a feel of coherent code formatting in the project. Usually one does not look at the class constructor.

Additionally because of the 2 tabs indent some lines now reach the max length and are formatted completely different, even though it's the same code within the same amount of scope levels.

@paul-dingemans
Copy link
Collaborator Author

Giving my feedback on this topic, I find it quite strange that classes with annotated constructor causes the class body to have 2 indents.

All of a sudden I find some classes with 2 tabs and others with 1 tab. Instead of all classes looking similar, it seems quite random and does not give a feel of coherent code formatting in the project. Usually one does not look at the class constructor.

Additionally because of the 2 tabs indent some lines now reach the max length and are formatted completely different, even though it's the same code within the same amount of scope levels.

Please see for similar feedback and backgrounds of this change in #2138.

@ericloe-cash
Copy link

Has this regressed? In 1.2.1 I have a class that was formatted to:

class MyClass
  @Inject
  constructor(
    private val arg : Any
  ) {
    fun myfun(
      arg : Any
    ): Any {
      /// more body
    }
  }

But I expect it (and so does intellij kotlin formatter) to be:

class MyClass
  @Inject
  constructor(
    private val arg : Any
  ) {
  fun myfun(
    arg : Any
  ): Any {
    /// more body
  }
}

@paul-dingemans
Copy link
Collaborator Author

This is not a regression bug but an intentional change in the ktlint_official code style. Please see comment above for references to unit tests, and similar issue where some people disagree with the "double" indentation.

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

Successfully merging a pull request may close this issue.

4 participants