Annotation wrapping before explicit constructor (ktlint_official)

[meyertime]

Expected Behavior

Our Android project contains many class declarations that look like this:

class Foo @Inject constructor(
    private val someDependency: SomeDependency,
    private val anotherDependency: AnotherDependency,
) : Bar {
    //...
}

In my opinion, this should pass linting using the ktlint_official style. The official Kotlin and Android style guides actually contain no examples of annotations on a constructor, so I don't think they even thought about it when writing the style guide. However, the Kotlin guide does say, "A single annotation without arguments may be placed on the same line as the corresponding declaration," and the Android guide says something similar. Therefore, the above example should pass.

Observed Behavior

Starting with ktlint v0.49.0, the above code produces the following linting errors:

Linting error > [standard:annotation] Expected newline before annotation
Linting error > [standard:annotation] Expected newline after last annotation

Further, an auto-correction of the above code looks like this:

class Foo
    @Inject
    constructor(
        private val someDependency: SomeDependency,
        private val anotherDependency: AnotherDependency,
    ) : Bar {
        //...
    }

This is really ugly and hard to read. Linting should make our code better, not worse. The issue that introduced this feature suggests the following in the case of a short parameter list:

class Foo
    @Inject
    constructor(private val someDependency: SomeDependency): Bar {
    //...
}

This is even worse. The indentation obscures where the opening brace is.

Steps to Reproduce

Run ktlint v0.49.0 or newer against the above code example with the ktlint_official style.

Your Environment

• Version of ktlint used: 0.49.1
• ktlint_official
• Name and version (or code for custom task) of integration used (Gradle plugin, Maven plugin, command line, custom Gradle task): org.jmailen.kotlinter version 3.15.0
• Version of Gradle used (if applicable): 8.2.1
• Operating System and version: Arch Linux

[paul-dingemans]

In my opinion, this should pass linting using the ktlint_official style. The official Kotlin and Android style guides actually contain no examples of annotations on a constructor, so I don't think they even thought about it when writing the style guide. However, the Kotlin guide does say, "A single annotation without arguments may be placed on the same line as the corresponding declaration," and the Android guide says something similar. Therefore, the above example should pass.

ktlint_official aims to produce more consistent code than the current code styles. Annotations with arguments are for example always wrapped, regardless on which construct they are used. To avoid inconsistencies between constructors with an annotation not having parameters, and constructors with an annotation having parameters, I have decided to wrap explicit constructors preceded by an annotation or modifier.

class Foo
    @Inject
    internal constructor()

and

data class Foo
    @Bar1
    @Bar2("bar")
    @Bar3
    @Bar4
    constructor(private val foobar: Int) {
        fun foo(): String = "foo"
    }

This is really ugly and hard to read. Linting should make our code better, not worse.

So, indeed the code looks quite different from before and it might need time to get used to it. Qualifiers like 'ugly', 'hard', 'better' and 'worse' are subjective and always related to what you are used to see. Also, I am still getting used to this new format.

class Foo
    @Inject
    constructor(private val someDependency: SomeDependency): Bar {
    //...
}

This example, I have been doubting about. In some other context, the Kotlin style guide suggest two possibilties:

Option 1:

For classes with a long supertype list, put a line break after the colon and align all supertype names horizontally:

class MyFavouriteVeryLongClassHolder :
    MyLongHolder<MyFavouriteVeryLongClass>(),
    SomeOtherInterface,
    AndAnotherOne {

    fun foo() { /*...*/ }
}

or

Option 2:

To clearly separate the class header and body when the class header is long, either put a blank line following the class header (as in the example above), or put the opening curly brace on a separate line:

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

I have chosen the first option. Due to the rule that removes blank lines at the start of a class the result becomes:

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

Allowing a blank line in at start of class above would not be consistent with that rule.

I am not inclined to change the formatting at this moment. I will leave the issue open for a while to collect other opinions.

[lyallcooper]

Also coming from a project that heavily uses @Inject-annotated constructors, I agree with @meyertime that this new formatting feels like a regression. While it's definitely hard to ignore the "I don't like change!" aspect of a something like this, I do think multi-line formatting decreases readability here, apart from just being unfamiliar, so I'll try to justify that.

Taking the original example:

class Foo @Inject constructor(
    private val someDependency: SomeDependency,
    private val anotherDependency: AnotherDependency,
) : Bar {
    //...
}

