[#250 clone] Run `ktlint` in process isolation + allow overriding `ktlint` version #279

mateuszkwiecinski · 2022-09-10T21:29:22Z

Clone of #250 with my suggested improvements/changes

new configurations will be automatically configured, even for custom tasks
It's possible to override ktlint version
@hrach's custom rules were migrated to new ktlint 0.47.1 apis
I slightly changed the way errors are reported back
Plugin itself won't expose ktlint dependency anymore to its consumer (it's defined as compileOnly now)
default runtime ktlint version is kept in sync with the one compilation uses
[new] It doesn't only uses ktlint APIs, but also resolves rules from the ktlint version provided via plugin extension

Fixes #280
Fixes #34

mateuszkwiecinski · 2022-09-14T08:02:15Z

Potential blocker: gradle/gradle#21964

Karn · 2022-09-30T19:06:30Z

Hey @mateuszkwiecinski! Looking forward to this PR! I'm wondering if you have considered just manually loading the custom rules using something like:

fun ktlintRulesetsFromClasspath(classpath: ConfigurableFileCollection): List<RuleSetProviderV2> {
    // Load the files from the classpath into a new ClassLoader
    @Suppress("DEPRECATION")
    val fileUris = classpath.map { it.toURL() }.toTypedArray()
    val classLoader = URLClassLoader(fileUris, Thread.currentThread().contextClassLoader)
    return ServiceLoader.load(RuleSetProviderV2::class.java, classLoader).toList()
}

Where classpath here is the ktlintClasspath. That way we can bypass the processIsolation issues, at least temporarily.

mateuszkwiecinski · 2022-10-01T11:00:47Z

@Karn Thanks for sharing the idea💡
I haven't considered any workarounds yet, mainly because the process isolation works just fine. I did some manual testing, and it all worked like a charm. The blocker I stumbled upon turned out not being related to the process isolation, but rather the quirky behavior of Gradle TestKit testing framework 🤷
I disabled problematic tests on Windows, and the PR should soon reach reviewable state :)

.github/workflows/default.yml

mateuszkwiecinski · 2022-10-15T15:22:53Z

README.md

