Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump Ktlint to 0.47.1 (+ bump Kotlin to 1.7.10) #275

Merged
merged 4 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import org.jetbrains.kotlin.gradle.plugin.getKotlinPluginVersion
import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
kotlin("jvm") version "1.7.0"
kotlin("jvm") version "1.7.10"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, do we still work with 1.7.0 or do we need to update the compatibility matrix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified we're still compatible with kotlin 1.7.0

id("com.gradle.plugin-publish") version "0.21.0"
`java-gradle-plugin`
`maven-publish`
Expand All @@ -27,7 +27,7 @@ description = projectDescription
object Versions {
const val androidTools = "7.2.1"
const val junit = "4.13.2"
const val ktlint = "0.46.1"
const val ktlint = "0.47.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go to 0.47.1!
I've gotten into the habit of waiting a few days after each ktlint release because there's always a flurry of error reports and fast fixes. Seems like some important ones to pick up.

const val mockitoKotlin = "4.0.0"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ internal fun editorConfigOverride(ktLintParams: KtLintParams): EditorConfigOverr
return if (rules.isEmpty()) {
EditorConfigOverride.emptyEditorConfigOverride
} else {
EditorConfigOverride.from(DefaultEditorConfigProperties.disabledRulesProperty to rules.joinToString(separator = ","))
EditorConfigOverride.from(DefaultEditorConfigProperties.ktlintDisabledRulesProperty to rules.joinToString(separator = ","))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

Expand Down
40 changes: 20 additions & 20 deletions src/main/kotlin/org/jmailen/gradle/kotlinter/support/RuleSets.kt
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
package org.jmailen.gradle.kotlinter.support

import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleSetProvider
import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.RuleSetProviderV2
import com.pinterest.ktlint.ruleset.experimental.ExperimentalRuleSetProvider
import java.util.ServiceLoader
import kotlin.comparisons.compareBy

fun resolveRuleSets(
providers: Iterable<RuleSetProvider>,
internal fun resolveRuleProviders(
providers: Iterable<RuleSetProviderV2>,
includeExperimentalRules: Boolean = false,
): List<RuleSet> {
return providers
.filter { includeExperimentalRules || it !is ExperimentalRuleSetProvider }
.map { it.get() }
.sortedWith(
compareBy {
when (it.id) {
"standard" -> 0
else -> 1
}
},
)
}
): Set<RuleProvider> = providers
jeremymailen marked this conversation as resolved.
Show resolved Hide resolved
.asSequence()
.filter { includeExperimentalRules || it !is ExperimentalRuleSetProvider }
.sortedWith(
compareBy {
when (it.id) {
"standard" -> 0
else -> 1
}
},
)
Comment on lines +14 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't understand why sorting was required, but I preserved the behavior, despite the new returned type is a Set

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At one point way back we intentionally put standard rules first in the list although I can't dig up the reason why. It might have been to report them first, but this might be an obsolete concern.

.map(RuleSetProviderV2::getRuleProviders)
.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<RuleSetProvider> =
ServiceLoader.load(RuleSetProvider::class.java).toList()
val defaultRuleSetProviders: List<RuleSetProviderV2> =
ServiceLoader.load(RuleSetProviderV2::class.java).toList()
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package org.jmailen.gradle.kotlinter.tasks.format

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleProvider
import org.gradle.api.logging.LogLevel
import org.gradle.api.logging.Logger
import org.gradle.api.logging.Logging
Expand All @@ -12,7 +12,7 @@ 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.resolveRuleSets
import org.jmailen.gradle.kotlinter.support.resolveRuleProviders
import org.jmailen.gradle.kotlinter.tasks.FormatTask
import java.io.File

Expand All @@ -28,7 +28,7 @@ abstract class FormatWorkerAction : WorkAction<FormatWorkerParameters> {
val fixes = mutableListOf<String>()
try {
files.forEach { file ->
val ruleSets = resolveRuleSets(defaultRuleSetProviders, ktLintParams.experimentalRules)
val ruleSets = resolveRuleProviders(defaultRuleSetProviders, ktLintParams.experimentalRules)
val sourceText = file.readText()
val relativePath = file.toRelativeString(projectDirectory)

Expand Down Expand Up @@ -68,18 +68,18 @@ abstract class FormatWorkerAction : WorkAction<FormatWorkerParameters> {
)
}

private fun formatKt(file: File, ruleSets: List<RuleSet>, onError: ErrorHandler) =
private fun formatKt(file: File, ruleSets: Set<RuleProvider>, onError: ErrorHandler) =
format(file, ruleSets, onError, false)

private fun formatKts(file: File, ruleSets: List<RuleSet>, onError: ErrorHandler) =
private fun formatKts(file: File, ruleSets: Set<RuleProvider>, onError: ErrorHandler) =
format(file, ruleSets, onError, true)

private fun format(file: File, ruleSets: List<RuleSet>, onError: ErrorHandler, script: Boolean): String {
private fun format(file: File, ruleProviders: Set<RuleProvider>, onError: ErrorHandler, script: Boolean): String {
return KtLint.format(
KtLint.ExperimentalParams(
fileName = file.path,
text = file.readText(),
ruleSets = ruleSets,
ruleProviders = ruleProviders,
script = script,
editorConfigOverride = editorConfigOverride(ktLintParams),
cb = { error, corrected ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package org.jmailen.gradle.kotlinter.tasks.lint
import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.core.Reporter
import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleProvider
import org.gradle.api.logging.Logger
import org.gradle.api.logging.Logging
import org.gradle.internal.logging.slf4j.DefaultContextAwareTaskLogger
Expand All @@ -15,7 +15,7 @@ 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
import org.jmailen.gradle.kotlinter.support.resolveRuleSets
import org.jmailen.gradle.kotlinter.support.resolveRuleProviders
import org.jmailen.gradle.kotlinter.tasks.LintTask
import java.io.File

Expand All @@ -35,7 +35,7 @@ abstract class LintWorkerAction : WorkAction<LintWorkerParameters> {
try {
reporters.onEach { it.beforeAll() }
files.forEach { file ->
val ruleSets = resolveRuleSets(defaultRuleSetProviders, ktLintParams.experimentalRules)
val ruleSets = resolveRuleProviders(defaultRuleSetProviders, ktLintParams.experimentalRules)
val relativePath = file.toRelativeString(projectDirectory)
reporters.onEach { it.before(relativePath) }
logger.debug("$name linting: $relativePath")
Expand Down Expand Up @@ -68,18 +68,18 @@ abstract class LintWorkerAction : WorkAction<LintWorkerParameters> {
}
}

private fun lintKt(file: File, ruleSets: List<RuleSet>, onError: (error: LintError) -> Unit) =
private fun lintKt(file: File, ruleSets: Set<RuleProvider>, onError: (error: LintError) -> Unit) =
lint(file, ruleSets, onError, false)

private fun lintKts(file: File, ruleSets: List<RuleSet>, onError: (error: LintError) -> Unit) =
private fun lintKts(file: File, ruleSets: Set<RuleProvider>, onError: (error: LintError) -> Unit) =
lint(file, ruleSets, onError, true)

private fun lint(file: File, ruleSets: List<RuleSet>, onError: ErrorHandler, script: Boolean) =
private fun lint(file: File, ruleProviders: Set<RuleProvider>, onError: ErrorHandler, script: Boolean) =
KtLint.lint(
KtLint.ExperimentalParams(
fileName = file.path,
text = file.readText(),
ruleSets = ruleSets,
ruleProviders = ruleProviders,
script = script,
editorConfigOverride = editorConfigOverride(ktLintParams),
cb = { error, _ -> onError(error) },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ package org.jmailen.gradle.kotlinter.support

import com.pinterest.ktlint.core.KtLint
import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleSetProvider
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.RuleSetProviderV2
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Test
Expand All @@ -13,29 +12,24 @@ class RuleSetsTest {

@Test
fun `resolveRuleSets loads from classpath providers`() {
val result = resolveRuleSets(defaultRuleSetProviders)
val standardOnly = resolveRuleProviders(defaultRuleSetProviders, includeExperimentalRules = false)
val withExperimentalRules = resolveRuleProviders(defaultRuleSetProviders, includeExperimentalRules = true)

assertEquals(listOf("standard"), result.map { it.id })
}

@Test
fun `resolveRuleSets loads from classpath providers including experimental rules`() {
jeremymailen marked this conversation as resolved.
Show resolved Hide resolved
val result = resolveRuleSets(defaultRuleSetProviders, true)

assertEquals(listOf("standard", "experimental"), result.map { it.id })
assertTrue(standardOnly.isNotEmpty())
assertTrue(standardOnly.size < withExperimentalRules.size)
}

@Test
fun `resolveRuleSets puts standard rules first`() {
val standard = TestRuleSetProvider(RuleSet("standard", TestRule("one")))
val extra1 = TestRuleSetProvider(RuleSet("extra-one", TestRule("two")))
val extra2 = TestRuleSetProvider(RuleSet("extra-two", TestRule("three")))
val standard = TestRuleSetProvider("standard", setOf(TestRule("one")))
val extra1 = TestRuleSetProvider("extra-one", setOf(TestRule("two")))
val extra2 = TestRuleSetProvider("extra-two", setOf(TestRule("three")))

val result = resolveRuleSets(providers = listOf(extra2, standard, extra1))
val result = resolveRuleProviders(providers = listOf(extra2, standard, extra1)).map { it.createNewRuleInstance() }

assertEquals(3, result.size)
assertEquals(standard.ruleSet, result.first())
assertTrue(result.containsAll(listOf(extra1.ruleSet, extra2.ruleSet)))
assertEquals(standard.ruleSet.single(), result.first())
assertTrue(result.containsAll(listOf(extra1.ruleSet.single(), extra2.ruleSet.single())))
}

@Test
Expand All @@ -53,21 +47,24 @@ class RuleSetsTest {
}

""".trimIndent(),
ruleSets = resolveRuleSets(defaultRuleSetProviders),
ruleProviders = resolveRuleProviders(defaultRuleSetProviders),
cb = { _, _ -> },
),
)
}
}

class TestRuleSetProvider(val ruleSet: RuleSet) : RuleSetProvider {
override fun get() = ruleSet
class TestRuleSetProvider(id: String, val ruleSet: Set<Rule>) : RuleSetProviderV2(
id = id,
about = About(
maintainer = "stub-maintainer",
description = "stub-description",
license = "stub-license",
repositoryUrl = "stub-repositoryUrl",
issueTrackerUrl = "stub-issueTrackerUrl",
),
) {
override fun getRuleProviders() = ruleSet.map { rule -> RuleProvider(provider = { rule }) }.toSet()
}

class TestRule(id: String) : Rule(id) {
override fun visit(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) = Unit
}
class TestRule(id: String) : Rule(id)
2 changes: 1 addition & 1 deletion test-project-android/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
plugins {
kotlin("android") version "1.7.0"
kotlin("android") version "1.7.10"
id("com.android.library")
id("org.jmailen.kotlinter")
}
Expand Down
2 changes: 1 addition & 1 deletion test-project/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
plugins {
kotlin("jvm") version "1.7.0"
kotlin("jvm") version "1.7.10"
id("org.jmailen.kotlinter")
}