In terms of token saliency when reading, I would argue class, Foo, someDependency: SomeDependency, anotherDependency: AnotherDependency and Bar are top, while @Inject, constructor, private and val are all lower.

Looking at the post-formatted example, I would argue that @Inject and constructor specifically are given undue prominence. The two extra lines used also means the eye has to jump further to get to the next important pieces of information after the class name.

class Foo
    @Inject
    constructor(
        private val someDependency: SomeDependency,
        private val anotherDependency: AnotherDependency,
    ) : Bar {
        //...
    }

Finally, this approach also means the whole class contents are shifted over by an additional indent, which feels a little wasteful.

In our codebase, the ...@Bar1 @Bar2("bar") @Bar3 @Bar4 constructor... pattern of having a bunch of annotations on the constructor is very uncommon, while the above pattern of a single @Inject annotation is extremely common, so the readability/aesthetics vs. consistency tradeoff would not be worth it for us.

I would suggest maybe enforcing the wrapping maybe only if more than one annotation is present.

[paul-dingemans]

Also coming from a project that heavily uses @Inject-annotated constructors, I agree with @meyertime that this new formatting feels like a regression. While it's definitely hard to ignore the "I don't like change!" aspect of a something like this, I do think multi-line formatting decreases readability here, apart from just being unfamiliar, so I'll try to justify that.

...

In terms of token saliency when reading, I would argue class, Foo, someDependency: SomeDependency, anotherDependency: AnotherDependency and Bar are top, while @Inject, constructor, private and val are all lower.

I like your style of reasoning. Despite that, there are other arguments not to allow to make an exception for the case of a single annotation.

From a viewpoint of consistency, the rules which are applied on functions, are also applied on constructors. I know that constructors and functions are different beasts. But as with cars, they have both four wheels and some doors.

The case of the @Inject constructor can not be compared to all other possible types of annotations which might a bigger impact and therefore would require more attention. Some other examples, that I have considered are:

class SecondLevelBeanFactoryQuery @Autowired(required = false) constructor(internal val topLevelValue: String)

data class GraphQLBatchRequest @JsonCreator constructor(@get:JsonValue val requests: List<GraphQLRequest>)

public class SerializersModuleBuilder @PublishedApi internal constructor() : SerializersModuleCollector

