Add maxRuleVersion configuration for version-based rule filtering #943
Conversation
|
For reference, the equivalent change in Ktlint has been merged - pinterest/ktlint#3101 |
| * @param maxVersion The maximum allowed version (from maxRuleVersion configuration) | ||
| * @return true if the rule version is compatible (should be included), false otherwise | ||
| */ | ||
| private fun isVersionCompatible(ruleVersion: String, maxVersion: String): Boolean { |
There was a problem hiding this comment.
we use net.swiftzer.semver.SemVer elsewhere in this project, so might as well use it here
|
Thanks for the contribution! |
|
Appreciate the feedback, I've gone ahead and implemented that, let me know your thoughts on it |
|
Looking good. one more question: how will this interact with versions of ktlint from before the annotation was retained at runtime? I think it should fail "open" in that case, and just include all rules. We support using ktlint versions back to 1.0.0, so this must be considered. |
|
That's a very good point - I was thinking more of in the future, if someone doesn't include it, but in practice, my change guarantees (with a reflection based test) that all rules will have the annotation, the issue is definitively the backwards compatibility, where some of them do not have it. I'll ensure that if it doesn't find it (the current scenario on 1.7.1 and below), it will accept all of them. I'll also make that clear in the javadoc of the new parameter. |
|
Ok actually the backwards compatibility was already baked in. I've updated the comments and added the @SInCE in the API itself, as the code was already merged and is currently planned as part of the 1.8.0 release |
| @DisplayName("Should allow setting maxRuleVersion") | ||
| @CommonTest | ||
| fun allowMaxRuleVersionConfiguration(gradleVersion: GradleVersion) { | ||
| project(gradleVersion) { | ||
| withCleanSources() | ||
| //language=Groovy | ||
| buildGradle.appendText( | ||
| """ | ||
| ktlint { | ||
| maxRuleVersion = "1.0.0" | ||
| } | ||
| """.trimIndent() | ||
| ) | ||
|
|
||
| build(CHECK_PARENT_TASK_NAME) { | ||
| assertThat(task(":$mainSourceSetCheckTaskName")?.outcome).isEqualTo(TaskOutcome.SUCCESS) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @DisplayName("Should work without maxRuleVersion set") | ||
| @CommonTest | ||
| fun workWithoutMaxRuleVersion(gradleVersion: GradleVersion) { | ||
| project(gradleVersion) { | ||
| withCleanSources() | ||
|
|
||
| build(CHECK_PARENT_TASK_NAME) { | ||
| assertThat(task(":$mainSourceSetCheckTaskName")?.outcome).isEqualTo(TaskOutcome.SUCCESS) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
So... These test that the maxRuleVersion can be set, but not that it behaves in a way that is expected. We should have a test for that.
There was a problem hiding this comment.
Totally agree, but for that, I think we'll need to park this PR until Ktlint 1.8.0 gets published, as the annotation isn't present at runtime (what we need) before. Or do you know of any other way to build against the unpublished Ktlint version?
| private fun isRuleCompatibleWithVersion(ruleProvider: RuleProvider, maxVersion: String): Boolean { | ||
| // Use reflection to check for @SinceKtlint annotation | ||
| val ruleClass = ruleProvider.createNewRuleInstance()::class.java | ||
| val sinceAnnotation = ruleClass.getAnnotation(SinceKtlint::class.java) | ||
|
|
||
| return if (sinceAnnotation != null) { | ||
| isVersionCompatible(sinceAnnotation.version, maxVersion) | ||
| } else { | ||
| // If no annotation, assume it's from an older ktlint version (before annotations were included | ||
| // at runtime) and include it for backward compatibility | ||
| true | ||
| } | ||
| } |
There was a problem hiding this comment.
This filtering isn't a feature built into ktlint itself as a library? I guess we are doing the rule loading manually, so I suppose this makes sense.
There was a problem hiding this comment.
Correct - per Paul's comment, it's the Ktlint Gradle plugin that provides the rules, so it's up to Ktlint Gradle to filter them out (if required). See pinterest/ktlint#3099 (comment)
3 participants
Address the feature request in #942
Summary
Adds
maxRuleVersionconfiguration to allow filtering rules based on their@SinceKtlintversion annotation.Changes
maxRuleVersionproperty toKtlintExtensionKtLintInvocation100based onmaxRuleVersionsettingMore context in the issue mentioned above.