Upgrade ktlint support #595

fthouraud · 2022-07-02T09:56:13Z

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 · 2022-07-05T15:08:40Z

plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/worker/KtLintWorkAction.kt

+    private val androidProperty = UsesEditorConfigProperties.EditorConfigProperty(
+        type = PropertyType.LowerCasingPropertyType(
+            "android",
+            "A boolean value indicating that the project is an Android one.",
+            PropertyType.PropertyValueParser.IDENTITY_VALUE_PARSER,
+            emptySet()
+        ),
+        defaultValue = "false"
+    )


Can't this be inferred?

It seemed to me you were treating this property as a String but it seems feasible to be "inferred" when looking at how the indentSize property is handled in the ktlint API.

I can give it a try.

JLLeitschuh · 2022-07-05T15:11:47Z

plugin/src/test/kotlin/org/jlleitschuh/gradle/ktlint/testdsl/testAnnotations.kt

@@ -12,10 +12,10 @@ import kotlin.streams.asStream

 object TestVersions {
    const val minSupportedGradleVersion = KtlintBasePlugin.LOWEST_SUPPORTED_GRADLE_VERSION
-    const val maxSupportedGradleVersion = "7.1.1"
+    const val maxSupportedGradleVersion = "7.4.2"


We like keeping the same version range support as the Google android plugin in general. Is this still kept with this change?

If we're on the same page, the latest Android Gradle plugin version is 7.2 which supports Gradle 7.3.3 and onwards (source). Do you want me to lower the version to 7.3.3?

Support as high as possible. But make sure we're not loosing versions still supported by the AGP

fthouraud · 2022-07-15T13:43:13Z

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

JLLeitschuh · 2022-07-17T10:19:32Z

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 🙂

fthouraud · 2022-07-24T16:50:07Z

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.

JLLeitschuh · 2022-08-23T15:05:40Z

I believe that this is now outdated by #597, correct?

JLLeitschuh · 2022-08-23T15:06:29Z

I believe that this is now outdated by #597, correct?

G00fY2 · 2022-08-25T07:48:53Z

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

fthouraud · 2022-08-25T08:55:31Z

I'll give this a look during the week-end. I should be able to rebase it.

That's progress anyway. Thanks.

G00fY2 · 2022-08-25T09:24:22Z

Maybe you can even check whether ktlint 0.47.0 works with your changes. ♥️

fthouraud · 2022-08-28T19:38:16Z

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.

scottkennedy · 2022-09-02T23:43:58Z

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'm certainly in favor of this. Thanks for your effort.

jzbrooks · 2022-09-07T15:42:10Z

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

dev-weiqi · 2022-10-12T08:16:13Z

Any update on this PR?
Compose Rules requires ktlint versions over 0.46.0 🥲

davidburstrom · 2022-10-14T08:33:25Z

You might want to do something similar to diffplug/spotless#1303 to ensure it's easy to maintain support for multiple versions of KtLint.

wakingrufus · 2023-02-02T22:02:46Z

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

JLLeitschuh reviewed Jul 5, 2022

View reviewed changes

scana added a commit to scana/ktlint-gradle that referenced this pull request Jul 7, 2022

Support rules disabling - taken from JLLeitschuh#595 PR

96d6c5b

scana mentioned this pull request Jul 7, 2022

Migrate to ktlint 0.46.0 #593

Closed

4 tasks

fthouraud force-pushed the upgrade-ktlint-support branch 2 times, most recently from 95136c3 to 3da35ac Compare July 8, 2022 18:27

AleksanderBrzozowski mentioned this pull request Jul 8, 2022

bump kotlin to 1.5.31, bump ktlint to 0.43.2 #597

Merged

fthouraud force-pushed the upgrade-ktlint-support branch from 3da35ac to 0767c95 Compare July 24, 2022 16:28

fthouraud force-pushed the upgrade-ktlint-support branch from 4e0fd59 to df6eeb4 Compare July 25, 2022 05:40

fthouraud force-pushed the upgrade-ktlint-support branch 2 times, most recently from 1690834 to 656cd45 Compare August 2, 2022 06:20

G00fY2 mentioned this pull request Aug 25, 2022

The plugin does not work when using Ktlint 0.46.0 #589

Closed

fthouraud added 6 commits August 28, 2022 20:04

Upgrade ktlint supported version up to 0.44.0

4e3ab73

gradle 7.3.3

ca6a6b8

Upgrade Kotlin up to 1.6.21

9392b94

Upgrade ktlint supported version up to 0.45.2

b5bd553

Upgrade ktlint supported version up to 0.46.1

be947e2

Remove some Ktlint version check

ef520c2

fthouraud force-pushed the upgrade-ktlint-support branch from 656cd45 to ef520c2 Compare August 28, 2022 19:03

LionZXY mentioned this pull request Oct 13, 2022

Add Compose rules flipperdevices/Flipper-Android-App#412

Closed

mrmans0n mentioned this pull request Oct 28, 2022

A failure occurred while executing org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction > com/pinterest/ktlint/core/RuleSetProviderV2. twitter/compose-rules#108

Closed

luizgrp mentioned this pull request Dec 13, 2022

Add twitter compose rules check google/horologist#831

Open

fthouraud closed this Feb 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade ktlint support #595

Upgrade ktlint support #595

fthouraud commented Jul 2, 2022

JLLeitschuh Jul 5, 2022

fthouraud Jul 5, 2022

JLLeitschuh Jul 5, 2022

fthouraud Jul 5, 2022 •

edited

JLLeitschuh Jul 8, 2022

fthouraud commented Jul 15, 2022

JLLeitschuh commented Jul 17, 2022

fthouraud commented Jul 24, 2022

JLLeitschuh commented Aug 23, 2022

JLLeitschuh commented Aug 23, 2022

G00fY2 commented Aug 25, 2022 •

edited

fthouraud commented Aug 25, 2022

G00fY2 commented Aug 25, 2022

fthouraud commented Aug 28, 2022

scottkennedy commented Sep 2, 2022

jzbrooks commented Sep 7, 2022

dev-weiqi commented Oct 12, 2022

davidburstrom commented Oct 14, 2022

wakingrufus commented Feb 2, 2023

Upgrade ktlint support #595

Upgrade ktlint support #595

Conversation

fthouraud commented Jul 2, 2022

JLLeitschuh Jul 5, 2022

Choose a reason for hiding this comment

fthouraud Jul 5, 2022

Choose a reason for hiding this comment

JLLeitschuh Jul 5, 2022

Choose a reason for hiding this comment

fthouraud Jul 5, 2022 • edited

Choose a reason for hiding this comment

JLLeitschuh Jul 8, 2022

Choose a reason for hiding this comment

fthouraud commented Jul 15, 2022

JLLeitschuh commented Jul 17, 2022

fthouraud commented Jul 24, 2022

JLLeitschuh commented Aug 23, 2022

JLLeitschuh commented Aug 23, 2022

G00fY2 commented Aug 25, 2022 • edited

fthouraud commented Aug 25, 2022

G00fY2 commented Aug 25, 2022

fthouraud commented Aug 28, 2022

scottkennedy commented Sep 2, 2022

jzbrooks commented Sep 7, 2022

dev-weiqi commented Oct 12, 2022

davidburstrom commented Oct 14, 2022

wakingrufus commented Feb 2, 2023

fthouraud Jul 5, 2022 •

edited

G00fY2 commented Aug 25, 2022 •

edited