Skip to content

Commit

Permalink
#292 Fix custom sources location not being recognized by ktfmt (#328)
Browse files Browse the repository at this point in the history
* #292 add test to reproduce issues with changed sources location and fix it by evaluating it lazy with providers
* #292 add a new regex extension property to specify what files should be ignored. Set this value by default to the build directory
* #292 update the api specification with the new property
* #292 update documentation

---------

Co-authored-by: Nicola Corti <corti.nico@gmail.com>
  • Loading branch information
simonhauck and cortinico authored Aug 25, 2024
1 parent 2025515 commit 8f28a03
Show file tree
Hide file tree
Showing 14 changed files with 312 additions and 77 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ Please add your entries according to this format.

## Unreleased

- Add new regex property to exclude sourceSets from analysis (#328)
- Fix custom KtFmt tasks not compatible with configuration cache (#290)
- Fix custom source directories not correctly recognized (#292)
- Fix mixed up task descriptions (#342)
- KtFmt to 0.52

Expand Down
18 changes: 16 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,24 @@ ktfmt {
}
```

By default, sourceSets that are in your `build` folder are ignored. To customize this, you can set a regex pattern to
exclude certain sourceSets:

```kotlin
ktfmt {
srcSetPathExclusionPattern = Regex(".*generated.*")
}
```

Please note, that this property will only affect the sourceSet path and not the individual files inside the sourceSet.
To include or exclude files inside a sourceSet, use the include and exclude properties of the ktfmt tasks.

## Using with a pre-commit hook 🎣

You can leverage the `--include-only` to let ktfmt-gradle run only on a specific subset of files.

To this you can register a simple task of type `KtfmtCheckTask` or `KtfmtFormatTask` in your `build.gradle.kts` as follows:
To do this you can register a simple task of type `KtfmtCheckTask` or `KtfmtFormatTask` in your `build.gradle.kts` as
follows:

```kotlin
import com.ncorti.ktfmt.gradle.tasks.*
Expand All @@ -129,7 +142,8 @@ tasks.register<KtfmtFormatTask>("ktfmtPrecommit") {
}
```

You can then invoke the task with `--include-only` and a comma-separated (or colon-separated) list of relative path of files:
You can then invoke the task with `--include-only` and a comma-separated (or colon-separated) list of relative path of
files:

```
./gradlew ktfmtPrecommit --include-only=src/main/java/File1.kt:src/main/java/File2.kt
Expand Down
1 change: 0 additions & 1 deletion example/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,3 @@ tasks.withType<Test> { useJUnitPlatform() }
tasks.withType<VerifyMigrationTask> { enabled = false }

sqldelight { databases { create("Database") { packageName.set("com.example") } } }

3 changes: 2 additions & 1 deletion plugin-build/plugin/api/plugin.api
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ public abstract class com/ncorti/ktfmt/gradle/KtfmtExtension {
public abstract fun getManageTrailingCommas ()Lorg/gradle/api/provider/Property;
public abstract fun getMaxWidth ()Lorg/gradle/api/provider/Property;
public abstract fun getRemoveUnusedImports ()Lorg/gradle/api/provider/Property;
public abstract fun getSrcSetPathExclusionPattern ()Lorg/gradle/api/provider/Property;
public final fun googleStyle ()V
public final fun kotlinLangStyle ()V
}
Expand All @@ -23,6 +24,7 @@ public final class com/ncorti/ktfmt/gradle/KtfmtPlugin$Companion {
public abstract class com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask : org/gradle/api/tasks/SourceTask {
protected abstract fun execute (Lorg/gradle/workers/WorkQueue;)V
public abstract fun getIncludeOnly ()Lorg/gradle/api/provider/Property;
public fun getSource ()Lorg/gradle/api/file/FileTree;
}

public abstract class com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTask : com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask {
Expand All @@ -32,6 +34,5 @@ public abstract class com/ncorti/ktfmt/gradle/tasks/KtfmtCheckTask : com/ncorti/

public abstract class com/ncorti/ktfmt/gradle/tasks/KtfmtFormatTask : com/ncorti/ktfmt/gradle/tasks/KtfmtBaseTask {
protected fun execute (Lorg/gradle/workers/WorkQueue;)V
protected final fun getOutputFiles ()Lorg/gradle/api/file/FileCollection;
}

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ internal object KtfmtAndroidUtils {
project: Project,
topLevelFormat: TaskProvider<Task>,
topLevelCheck: TaskProvider<Task>,
ktfmtExtension: KtfmtExtension,
isKmpProject: Boolean = false
) {
fun applyKtfmtForAndroid() {
Expand Down Expand Up @@ -43,6 +44,7 @@ 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 @@ -12,6 +12,7 @@ abstract class KtfmtExtension {
removeUnusedImports.convention(DEFAULT_REMOVE_UNUSED_IMPORTS)
debuggingPrintOpsAfterFormatting.convention(DEFAULT_DEBUGGING_PRINT_OPTS)
manageTrailingCommas.convention(DEFAULT_MANAGE_TRAILING_COMMAS)
srcSetPathExclusionPattern.convention(DEFAULT_SRC_SET_PATH_EXCLUSION_PATTERN)
}

/** ktfmt breaks lines longer than maxWidth. Default 100. */
Expand Down Expand Up @@ -53,6 +54,15 @@ abstract class KtfmtExtension {
/** Whether ktfmt should remove imports that are not used. */
abstract val removeUnusedImports: Property<Boolean>

/**
* Regex to define what sourceSets paths should not be formatted
*
* For example the sourceSet "main" in the "build" folder would be excluded with the regex
* "^(.*[\\/])?build([\\/].*)?$". This does not affect files inside a sourceSet. To exclude a
* file inside a sourceSet use the include & exclude options on the ktfmt task.
*/
abstract val srcSetPathExclusionPattern: Property<Regex>

/**
* Print the Ops generated by KotlinInputAstVisitor to help reason about formatting (i.e.,
* newline) decisions
Expand Down Expand Up @@ -94,5 +104,7 @@ abstract class KtfmtExtension {
internal const val DEFAULT_REMOVE_UNUSED_IMPORTS: Boolean = true
internal const val DEFAULT_DEBUGGING_PRINT_OPTS: Boolean = false
internal const val DEFAULT_MANAGE_TRAILING_COMMAS: Boolean = false
internal val DEFAULT_SRC_SET_PATH_EXCLUSION_PATTERN =
Regex("^(.*[\\\\/])?build([\\\\/].*)?\$")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,34 +56,35 @@ abstract class KtfmtPlugin : Plugin<Project> {
topLevelFormat = createTopLevelFormatTask(project)
topLevelCheck = createTopLevelCheckTask(project)

project.plugins.withId("kotlin") { applyKtfmt(project) }
project.plugins.withId("kotlin") { applyKtfmt(project, ktfmtExtension) }
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)
applyKtfmtToAndroidProject(project, topLevelFormat, topLevelCheck, ktfmtExtension)
}
}
project.plugins.withId("org.jetbrains.kotlin.js") { applyKtfmt(project) }
project.plugins.withId("org.jetbrains.kotlin.js") { applyKtfmt(project, ktfmtExtension) }
project.plugins.withId("org.jetbrains.kotlin.multiplatform") {
applyKtfmtToMultiplatformProject(project)
applyKtfmtToMultiplatformProject(project, ktfmtExtension)
}
}

private fun applyKtfmt(project: Project) {
private fun applyKtfmt(project: Project, ktfmtExtension: KtfmtExtension) {
val extension = project.extensions.getByType(KotlinProjectExtension::class.java)
extension.sourceSets.all {
createTasksForSourceSet(
project,
it.name,
it.kotlin.sourceDirectories,
ktfmtExtension,
topLevelFormat,
topLevelCheck,
)
}
}

private fun applyKtfmtToMultiplatformProject(project: Project) {
private fun applyKtfmtToMultiplatformProject(project: Project, ktfmtExtension: KtfmtExtension) {
val extension = project.extensions.getByType(KotlinMultiplatformExtension::class.java)
extension.sourceSets.all {
val name = "kmp ${it.name}"
Expand All @@ -96,6 +97,7 @@ abstract class KtfmtPlugin : Plugin<Project> {
project,
name,
it.kotlin.sourceDirectories,
ktfmtExtension,
topLevelFormat,
topLevelCheck,
)
Expand All @@ -104,7 +106,7 @@ abstract class KtfmtPlugin : Plugin<Project> {
extension.targets.all { kotlinTarget ->
if (kotlinTarget.platformType == KotlinPlatformType.androidJvm) {
applyKtfmtToAndroidProject(
project, topLevelFormat, topLevelCheck, isKmpProject = true)
project, topLevelFormat, topLevelCheck, ktfmtExtension, isKmpProject = true)
}
}
}
Expand All @@ -125,8 +127,6 @@ abstract class KtfmtPlugin : Plugin<Project> {
}

companion object {
internal val defaultExcludes = listOf("**/build/**")
internal val defaultExcludesRegex = Regex("^(.*[\\\\/])?build([\\\\/].*)?\$")
internal val defaultIncludes = listOf("**/*.kt", "**/*.kts")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package com.ncorti.ktfmt.gradle

import com.ncorti.ktfmt.gradle.tasks.KtfmtCheckTask
import com.ncorti.ktfmt.gradle.tasks.KtfmtFormatTask
import java.io.File
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.file.FileCollection
import org.gradle.api.provider.Provider
import org.gradle.api.tasks.TaskProvider
import org.gradle.language.base.plugins.LifecycleBasePlugin
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
Expand Down Expand Up @@ -34,15 +36,16 @@ 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, srcSetName, srcSetDir)
val srcFormatTask = createFormatTask(project, srcSetName, srcSetDir)
val srcCheckTask = createCheckTask(project, srcSetName, srcSetDir, ktfmtExtension)
val srcFormatTask = createFormatTask(project, srcSetName, srcSetDir, ktfmtExtension)

// When running together with compileKotlin, ktfmt tasks should have precedence as
// they're editing the source code
Expand All @@ -63,7 +66,8 @@ internal object KtfmtPluginUtils {
private fun createCheckTask(
project: Project,
name: String,
srcDir: FileCollection
srcDir: FileCollection,
ktfmtExtension: KtfmtExtension,
): TaskProvider<KtfmtCheckTask> {
val capitalizedName =
name.split(" ").joinToString("") {
Expand All @@ -77,20 +81,21 @@ internal object KtfmtPluginUtils {
charArray.concatToString()
}
val taskName = "$TASK_NAME_CHECK$capitalizedName"
val inputDirs = srcDir.toList()

val inputDirs = project.getSelectedSrcSets(srcDir, ktfmtExtension)
return project.tasks.register(taskName, KtfmtCheckTask::class.java) {
it.description =
"Run Ktfmt formatter validation for sourceSet '$name' on project '${project.name}'"
it.setSource(inputDirs)
it.setIncludes(KtfmtPlugin.defaultIncludes)
it.setExcludes(KtfmtPlugin.defaultExcludes)
}
}

private fun createFormatTask(
project: Project,
name: String,
srcDir: FileCollection
srcDir: FileCollection,
ktfmtExtension: KtfmtExtension,
): TaskProvider<KtfmtFormatTask> {
val srcSetName =
name.split(" ").joinToString("") {
Expand All @@ -104,13 +109,24 @@ internal object KtfmtPluginUtils {
charArray.concatToString()
}
val taskName = "$TASK_NAME_FORMAT$srcSetName"
val inputDirs = srcDir.toList()

val inputDirs = project.getSelectedSrcSets(srcDir, ktfmtExtension)
return project.tasks.register(taskName, KtfmtFormatTask::class.java) {
it.description =
"Run Ktfmt formatter for sourceSet '$name' on project '${project.name}'"
it.setSource(inputDirs)
it.setIncludes(KtfmtPlugin.defaultIncludes)
it.setExcludes(KtfmtPlugin.defaultExcludes)
}
}

private fun Project.getSelectedSrcSets(
srcDir: FileCollection,
ktfmtExtension: KtfmtExtension
): Provider<List<File>> {
val excludedSourceSets = ktfmtExtension.srcSetPathExclusionPattern

return provider {
srcDir.toList().filterNot { it.absolutePath.matches(excludedSourceSets.get()) }
}
}
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package com.ncorti.ktfmt.gradle.tasks

import com.ncorti.ktfmt.gradle.FormattingOptionsBean
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 java.util.UUID
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.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
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.SkipWhenEmpty
import org.gradle.api.tasks.SourceTask
import org.gradle.api.tasks.TaskAction
import org.gradle.api.tasks.options.Option
Expand Down Expand Up @@ -49,11 +47,11 @@ internal constructor(
@get:Input
abstract val includeOnly: Property<String>

@get:PathSensitive(PathSensitivity.RELATIVE)
@get:InputFiles
@get:IgnoreEmptyDirectories
internal val inputFiles: FileCollection
get() = super.getSource().filter(defaultExcludesFilter)
@PathSensitive(PathSensitivity.RELATIVE)
@InputFiles
@IgnoreEmptyDirectories
@SkipWhenEmpty
override fun getSource(): FileTree = super.getSource()

@TaskAction
internal fun taskAction() {
Expand All @@ -64,16 +62,6 @@ internal constructor(

protected abstract fun execute(workQueue: WorkQueue)

@get:Internal
internal val defaultExcludesFilter: Spec<File> =
Spec<File> {
if (this.excludes.containsAll(KtfmtPlugin.defaultExcludes) && this.excludes.size == 1) {
it.absolutePath.matches(KtfmtPlugin.defaultExcludesRegex).not()
} else {
true
}
}

internal fun <T : KtfmtWorkAction> FileCollection.submitToQueue(
queue: WorkQueue,
action: Class<T>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal constructor(workerExecutor: WorkerExecutor, layout: ProjectLayout) :
}

override fun execute(workQueue: WorkQueue) {
val results = inputFiles.submitToQueue(workQueue, KtfmtCheckAction::class.java)
val results = source.submitToQueue(workQueue, KtfmtCheckAction::class.java)

logger.d("Check results: $results")
val failures = results.filterIsInstance<Result.Failure>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ import com.ncorti.ktfmt.gradle.util.KtfmtUtils
import com.ncorti.ktfmt.gradle.util.d
import com.ncorti.ktfmt.gradle.util.i
import javax.inject.Inject
import org.gradle.api.file.FileCollection
import org.gradle.api.file.ProjectLayout
import org.gradle.api.tasks.OutputFiles
import org.gradle.workers.WorkQueue
import org.gradle.workers.WorkerExecutor

Expand All @@ -18,16 +16,14 @@ abstract class KtfmtFormatTask
internal constructor(workerExecutor: WorkerExecutor, layout: ProjectLayout) :
KtfmtBaseTask(workerExecutor, layout) {

@get:OutputFiles
protected val outputFiles: FileCollection
get() = inputFiles

init {
group = KtfmtUtils.GROUP_FORMATTING
// Since the input files are also the output files, we do not have to specify outputs again
outputs.upToDateWhen { true }
}

override fun execute(workQueue: WorkQueue) {
val results = inputFiles.submitToQueue(workQueue, KtfmtFormatAction::class.java)
val results = source.submitToQueue(workQueue, KtfmtFormatAction::class.java)

logger.d("Format results: $results")
val failures = results.filterIsInstance<Result.Failure>()
Expand Down
Loading

0 comments on commit 8f28a03

Please sign in to comment.