ktlint-gradle
ktlint-gradle copied to clipboard
Upgrade ktlint support
Hi 👋
This MR is an attempt on upgrading support of Ktlint up to 0.46.1 (#589).
I started to work on this before this MR #593 arrived. So I opened a new one anyway at least to provide help.
I've split my work into steps in order to test each version from 0.42.1 to 0.46.1.
I also upgraded Gradle to 7.4.2 and Kotlin to 1.6.21.
@JLLeitschuh how do you see the Ktlint support of this plugin? As is, it only supports the 0.46.1
due to the breaking changes.
If you still want to support the full range of versions from 0.34.0
then I think there should be a module for all versions prior to 0.46.1
and a second one for all versions from 0.46.1
. The current plugin
module could handle all common work which does not require references to a Ktlint that has been broken. This is a tremendous amount of work but doable I think.
IMHO, I think the support for the older versions could be dropped with a new major release of this plugin. Ktlint is a quality tool that should not harm any project by upgrading it. Moreover, it is configurable so that any new rule could be disabled.
What's your opinion on this?
I guess one solution is to simply say that if someone requires backwards compatibility, they are welcome to contribute it.
Under that pretense, please be sure to clearly document that this will be a breaking change and move forward with the change 🙂
I apologize for the delay, I got an increased amount of work lately so I'm lacking time on this.
I updated the PR with a new minimal supported version to 0.45.2
. I've had to drop the 0.45.1
due to the lack of EditorConfigOverrides
.
I also stepped back on the main
property deprecation (JavaExecTask#main) as this would mean dropping the support for Gradle 6.0
but I think there's still time before the 8.0
.
I believe that this is now outdated by #597, correct?
I believe that this is now outdated by #597, correct?
@JLLeitschuh I don't get the point of the 11.0.0 release (which contains #597).:monocle_face: This only bumped Kotlin and Gradle. I thought the goal was to make ktlint-gradle ready for ktlint 0.46.+ (which this PR does)?
Maybe I am missing something?
I'll give this a look during the week-end. I should be able to rebase it.
That's progress anyway. Thanks.
Maybe you can even check whether ktlint 0.47.0 works with your changes. :hearts:
I've managed to rebase this PR without trouble.
Regarding the upgrade to Ktlint 0.47.0
, it will require more work than expected.
First, the RuleProvider
interface is now deprecated and marked for deletion on 0.48.0
. Its replacement, RuleProviderV2
, is loadable using the same way (i.e. ServiceLoader
). It's not very difficult to change that. I don't think it is reasonable to provide support for the old RuleProvider
due to its imminent deletion.
The second change concerns the Baseline
class which has been moved and refactored a bit. Nothing really difficult here.
Last, the generation of IDEA configuration is no longer provided by Ktlint. It is up to this plugin to also drop the support or to implement it directly.
I had some other failing tests I didn't look at closely but I don't think there are other significant issues.
I'll be glad to continue the upgrade process but I think this could be done in a separate MR due to the new changes it involves.
Please let me know what's your opinion on that.
I've managed to rebase this PR without trouble.
Regarding the upgrade to Ktlint
0.47.0
, it will require more work than expected.First, the
RuleProvider
interface is now deprecated and marked for deletion on0.48.0
. Its replacement,RuleProviderV2
, is loadable using the same way (i.e.ServiceLoader
). It's not very difficult to change that. I don't think it is reasonable to provide support for the oldRuleProvider
due to its imminent deletion.The second change concerns the
Baseline
class which has been moved and refactored a bit. Nothing really difficult here.Last, the generation of IDEA configuration is no longer provided by Ktlint. It is up to this plugin to also drop the support or to implement it directly.
I had some other failing tests I didn't look at closely but I don't think there are other significant issues.
I'll be glad to continue the upgrade process but I think this could be done in a separate MR due to the new changes it involves.
Please let me know what's your opinion on that.
I'm certainly in favor of this. Thanks for your effort.
It is up to this plugin to also drop the support or to implement it directly.
Dropping support in parity with the ktlint version seems reasonable. Especially if the major version was revised again and the minimum supported ktlint version was set to 0.47 (or perhaps 0.48 by the time this is merged). It would be a big, breaking change—but reducing the maintenance cost of this plugin seems valuable until it can get some new maintainers onboard.
There's also an argument to be made that IntelliJ should be responsible for its own configuration, so the kotlin plugin could provide an "official" and "android official" style preset, which would obviate the need for the ktlint/ktlint-gradle implementation.
Here's the relevant rationale upstream: pinterest/ktlint/issues/701
Any update on this PR? Compose Rules requires ktlint versions over 0.46.0 🥲
You might want to do something similar to https://github.com/diffplug/spotless/pull/1303 to ensure it's easy to maintain support for multiple versions of KtLint.
I think we can close this PR, since I have added support for newer versions of ktlint in a way that is backward compatible via reflection