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

Multiple annotations should be allowed on the same line when construct is a constructor #673

Closed
ZacSweers opened this issue Dec 19, 2019 · 9 comments

Comments

@ZacSweers
Copy link

Consider the following example:

class Foo @JvmOverloads @SuppressLint("CheckResult") constructor(...)

There alternative is basically this

class Foo
@JvmOverloads
@SuppressLint("CheckResult")
constructor(...)

Which I'm not sure is better. From experimental:annotation

@Tapchicoma
Copy link
Collaborator

I think in this case behaviour of experimental:annotation is correct. According to https://kotlinlang.org/docs/reference/coding-conventions.html#annotation-formatting:

A single annotation without arguments may be placed on the same line as the corresponding declaration:

Another problem having multiple annotations for constructor on the same line - this line could more easily hit max-line limit, especially with long class name.

@ZacSweers
Copy link
Author

I think it makes sense for the declaration examples they have there, but not constructors. Yes it increases line length, but unlike the examples they have there, new lines actually have a destructive effect on the class header declaration here.

@3flex
Copy link
Contributor

3flex commented Oct 13, 2021

Came here to report the same... I think without clear examples on the Kotlin coding conventions page for constructor annotations, this isn't something that should be flagged by the rule.

@lwasyl
Copy link

lwasyl commented Feb 22, 2022

I'd like to expand the discussion to not only constructor annotations, but annotations for method parameters too. Currently (0.44.0) this is reported as an error:

@Component(modules = [Foo::class, Foo::class])
interface Foo {

    @Component.Factory
    interface Factory {

        fun create(
            @BindsInstance context: Context,
            @BindsInstance @Bar bar: String,
        ): Foo
    }
}

annotation class Bar

and I'm not sure what would be the suggested formatting. The following is the only variant that KtLint doesn't complain about, and I argue it's much less readable

@Component(modules = [Foo::class, Foo::class])
interface Foo {

    @Component.Factory
    interface Factory {

        fun create(
            @BindsInstance context: Context,
            @BindsInstance
            @Bar
            bar: String,
        ): Foo
    }
}

annotation class Bar

Since the annotations will most likely fit in a single line, it seems unnecessary to put them in separate lines like that

@hanrw
Copy link

hanrw commented Jun 29, 2022

I have same issue with - Multiple annotations should not be placed on the same line as the annotated construct (annotation)
E.g for better readable, if we can accept the same line for Multiple annotations

fun update(@Valid @Argument("input") input: UpdateRequest): UpdateResponse {}

@hantsy
Copy link

hantsy commented Jul 28, 2022

How to disable this rule?

@paul-dingemans
Copy link
Collaborator

fun update(@Valid @Argument("input") input: UpdateRequest): UpdateResponse {}

Please read the documentation: https://pinterest.github.io/ktlint/faq/#how-do-i-suppress-errors-for-a-lineblockfile

@Vadgyik
Copy link

Vadgyik commented Dec 14, 2022

another example when this rule is behaving incorrectly (validation annotation on collection type):

data class FooRequestDto(
    val data: List<@Valid @NotNull FooDto>,
)

@paul-dingemans
Copy link
Collaborator

Behavior of the annotation rules has been made more consistent in PR's #1916 and #1917 albeit not in the way most commentors in this issue would like it to be changed. For a consistency viewpoint, annotation wrapping should be done as consistent/deterministic as possible. Unfortunately, it is not possible to create a formatter which makes everybody happy without having hundreds of parameters to tweak the behavior.

@paul-dingemans paul-dingemans closed this as not planned Won't fix, can't repro, duplicate, stale Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants