Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
14 changes: 14 additions & 0 deletions .github/actions/minor-version-bump/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
name: Minor version bump
description: Verifies branch is ready to merge before minor version bump
runs:
using: composite
steps:
- name: Checkout repository
uses: actions/checkout@v4

- name: Configure Gradle
uses: awslabs/aws-kotlin-repo-tools/.github/actions/configure-gradle@main

- name: Verify minor version bump
shell: bash
run: ./gradlew minorVersionBumpScan
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package aws.sdk.kotlin.gradle.dsl

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.*
import org.gradle.language.base.plugins.LifecycleBasePlugin

/**
* Configures Gradle tasks that run or fix minor-version-bump-specific Ktlint rules.
*/
fun Project.configureMinorVersionStrategyRules(lintPaths: List<String>) {
verifyRootProject { "Task configuration is expected to be configured on the root project" }

val ktlintVersion = object {} // Can't use Project.javaClass because that's using the Gradle classloader
.javaClass
.getResource("ktlint-version.txt")
?.readText()
?: error("Missing ktlint-version.txt")

val minorVersionBumpKtlint by configurations.creating

dependencies {
minorVersionBumpKtlint("com.pinterest.ktlint:ktlint-cli:$ktlintVersion") {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.SHADOWED))
}
}
minorVersionBumpKtlint(project(":ktlint-rules:minor-version-strategy"))
}

tasks.register<JavaExec>("minorVersionBumpScan") {
Copy link
Member

Choose a reason for hiding this comment

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

nit/naming: validateMinorVersionBump?

group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Check minor version bump rules"
classpath = minorVersionBumpKtlint
mainClass.set("com.pinterest.ktlint.Main")
args = lintPaths
}

tasks.register<JavaExec>("minorVersionBumpFix") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Check minor version bump rules"
classpath = minorVersionBumpKtlint
mainClass.set("com.pinterest.ktlint.Main")
args = listOf("-F") + lintPaths
jvmArgs("--add-opens=java.base/java.lang=ALL-UNNAMED")
}
}
4 changes: 3 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,21 @@ smithy-version = "1.60.2"
smithy-gradle-plugin-version = "1.3.0"
junit-version = "5.10.1"
coroutines-version = "1.10.2"
slf4j = "2.0.17"

[libraries]
aws-sdk-cloudwatch = { module = "aws.sdk.kotlin:cloudwatch", version.ref = "aws-sdk-version" }
aws-sdk-s3 = { module = "aws.sdk.kotlin:s3", version.ref = "aws-sdk-version" }
ktlint-cli = { module = "com.pinterest.ktlint:ktlint-cli", 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" }
ktlint-rule-engine = { module = "com.pinterest.ktlint:ktlint-rule-engine", version.ref = "ktlint" }
nexus-publish-plugin = { module = "io.github.gradle-nexus:publish-plugin", version.ref = "nexus-plugin-version" }
jreleaser-plugin = { module = "org.jreleaser:jreleaser-gradle-plugin", version.ref = "jreleaser-plugin-version" }
smithy-model = { module = "software.amazon.smithy:smithy-model", version.ref = "smithy-version" }
smithy-gradle-base-plugin = { module = "software.amazon.smithy.gradle:smithy-base", version.ref = "smithy-gradle-plugin-version" }
junit-jupiter = { module = "org.junit.jupiter:junit-jupiter", version.ref = "junit-version" }
kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "coroutines-version" }
slf4j-simple = { module = "org.slf4j:slf4j-simple", version.ref = "slf4j" }

[plugins]
kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin-version" }
Expand Down
17 changes: 15 additions & 2 deletions ktlint-rules/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,31 @@ kotlin {
sourceSets {
main {
dependencies {
implementation(libs.ktlint.cli.ruleset.core)
api(libs.ktlint.cli.ruleset.core)
Copy link
Member

Choose a reason for hiding this comment

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

Was this never needed as implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It worked as an implementation before, but now it's an API so downstream consumers of this module don’t also need to depend on libs.ktlint.cli.ruleset.core.

https://github.com/aws/aws-sdk-kotlin/blob/minor-version-strategy-sdk/ktlint-rules/minor-version-strategy/build.gradle.kts#L14

}
}

test {
dependencies {
implementation(libs.ktlint.test)
implementation(kotlin("test"))
implementation(libs.ktlint.rule.engine)
implementation(libs.slf4j.simple) // Required by ktlint rule engine tests
}
}
}
}

tasks.test {
useJUnitPlatform()
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't need this useJUnitPlatform() call

testLogging {
events("passed", "skipped", "failed")
showStandardStreams = true
showStackTraces = true
showExceptions = true
exceptionFormat = org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL
}
}

