From fc30ff138cc88087c2f9aef4cd90aaa0fc8ad3c4 Mon Sep 17 00:00:00 2001 From: simonhauck Date: Thu, 18 Jul 2024 00:38:04 +0200 Subject: [PATCH] #290 plugin not compatible with gradle configuration cache (#312) * #290 Add tests to reproduce issues with configuration cache and custom format tasks * #290 provide formatting options bean as property instead to be compatible with configuration cache * #290 extend tests to check if formatting works with the formatted styles * #290 add a changelog entry for configuration cache fix for custom ktfmt tasks * #290 Remove run call from project * #290 rename toBean method to be better readable, removing obsolete mapping function in KtfmtPlugin --- CHANGELOG.md | 6 +- .../ncorti/ktfmt/gradle/KtfmtAndroidUtils.kt | 2 - .../com/ncorti/ktfmt/gradle/KtfmtExtension.kt | 2 +- .../com/ncorti/ktfmt/gradle/KtfmtPlugin.kt | 78 ++++++++-------- .../ncorti/ktfmt/gradle/KtfmtPluginUtils.kt | 9 +- .../ktfmt/gradle/tasks/KtfmtBaseTask.kt | 20 +--- .../ncorti/ktfmt/gradle/KtfmtExtensionTest.kt | 4 +- .../tasks/KtfmtCheckTaskIntegrationTest.kt | 22 +++++ .../tasks/KtfmtFormatTaskIntegrationTest.kt | 92 +++++++++++++++++++ 9 files changed, 161 insertions(+), 74 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c0b491a..4f9b5978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,9 +5,7 @@ Please add your entries according to this format. ## Unreleased -- Update formatting of `build.gradle.kts` with ktfmt 0.51 -- Replace deprecated methods in `build.gradle.kts` files -- Prepare for Gradle 8.9 +- Fix custom KtFmt tasks not compatible with configuration cache (#290) ## Version 0.19.0 _(2024-07-03)_ @@ -173,4 +171,4 @@ Please add your entries according to this format. ## Version 0.1.0 _(2020-12-22)_ -That's the first version of `ktfmt-gradle` +That's the first version of `ktfmt-gradle` \ No newline at end of file diff --git a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtAndroidUtils.kt b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtAndroidUtils.kt index b625a897..b6ccdbcc 100644 --- a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtAndroidUtils.kt +++ b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtAndroidUtils.kt @@ -12,7 +12,6 @@ internal object KtfmtAndroidUtils { internal fun applyKtfmtToAndroidProject( project: Project, - ktfmtExtension: KtfmtExtension, topLevelFormat: TaskProvider, topLevelCheck: TaskProvider, isKmpProject: Boolean = false @@ -44,7 +43,6 @@ internal object KtfmtAndroidUtils { project, sourceSetName, project.files(Callable { srcDirs }), - ktfmtExtension, topLevelFormat, topLevelCheck, ) diff --git a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt index 9ebdb000..c9918f8d 100644 --- a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt +++ b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtExtension.kt @@ -77,7 +77,7 @@ abstract class KtfmtExtension { continuationIndent.set(4) } - internal fun toBean(): FormattingOptionsBean = + internal fun toFormattingOptions(): FormattingOptionsBean = FormattingOptionsBean( maxWidth = maxWidth.get(), blockIndent = blockIndent.get(), diff --git a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPlugin.kt b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPlugin.kt index aee1d3a3..71b75e74 100644 --- a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPlugin.kt +++ b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPlugin.kt @@ -26,50 +26,49 @@ abstract class KtfmtPlugin : Plugin { private lateinit var topLevelFormat: TaskProvider private lateinit var topLevelCheck: TaskProvider - override fun apply(project: Project) = - project.run { - ktfmtExtension = project.extensions.create(EXTENSION_NAME, KtfmtExtension::class.java) - - // setup to pull in ktfmt separately to run on an isolated classloader - val ktFmt = - configurations.create("ktfmt").apply { - attributes.apply { - attribute( - Usage.USAGE_ATTRIBUTE, - project.objects.named(Usage::class.java, Usage.JAVA_RUNTIME)) - } - isVisible = false - isCanBeConsumed = false + override fun apply(project: Project) { + ktfmtExtension = project.extensions.create(EXTENSION_NAME, KtfmtExtension::class.java) + + // setup to pull in ktfmt separately to run on an isolated classloader + val ktFmt = + project.configurations.create("ktfmt").apply { + attributes.apply { + attribute( + Usage.USAGE_ATTRIBUTE, + project.objects.named(Usage::class.java, Usage.JAVA_RUNTIME)) } + isVisible = false + isCanBeConsumed = false + } - val resourceUri = - KtfmtPlugin::class.java.getResource("ktfmt-version.txt") - ?: error("Missing ktfmt version") - val ktfmtVersion = resources.text.fromUri(resourceUri).asString() + val resourceUri = + KtfmtPlugin::class.java.getResource("ktfmt-version.txt") + ?: error("Missing ktfmt version") + val ktfmtVersion = project.resources.text.fromUri(resourceUri).asString() - project.dependencies.add(ktFmt.name, ktfmtVersion) + project.dependencies.add(ktFmt.name, ktfmtVersion) - project.tasks.withType(KtfmtBaseTask::class.java).configureEach { - it.ktfmtClasspath.from(ktFmt) - } + project.tasks.withType(KtfmtBaseTask::class.java).configureEach { + it.ktfmtClasspath.from(ktFmt) + it.formattingOptionsBean.set(ktfmtExtension.toFormattingOptions()) + } - topLevelFormat = createTopLevelFormatTask(project) - topLevelCheck = createTopLevelCheckTask(project) + topLevelFormat = createTopLevelFormatTask(project) + topLevelCheck = createTopLevelCheckTask(project) - project.plugins.withId("kotlin") { applyKtfmt(project) } - project.plugins.withId("kotlin-android") { - if (project.plugins.hasPlugin("org.jetbrains.kotlin.multiplatform")) { - project.logger.i("Skipping Android task creation, as KMP is applied") - } else { - applyKtfmtToAndroidProject( - project, ktfmtExtension, topLevelFormat, topLevelCheck) - } - } - project.plugins.withId("org.jetbrains.kotlin.js") { applyKtfmt(project) } - project.plugins.withId("org.jetbrains.kotlin.multiplatform") { - applyKtfmtToMultiplatformProject(project) + project.plugins.withId("kotlin") { applyKtfmt(project) } + project.plugins.withId("kotlin-android") { + if (project.plugins.hasPlugin("org.jetbrains.kotlin.multiplatform")) { + project.logger.i("Skipping Android task creation, as KMP is applied") + } else { + applyKtfmtToAndroidProject(project, topLevelFormat, topLevelCheck) } } + project.plugins.withId("org.jetbrains.kotlin.js") { applyKtfmt(project) } + project.plugins.withId("org.jetbrains.kotlin.multiplatform") { + applyKtfmtToMultiplatformProject(project) + } + } private fun applyKtfmt(project: Project) { val extension = project.extensions.getByType(KotlinProjectExtension::class.java) @@ -78,9 +77,9 @@ abstract class KtfmtPlugin : Plugin { project, it.name, it.kotlin.sourceDirectories, - ktfmtExtension, topLevelFormat, - topLevelCheck) + topLevelCheck, + ) } } @@ -97,7 +96,6 @@ abstract class KtfmtPlugin : Plugin { project, name, it.kotlin.sourceDirectories, - ktfmtExtension, topLevelFormat, topLevelCheck, ) @@ -106,7 +104,7 @@ abstract class KtfmtPlugin : Plugin { extension.targets.all { kotlinTarget -> if (kotlinTarget.platformType == KotlinPlatformType.androidJvm) { applyKtfmtToAndroidProject( - project, ktfmtExtension, topLevelFormat, topLevelCheck, isKmpProject = true) + project, topLevelFormat, topLevelCheck, isKmpProject = true) } } } diff --git a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPluginUtils.kt b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPluginUtils.kt index 69ebc578..778e77a7 100644 --- a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPluginUtils.kt +++ b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/KtfmtPluginUtils.kt @@ -34,7 +34,6 @@ internal object KtfmtPluginUtils { project: Project, srcSetName: String, srcSetDir: FileCollection, - ktfmtExtension: KtfmtExtension, topLevelFormat: TaskProvider, topLevelCheck: TaskProvider ) { @@ -42,8 +41,8 @@ internal object KtfmtPluginUtils { return } - val srcCheckTask = createCheckTask(project, ktfmtExtension, srcSetName, srcSetDir) - val srcFormatTask = createFormatTask(project, ktfmtExtension, srcSetName, srcSetDir) + val srcCheckTask = createCheckTask(project, srcSetName, srcSetDir) + val srcFormatTask = createFormatTask(project, srcSetName, srcSetDir) // When running together with compileKotlin, ktfmt tasks should have precedence as // they're editing the source code @@ -63,7 +62,6 @@ internal object KtfmtPluginUtils { private fun createCheckTask( project: Project, - ktfmtExtension: KtfmtExtension, name: String, srcDir: FileCollection ): TaskProvider { @@ -86,13 +84,11 @@ internal object KtfmtPluginUtils { it.setSource(inputDirs) it.setIncludes(KtfmtPlugin.defaultIncludes) it.setExcludes(KtfmtPlugin.defaultExcludes) - it.bean = ktfmtExtension.toBean() } } private fun createFormatTask( project: Project, - ktfmtExtension: KtfmtExtension, name: String, srcDir: FileCollection ): TaskProvider { @@ -115,7 +111,6 @@ internal object KtfmtPluginUtils { it.setSource(inputDirs) it.setIncludes(KtfmtPlugin.defaultIncludes) it.setExcludes(KtfmtPlugin.defaultExcludes) - it.bean = ktfmtExtension.toBean() } } } diff --git a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask.kt b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask.kt index c6488ec4..3806a747 100644 --- a/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask.kt +++ b/plugin-build/plugin/src/main/java/com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask.kt @@ -1,14 +1,12 @@ package com.ncorti.ktfmt.gradle.tasks import com.ncorti.ktfmt.gradle.FormattingOptionsBean -import com.ncorti.ktfmt.gradle.KtfmtExtension import com.ncorti.ktfmt.gradle.KtfmtPlugin import com.ncorti.ktfmt.gradle.tasks.worker.KtfmtWorkAction import com.ncorti.ktfmt.gradle.tasks.worker.Result import com.ncorti.ktfmt.gradle.util.d import java.io.File import java.util.* -import org.gradle.api.UnknownDomainObjectException import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.FileCollection import org.gradle.api.file.ProjectLayout @@ -19,7 +17,6 @@ import org.gradle.api.tasks.IgnoreEmptyDirectories import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputFiles import org.gradle.api.tasks.Internal -import org.gradle.api.tasks.Optional import org.gradle.api.tasks.PathSensitive import org.gradle.api.tasks.PathSensitivity import org.gradle.api.tasks.SourceTask @@ -42,7 +39,7 @@ internal constructor( @get:Classpath @get:InputFiles internal abstract val ktfmtClasspath: ConfigurableFileCollection - @get:Input @get:Optional internal var bean: FormattingOptionsBean? = null + @get:Input internal abstract val formattingOptionsBean: Property @get:Option( option = "include-only", @@ -67,19 +64,6 @@ internal constructor( protected abstract fun execute(workQueue: WorkQueue) - private fun formattingOptions(): FormattingOptionsBean { - if (bean == null) { - // TODO - set this as property / convention - bean = - try { - project.extensions.getByType(KtfmtExtension::class.java).toBean() - } catch (ignored: UnknownDomainObjectException) { - FormattingOptionsBean() - } - } - return bean!! - } - @get:Internal internal val defaultExcludesFilter: Spec = Spec { @@ -104,7 +88,7 @@ internal constructor( forEach { queue.submit(action) { parameters -> parameters.sourceFile.set(it) - parameters.formattingOptions.set(formattingOptions()) + parameters.formattingOptions.set(formattingOptionsBean.get()) parameters.includedFiles.set(includedFiles) parameters.projectDir.set(layout.projectDirectory) parameters.workingDir.set(workingDir) diff --git a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/KtfmtExtensionTest.kt b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/KtfmtExtensionTest.kt index 7e102aff..f8611296 100644 --- a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/KtfmtExtensionTest.kt +++ b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/KtfmtExtensionTest.kt @@ -68,7 +68,7 @@ class KtfmtExtensionTest { } @Test - fun `toBean copies fields correctly`() { + fun `toFormattingOptions copies fields correctly`() { val extension = createExtension() extension.maxWidth.set(42) @@ -78,7 +78,7 @@ class KtfmtExtensionTest { extension.manageTrailingCommas.set(true) extension.debuggingPrintOpsAfterFormatting.set(true) - val bean = extension.toBean() + val bean = extension.toFormattingOptions() assertThat(bean.maxWidth).isEqualTo(42) assertThat(bean.blockIndent).isEqualTo(43) diff --git a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTaskIntegrationTest.kt b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTaskIntegrationTest.kt index b649582d..3b7b9383 100644 --- a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTaskIntegrationTest.kt +++ b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTaskIntegrationTest.kt @@ -257,6 +257,28 @@ internal class KtfmtCheckTaskIntegrationTest { assertThat(result!!.output).contains("Reusing configuration cache.") } + @Test + fun `custom formatCheck task should be compatible with configuration cache`() { + createTempFile(content = "val answer = 42\n") + + tempDir + .resolve("build.gradle.kts") + .appendText( + """ + | + |tasks.register("customFormatCheck") { + | source = fileTree("src/main/java") + |} + """ + .trimMargin()) + + GradleRunner.create() + .withProjectDir(tempDir) + .withPluginClasspath() + .withArguments("customFormatCheck", "--configuration-cache") + .build() + } + private fun createTempFile( @Language("kotlin") content: String, fileName: String = "TestFile.kt", diff --git a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtFormatTaskIntegrationTest.kt b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtFormatTaskIntegrationTest.kt index 5d770f26..fd165359 100644 --- a/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtFormatTaskIntegrationTest.kt +++ b/plugin-build/plugin/src/test/java/com/ncorti/ktfmt/gradle/tasks/KtfmtFormatTaskIntegrationTest.kt @@ -258,6 +258,91 @@ internal class KtfmtFormatTaskIntegrationTest { assertThat(result.task(":ktfmtFormatMain")?.outcome).isEqualTo(SUCCESS) } + @Test + fun `custom format task should be compatible with configuration cache`() { + createTempFile(content = "val answer = 42\n") + + tempDir + .resolve("build.gradle.kts") + .appendText( + """ + | + |tasks.register("customFormatTask") { + | source = fileTree("src/main/java") + |} + """ + .trimMargin()) + + GradleRunner.create() + .withProjectDir(tempDir) + .withPluginClasspath() + .withArguments("customFormatTask", "--configuration-cache") + .build() + } + + @Test + fun `should format the files in kotlinLang style with a 4 space indentation`() { + val file = + createTempFile( + content = + """ + |fun someFun(){ + |println("Hello, World!") + |println("HelloWorld2") + |} + """ + .trimMargin()) + + appendToBuildGradle("ktfmt { kotlinLangStyle() }") + + GradleRunner.create() + .withProjectDir(tempDir) + .withPluginClasspath() + .withArguments("ktfmtFormatMain") + .forwardOutput() + .build() + + val actual = file.readLines() + assertThat(actual) + .containsExactly( + "fun someFun() {", + " println(\"Hello, World!\")", + " println(\"HelloWorld2\")", + "}") + } + + @Test + fun `should format the files in googleStyle style with a 2 space indentation`() { + val file = + createTempFile( + content = + """ + |fun someFun(){ + |println("Hello, World!") + |println("HelloWorld2") + |} + """ + .trimMargin()) + + appendToBuildGradle("ktfmt { googleStyle() }") + + GradleRunner.create() + .withProjectDir(tempDir) + .withPluginClasspath() + .withArguments("ktfmtFormatMain") + .forwardOutput() + .build() + + val actual = file.readLines() + + assertThat(actual) + .containsExactly( + "fun someFun() {", + " println(\"Hello, World!\")", + " println(\"HelloWorld2\")", + "}") + } + private fun createTempFile( @Language("kotlin") content: String, fileName: String = "TestFile.kt", @@ -267,4 +352,11 @@ internal class KtfmtFormatTaskIntegrationTest { createNewFile() writeText(content) } + + private fun appendToBuildGradle(content: String) { + tempDir.resolve("build.gradle.kts").apply { + appendText(System.lineSeparator()) + appendText(content) + } + } }