Skip to content

Ktlint upgrade #42

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jun 26, 2024
Merged

Ktlint upgrade #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9961af8
upgrade
0marperez May 9, 2024
2ac88cd
upgrade ktlint to latest version
0marperez May 9, 2024
83d8476
remove manual tracking of ktlint version in code
0marperez May 9, 2024
a14c0f6
try to make ktlint task work on consumers
0marperez May 10, 2024
dd50396
clean up
0marperez May 10, 2024
b00efff
Merge branch 'main' of github.com:awslabs/aws-kotlin-repo-tools into …
lauzadis Jun 25, 2024
0950d9b
ktlint is running
lauzadis Jun 26, 2024
3251c17
ktlint is running (in aws-kotlin-repo-tools too now)
lauzadis Jun 26, 2024
c0ac365
jvmArgs change
lauzadis Jun 26, 2024
67b3607
ignore backing property rule
lauzadis Jun 26, 2024
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
3 changes: 3 additions & 0 deletions .editorconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
[*.{kt,kts}]
ktlint_code_style = intellij_idea

# ktlint rules to disable
ktlint_standard_no-wildcard-imports = disabled
ktlint_standard_filename = disabled
ktlint_standard_backing-property-naming = disabled

# enable trailing commas per JetBrains recommendation
# (https://kotlinlang.org/docs/coding-conventions.html#trailing-commas)
Expand Down
34 changes: 16 additions & 18 deletions build-plugins/build-support/src/main/kotlin/aws/sdk/kotlin/gradle/dsl/CodeStyle.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ import aws.sdk.kotlin.gradle.util.verifyRootProject
import org.gradle.api.Project
import org.gradle.api.attributes.Bundling
import org.gradle.api.tasks.JavaExec
import org.gradle.kotlin.dsl.dependencies
import org.gradle.kotlin.dsl.named
import org.gradle.kotlin.dsl.register
import org.gradle.kotlin.dsl.*
import org.gradle.language.base.plugins.LifecycleBasePlugin

/**
* Configure lint rules for the project
Expand All @@ -19,36 +18,35 @@ import org.gradle.kotlin.dsl.register
fun Project.configureLinting(lintPaths: List<String>) {
verifyRootProject { "Kotlin SDK lint configuration is expected to be configured on the root project" }

val ktlint = configurations.create("ktlint") {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL))
}
}
val ktlint by configurations.creating

// TODO - is there anyway to align this with the version from libs.versions.toml in this project/repo
val ktlintVersion = "0.48.1"
dependencies {
ktlint("com.pinterest:ktlint:$ktlintVersion")
val ktlintVersion = "1.3.0"
ktlint("com.pinterest.ktlint:ktlint-cli:$ktlintVersion") {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL))
}
}
}

// add the buildscript classpath which should pickup our custom ktlint-rules (via runtimeOnly dep on this plugin)
// plus any custom rules added by consumer
val execKtlintClaspath = ktlint + buildscript.configurations.getByName("classpath")
val execKtlintClasspath = ktlint + buildscript.configurations.getByName("classpath")

tasks.register<JavaExec>("ktlint") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Check Kotlin code style."
group = "Verification"
classpath = execKtlintClaspath
classpath = execKtlintClasspath
mainClass.set("com.pinterest.ktlint.Main")
args = lintPaths
jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED")
Copy link
Contributor Author

@0marperez 0marperez Jun 26, 2024

Choose a reason for hiding this comment

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

question: why remove the jvm args just for ktlint?

Copy link
Member

@lauzadis lauzadis Jun 26, 2024

Choose a reason for hiding this comment

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

Just following the guide on ktlint docs: https://pinterest.github.io/ktlint/latest/install/integrations/#custom-gradle-integration-with-kotlin-dsl

Copy link
Contributor Author

@0marperez 0marperez Jun 26, 2024

Choose a reason for hiding this comment

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

I would lean towards keeping it since we don't know if it was necessary but I don't feel strongly about it. A good way to test this would be to run ktlint twice, once with and once without

Copy link
Member

@lauzadis lauzadis Jun 26, 2024

Choose a reason for hiding this comment

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

Sorry, just saw this comment after I already merged the PR. I had already tested with and without the jvmArgs locally, there was no difference. In general I think we should trust our dependencies' onboarding guides and if they end up being incorrect, send them a PR to fix the documentation.

}

tasks.register<JavaExec>("ktlintFormat") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Auto fix Kotlin code style violations"
group = "formatting"
classpath = execKtlintClaspath
classpath = execKtlintClasspath
mainClass.set("com.pinterest.ktlint.Main")
args = listOf("-F") + lintPaths
jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED")
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
}
}
4 changes: 2 additions & 2 deletions ...in/kotlin/aws/sdk/kotlin/gradle/plugins/artifactsizemetrics/AnalyzeArtifactSizeMetrics.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ internal abstract class AnalyzeArtifactSizeMetrics : DefaultTask() {

val changeHappened = artifactSizeMetrics.values.any { it.delta.isNotaFluctuation() }
val significantChange = artifactSizeMetrics.values.any {
(it.percentage > pluginConfig.significantChangeThresholdPercentage) || // Increase in size above threshold
(it.latestReleaseSize == 0L) // New artifact
// Increase in size above threshold or new artifact
(it.percentage > pluginConfig.significantChangeThresholdPercentage) || (it.latestReleaseSize == 0L)
}

return ArtifactSizeMetricsAnalysis(artifactSizeMetrics, significantChange, changeHappened)
Expand Down
24 changes: 11 additions & 13 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,14 @@ fun propertyOrEnv(propName: String, envName: String): String? {
return findProperty(propName) as? String ?: env[envName]
}

// chicken and egg problem, we can't use the kotlinter gradle plugin here AND use our custom rules
val ktlint by configurations.creating {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL))
}
}
val ktlint by configurations.creating

dependencies {
ktlint(libs.ktlint)
ktlint(libs.ktlint.cli) {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.EXTERNAL))
}
}
ktlint(project(":ktlint-rules"))
}

Expand All @@ -61,19 +60,18 @@ val lintPaths = listOf(
)

tasks.register<JavaExec>("ktlint") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Check Kotlin code style."
group = "Verification"
classpath = configurations.getByName("ktlint")
classpath = ktlint
mainClass.set("com.pinterest.ktlint.Main")
args = lintPaths
jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED")
}

tasks.register<JavaExec>("ktlintFormat") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Auto fix Kotlin code style violations"
group = "formatting"
classpath = configurations.getByName("ktlint")
classpath = ktlint
mainClass.set("com.pinterest.ktlint.Main")
args = listOf("-F") + lintPaths
jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED")
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
Copy link
Contributor Author

@0marperez 0marperez Jun 26, 2024

Choose a reason for hiding this comment

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

nit/style: the same change could be made in CodeStyle.kt

}
7 changes: 4 additions & 3 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
[versions]
ktlint = "0.48.1"
ktlint = "1.3.0"
smithy-version = "1.42.0"
smithy-gradle-plugin-version = "0.9.0"
junit-version = "5.10.1"

[libraries]
ktlint = { module = "com.pinterest:ktlint", version.ref = "ktlint" }
ktlint-core = { module = "com.pinterest.ktlint:ktlint-core", version.ref = "ktlint" }
ktlint-cli = { module = "com.pinterest.ktlint:ktlint-cli", version.ref = "ktlint" }
ktlint-rule-engine-core = { module = "com.pinterest.ktlint:ktlint-rule-engine-core", version.ref = "ktlint" }
ktlint-cli-ruleset-core = { module = "com.pinterest.ktlint:ktlint-cli-ruleset-core", version.ref = "ktlint" }
ktlint-test = {module = "com.pinterest.ktlint:ktlint-test", version.ref = "ktlint" }
nexusPublishPlugin = { module = "io.github.gradle-nexus:publish-plugin", version = "1.3.0" }
smithy-model = { module = "software.amazon.smithy:smithy-model", version.ref = "smithy-version" }
Expand Down
2 changes: 1 addition & 1 deletion ktlint-rules/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ kotlin {
sourceSets {
main {
dependencies {
implementation(libs.ktlint.core)
implementation(libs.ktlint.cli.ruleset.core)
}
}

Expand Down
5 changes: 3 additions & 2 deletions ktlint-rules/src/main/kotlin/software/aws/ktlint/rules/CopyrightHeaderRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
*/
package software.aws.ktlint.rules

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiCommentImpl
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.stubs.elements.KtFileElementType

