pinterest / ktlint

An anti-bikeshedding Kotlin linter with built-in formatter
https://pinterest.github.io/ktlint/
MIT License
6.23k stars 512 forks source link

Formatting does not work well in classes with generics and explicit constructor #2360

Closed jreij closed 11 months ago

jreij commented 11 months ago

Expected Behavior

I have some classes with generics for which I need to explicitly declare the constructor, either to document it, annotate it or add a visibility modifier to it. I expect to find a way to format these without getting ktlint errors.

Observed Behavior

I am getting errors from the type-parameter-list-spacing and discouraged-comment-location rules.

Steps to Reproduce

A few code examples:

class ClassWithGenericsAndAVeryLongName<FirstGeneric : FirstGenericType, SecondGeneric : SecondGenericType>
internal constructor(param: SomeType)
// throws `type-parameter-list-spacing`
class ClassWithGenericsAndAVeryLongName<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType
    >
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
constructor(param: SomeType)
// throws `type-parameter-list-spacing`
/**
 * Documentation for ClassWithGenericsAndAVeryLongName.
 */
class ClassWithGenericsAndAVeryLongName<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType
    >
/**
 * Constructor to initialize ClassWithGenericsAndAVeryLongName. Should only show if you use this constructor.
 * There's another constructor inside the class.
 */
constructor(param: SomeType)
// throws `type-parameter-list-spacing` and `discouraged-comment-location`.

Your Environment

hanggrian commented 11 months ago

I think this is related to the indentation rule. Consider the code snippets below:

@file:Suppress("ktlint:standard:max-line-length")

class ClassWithGenericsAndAVeryLongName1<FirstGeneric : FirstGenericType, SecondGeneric : SecondGenericType>
    internal constructor(param: SomeType)

class ClassWithGenericsAndAVeryLongName2<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType,
    >
    @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
    constructor(param: SomeType)

/**
 * @constructor initialize ClassWithGenericsAndAVeryLongName.
 */
class ClassWithGenericsAndAVeryLongName3<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType,
    >
    constructor(param: SomeType)

They run fine because they comply with Kotlin guidelines about class headers. Also, note that the comment location has changed.

paul-dingemans commented 11 months ago

@jreij Tnx for the interesting examples. Can you please provide the entire .editorconfig? Especially, I want to know whether the experimental rules are enabled, and if other rules have been enabled/disabled.

Rules type-parameter-list-spacing and indent do not agree whether the line after the closing > should or should not be indented. The first example is accepted when rewritten to:

class ClassWithGenericsAndAVeryLongName<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType
    > internal constructor(param: SomeType)

But I do not think that code above is well formatted. Below would be more consistent in my opionion:

class ClassWithGenericsAndAVeryLongName<
        FirstGeneric : FirstGenericType,
        SecondGeneric : SecondGenericType
    >
    internal constructor(param: SomeType)

Your second example is comparable to the first. But there is a debate in #2138 how to handle this.

For discourage comment locations in your third exmaple, please see faq

jreij commented 11 months ago

@hendraanggrian @paul-dingemans thanks for the reply.

Here's the full editorconfig. I'm using Android Studio with ktlint_code_style = android_studio.

Just a few comments:

paul-dingemans commented 11 months ago
  • I don't see a difference between the 2 examples shared by @paul-dingemans, maybe github formatted them wrong?

Indeed, something went wrong. Fixed it in the comment.

paul-dingemans commented 11 months ago

The discouraged-comment-location rule has been refactored and split (see #2371). The KDoc in your third example is now accepted, and code will be formatted with current snapshot version as follows:

/**
 * Documentation for ClassWithGenericsAndAVeryLongName.
 */
class ClassWithGenericsAndAVeryLongName<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType,
> /**
 * Constructor to initialize ClassWithGenericsAndAVeryLongName. Should only show if you use this constructor.
 * There's another constructor inside the class.
 */
    constructor(
        param: SomeType,
    )

But this still results in a lint violation because the KDoc should start on a newline.

Although not nice, for now you can work around it as follows (assuming that you only have a few occurences of this problem):

/**
 * Documentation for ClassWithGenericsAndAVeryLongName.
 */
@Suppress(
    "ktlint:standard:kdoc-wrapping",
    "ktlint:standard:discouraged-comment-location",
    "ktlint:standard:type-parameter-list-spacing"
)
class ClassWithGenericsAndAVeryLongName<
    FirstGeneric : FirstGenericType,
    SecondGeneric : SecondGenericType
    >
/**
 * Constructor to initialize ClassWithGenericsAndAVeryLongName. Should only show if you use this constructor.
 * There's another constructor inside the class.
 */
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
constructor(
    param: SomeType
)