Skip to content

Commit

Permalink
Load RuleSetProviders from resolved classpach. Remove unused depend…
Browse files Browse the repository at this point in the history
…encies. Add tests covering new features
  • Loading branch information
mateuszkwiecinski committed Oct 22, 2022
1 parent 3a5faab commit 91a7f56
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 89 deletions.
4 changes: 1 addition & 3 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ dependencies {
"ktlint-reporter-json",
"ktlint-reporter-html",
"ktlint-reporter-plain",
"ktlint-reporter-sarif",
"ktlint-ruleset-experimental",
"ktlint-ruleset-standard"
"ktlint-reporter-sarif"
).forEach { module ->
compileOnly("com.pinterest.ktlint:$module:${Versions.ktlint}")
testImplementation("com.pinterest.ktlint:$module:${Versions.ktlint}")
Expand Down
14 changes: 7 additions & 7 deletions src/main/kotlin/org/jmailen/gradle/kotlinter/support/RuleSets.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ package org.jmailen.gradle.kotlinter.support

import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.RuleSetProviderV2
import com.pinterest.ktlint.ruleset.experimental.ExperimentalRuleSetProvider
import java.util.ServiceLoader

internal fun resolveRuleProviders(
providers: Iterable<RuleSetProviderV2>,
includeExperimentalRules: Boolean = false,
): Set<RuleProvider> = providers
): Set<RuleProvider> = defaultRuleSetProviders()
.asSequence()
.filter { includeExperimentalRules || it !is ExperimentalRuleSetProvider }
.filter { includeExperimentalRules || it.id != "experimental" }
.sortedWith(
compareBy {
when (it.id) {
Expand All @@ -23,7 +21,9 @@ internal fun resolveRuleProviders(
.flatten()
.toSet()

// statically resolve providers from plugin classpath. ServiceLoader#load alone resolves classes lazily which fails when run in parallel
// https://github.com/jeremymailen/kotlinter-gradle/issues/101
val defaultRuleSetProviders: List<RuleSetProviderV2> =
/**
* Make sure this gets called with proper classpath (i.e. within Gradle Worker class)
* `toList()` call prevents concurrency issues: https://github.com/jeremymailen/kotlinter-gradle/issues/101
*/
private fun defaultRuleSetProviders(): List<RuleSetProviderV2> =
ServiceLoader.load(RuleSetProviderV2::class.java).toList()
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ package org.jmailen.gradle.kotlinter.tasks.format
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.RuleProvider
import org.gradle.api.logging.LogLevel
import org.gradle.api.logging.Logger
import org.gradle.api.logging.Logging
import org.gradle.internal.logging.slf4j.DefaultContextAwareTaskLogger
import org.gradle.workers.WorkAction
import org.jmailen.gradle.kotlinter.support.KotlinterError
import org.jmailen.gradle.kotlinter.support.KtLintParams
import org.jmailen.gradle.kotlinter.support.defaultRuleSetProviders
import org.jmailen.gradle.kotlinter.support.editorConfigOverride
import org.jmailen.gradle.kotlinter.support.resetEditorconfigCacheIfNeeded
import org.jmailen.gradle.kotlinter.support.resolveRuleProviders
Expand All @@ -30,21 +28,22 @@ abstract class FormatWorkerAction : WorkAction<FormatWorkerParameters> {
changedEditorconfigFiles = parameters.changedEditorConfigFiles,
logger = logger,
)
val ruleSets = resolveRuleProviders(includeExperimentalRules = ktLintParams.experimentalRules)
logger.info("Resolved ${ruleSets.size} RuleSetProviders")

val fixes = mutableListOf<String>()
try {
files.forEach { file ->
val ruleSets = resolveRuleProviders(defaultRuleSetProviders, ktLintParams.experimentalRules)
val sourceText = file.readText()
val relativePath = file.toRelativeString(projectDirectory)

logger.log(LogLevel.DEBUG, "$name checking format: $relativePath")
logger.debug("$name checking format: $relativePath")

when (file.extension) {
"kt" -> this::formatKt
"kts" -> this::formatKts
else -> {
logger.log(LogLevel.DEBUG, "$name ignoring non Kotlin file: $relativePath")
logger.debug("$name ignoring non Kotlin file: $relativePath")
null
}
}?.let { formatFunc ->
Expand All @@ -53,11 +52,11 @@ abstract class FormatWorkerAction : WorkAction<FormatWorkerParameters> {
true -> "${file.path}:${error.line}:${error.col}: Format fixed > [${error.ruleId}] ${error.detail}"
false -> "${file.path}:${error.line}:${error.col}: Format could not fix > [${error.ruleId}] ${error.detail}"
}
logger.log(LogLevel.QUIET, msg)
logger.quiet(msg)
fixes.add(msg)
}
if (!formattedText.contentEquals(sourceText)) {
logger.log(LogLevel.QUIET, "${file.path}: Format fixed")
logger.quiet("${file.path}: Format fixed")
file.writeText(formattedText)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import org.gradle.internal.logging.slf4j.DefaultContextAwareTaskLogger
import org.gradle.workers.WorkAction
import org.jmailen.gradle.kotlinter.support.KotlinterError
import org.jmailen.gradle.kotlinter.support.KtLintParams
import org.jmailen.gradle.kotlinter.support.defaultRuleSetProviders
import org.jmailen.gradle.kotlinter.support.editorConfigOverride
import org.jmailen.gradle.kotlinter.support.reporterFor
import org.jmailen.gradle.kotlinter.support.reporterPathFor
Expand All @@ -34,12 +33,17 @@ abstract class LintWorkerAction : WorkAction<LintWorkerParameters> {
changedEditorconfigFiles = parameters.changedEditorconfigFiles,
logger = logger,
)
val ruleSets = resolveRuleProviders(includeExperimentalRules = ktLintParams.experimentalRules)
logger.info("Resolved ${ruleSets.size} RuleSetProviders")
if (logger.isDebugEnabled) {
logger.debug("Resolved RuleSetProviders = ${ruleSets.joinToString { it.createNewRuleInstance().id }}")
}

var hasError = false

try {
reporters.onEach { it.beforeAll() }
files.forEach { file ->
val ruleSets = resolveRuleProviders(defaultRuleSetProviders, ktLintParams.experimentalRules)
val relativePath = file.toRelativeString(projectDirectory)
reporters.onEach { it.before(relativePath) }
logger.debug("$name linting: $relativePath")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,43 @@ internal class ExtensionTest : WithGradleTest.Kotlin() {
assertEquals(TaskOutcome.SUCCESS, task(":lintKotlinMain")?.outcome)
}
}

@Test
fun `can override ktlint version`() {
projectRoot.resolve("build.gradle") {
// language=groovy
val buildScript =
"""
plugins {
id 'kotlin'
id 'org.jmailen.kotlinter'
}
repositories {
mavenCentral()
}
kotlinter {
ktlintVersion = "0.46.0"
}
""".trimIndent()
writeText(buildScript)
}
projectRoot.resolve("src/main/kotlin/FileName.kt") {
writeText(kotlinClass("FileName"))
}

buildAndFail("lintKotlin").apply {
assertEquals(TaskOutcome.FAILED, task(":lintKotlinMain")?.outcome) { "should fail due to incompatibility" }
val expectedMessage = "Caused by: java.lang.NoSuchMethodError: 'void com.pinterest.ktlint.core.KtLint\$ExperimentalParams."
assertTrue(output.contains(expectedMessage)) { "should explain the incompatibility" }
}
// remove `--configuration-cache-problems=warn` when upgrading to Gradle 7.6 https://github.com/gradle/gradle/issues/17470
build("dependencies", "--configuration", "ktlint", "--configuration-cache-problems=warn").apply {
assertTrue(output.contains("com.pinterest:ktlint:0.46.0")) {
"should include overridden ktlin version in `ktlint` configuration"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,30 @@ internal class KotlinProjectTest : WithGradleTest.Kotlin() {
}
}

@Test
fun `plugin resolves dynamically loaded RulesetProviders`() {
settingsFile()
buildFile()
editorConfig()
kotlinSourceFile(
"CustomObject.kt",
"""
object CustomObject
""".trimIndent(),
)

build("lintKotlin", "--info").apply {
assertEquals(SUCCESS, task(":lintKotlin")?.outcome)
assertTrue(output.contains("Resolved 45 RuleSetProviders"))
}

build("formatKotlin", "--info").apply {
assertEquals(SUCCESS, task(":formatKotlin")?.outcome)
assertTrue(output.contains("Resolved 45 RuleSetProviders"))
}
}

private fun settingsFile() = settingsFile.apply {
writeText("rootProject.name = 'kotlinter'")
}
Expand Down

This file was deleted.

0 comments on commit 91a7f56

Please sign in to comment.