tasks.withType<KotlinCompile> {
compilerOptions {
jvmTarget.set(JvmTarget.JVM_1_8)
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

naming: ApisScheduledForRemovalRule is awkward and long, how about DeprecatedApiRule?

Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.aws.ktlint.rules

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

/**
* Matches @DeprecatedUntilVersion with either named args (major=x, minor=y) or positional args (x, y)
*/
internal fun deprecatedUntilVersionRegex(major: Int, minor: Int): Regex =
Regex(
"""@DeprecatedUntilVersion\s*\(\s*(?:major\s*=\s*$major\s*,\s*minor\s*=\s*$minor\s*|\s*$major\s*,\s*$minor\s*)\s*\)""",
)

/**
* Creates a ktlint rule that detects APIs annotated with @DeprecatedUntilVersion for the specified versions.
* If autocorrect is enabled, the API will be deleted.
*/
fun apisScheduledForRemovalRule(major: Int, minor: Int): Rule =
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this rule provider function gets connected to the Gradle task. Where do we pass the major / minor versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not used in the Gradle task, it's for the downstream ktlint-rules:minor-version-strategy modules to create the same rule with different major/minor versions.

https://github.com/aws/aws-sdk-kotlin/blob/minor-version-strategy-sdk/ktlint-rules/minor-version-strategy/src/main/kotlin/aws/sdk/kotlin/ktlintrules/minorversionstrategy/MinorVersionStrategyRuleSetProvider.kt#L23-L24

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I saw that after reviewing your aws-sdk-kotlin PR. I wish the rule could be registered here for a given Project

object : Rule(RuleId("minor-version-strategy:apis-scheduled-for-removal"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
if (node.elementType == ElementType.ANNOTATION_ENTRY) {
val regex = deprecatedUntilVersionRegex(major, minor)
if (regex.containsMatchIn(node.text)) {
emit(
node.startOffset,
"The deprecated API is scheduled for removal, please remove it before releasing the next minor version.",
true,
)

if (autoCorrect) {
node.treeParent.treeParent?.let {
it.treeParent.removeChild(it)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove autocorrect from this rule, deleting APIs should be an intentional action, not something our linter does

Copy link
Contributor Author

@0marperez 0marperez Sep 17, 2025

Choose a reason for hiding this comment

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

It won't run on every lint, only when someone executes the new Gradle task (fixMinorVersionBump). We should delete deprecated APIs when they're scheduled for removal, why not automate it?

Copy link
Member

Choose a reason for hiding this comment

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

Deleting APIs is a sensitive operation and should not be automated

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, we'll likely have to make other manual changes after deleting a deprecated API such as replacing its usage in calling functions.

}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.aws.ktlint.rules

import com.pinterest.ktlint.rule.engine.api.Code
import com.pinterest.ktlint.rule.engine.api.KtLintRuleEngine
import com.pinterest.ktlint.rule.engine.core.api.AutocorrectDecision
import com.pinterest.ktlint.rule.engine.core.api.RuleProvider
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class ApisScheduledForRemovalRuleTest {
val ruleEngine = KtLintRuleEngine(
ruleProviders = setOf(
RuleProvider { apisScheduledForRemovalRule(1, 2) },
),
)

private fun runAutoCorrectTest(codeSnippet: String, expected: String) {
val unformattedCode = Code.fromSnippet(codeSnippet)
val formattedCode = ruleEngine
.format(unformattedCode) { AutocorrectDecision.ALLOW_AUTOCORRECT }
.trimStart()
.trimEnd()

assertEquals(expected, formattedCode)
}

@Test
fun testAutoCorrect() {
runAutoCorrectTest(
"""
@DeprecatedUntilVersion(1, 2)
class Foo {
fun foo() {}
}

class Bar {
fun bar() {}
}
""".trimIndent(),
"""
class Bar {
fun bar() {}
}
""".trimIndent(),
)

runAutoCorrectTest(
"""
@DeprecatedUntilVersion(1, 2)
class Foo {
fun foo() {}
}
""".trimIndent(),
"""
""".trimIndent(),
)

runAutoCorrectTest(
"""
@DeprecatedUntilVersion(1, 2)
fun foo() {
// foo foo
}

class Bar {
fun bar() {}
}
""".trimIndent(),
"""
class Bar {
fun bar() {}
}
""".trimIndent(),
)
}

fun runRegexTestCases(minor: Int, major: Int) {
val regex = deprecatedUntilVersionRegex(major, minor)

assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion($major,$minor)"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion($major,$minor )"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion($major, $minor)"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion($major ,$minor)"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion( $major,$minor)"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion( $major , $minor )"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion(major=$major,minor=$minor)"))
assertTrue(regex.containsMatchIn("@DeprecatedUntilVersion( major= $major , minor= $minor )"))

assertFalse(regex.containsMatchIn("@DeprecatedUntilVersion"))
assertFalse(regex.containsMatchIn("@DeprecatedUntilVersion()"))
assertFalse(regex.containsMatchIn("@DeprecatedUntilVersion($minor,$minor)"))
assertFalse(regex.containsMatchIn("@DeprecatedUntilVersion($major,$major)"))
assertFalse(regex.containsMatchIn("@DeprecatedUntilVersion($minor,$major)"))
}

@Test
fun testRegex() {
runRegexTestCases(0, 1)
runRegexTestCases(1, 70)
runRegexTestCases(100, 1_000_000)
}
}
Loading