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

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

Closed
jreij opened this issue Nov 17, 2023 · 5 comments · Fixed by #2381
Closed

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

jreij opened this issue Nov 17, 2023 · 5 comments · Fixed by #2381

Comments

@jreij
Copy link

jreij commented Nov 17, 2023

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

  • Version of ktlint used: 1.0.1
  • Relevant parts of the .editorconfig settings: ktlint_code_style = android_studio
  • Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): Gradle with Android studio, for an Android library.
  • Version of Gradle used (if applicable): 8.4
  • Operating System and version: MacOS Sonoma.
@hendraanggrian
Copy link
Contributor

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
Copy link
Collaborator

paul-dingemans commented Nov 18, 2023

@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
Copy link
Author

jreij commented Nov 22, 2023

@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:

  • I don't see a difference between the 2 examples shared by @paul-dingemans, maybe github formatted them wrong?
  • The second example from @hendraanggrian still gives me a type-parameter-list-spacing error.
  • For the example with the @RestrictTo annotation, is there any fix or do you suggest I keep suppressing the rule for now?
  • Concerning the comment location issue, the idea here is to document the constructor and the class separately. This class has 2 constructors, one of them is the primary constructor because kotlin forces us to have one but in fact both of them are interchangeable. And the class itself needs some documentation that doesn't apply to any of the constructors. So ideally we would have kdocs for the class and for each constructor (updated my example to show that).
  • I wouldn't put the class name in a single line because it's even longer than the example I sent.
  • In my initial examples I omitted the indentation before the > by mistake but I have them in the actual code, I edited the examples to fix that.

@paul-dingemans
Copy link
Collaborator

paul-dingemans commented Nov 24, 2023

  • 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
Copy link
Collaborator

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
)

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.

3 participants