@@ -157,6 +157,7 @@ kotlinter {
    reporters = arrayOf("checkstyle", "plain")
    experimentalRules = false
    disabledRules = emptyArray()
+    ktlintVersion = "x.y.z"


I wasn't sure how to reflect the ktlint version can be overriden. the text block above mentions these are the defaults, and the default value is the current ktlint version (the one kotlinter is compiled with, to be more precise)

As an alternative, I can propose having some sort of hardcoded value that will be mapped into "our" ktlint version, i.e.

Suggested change

ktlintVersion = "x.y.z"

ktlintVersion = "default" // or `latest` or `current`? but that might be too confusing

I usually show the default value if you don't configure anything.
Which I suppose also causes a little confusion because people explicitly configure it not realizing that's what they get by default.

README.md

build.gradle.kts

src/main/kotlin/org/jmailen/gradle/kotlinter/support/RuleSets.kt

src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt

mateuszkwiecinski · 2022-10-15T16:01:35Z

src/test/kotlin/org/jmailen/gradle/kotlinter/functional/KotlinProjectTest.kt

+        build("lintKotlin", "--info").apply {
+            assertEquals(SUCCESS, task(":lintKotlin")?.outcome)
+            assertTrue(output.contains("Resolved 45 RuleSetProviders"))
+        }
+
+        build("formatKotlin", "--info").apply {
+            assertEquals(SUCCESS, task(":formatKotlin")?.outcome)
+            assertTrue(output.contains("Resolved 45 RuleSetProviders"))
+        }


This is not ideal tests, and will likely fail when upgrading ktlint version, but I was out of ideas and decided to defer writing better assertion to somewhen later 😬
What I want to test here is to somehow replace the old test relying on test classpath, with something that covers the dynamic loading part.

test-project-android/app/build.gradle.kts

hrach · 2022-12-05T14:33:32Z

Friendly reminder - it would be great to have the functionality for custom rules. Thank you.

… sync

…encies. Add tests covering new features

jeremymailen · 2024-12-12T08:38:06Z

Thanks, this example was very helpful. The feature has been incorporated in 5.0.0. Cheers

Base automatically changed from bump_kotlinter to master September 10, 2022 22:37

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 9584ae9 to 2fd861d Compare September 11, 2022 12:23

hrach mentioned this pull request Sep 17, 2022

introduce ktlintRuleSet configuration (BC break!) #250

Closed

mateuszkwiecinski mentioned this pull request Sep 28, 2022

Migrate to JUnit5 + bump AGP to 7.3.0 + bump Kotlin to 1.7.20 #285

Merged

mateuszkwiecinski changed the base branch from master to mateuszkwiecinski/migrate_to_junit5 October 1, 2022 10:51

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 2fd861d to fe8c721 Compare October 1, 2022 10:51

jeremymailen mentioned this pull request Oct 3, 2022

If we can provide a property in KotlinterExtension to override ktlint version? #280

Open

Base automatically changed from mateuszkwiecinski/migrate_to_junit5 to master October 3, 2022 15:47

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from fe8c721 to 3ebc2a7 Compare October 9, 2022 18:02

mateuszkwiecinski changed the title ~~[#250 clone] Run ktlint in process isolation~~ [#250 clone] Run ktlint in process isolation + allow overriding ktlint version Oct 9, 2022

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 869e1c7 to 4eb8585 Compare October 15, 2022 16:04

mateuszkwiecinski commented Oct 15, 2022

View reviewed changes

mateuszkwiecinski marked this pull request as ready for review October 15, 2022 16:10

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 4eb8585 to 203414b Compare October 22, 2022 20:05

mateuszkwiecinski mentioned this pull request Oct 22, 2022

Load Reporters dynamically, using provided ktlint version #294

Closed

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 203414b to 9c8f09d Compare October 23, 2022 21:30

Karn mentioned this pull request Dec 22, 2022

Add support for custom Ktlint RuleSets Karn/kotlinter-gradle#2

Closed

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 9c8f09d to dd2b917 Compare January 5, 2023 06:37

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from dd2b917 to 1036d33 Compare January 20, 2023 18:40

jeremymailen mentioned this pull request Jan 22, 2023

Bump ktlint to 0.48.1 #304

Merged

hrach and others added 6 commits January 24, 2023 19:56

introduce ktlintRuleSet configuration (BC break!)

2c49b3e

Move custom-rules-test into test-project-android

df272cd

Define ktlint dependency as compileOnly

ba778e3

Update test matrix

f80a704

Unify plugin errors

c63c9a3

Make ktlintVersion customizable via plugin extension

dd533a8

mateuszkwiecinski added 7 commits January 24, 2023 19:57

Set ktlintClasspath and ruleSetsClasspath for all kotlinter tasks

ef005c9

Restore failed sourceSet name

bdc4f06

Ensure compileOnly classpath and default ktlint runtime are kept in…

c10f800

… sync

Disable failing tests on Windows

f325b98

Load RuleSetProviders from resolved classpach. Remove unused depend…

c9068cd

…encies. Add tests covering new features

Update README to reflect latest behavior changes

e8bce9d

Fix Gradle 8.0 compatibility

ff27fdf

mateuszkwiecinski force-pushed the run_lint_with_process_isolation branch from 6b229e8 to ff27fdf Compare January 24, 2023 19:11

jeremymailen mentioned this pull request Mar 9, 2023

kotlinter 3.13.0 crashes when running with ktlint 0.48.2 #316

Closed

Karn mentioned this pull request May 17, 2023

Add support for custom Ktlint RuleSets Karn/kotlinter-gradle#3

Closed

jeremymailen closed this Dec 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#250 clone] Run `ktlint` in process isolation + allow overriding `ktlint` version #279

[#250 clone] Run `ktlint` in process isolation + allow overriding `ktlint` version #279

mateuszkwiecinski commented Sep 10, 2022 •

edited

Loading

mateuszkwiecinski commented Sep 14, 2022

Karn commented Sep 30, 2022

mateuszkwiecinski commented Oct 1, 2022 •

edited

Loading

mateuszkwiecinski Oct 15, 2022

mateuszkwiecinski Oct 17, 2022

jeremymailen Jan 30, 2023

mateuszkwiecinski Oct 15, 2022

hrach commented Dec 5, 2022

jeremymailen commented Dec 12, 2024

	ktlintVersion = "x.y.z"
	ktlintVersion = "default" // or `latest` or `current`? but that might be too confusing

[#250 clone] Run ktlint in process isolation + allow overriding ktlint version #279

[#250 clone] Run ktlint in process isolation + allow overriding ktlint version #279

Conversation

mateuszkwiecinski commented Sep 10, 2022 • edited Loading

mateuszkwiecinski commented Sep 14, 2022

Karn commented Sep 30, 2022

mateuszkwiecinski commented Oct 1, 2022 • edited Loading

mateuszkwiecinski Oct 15, 2022

Choose a reason for hiding this comment

mateuszkwiecinski Oct 17, 2022

Choose a reason for hiding this comment

jeremymailen Jan 30, 2023

Choose a reason for hiding this comment

mateuszkwiecinski Oct 15, 2022

Choose a reason for hiding this comment

hrach commented Dec 5, 2022

jeremymailen commented Dec 12, 2024

[#250 clone] Run `ktlint` in process isolation + allow overriding `ktlint` version #279

[#250 clone] Run `ktlint` in process isolation + allow overriding `ktlint` version #279

mateuszkwiecinski commented Sep 10, 2022 •

edited

Loading

mateuszkwiecinski commented Oct 1, 2022 •

edited

Loading