Skip to content

Commit

Permalink
cortinico#290 plugin not compatible with gradle configuration cache (c…
Browse files Browse the repository at this point in the history
…ortinico#312)

* cortinico#290 Add tests to reproduce issues with configuration cache and custom format tasks

* cortinico#290 provide formatting options bean as property instead to be compatible with configuration cache

* cortinico#290 extend tests to check if formatting works with the formatted styles

* cortinico#290 add a changelog entry for configuration cache fix for custom ktfmt tasks

* cortinico#290 Remove run call from project

* cortinico#290 rename toBean method to be better readable, removing obsolete mapping function in KtfmtPlugin
  • Loading branch information
simonhauck authored Jul 17, 2024
1 parent c993aee commit fc30ff1
Show file tree
Hide file tree
Showing 9 changed files with 161 additions and 74 deletions.
6 changes: 2 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)_

Expand Down Expand Up @@ -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`
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ internal object KtfmtAndroidUtils {

internal fun applyKtfmtToAndroidProject(
project: Project,
ktfmtExtension: KtfmtExtension,
topLevelFormat: TaskProvider<Task>,
topLevelCheck: TaskProvider<Task>,
isKmpProject: Boolean = false
Expand Down Expand Up @@ -44,7 +43,6 @@ internal object KtfmtAndroidUtils {
project,
sourceSetName,
project.files(Callable { srcDirs }),
ktfmtExtension,
topLevelFormat,
topLevelCheck,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,50 +26,49 @@ abstract class KtfmtPlugin : Plugin<Project> {
private lateinit var topLevelFormat: TaskProvider<Task>
private lateinit var topLevelCheck: TaskProvider<Task>

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)
Expand All @@ -78,9 +77,9 @@ abstract class KtfmtPlugin : Plugin<Project> {
project,
it.name,
it.kotlin.sourceDirectories,
ktfmtExtension,
topLevelFormat,
topLevelCheck)
topLevelCheck,
)
}
}

Expand All @@ -97,7 +96,6 @@ abstract class KtfmtPlugin : Plugin<Project> {
project,
name,
it.kotlin.sourceDirectories,
ktfmtExtension,
topLevelFormat,
topLevelCheck,
)
Expand All @@ -106,7 +104,7 @@ abstract class KtfmtPlugin : Plugin<Project> {
extension.targets.all { kotlinTarget ->
if (kotlinTarget.platformType == KotlinPlatformType.androidJvm) {
applyKtfmtToAndroidProject(
project, ktfmtExtension, topLevelFormat, topLevelCheck, isKmpProject = true)
project, topLevelFormat, topLevelCheck, isKmpProject = true)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,15 @@ internal object KtfmtPluginUtils {
project: Project,
srcSetName: String,
srcSetDir: FileCollection,
ktfmtExtension: KtfmtExtension,
topLevelFormat: TaskProvider<Task>,
topLevelCheck: TaskProvider<Task>
) {
if (shouldCreateTasks(srcSetName).not()) {
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
Expand All @@ -63,7 +62,6 @@ internal object KtfmtPluginUtils {

private fun createCheckTask(
project: Project,
ktfmtExtension: KtfmtExtension,
name: String,
srcDir: FileCollection
): TaskProvider<KtfmtCheckTask> {
Expand All @@ -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<KtfmtFormatTask> {
Expand All @@ -115,7 +111,6 @@ internal object KtfmtPluginUtils {
it.setSource(inputDirs)
it.setIncludes(KtfmtPlugin.defaultIncludes)
it.setExcludes(KtfmtPlugin.defaultExcludes)
it.bean = ktfmtExtension.toBean()
}
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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<FormattingOptionsBean>

@get:Option(
option = "include-only",
Expand All @@ -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<File> =
Spec<File> {
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class KtfmtExtensionTest {
}

@Test
fun `toBean copies fields correctly`() {
fun `toFormattingOptions copies fields correctly`() {
val extension = createExtension()

extension.maxWidth.set(42)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<com.ncorti.ktfmt.gradle.tasks.KtfmtCheckTask>("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",
Expand Down
Loading

0 comments on commit fc30ff1

Please sign in to comment.