Skip to content
New issue

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

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

Already on GitHub? # to your account

Multiline class declaration with superclass + interface: Interface gets moved to newline #1713

Closed
carstenhag opened this issue Nov 22, 2022 · 4 comments
Assignees
Milestone

Comments

@carstenhag
Copy link

carstenhag commented Nov 22, 2022

Expected Behavior

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
    xyz: Xyz
) : FillingStationDetailsViewModel(
    mapService,
    xyz,
), IHeaderContentViewModel {

is accepted formatting.

https://kotlinlang.org/docs/coding-conventions.html#class-headers mentions similar cases but not this exact one. Keeping it in the same line seems to be the most reasonable solution to me.

class Person(
    id: Int,
    name: String,
    surname: String
) : Human(id, name),
    KotlinMaker { /*...*/ }

Observed Behavior

Missing newline after "," (wrapping) is reported. ktLintFormat formats it to

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
) : FillingStationDetailsViewModel(
    mapService,
),
    IHeaderContentViewModel {

which is not that pretty imo :D

Steps to Reproduce

Format the above

Your Environment

  • Version of ktlint used: 0.47.1
  • Relevant parts of the .editorconfig settings:
[*.{kt,kts}]
indent_size=4
insert_final_newline=true
max_line_length=120
ktlint_disabled_rules=trailing-comma-on-declaration-site,trailing-comma-on-call-site
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task):
  • Version of Gradle used (if applicable): 7.5.1
  • Operating System and version: macOS 12.3
@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Nov 22, 2022

I agree that code below does not look well formatted:

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
) : FillingStationDetailsViewModel(
    mapService,
),
    IHeaderContentViewModel {
   // do something
}

The format you prefer was actually the format until and including 0.40.x (I have not analysed why it was changed).

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
) : FillingStationDetailsViewModel(
    mapService,
), IHeaderContentViewModel {
   // do something
}

Althought this format looks a bit better, it is less consistent with how other argument lists are formatted.

Based on example below (see https://kotlinlang.org/docs/coding-conventions.html#class-headers):

class MyFavouriteVeryLongClassHolder :
    MyLongHolder<MyFavouriteVeryLongClass>(),
    SomeOtherInterface,
    AndAnotherOne
{
    fun foo() { /*...*/ }
}

I would say that the code should have been formatted as follows:

class HeaderContentViewModel(
    mapService: IMapService,
    xyz: Xyz,
) :
    FillingStationDetailsViewModel(
        mapService,
        xyz
    ),
    IHeaderContentViewModel
{
    // do something
}

IntelliJ IDEA default formatting however joins the opening brace { with last super type, resulting in code below:

class HeaderContentViewModel(
    mapService: IMapService,
    xyz: Xyz,
) :
    FillingStationDetailsViewModel(
        mapService,
        xyz
    ),
    IHeaderContentViewModel {
    // do something
}

This last code sample is also accepted by KtLint 0.47.x and current snapshot for 0.48.x. So for now, my suggestion would be to follow that format.

@paul-dingemans
Copy link
Collaborator

Related to #1349

@paul-dingemans
Copy link
Collaborator

The new class-signature (#1349, #875) rewrites the code sample below:

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
    xyz: Xyz
) : FillingStationDetailsViewModel(
    mapService,
    xyz,
), IHeaderContentViewModel {
    // body
}

with code style ktlint_official to:

class HeaderContentViewModel
    @Inject
    constructor(
        mapService: IMapService,
        xyz: Xyz,
    ) : FillingStationDetailsViewModel(
            mapService,
            xyz,
        ),
        IHeaderContentViewModel {
    // body
}

with code styles intellij_idea and android_studio the format is kept unchanged and less optiomal as:

class HeaderContentViewModel @Inject constructor(
    mapService: IMapService,
    xyz: Xyz,
) : FillingStationDetailsViewModel(
    mapService,
    xyz,
),
    IHeaderContentViewModel {
    // body
}

@paul-dingemans
Copy link
Collaborator

See #2119

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

No branches or pull requests

2 participants