From b68c994c446d8330232219a2d74c8da1b83b82bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Kwiecin=CC=81ski?= Date: Sun, 11 Sep 2022 08:49:12 +0200 Subject: [PATCH 1/3] Make lint task incremental --- .../kotlinter/tasks/ConfigurableKtLintTask.kt | 43 ++++++++++++++++--- .../gradle/kotlinter/tasks/FormatTask.kt | 2 +- .../gradle/kotlinter/tasks/LintTask.kt | 10 +---- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt index 44e25dc6..2a70418c 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt @@ -2,12 +2,14 @@ package org.jmailen.gradle.kotlinter.tasks import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.FileCollection +import org.gradle.api.file.FileTree import org.gradle.api.file.ProjectLayout import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.ListProperty import org.gradle.api.provider.MapProperty import org.gradle.api.provider.Property import org.gradle.api.tasks.Classpath +import org.gradle.api.tasks.IgnoreEmptyDirectories import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.Internal @@ -49,18 +51,31 @@ abstract class ConfigurableKtLintTask( @Classpath val ruleSetsClasspath: ConfigurableFileCollection = objectFactory.fileCollection() + @Internal + val sourceFiles = project.objects.fileCollection() + + @SkipWhenEmpty // Marks the input incremental: https://github.com/gradle/gradle/issues/17593 + @InputFiles + @PathSensitive(PathSensitivity.RELATIVE) + @IgnoreEmptyDirectories + val source = objectFactory.fileCollection().from({ sourceFiles.asFileTree.matching(patternSet) }) + + override fun source(vararg sources: Any?): SourceTask { + sourceFiles.setFrom(*sources) + return this + } + + override fun getSource(): FileTree = source.asFileTree + + override fun setSource(source: Any) { + sourceFiles.from(source) + } + @Internal protected fun getKtLintParams(): KtLintParams = KtLintParams( experimentalRules = experimentalRules.get(), disabledRules = disabledRules.get(), ) - - protected fun getChangedEditorconfigFiles(inputChanges: InputChanges) = - if (inputChanges.isIncremental) { - inputChanges.getFileChanges(editorconfigFiles).map(FileChange::getFile) - } else { - emptyList() - } } internal inline fun ObjectFactory.property(default: T? = null): Property = @@ -77,3 +92,17 @@ internal inline fun ObjectFactory.mapProperty(default: Ma mapProperty(K::class.java, V::class.java).apply { set(default) } + +internal fun ConfigurableKtLintTask.getChangedEditorconfigFiles(inputChanges: InputChanges) = + if (inputChanges.isIncremental) { + inputChanges.getFileChanges(editorconfigFiles).map(FileChange::getFile) + } else { + emptyList() + } + +internal fun ConfigurableKtLintTask.getChangedSources(inputChanges: InputChanges) = + if (inputChanges.isIncremental && inputChanges.getFileChanges(editorconfigFiles).none()) { + inputChanges.getFileChanges(source).map(FileChange::getFile) + } else { + source + } diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt index 8182662d..b64cf97c 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt @@ -40,7 +40,7 @@ open class FormatTask @Inject constructor( workQueue.submit(FormatWorkerAction::class.java) { p -> p.name.set(name) - p.files.from(source) + p.files.from(getChangedSources(inputChanges)) p.projectDirectory.set(projectLayout.projectDirectory.asFile) p.ktLintParams.set(getKtLintParams()) p.output.set(report) diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt index fbcca7cc..0a88dc92 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/LintTask.kt @@ -1,16 +1,12 @@ package org.jmailen.gradle.kotlinter.tasks -import org.gradle.api.file.FileTree import org.gradle.api.file.ProjectLayout import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.MapProperty import org.gradle.api.provider.Property import org.gradle.api.tasks.CacheableTask import org.gradle.api.tasks.Input -import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.OutputFiles -import org.gradle.api.tasks.PathSensitive -import org.gradle.api.tasks.PathSensitivity import org.gradle.api.tasks.TaskAction import org.gradle.work.InputChanges import org.gradle.workers.WorkerExecutor @@ -32,10 +28,6 @@ open class LintTask @Inject constructor( @OutputFiles val reports: MapProperty = objectFactory.mapProperty(default = emptyMap()) - @InputFiles - @PathSensitive(PathSensitivity.RELATIVE) - override fun getSource(): FileTree = super.getSource() - @Input val ignoreFailures: Property = objectFactory.property(default = DEFAULT_IGNORE_FAILURES) @@ -50,7 +42,7 @@ open class LintTask @Inject constructor( workQueue.submit(LintWorkerAction::class.java) { p -> p.name.set(name) - p.files.from(source) + p.files.from(getChangedSources(inputChanges)) p.projectDirectory.set(projectLayout.projectDirectory.asFile) p.reporters.putAll(reports) p.ktLintParams.set(getKtLintParams()) From a05ebaf6a7c6ef663b1f695a00420ba87b10344c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Kwiecin=CC=81ski?= Date: Sun, 16 Oct 2022 15:20:47 +0200 Subject: [PATCH 2/3] Migrate away from using SourceTask https://github.com/gradle/gradle/issues/11461 and https://github.com/gradle/gradle/issues/12318 --- .../kotlinter/tasks/ConfigurableKtLintTask.kt | 49 +++++++++++++------ 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt index 2a70418c..94cd2b52 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/ConfigurableKtLintTask.kt @@ -1,13 +1,16 @@ package org.jmailen.gradle.kotlinter.tasks +import groovy.lang.Closure +import org.gradle.api.DefaultTask import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.FileCollection -import org.gradle.api.file.FileTree +import org.gradle.api.file.FileTreeElement import org.gradle.api.file.ProjectLayout import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.ListProperty import org.gradle.api.provider.MapProperty import org.gradle.api.provider.Property +import org.gradle.api.specs.Spec import org.gradle.api.tasks.Classpath import org.gradle.api.tasks.IgnoreEmptyDirectories import org.gradle.api.tasks.Input @@ -15,7 +18,9 @@ import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.Internal import org.gradle.api.tasks.PathSensitive import org.gradle.api.tasks.PathSensitivity -import org.gradle.api.tasks.SourceTask +import org.gradle.api.tasks.SkipWhenEmpty +import org.gradle.api.tasks.util.PatternFilterable +import org.gradle.api.tasks.util.PatternSet import org.gradle.work.FileChange import org.gradle.work.Incremental import org.gradle.work.InputChanges @@ -23,11 +28,12 @@ import org.jmailen.gradle.kotlinter.KotlinterExtension.Companion.DEFAULT_DISABLE import org.jmailen.gradle.kotlinter.KotlinterExtension.Companion.DEFAULT_EXPERIMENTAL_RULES import org.jmailen.gradle.kotlinter.support.KtLintParams import org.jmailen.gradle.kotlinter.support.findApplicableEditorConfigFiles +import java.util.concurrent.Callable abstract class ConfigurableKtLintTask( projectLayout: ProjectLayout, objectFactory: ObjectFactory, -) : SourceTask() { +) : DefaultTask(), PatternFilterable { @Input val experimentalRules: Property = objectFactory.property(default = DEFAULT_EXPERIMENTAL_RULES) @@ -51,24 +57,22 @@ abstract class ConfigurableKtLintTask( @Classpath val ruleSetsClasspath: ConfigurableFileCollection = objectFactory.fileCollection() - @Internal - val sourceFiles = project.objects.fileCollection() + private val allSourceFiles = project.objects.fileCollection() + + @get:Internal + internal val patternFilterable: PatternFilterable = PatternSet() @SkipWhenEmpty // Marks the input incremental: https://github.com/gradle/gradle/issues/17593 @InputFiles @PathSensitive(PathSensitivity.RELATIVE) @IgnoreEmptyDirectories - val source = objectFactory.fileCollection().from({ sourceFiles.asFileTree.matching(patternSet) }) - - override fun source(vararg sources: Any?): SourceTask { - sourceFiles.setFrom(*sources) - return this - } + val source: FileCollection = objectFactory.fileCollection() + .from(Callable { allSourceFiles.asFileTree.matching(patternFilterable) }) - override fun getSource(): FileTree = source.asFileTree + fun source(vararg sources: Any?) = also { allSourceFiles.setFrom(*sources) } - override fun setSource(source: Any) { - sourceFiles.from(source) + fun setSource(source: Any) { + allSourceFiles.setFrom(source) } @Internal @@ -76,6 +80,23 @@ abstract class ConfigurableKtLintTask( experimentalRules = experimentalRules.get(), disabledRules = disabledRules.get(), ) + + @Internal + override fun getIncludes(): MutableSet = patternFilterable.includes + + @Internal + override fun getExcludes(): MutableSet = patternFilterable.excludes + + override fun setIncludes(includes: MutableIterable) = also { patternFilterable.setIncludes(includes) } + override fun setExcludes(excludes: MutableIterable) = also { patternFilterable.setExcludes(excludes) } + override fun include(vararg includes: String?) = also { patternFilterable.include(*includes) } + override fun include(includes: MutableIterable) = also { patternFilterable.include(includes) } + override fun include(includeSpec: Spec) = also { patternFilterable.include(includeSpec) } + override fun include(includeSpec: Closure<*>) = also { patternFilterable.include(includeSpec) } + override fun exclude(vararg excludes: String?) = also { patternFilterable.exclude(*excludes) } + override fun exclude(excludes: MutableIterable) = also { patternFilterable.exclude(excludes) } + override fun exclude(excludeSpec: Spec) = also { patternFilterable.exclude(excludeSpec) } + override fun exclude(excludeSpec: Closure<*>) = also { patternFilterable.exclude(excludeSpec) } } internal inline fun ObjectFactory.property(default: T? = null): Property = From 819dda68d9956213c4f08afa4082ed9e1b132d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Kwiecin=CC=81ski?= Date: Sun, 16 Oct 2022 21:30:16 +0200 Subject: [PATCH 3/3] Add tests for `LintTask` being incremental --- .../gradle/kotlinter/tasks/FormatTask.kt | 2 +- .../tasks/format/FormatWorkerAction.kt | 3 +- .../kotlinter/tasks/lint/LintWorkerAction.kt | 3 +- .../kotlinter/functional/KotlinProjectTest.kt | 77 +++++++++++++++++-- 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt index b64cf97c..8182662d 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/FormatTask.kt @@ -40,7 +40,7 @@ open class FormatTask @Inject constructor( workQueue.submit(FormatWorkerAction::class.java) { p -> p.name.set(name) - p.files.from(getChangedSources(inputChanges)) + p.files.from(source) p.projectDirectory.set(projectLayout.projectDirectory.asFile) p.ktLintParams.set(getKtLintParams()) p.output.set(report) diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt index e39c6330..9418995a 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/format/FormatWorkerAction.kt @@ -27,7 +27,8 @@ abstract class FormatWorkerAction : WorkAction { changedEditorconfigFiles = parameters.changedEditorConfigFiles, logger = logger, ) - logger.info("Resolved ${ktLintEngine.ruleProviders.size} RuleProviders") + logger.info("$name - resolved ${ktLintEngine.ruleProviders.size} RuleProviders") + logger.info("$name - executing against ${files.size} file(s)") val fixes = mutableListOf() try { diff --git a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt index 53fcba58..e827051a 100644 --- a/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt +++ b/src/main/kotlin/org/jmailen/gradle/kotlinter/tasks/lint/LintWorkerAction.kt @@ -31,7 +31,8 @@ abstract class LintWorkerAction : WorkAction { changedEditorconfigFiles = parameters.changedEditorConfigFiles, logger = logger, ) - logger.info("Resolved ${ktLintEngine.ruleProviders.size} RuleProviders") + logger.info("$name - resolved ${ktLintEngine.ruleProviders.size} RuleProviders") + logger.info("$name - executing against ${files.size} file(s)") if (logger.isDebugEnabled) { logger.debug("Resolved RuleSetProviders = ${ktLintEngine.ruleProviders.joinToString { it.createNewRuleInstance().id }}") } diff --git a/src/test/kotlin/org/jmailen/gradle/kotlinter/functional/KotlinProjectTest.kt b/src/test/kotlin/org/jmailen/gradle/kotlinter/functional/KotlinProjectTest.kt index 7811bfec..8187132f 100644 --- a/src/test/kotlin/org/jmailen/gradle/kotlinter/functional/KotlinProjectTest.kt +++ b/src/test/kotlin/org/jmailen/gradle/kotlinter/functional/KotlinProjectTest.kt @@ -1,6 +1,7 @@ package org.jmailen.gradle.kotlinter.functional import org.gradle.testkit.runner.TaskOutcome.FAILED +import org.gradle.testkit.runner.TaskOutcome.NO_SOURCE import org.gradle.testkit.runner.TaskOutcome.SUCCESS import org.gradle.testkit.runner.TaskOutcome.UP_TO_DATE import org.jmailen.gradle.kotlinter.functional.utils.editorConfig @@ -193,6 +194,66 @@ internal class KotlinProjectTest : WithGradleTest.Kotlin() { } } + @Test + fun `lint task is incremental`() { + settingsFile() + buildFile() + editorConfig() + kotlinSourceFile( + "CustomObject.kt", + """ + object CustomObject + + """.trimIndent(), + ) + kotlinSourceFile( + "CustomClass.kt", + """ + data class CustomClass(val value: Int) + + """.trimIndent(), + ) + + build("lintKotlin", "--info").apply { + assertEquals(SUCCESS, task(":lintKotlin")?.outcome) + assertEquals(SUCCESS, task(":lintKotlinMain")?.outcome) + assertEquals(NO_SOURCE, task(":lintKotlinTest")?.outcome) + assertTrue(output.contains("lintKotlinMain - executing against 2 file(s)")) + } + + kotlinSourceFile( + "CustomClass.kt", + """ + data class CustomClass(val modified: Int) + + """.trimIndent(), + ) + build("lintKotlin", "--info").apply { + assertEquals(SUCCESS, task(":lintKotlin")?.outcome) + assertEquals(SUCCESS, task(":lintKotlinMain")?.outcome) + assertEquals(NO_SOURCE, task(":lintKotlinTest")?.outcome) + assertTrue(output.contains("lintKotlinMain - executing against 1 file(s)")) + } + + editorconfigFile.appendText("content=updated") + build("lintKotlin", "--info").apply { + assertEquals(SUCCESS, task(":lintKotlin")?.outcome) + assertTrue(output.contains("lintKotlinMain - executing against 2 file(s)")) + } + + kotlinSourceFile( + "CustomClass.kt", + """ + data class CustomClass(val modifiedEditorconfig: Int) + + """.trimIndent(), + ) + build("lintKotlin", "--info").apply { + assertEquals(SUCCESS, task(":lintKotlin")?.outcome) + assertTrue(output.contains("lintKotlinMain - executing against 1 file(s)")) + } + } + @Test fun `plugin is compatible with configuration cache`() { settingsFile() @@ -225,7 +286,7 @@ internal class KotlinProjectTest : WithGradleTest.Kotlin() { } @Test - fun `plugin resolves dynamically loaded RulesetProviders`() { + fun `plugin resolves dynamically loaded RuleSetProviders`() { settingsFile() buildFile() editorConfig() @@ -237,18 +298,22 @@ internal class KotlinProjectTest : WithGradleTest.Kotlin() { """.trimIndent(), ) - val matcher = "Resolved (\\d+) RuleProviders".toRegex() + fun String.findResolvedRuleProvidersCount(taskName: String): Int { + val matcher = "$taskName - resolved (\\d+) RuleProviders".toRegex() + + return matcher.find(this)?.groups?.get(1)?.value?.toIntOrNull() ?: 0 + } build("lintKotlin", "--info").apply { assertEquals(SUCCESS, task(":lintKotlin")?.outcome) - val resolvedRulesCount = matcher.find(output)?.groups?.get(1)?.value?.toIntOrNull() ?: 0 - assertTrue(resolvedRulesCount > 0) + val resolvedRulesCount = output.findResolvedRuleProvidersCount("lintKotlinMain") + assertTrue(resolvedRulesCount > 0) { "expected to find more than 0 resolved RuleProviders, was=$resolvedRulesCount" } } build("formatKotlin", "--info").apply { assertEquals(SUCCESS, task(":formatKotlin")?.outcome) - val resolvedRulesCount = matcher.find(output)?.groups?.get(1)?.value?.toIntOrNull() ?: 0 - assertTrue(resolvedRulesCount > 0) + val resolvedRulesCount = output.findResolvedRuleProvidersCount("formatKotlinMain") + assertTrue(resolvedRulesCount > 0) { "expected to find more than 0 resolved RuleProviders, was=$resolvedRulesCount" } } }