Should we remove detekt-formatting (KtLint Wrapper)

[cortinico]

Hi all,
I'm opening this poll to ask the community whether we should remove the support for detekt-formatting or not.

The detekt formatting ruleset is a first party collection of rule we maintain that effectively wraps KtLint (see https://detekt.dev/docs/rules/formatting/).

It's shipped as a separate artifact that developers have to include it with a detektPlugins dependency: https://detekt.dev/docs/introduction/extensions/#let-detekt-know-about-your-extensions

This ruleset has always been controversial for a number of reasons:

• It breaks the single responsibility principle. Users that want to run more than one tool should use Spotless or other mechanism to orchestrate rules.
• It adds maintenance burden for us as every KtLint bump is non-trivial (e.g. this one Upgrade ktlint to 0.47.1 #5312)
• We do have rules inside style that provide the same functionalities as the equivalent formatting rules, like WildcardImport vs NoWildcardImports or UnusedImports vs NoUnusedImports. This is confusing for users as they don't immediately understand why they do have two similar rule in the config files.
• Historically, it was harder to suppress rules from the formatting ruleset due to how KtLint exposes reporting information (fixed in 1.23.x Add support for local suppression inside formatting #5876 though).

We're going to publish a detekt-formatting artifact for 1.23.x, but we're wondering if this is used and also how (like what's your use case). If there are no meaningful use cases, we'll probably consider dropping it.

So please vote to this poll + share your concerns and use case.

[cromefire]

I am using it, but if most stuff is natively supported in detekt I'd still be happy to just drop it. (Auto correction with ktlint is pretty great)

[PoisonedYouth]

I use detekt + ktlint in all my projects separately so there was no need for using format rules of detekt.

[tfcporciuncula]

We're using it since for formatting rules we prefer ktlint's auto correct capabilities (and it's super cool how detekt's IntelliJ plugin works well with that). The first point about using Spotless instead makes a lot of sense, though, and maybe we should've been doing that instead. But since we just have detekt and ktlint, it's really convenient to be able to use detekt's wrapper and not bother with anything else.

I also totally understand how it can be confusing to have overlapping between rules, but I would assume anyone interested in using both detekt and ktlint should be aware about it, and I personally think detekt's docs on the formatting ruleset do a great job making that explicit (with the "This rules overlaps with..." sentences there).

[mklnwt]

The formatting wrapper is amazing. It allows easier customisation than ktlint natively and the IntelliJ plugin works like a charm. The formatter is the main reason we started using detekt do begin with. I would for sure be missing it.

[mustafaozhan]

If it is also affecting the IDE plugin, I use it and we as a team love it because it teaches formatting to newcomers during they write their codes. It helps a lot in PR reviews since the formattings are already filtered out with the plugin.

[mklnwt]

The ktlint plugin is unofficial though

[cromefire]

And so is the ktlint-detekt integration.

[mklnwt]

From the view of ktlint, yes. From the view of detekt, it's part of this repo.

[cromefire]

Well but ktlint is doing the changes to ktlint not detekt, so it's unofficial, both might break after an ktlint update, but so far both were very stable for me.

[mklnwt]

We recently switched to using ktlint & detekt separately.
The unofficial plugin now has contributions from the the main ktlint contributor.

[3flex]

If detekt maintainers decide to drop the formatting plugin then we can ask community members or an organisation to take over.

It adds maintenance burden for us as every KtLint bump is non-trivial

This pain point should actually go away very soon as the next planned ktlint release is version 1.0. The 0.49.0 release notes say that there are no more breaking changes planned.

[mklnwt]

I also believe the stability with 1.0 should increase.

[BraisGabin]

If detekt maintainers decide to drop the formatting plugin then we can ask community members or an organisation to take over.

Yes, the idea would not be remove it. We would move it to a different repo and ask for someone to take over.

[mklnwt]

Would it make sense to drop the style rules in favor of ktlint to reduce the maintenance burden?

With version 1.0 and ktlint_official the ktlint team aims to achieve a more consistent formatting.
It would be sad to see the styles diverge. Especially if people are using detekt and ktlint together.

In terms of maintenance burden and single responsibility I'm wondering if the wrapper would be the better option since detekt would pass the responsibility of style fully to ktlint. The wrapper itself is just a wrapper, not a direct responsibility.

In terms of using spotless there is no support for detekt afaik. Imo detekt is rather an alternative to spotless (a bundle of different rulesets to run) than just a ruleset on the same level as ktlint.

The overall plugin support (being able to run different rulesets and display the result directly in the IDE) plus the autofixes (globally or per file) are the biggest value adds of detekt to me.

[cortinico]

In terms of using spotless there is no support for detekt afaik

Yup you're right. We should add support for Detekt in Spotless:

• Support for Kotlin Formatter Detekt diffplug/spotless#143
  This is also up for grab if someone wants to contribute.

Would it make sense to drop the style rules in favor of ktlint to reduce the maintenance burden?

I'm unsure what are you suggesting here. A lot of the rules in style are missing in KtLint

[mklnwt]

Would it be an option to suggest/add the missing rules to ktlint?
Isn't ktlint supposed to be a complete styling?

When talking about single responsibility to me it would make more sense to offload that responsibility to ktlint rather than dropping the support and maintaining a similar ruleset separately.

I would much prefer to use ktlint over detekt than interesting spotless with detekt and ktlint and then having to use both ktlint and detekt plugins. Currently one just has to integrate detekt (from a user perspective ktlint comes baked in) and use the detekt plugin. That's 2 vs 5 dependencies to consider.

The one benefit I see to this is easier version updates of ktlint since detekt is lagging behind.

[cortinico]

When talking about single responsibility to me

The problem is already that ktlint is doing more than a formatter and has some rules which are part of the Static Analysis domain.

Would it be an option to suggest/add the missing rules to ktlint?

I mean, we could, but we probably want to retain some of the rules there. Plus, we offer more configuration options than ktlint.

I would much prefer to use ktlint over detekt than interesting spotless with detekt and ktlint and then having to use both ktlint and detekt plugins.

We can also just move detekt-formatting outside of the Detekt repo and follow a different upgrade schedule or find someone else that is willing to help maintaining it.

[BraisGabin]

I think that detekt style rule set goes beyond a formatter. Rules like unnecessary let doesn't belong to ktlint. So I don't think we should remove it out try to pass them to ktlint.

Imo detekt is rather an alternative to spotless (a bundle of different rulesets to run) than just a ruleset on the same level as ktlint.

I don't think of detekt as a ruleset runner. That's what spotless does. Detekt is a static analysis tool focused on code smells. Find potential problems on the analyzed code. Ktlint works on the formatting level. For that reason I feel normal to enable two different plugins for two different tasks.

And I understand, this wrapper can be handy. But should be supported as a first party plugin? Couldn't it be maintained by a third party?

[haluzpav]

Did you consider making the formatting wrapper "smarter"? For example, you could remove the duplicate rules, show a warning when the the both duplicate rules are enabled, rename rules etc. To make it less confusing, you could market it not as a "KtLint wrapper", but as a "ruleset based on KtLint".

Not sure about this, just throwing out an idea. 🙂

[BraisGabin]

A really important point to note here. This poll is to decide if we should keep maintaining this wrapper. If we decide to drop the support of the wrapper we will not remove it. We will move it to a different repository and ask for some one to take it over. We know that some people use this plugin we would try to make the transition as easy as possible.

[eygraber]

Having formatting get auto corrected is one of my favorite features of detekt. I know that I could just run ktlint or something like spotless instead, but I much prefer doing everything through detekt.

[eygraber]

I'd actually like to change my answer. I moved to using ktlint directly a few months ago, and while I think that's the better overall approach, I'm not thrilled with the new direction that ktlint has headed in. The new maintainer is super opinionated, doesn't always follow the style guide, and allows almost 0 configuration.

I'd be super happy with a formatting tool built with detekt without any reliance on ktlint. The API for implementing rules is much nicer, and the usage of configuration remains useful, but not a free for all.

[JavierSegoviaCordoba]

I have the hope to get ktfmt support on detekt some day so I can drop an additional plugin.

[3flex]

That request should be made separately however, I don't think that's a good fit for detekt. ktfmt is a code formatter while detekt is primarily a linter.

Spotless already supports ktlint and ktfmt, and there's an open issue re: adding detekt support, so I'd suggest that's the best option if you want to use a mix of Kotlin tools in a single plugin.

[cortinico]

As an alternative, you can create your own ktfmt wrapper as a 3rd party rule. We'll be happy to add it it to the Marketplace

[hrach]

We have just started using this. Because:

• other ktlint Gradle wrappers are not supporting 0.49 (👏)
• detekt has great support for custom rules
• having one tool is better than two (we've been using detekt without it)

[matejdro]

We started using ktlint-gradle, then we adopted Detekt and were running ktlint-gradle + detekt and then finally switched to just using detekt for everything.

IMO it is very beneficial to have everything in one package:

• Much easier setup (no need to configure and maintain gradle / git hook / CI configuration etc. for the two different static analysis plugins)
• Single output (no need to read two different reports, detekt contains everything)
• Much easier to configure (Detekt configuration style + ability to trivially easy turn off various rules is miles ahead of everything else for ktlint)

I do agree thought, that duplicate rules are a problem. I feel like detekt variant of duplicate rules is generally better than ktlint variant (more configurability, better documentation), so maybe duplicate ktlint rules could receive Ktlint suffix (to avoid naming confusion) and be disabled by default?

[rmarquis]

I do agree thought, that duplicate rules are a problem. I feel like detekt variant of duplicate rules is generally better than ktlint variant (more configurability, better documentation), so maybe duplicate ktlint rules could receive Ktlint suffix (to avoid naming confusion) and be disabled by default?

I don't feel this would solve the duplicate issue. I know I might not be part of the majority, but I don't feel having more configurability is an advantage for detekt rules compared to the more strict and "no configuration needed and enforce Kotlin conventions" approach of Ktlint. And when configuration is possible, I'd rather have to edit a single .editorconfig file than messing with both detekt and Ktlint configuartion.

In other words, the duplicates need to go away. It might be possible to port that extra bit of config to the Ktlint ruleset instead, but I'd overall prefer to lose these small bits and not have overlapping here.

[matejdro]

But you don't need to edit .editorconfig at all. Everything is in detekt's config. So current integrated solution also only needs single config file.

[G00fY2]

Since I expect most Kotlin devs using Gradle, there are actually not many reliable ktlint Gradle integrations that are 1. actively maintained and 2. support Android projects.

Previously we used the ktlint plugin from JLLeitschuh which wasn't catching up with ktlint updates and Gradle API changes (configuration cache etc.). Therefore the detekt ktlint wrapper was the perfect match for all our projects and we completely dropped other formatting plugins.

Because of that I voted for "No, please keep it. We're using it". But if ktlint integration would just be moved into a separate repo and could be easily integrated like e.g. compose-rules than I would also welcome it (should make no difference for consumers). This would even allow using ktlint updates without waiting for detekt releases, right? If this is the case, than maybe the poll is a bit misleading.

[cortinico]

Thanks for your feedback @G00fY2
I believe initially we just wanted to remove the KtLint integration, as it wasn't clear who was using it or not. Agree that the poll as it can be confusing, as we might just consider moving ktlint integration to a separate repo. Still, this discussion is valuable for us as we can get an idea of how our APIs are used and how we can support our users better.

[ColtonIdle]

IMO it makes no sense to keep ktlint in detekt. they're two different tools and (although convenient) I can make the same case about it being convenient for the detekt team to add ktfmt (which has some pros over ktlint).

keep the library focussed IMO and make it easier for other people to understand (my team gets confused with why detekt does formatting if its a static code analyzer)

[rocketraman]

My vote lies somewhere between:

• Yes, please remove it. We're not using it.
• No, please keep it. We're using it.

My vote would be:

• Remove it, despite the fact that I'm using it.

The problem is that since ktlint is integrated into detekt, when we run into issues users naturally report them against detekt -- as end-users, we don't always know or care that another library is performing some of the function of the library we are using.

Furthermore, we can run into issues like #6615 in which the detekt team says: "that's not our issue, that's ktlint, report it there", and then the ktlint team says "we can't reproduce it with ktlint, and we know nothing about detekt, so bugger off" (pinterest/ktlint#2351 (comment)).

I'd rather just use ktlint directly so I am aware as a consumer of what I am using, and I can intelligently debug two simple and independent things and know straight-away where an issue lies, rather than one complected thing where the boundaries and responsibilities for issues are unclear -- is it detekt, is it ktlint, or is it the integration between detekt and ktlint?

Either that, or the detekt team takes responsibility for its dependencies and deals with reports directly, and itself takes on the responsibility of reporting issues upstream when indeed the problem is upstream.

[3flex]

I'd rather just use ktlint directly

Of course this is always an option regardless of what we decide to do with the integration.

the detekt team... takes on the responsibility of reporting issues upstream when indeed the problem is upstream

Assume this won't happen. Please remember that maintainers are volunteering their time and that detekt, including the ktlint wrapper, is offered as is with no warranties.

[mgroth0]

Reading through this discussion, I did not see any mention of performance.

I have some questions:

1. In theory, should we expect that running ktlint inside of Detekt is faster than running ktlint and detekt separately?
2. In practice, do measurements exist that verify this?
3. In what ways can including ktlint inside of detekt lead to performance gains? Can they share any caches? Do they share parsing results and avoid redundant parsing?

[BraisGabin]

1. In theory, should we expect that running ktlint inside of Detekt is faster than running ktlint and detekt separately?

Maybe? If we talk about executing detekt cli with formatting vs detekt cli without formatting + ktlint cli... probably detekt is a bit faster because it avoids to parse the files twice. But if we enter in the gradle land there are A LOT of other things to think about when we talk about "speed". ktlint only cares about the files, if a dependency changes ktlint doesn't need to reexecute, detekt does. So it's nearly impossible to answer this question.

2. In practice, do measurements exist that verify this?

Nope. And we don't have nor support detekt-formatting for performance reasons. And as I said it's difficult to compare.

3. In what ways can including ktlint inside of detekt lead to performance gains? Can they share any caches? Do they share parsing results and avoid redundant parsing?

They can't share any caches. That's completely the oposite, if you run ktlint with (for example) spotless and detekt you will get faster incremental builds than just using detekt, probably. And yes, they share parsing when using detekt-formatting.

[mgroth0]

Thanks for your answers.

ktlint only cares about the files, if a dependency changes ktlint doesn't need to reexecute, detekt does

For me, this is a great reason to keep the tasks seperate. I wonder though, does this have anything to do with the discussions I have been seeing about type resolution vs no type resolution detekt executions? If Detekt ends up having a task that is designed to explicitly not use type resolution, does that mean it does not care about dependencies and can be like ktlint in that it only cares about the direct content of the kotlin files?

And yes, they share parsing when using detekt-formatting

This is interesting though and persuasive in the other direction that embeding ktlint in detekt would, in theory, have a performance gain.

So if I understand correctly, if detekt ends up having tasks that explicitly do not use type resolution (and don't use dependency info as input) then they can include detekt-formatting and get the performance gains of reusing the parsing results without having the performance loss from rerunning ktlint when dependencies change. Do I have that right?

[BraisGabin]

I wonder though, does this have anything to do with the discussions I have been seeing about type resolution vs no type resolution detekt executions

Yes. Partially related. The thing is that we are not going to have 2 separated task one with type solving and another without. We would have one without type solving and another with all the rules. For sure someone could fine tune it make what you say but probably no worth the effort.

Also, we took a decision here already (if nothing else appear to change our mine). We will move detekt-formatting outside the main repo but we will not remove it.