public class JsonConfiguration @OptIn(ExperimentalSerializationApi::class) internal constructor(

I do agree that the identation feels awkward. But I would wrap, and keep IntelliJ IDEA formatting, the result would be:

class Foo
@Inject
constructor(
    private val someDependency: SomeDependency,
    private val anotherDependency: AnotherDependency,
) : Bar {
    //...
}

In this way the most important thing class Foo got lost. Especially when it would be preceded by another annotation.

[meyertime]

Qualifiers like 'ugly', 'hard', 'better' and 'worse' are subjective and always related to what you are used to see.

It's true that "ugly" is subjective, but my main point is that it is "hard to read", or in other words, "less readable", and that is not entirely subjective. "Better" and "worse" are also not entirely subjective in my opinion, as there are patterns that have proven to be more or less readable regardless of what people are used to.

Looking at the post-formatted example, I would argue that @Inject and constructor specifically are given undue prominence. The two extra lines used also means the eye has to jump further to get to the next important pieces of information after the class name.

Finally, this approach also means the whole class contents are shifted over by an additional indent, which feels a little wasteful.

Agreed! Thanks @lyallcooper for articulating this.

ktlint_official aims to produce more consistent code than the current code styles.

I understand the desire for consistency, but it is really a secondary concern. The primary concern is readability. Consistency generally improves readability, which is what makes consistency worthwhile. However, if something is done just to improve consistency and it actually reduces readability, then it defeats the purpose.

A machine will never be able to make the best decision for human readability in 100% of cases. There are always going to be some cases where a human will have to make a judgment call. The Kotlin guide says as much in the way it is worded. I previously quoted: "A single annotation without arguments may be placed on the same line as the corresponding declaration." Here it says "may", denoting that it is an option for the human programmer to decide, not a strict rule. I am suggesting that this be made an option in harmony with the Kotlin official guidance, not that it be enforced to be on the same line in that case.

From a viewpoint of consistency, the rules which are applied on functions, are also applied on constructors. I know that constructors and functions are different beasts. But as with cars, they have both four wheels and some doors.

Generally, yes, constructors are similar to functions. However, I could easily argue that Kotlin primary constructors are very different and have a much stronger link to the class definition, but I do not have to, because even in the case of a regular function, the Kotlin guide says that a single annotation without arguments can be placed on the same line as the function declaration, even offering this example:

@Test fun foo() { /*...*/ }

Further, the Android guide also states, "When only a single annotation without arguments is present, it may be placed on the same line as the declaration," and offers this example:

@Test fun selectAll() {
    // …
}

Obviously, there's a reason these guides agree and both saw fit to include this as an explicit option. There are definitely cases where it is most readable to put the single annotation without arguments on the same line, and my @Inject constructor case is one such example.

class Foo
@Inject
constructor(
    private val someDependency: SomeDependency,
    private val anotherDependency: AnotherDependency,
) : Bar {
    //...
}

Honestly, of all the different indentation examples where the annotation is on a separate line, I think this one is the most readable. class Foo does not get lost to me, because it's the first line, but as you said, if it were preceded by another annotation, it might be different.

However, I think every such example given here is deficient in some way. I don't think we can clearly say of any of them, "This is the best way to indent things in 100% of cases," so I don't think it makes sense to force it upon everyone. But this issue is really secondary to me, because I can avoid it altogether if I can put @Inject on the same line.

[paul-dingemans]

Let's agree to disagree on this topic for now. As documented:

This ktlint_official code style combines the best elements from the Kotlin Coding conventions and Android's Kotlin styleguide. This code style also provides additional formatting on topics which are not (explicitly) mentioned in those conventions and style guide.

For now, I will mark the issue as parked so that it can gather more opinions.

[eygraber]

This looks like a regression to me, for many of the same reasons stated in the comments above.

My primary reason for using ktlint is to format my code automatically so that it's readable. Of course consistency is important, but not at the expense of readability.

It's also concerning that consistency is decided in a vacuum. @paul-dingemans you mention that:

I know that constructors and functions are different beasts. But as with cars, they have both four wheels and some doors.

which sounds nice, but comes with the presupposition that constructors and functions are both cars. Considering that's a matter of opinion, this probably shouldn't have been decided on unilaterally (or if there was more involvement, then it should've been in the open).

[ephemient]

We are using .editorconfig

[*.{kt,kts}]
ktlint_standard_annotation = disabled

because this part of this rule is simply unusable for our codebase.

[meyertime]

We've gotten around this for now by using the android_studio style rather than ktlint_official, but it seems like it's not strict enough in some ways. It would be nice if it was more configurable. The stricter you get, the more people are going to disagree on which way it should be. At least with the intellij_idea and android_studio styles, you could point to the official style guides to resolve any dispute.

It's also concerning that consistency is decided in a vacuum. @paul-dingemans you mention that:

I know that constructors and functions are different beasts. But as with cars, they have both four wheels and some doors.

which sounds nice, but comes with the presupposition that constructors and functions are both cars. Considering that's a matter of opinion, this probably shouldn't have been decided on unilaterally (or if there was more involvement, then it should've been in the open).

Not only are constructors and functions "different beasts", primary constructors in Kotlin are even further removed from normal functions for several reasons:

• You can define class-level properties in primary constructors, but not in normal functions.
• There can only be one primary constructor for a class, whereas you can have many functions.
• You can only define the primary constructor as part of the class header, whereas functions can only be in the class body.
• A primary constructor cannot have a function body. What you find syntactically in place of the primary constructor's function body is actually the class body. Instead, property initializers and any init blocks are combined behind the scenes to generate the function body.

Any of these would be good reason for potentially different formatting rules to apply to a primary constructor than to a function. And, indeed, the fact that it has to be part of the class header necessitates it.

[paul-dingemans]

Considering that's a matter of opinion, this probably shouldn't have been decided on unilaterally (or if there was more involvement, then it should've been in the open).

In past I have tried to get feedback on proposal for changes via the issue tracker and via Slack. This resulted in almost no feedback at all. The feedback collected in this issue is valuable (that is the reason that the issue was not closed), but unfortunately also not representative to the entire user group. Users that disagree with decisions made are much more likely to reach out than people who agree.

I don't see any problem in unilatterly deciding about the new code style ktlint_official. The idea has been openly announced in this issue in November 2022. Also, all issues related to this code style haven been labeled and as of that were easily to follow or to provide feedback on. Next to that, it is not affecting the existing code styles that people are used to. Finally, considering that I am the developer who puts countless number of hours in evolving ktlint with almost no help from the community, I feel that I have a big say towards the direction that ktlint is evolving.

[paul-dingemans]

We've gotten around this for now by using the android_studio style rather than ktlint_official, but it seems like it's not strict enough in some ways. It would be nice if it was more configurable.

Please see https://pinterest.github.io/ktlint/0.50.0/faq/#can-a-new-toggle-be-added-to-optionally-enabledisable-format-code-in-a-particular-way. Also note that when your android_studio code style, that you still can enable rules of ktlint_official that you would like to use. But this will not help for the constructor annotation case.

The stricter you get, the more people are going to disagree on which way it should be.

Yes, I am aware of that. Code style ktlint_official values consistency more important than the other code styles.

At least with the intellij_idea and android_studio styles, you could point to the official style guides to resolve any dispute.

Please feel free to create a PR which provides this clarity.

Not only are constructors and functions "different beasts", primary constructors in Kotlin are even further removed from normal functions for several reasons:

You can define class-level properties in primary constructors, but not in normal functions. There can only be one primary constructor for a class, whereas you can have many functions. You can only define the primary constructor as part of the class header, whereas functions can only be in the class body. A primary constructor cannot have a function body. What you find syntactically in place of the primary constructor's function body is actually the class body. Instead, property initializers and any init blocks are combined behind the scenes to generate the function body. Any of these would be good reason for potentially different formatting rules to apply to a primary constructor than to a function. And, indeed, the fact that it has to be part of the class header necessitates it.

Above are indeed differences between constructors and functions. But the root cause of this issue is the handling of annotations on the constructor and how to format those annotations. According to the Kotlin Coding conventions:

Place annotations on separate lines before the declaration to which they are attached, and with the same indentation:

[eygraber]

This resulted in almost no feedback at all.

That's just the way it is. Most people don't think about the tool that's going to format their code, and just assume it will work.

Users that disagree with decisions made are much more likely to reach out than people who agree.

True, and it's also an effective way of getting feedback, but it also alienates users and lowers trust in the tool.

I don't see any problem in unilatterly deciding about the new code style ktlint_official.

Android Kotlin Style Guide Jetbrains Kotlin Style Guide

Neither of those are/were built with one person having unilateral decision power. It's also not very visible how much time and methodology went into making the decisions that they did. There's no way one person could have the context or info needed to make a decision that would apply across the ecosystem. I think you'll end seeing a lot more issues like this one crop up as more and more people adopt this style.

Of course it would be good to get community involvement in the decision making process, but it's not a topic that's ever going to get broad community involvement. It's a rock and a hard place situation.

considering that I am the developer who puts countless number of hours in evolving ktlint with almost no help from the community

:100: and personally I'm very thankful that you do this. I try to contribute by testing new versions and filing issues when I could, but most of the time I have to work on my projects and don't want to spend time thinking about the formatting tool.

I feel that I have a big say towards the direction that ktlint is evolving

Absolutely, but with great power comes great responsibility.

[meyertime]

Above are indeed differences between constructors and functions. But the root cause of this issue is the handling of annotations on the constructor and how to format those annotations. According to the Kotlin Coding conventions:

Place annotations on separate lines before the declaration to which they are attached, and with the same indentation:

Yes, this is the general rule in the Kotlin coding conventions, but it then proceeds to explain two explicit exceptions in that same section:

Annotations without arguments may be placed on the same line:

And:

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

And this is probably where you're going to bring up "consistency" and we'll be going in circles. But we can just as easily be consistent by enforcing that a single annotation without arguments always goes on the same line. These are called out in the coding conventions for a reason, so it should at least be an option (enforce separate line, enforce same line, or do not enforce either way).

[hantsy]

How can I use the original format? The new line break before/after annotation and intend make the codes look so ugly.

[hantsy]

Any lint tools instead? @meyertime

[meyertime]

@hantsy Use the intellij_idea or android_studio style. See https://pinterest.github.io/ktlint/1.0.0/rules/code-styles/

[PauGuillamon]

Comment copied from #2115, since I had missed this thread:

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.

---

END of original comment.

I don't even care that much about how the constructor looks like. Speaking of consistency mentioned in the thread, some classes having 2 tabs and some classes having 1 tab is not consistent and decreases readability. Indentation levels are usually a hint of scope level which is broken here.

[realdadfish]

Coming from an earlier ktlint version this also looks like a big regression for me, but not so much because I (subjectively) dislike the new style; rather because once the code base is automatically reformatted (and our devs use that feature a LOT), we basically break git blame history on the respective files, and that's simply a no-go.

Certainly, no one expected that a 0.x version of a software is still 100% compatible with a 1.x version, but if that 0.x version was out for years and people relied on it formatting their code in one particular way now see that the same tool in a stable version reformats their code completely, then this is not acceptable.

Sorry, I see your intent and all the work that went into this, but please, please make this configurable. Thanks!

[paul-dingemans]

Certainly, no one expected that a 0.x version of a software is still 100% compatible with a 1.x version, but if that 0.x version was out for years and people relied on it formatting their code in one particular way now see that the same tool in a stable version reformats their code completely, then this is not acceptable.

I do understand that this is unwanted. Did you notice that the default code style of Ktlint was changed with publication of 1.0? The default code style is now ktlint_official which indeed has a some big changes in the code style. You can try whether sticking on the old intellij_idea code style results in less violations and a better backwards compatibility. See https://pinterest.github.io/ktlint/latest/rules/code-styles/

[realdadfish]

I had weird results locally when changing the different .editorconfig settings, because none seem to have had any effect. Eventually this was caused by the plugin I used (kotlinter), because once I forced single-daemon mode, deactivating the offending rule worked, so I could keep otherwise ktlint-official "compliant".

[paul-dingemans]

I had weird results locally when changing the different .editorconfig settings, because none seem to have had any effect. Eventually this was caused by the plugin I used (kotlinter), because once I forced single-daemon mode, deactivating the offending rule worked, so I could keep otherwise ktlint-official "compliant".

I am not familiar with the Kotlinter integration with ktlint. But I am happy for you that it works.

[erdi]

The default code style is now ktlint_official which indeed has a some big changes in the code style. You can try whether sticking on the old intellij_idea code style results in less violations and a better backwards compatibility.

Even though it's my fault for not reading the changelog prior to upgrading I would like to flag the this was the crucial bit that I missed. My motivation for upgrading to 1.x was to include a fix for https://github.com/pinterest/ktlint/issues/2343 so I was surprised that so many new violations are being raised after upgrading. After reconfiguring back to using intellij_idea style post upgrade the violation this issue is about went away.

[ericloe-cash]

I don't even care that much about how the constructor looks like. Speaking of consistency mentioned in the thread, some classes having 2 tabs and some classes having 1 tab is not consistent and decreases readability. Indentation levels are usually a hint of scope level which is broken here.

This

Please fix it, there is no rationale. Experimental rule class-signature is also unusable because of the behavior with extends/implements in a new line causing double indent on the class body.

https://github.com/pinterest/ktlint/issues/2423

[horsmand]

I don't even care that much about how the constructor looks like. Speaking of consistency mentioned in the thread, some classes having 2 tabs and some classes having 1 tab is not consistent and decreases readability. Indentation levels are usually a hint of scope level which is broken here.

Yes! There should never be a linting rule that causes the closing curly brace for a class to be indented differently from the beginning of the class definition. It is confusing when you get to the end of a file and it ends still indented. It also just wastes line real estate when everything in the class needs an extra level of indentation.

[quinlam]

This issue is marked as open/parked. What is the intention? Is more feedback being sought in other channels? It seems there enough options to work around this rule, but I am concerned that is the default rule and agree with comments regarding reduced readability and unexpected indentation.

Its particularly stark coming from other languages and linting systems.

[paul-dingemans]

This issue is marked as open/parked. What is the intention?

The issue is parked as I am (still) not convinced by the arguments in this thread. The thread is kept open so that it is more conventient for users to find the thread, and to add additional insights. I am not actively researching the topic on any (other) channel.

We have to bear in mind that only a limited number of developers out of thousands of users have objected against this way of formatting. Changing the current format as requested in this thread will make other developers unhappy. I am only willing to do so, when I am convinced so that I can explain/defend a new policy.

[realdadfish]

Problem might be that ktlint eventually more and more moves away from the original intent of being a "non-bikeshedding" linter / formatter. Having different "styles" available already worked against that original intent, then allowing all rules to be configured / activated / deactivated worked against this as well and maybe there isn't even such a thing as an "non-bikeshedding" formatter and everything is fine as is. Maybe the official "ktlint" style is also just one biased version of a style and if we all agree to disagree on things and have ways to make "our own style" derived from whatever default style we're coming from, everybody is happy?

In the end it's still just code formatting we're all heavily argueing about :)

[paul-dingemans]

I do agree with most of what you say. Having different code styles for Android and non-Android is caused by having different sets of guidlines.

The kotlin_official coding style was created primarily to get rid of the requirement that code formatted by ktlint may not conflict with default Intellij IDEA formatting (even for cases in which that formatting did not apply with mentioned guidelines). Op top of that the kotlin_official code styles has some subjective additions to it, on which people indeed have varied opinions.

I am as restrictive as I can with configuration options for rules. Ideally I would get rid of all configuration properties, as well as the possibility to disable rules. But probably I would need to start an entire new project for that because the ktlint community will never be able to agree on one standard if they cannot tweak it to personal preferences. My advise is always to pick one of the code styles and to accept the default settings. It is up to the ktlint users whether they adhere to that.

[meyertime]

The irony is that this "anti-bikeshedding" linter doesn't actually eliminate bikeshedding; instead, the maintainer ends up dealing with all the bikeshedding whenever ktlint can't just be configured by the user.

If you really want to avoid bikeshedding, then embrace configuration. Then each user can have their way with it, and there won't be any bikeshedding. Everyone is happy. Most languages have a configurable linter; that is what is lacking for Kotlin and where ktlint could really be useful. When Pinterest took ownership of ktlint, the ability to disable rules was ktlint's most-requested feature. Clearly the community wants more configurability, not less.

Of course, I'm sure the bikeshedding referred to in ktlint's vision statement is what happens in code reviews when developers argue about code style. In my experience, this bikeshedding only actually occurs when there isn't a tool set up to enforce a particular standard. When a linter is set up, very rarely do we argue about its rules, and when we do, it's a brief conversation, and we implement it. For example, the issue discussed here would have been a brief conversation with the team, and everybody would have been like, "Yeah, that code looks awful. Let's change the lint rules." Instead, it's been an open issue for almost a year where various users take turns trying to convince the maintainer. Is that really "anti-bikeshedding"?

I picked up ktlint for our project because I wanted a way to apply and enforce the official style guidance, and at the time, ktlint did a good job of that. I did not want one person's opinionated style, and I highly doubt anybody wants that. (Also, why is a non-Pinterest employee unilaterally deciding the direction of Pinterest's open source project? :thinking:) I also did not want a deterministic formatter that takes away all choice from the programmer. Readability is way more important than determinism, and even if I did want that, ktfmt already exists for that purpose. There's a reason people are using ktlint instead of ktfmt; if you make it just like ktfmt, what reason will be left for ktlint to exist?

The only reason we're still using ktlint is because intellij_idea and android_studio are still options. However, these are clearly second-class features now, as some of the ktlint_official opinionated additions have crept into them over time, and we have had to disable a handful of rules to get it to allow things that the official guidelines either allow or are silent on. Disabling rules is also a sub-par solution, because ktlint may no longer enforce some things that the official guidelines do say. Configurability would be much better. It can still be turn-key and "no configuration required" like it is now for those who don't want to bother, so it would be a win-win.

I know this isn't a forum to discuss the overall direction of the project, but I believe the issue here is a symptom of a deeper problem with the way things have been handled over the last year or two.

But as far as the actual topic of this issue, I no longer have a pony in this race, because we're not using ktlint_official, and we won't as long as it remains one person's opinionated style. All that having been said, I think it's a false assumption that any developers would be unhappy if this change was made to ktlint_official. For one thing, of the "thousands" of users of ktlint, how many are using ktlint_official? And of those that are, how many have encountered this particular situation in their code? And of those, I would be shocked if you could find one person who would honestly say "I think the way ktlint formats it is better" and be upset if it was changed. On the other hand, several had the opposite reaction, found this issue, and put their 2 cents in, and we don't know how many just silently followed the advice here and either disabled the rule or ditched ktlint_official.

Finally, at a minimum, we have only asked for the option to configure this rule; you could do that and leave the default the way it is without affecting anyone who would supposedly be upset about the change. Though it would be better to improve the default. Most users would be pleased to see the readability of their code improve, and any outliers could configure it back to the way it was. Or, it could be implemented not to reformat code that was previously formatted the ugly way, but at the same time allow code that is formatted the nicer way. The point is, there are plenty of options that would avoid supposedly upsetting users that have been silent on this issue so far, so I don't buy that excuse.

This is really telling:

No other open issue on ktlint has nearly as many comments or :+1:s, and every comment from the community is in support of this change. If that's not consensus, I don't know what is. The community has spoken. The question is, are you listening? This goes beyond this one issue, because if you refuse to take clear feedback from the community, then as previously brought out, it "alienates users and lowers trust in the tool." Eventually, someone will fork ktlint, or at least build a custom ruleset that's either configurable or takes community feedback, and users will flock to it.

-- edit by @paul-dingemans : replaced references to my company with my user handle

-- edit by @meyertime : changed Paul's edits so that they didn't change the meaning of what I was saying

[paul-dingemans]

I did not want one xxx employee's opinionated style, and I highly doubt anybody wants that. (Also, why is a xxx employee unilaterally deciding the direction of Pinterest's open source project? 🤔)

If you take a look at the contribution statistics, you can see that the project only has two regular contributors in the last 18 months:

I am the only one who is willing to contribute structurally to the Ktlint project. If you want to contribute to the project, you're welcome if you think that you can do a better job.

[meyertime]

I resent you editing my comment. Your edit changed the point I was making. Why do you want to hide from this thread the fact that you don't work for Pinterest? It's in your public GitHub profile that you work for xxxx. I did not reveal any non-public information.

Edit by @paul-dingemans : removed company name as not being relevant

[paul-dingemans]

The company I work for is not relevant. It is is not a secret that I do not work for Pinterest. People can look up my resume easily.

Although Pinterest has fostered this project for a couple of years, only one of the maintainers was employed by Pinterest when I became a maintainer. I removed my company name as you insinuate that my employer is relevant to this thread. All decisions I have made were my personal decisions.

Please refrain from further escalations. According to The contribution guidelines of this project this should be a safe place for all. I did not attack you personally. But you are making it personal to me if you continue like this.

[meyertime]

Ok, I will concede that the name of your specific employer is not relevant to this discussion. I will respect your wishes and refrain from mentioning it, and I do apologize for my initial reaction.

However, the fact that you are not employed by Pinterest is relevant, because it gives us a potential recourse before resorting to forking: We can appeal to Pinterest to take action. Further, while it may not be a "secret" that you don't work for Pinterest, it's also not common knowledge. I had assumed that the sole maintainer and contributor of this Pinterest-owned project was a Pinterest employee until I found out otherwise, and I believe that would be a common assumption.

Also, holding you accountable for how you have been maintaining ktlint is fair and does not in itself constitute a personal attack, especially when you are not the owner (or employed by the owner). When you started maintaining it, you opened yourself up to fair criticism.

If you take a look at the contribution statistics, you can see that the project only has two regular contributors in the last 18 months:

I am the only one who is willing to contribute structurally to the Ktlint project.

You seem to be equating contributing with maintaining. Those are different roles. Yes, you've certainly done a lot of contributing. No one is saying otherwise. But maintaining is about making sure contributions are moving the project in the right direction.

Sure, I've seen plenty of "open source" projects that are really just one person (the sole maintainer and contributor) putting something they made out there. They have their own vision for the project and will do what they want regardless. That person has every right to do whatever they want, as it's their project. However, it won't be truly useful to a lot of people and gain a significant following unless it satisfies the needs of the community rather than the maintainer's personal whim.

However, this is not one of those projects. There was already a significant user base when you took over maintaining it. It didn't get that way because of your effort. Yet, you act as if your work on the project entitles you to do as you please and we should somehow just appreciate it. A maintainer going rogue isn't doing the community any favors, no matter how much they're contributing.

If you want to contribute to the project, you're welcome if you think that you can do a better job.

That is not true. If you refuse to accept clear feedback from the community, what reason do we have to believe that you would accept a PR that implements that feedback? At this point, appealing to Pinterest, or starting a community-driven fork of ktlint, are our only viable options, unless you start listening to the community.

[npouliquen]

This has clearly become a quite spirited discussion. I recently upgraded package versions at work, and I came across this issue, so here I am sharing my thoughts for the first time ever on a GitHub issue.

I am a bit perplexed regarding the resistance to making this rule configurable. There are other configurable rules in Ktlint already. There is clearly a desire from folks to have this configurable. This issue has more engagement than any other open issue of Ktlint, and it isn't particularly close. The current behavior can be left as a default. How is this in any way a bad thing or regression?

@paul-dingemans, you said the following in this thread:

Qualifiers like 'ugly', 'hard', 'better' and 'worse' are subjective and always related to what you are used to see

All decisions I have made were my personal decisions.

I don't see any problem in unilatterly deciding about the new code style ktlint_official

Code formatting is subjective. Given the above quotes and the fact that code formatting is purely subjective, is it fair to say that Ktlint just your personal preference on Kotlin code formatting? Is that the intended spirit of Ktlint? If the answer is yes to both, that's fine. If not, then it seems like adding some level of configurability for particularly controversial changes like this one would be a good solution.

In my opinion, this package probably shouldn't be based on the preferences of a single person. I understand that you have an aversion to configuration properties based on your quote

Ideally I would get rid of all configuration properties, as well as the possibility to disable rules.

But you immediately acknowledge that this idea isn't tenable:

But probably I would need to start an entire new project for that because the ktlint community will never be able to agree on one standard if they cannot tweak it to personal preferences.

Again, given the amount of push back on this particular formatting, how is this any different from other configuration properties? If the basis of other configuration properties comes from a lack of agreement on one standard, how is this meaningfully different? This entire thread is evidence of a lack of agreement upon a standard.

Edit by @npouliquen: formatting

[meyertime]

Ok, so things got a little off the rails up there. I also opened #2703 to discuss the direction of ktlint more broadly, and things got a little salty there as well. But fortunately, it ended on a positive note.

As far as this issue is concerned, I will work on making the annotation rule configurable. If it goes well, I will drive an effort to do the same with the other rules. That way, there will be less reason to argue about the default behavior, since users that don't agree can just change their configuration.

[paul-dingemans]

Please add you generic thoughts about configurability in discussion #2711. This current issue is intended solely for the configuration of the annotation wrapping before an explicit constructor.

[bcmedeiros]

I just bumped into this issue when adding @JvmOverloads to a class. because of @JvmOverloads constructor being added, my entire class body was indented with one tab.

I do agree that the identation feels awkward

This comment is spot on. I think ktlint has been taking tabs lightly recently. We should not have rules that change the entire indentation of a class that are not strictly related to the class itself. An "annotation" rule should not have the power to control anything outside the thing it's annotating.

[mtrakal]

I'm ok with keep indent and constructor on new lines (1), but just because you add annotated constructor + superclass it really should not move whole class body one tab to right (3 vs 4) and create whole class as changed.

In case, that constructor is needed to be on new line, I think, that super class / inheritance / interface should be on new line again as whole class and not behind constructor (2).

My expected result could be:

---

Compare Android Studio / IntelliJ default

abstract class MyParentClass(
    open val boolParam: Boolean,
)

@HiltViewModel
class MyClass @Inject constructor(
    override val boolParam: Boolean,
) : MyParentClass(
    boolParam = boolParam
) {
    private val myVar: String = "Hello"
}

vs formatted with ktlint (moved class body to right - use 8 spaces)

abstract class MyParentClass(
    open val boolParam: Boolean,
)

@HiltViewModel
class MyClass
    @Inject
    constructor(
        override val boolParam: Boolean,
    ) : MyParentClass(
            boolParam = boolParam,
        ) {
        private val myVar: String = "Hello"
    }

We want to adapt ktlint to our existing project, but it make hunders/thousands file changes just by moving body. Code in that case is not consistent, because when you just remove @Inject

it's like Intellij / AS

@HiltViewModel
class MyClass constructor(
    override val boolParam: Boolean,
) : MyParentClass(
    boolParam = boolParam,
) {
    private val myVar: String = "Hello"
}

after ktlint:

@HiltViewModel
class MyClass constructor(
    override val boolParam: Boolean,
) : MyParentClass(
        boolParam = boolParam,
    ) {
    private val myVar: String = "Hello"
}

Body persist on 4 spaces instead of 8 used by adding annotation. Only boolParam = boolParam, and next line ) { is moved by 4 spaces, rest keeps same. But still I think, that this code is not well formatted, because ) { should persist as it was formatted by IntelliJ

---

When I set: ktlint_standard_annotation = disabled

It keeps annotation as expected, but still there is inconsistency with 4 spaces

[bcmedeiros]

I'm ok with keep indent and constructor on new lines (1), but just because you add annotated constructor + superclass it really should not move whole class body one tab to right (3 vs 4) and create whole class as changed.

I couldn't agree more. There is no reason for the entire class body to have 2 tabs.