class CopyrightHeaderRule : Rule("copyright-header") {
class CopyrightHeaderRule : Rule(RuleId("copyright-header"), About()) {
companion object {
private val header = """
/*
Expand Down
7 changes: 4 additions & 3 deletions ktlint-rules/src/main/kotlin/software/aws/ktlint/rules/CustomRuleSetProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
*/
package software.aws.ktlint.rules

import com.pinterest.ktlint.core.RuleProvider
import com.pinterest.ktlint.core.RuleSetProviderV2
import com.pinterest.ktlint.cli.ruleset.core.api.RuleSetProviderV3
import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
import com.pinterest.ktlint.rule.engine.core.api.RuleSetId

class CustomRuleSetProvider : RuleSetProviderV2("custom-ktlint-rules", NO_ABOUT) {
class CustomRuleSetProvider : RuleSetProviderV3(RuleSetId("custom-ktlint-rules")) {
override fun getRuleProviders() = setOf(
RuleProvider { CopyrightHeaderRule() },
RuleProvider { ExpressionBodyRule() },
Expand Down
5 changes: 3 additions & 2 deletions ktlint-rules/src/main/kotlin/software/aws/ktlint/rules/ExpressionBodyRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@
*/
package software.aws.ktlint.rules

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtReturnExpression

class ExpressionBodyRule : Rule("expression-body") {
class ExpressionBodyRule : Rule(RuleId("expression-body"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
7 changes: 4 additions & 3 deletions ktlint-rules/src/main/kotlin/software/aws/ktlint/rules/MultilineIfElseBlockRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*/
package software.aws.ktlint.rules

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType
import com.pinterest.ktlint.rule.engine.core.api.ElementType
import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import org.jetbrains.kotlin.com.intellij.lang.ASTNode

class MultilineIfElseBlockRule : Rule("multiline-if-else-block") {
class MultilineIfElseBlockRule : Rule(RuleId("multiline-if-else-block"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
0 ...m.pinterest.ktlint.core.RuleSetProviderV2 → ...m.pinterest.ktlint.core.RuleSetProviderV3
View file
Open in desktop
File renamed without changes.