Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
9 changes: 6 additions & 3 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
[versions]
aws-sdk-version = "1.5.26"
kotlin-version = "2.2.0"
ktlint = "1.3.0"
ktlint-version = "1.3.0"
nexus-plugin-version = "2.0.0"
jreleaser-plugin-version = "1.18.0"
publish-plugin-version = "1.3.1"
smithy-version = "1.60.2"
smithy-gradle-plugin-version = "1.3.0"
junit-version = "5.10.1"
coroutines-version = "1.10.2"
slf4j-version = "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-cli = { module = "com.pinterest.ktlint:ktlint-cli", version.ref = "ktlint-version" }
ktlint-cli-ruleset-core = { module = "com.pinterest.ktlint:ktlint-cli-ruleset-core", version.ref = "ktlint-version" }
ktlint-rule-engine = { module = "com.pinterest.ktlint:ktlint-rule-engine", version.ref = "ktlint-version" }
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-version" }

[plugins]
kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin-version" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ 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 MinorVersionRuleSetProvider : RuleSetProviderV3(RuleSetId("minor-version-strategy-rules")) {
internal const val RULE_SET = "minor-version-strategy-rules"
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this RULE_SET refactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the MinorVersionRuleSetProvider one or in general?

Copy link
Member

Choose a reason for hiding this comment

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

In general, why do we need it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every rule ID needs to have a rule-set ID followed by : and then the name of the rule. This is just to stop repeating the rule-set ID so many times.


class MinorVersionRuleSetProvider : RuleSetProviderV3(RuleSetId(RULE_SET)) {
override fun getRuleProviders() = setOf(
RuleProvider { DeprecatedApiRule() },
RuleProvider { PlannedRemovalRule() },
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ import java.io.File
import java.util.Properties

/**
* Matches @DeprecatedUntilVersion with either named args (major=x, minor=y) or positional args (x, y)
* Matches @PlannedRemoval with either named args (major=x, minor=y) or positional args (x, y)
*/
internal fun deprecatedUntilVersionRegex(major: Int, minor: Int): Regex =
internal fun plannedRemovalRegex(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*\)""",
"""@PlannedRemoval\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 upcoming minor version.
* Creates a ktlint rule that detects APIs annotated with @PlannedRemoval for the upcoming minor version.
*/
class DeprecatedApiRule : Rule(RuleId("minor-version-strategy-rules:deprecated-apis"), About()) {
class PlannedRemovalRule : Rule(RuleId("$RULE_SET:planned-removal"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand All @@ -38,12 +38,12 @@ class DeprecatedApiRule : Rule(RuleId("minor-version-strategy-rules:deprecated-a
val majorVersion = sdkVersion[0].toInt()
val minorVersion = sdkVersion[1].toInt()

val regex = deprecatedUntilVersionRegex(majorVersion, minorVersion + 1)
val regex = plannedRemovalRegex(majorVersion, minorVersion + 1)
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,
"API is scheduled for removal, remove it before releasing the next minor version.",
false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to indicate if the rule can be auto-corrected. It should've been false from the beginning.

)
}
}
Expand Down

This file was deleted.

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

package software.aws.ktlint.rules

import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class PlannedRemovalRuleTest {
fun runRegexTestCases(minor: Int, major: Int) {
val regex = plannedRemovalRegex(major, minor)

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

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

@Test
fun testRegex() {
runRegexTestCases(0, 1)
runRegexTestCases(1, 70)
runRegexTestCases(100, 1_000_000)
}
}
17 changes: 17 additions & 0 deletions ktlint/style-rules/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,23 @@ kotlin {
implementation(libs.ktlint.cli.ruleset.core)
}
}
test {
dependencies {
implementation(kotlin("test"))
implementation(libs.ktlint.rule.engine)
implementation(libs.slf4j.simple) // Required by ktlint rule engine tests
}
}
}
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ 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(RuleId("aws-kotlin-repo-tools-rules:copyright-header"), About()) {
class CopyrightHeaderRule : Rule(RuleId("$RULE_SET:copyright-header"), About()) {
companion object {
private val header = """
/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.KtReturnExpression

class ExpressionBodyRule : Rule(RuleId("aws-kotlin-repo-tools-rules:expression-body"), About()) {
class ExpressionBodyRule : Rule(RuleId("$RULE_SET:expression-body"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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(RuleId("aws-kotlin-repo-tools-rules:multiline-if-else-block"), About()) {
class MultilineIfElseBlockRule : Rule(RuleId("$RULE_SET:multiline-if-else-block"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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

/**
* Creates a ktlint rule that forces APIs annotated with @PlannedRemoval to also be annotated with @Deprecated.
*/
class PlannedRemovalRule : Rule(RuleId("$RULE_SET:planned-removal"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit,
) {
if (node.elementType == ElementType.MODIFIER_LIST) {
val annotations = node.getChildren(null).filter { it.elementType == ElementType.ANNOTATION_ENTRY }
val deprecated = annotations.any { it.text.startsWith("@Deprecated") }
val plannedRemoval = annotations.any { it.text.startsWith("@PlannedRemoval") }

if (plannedRemoval && !deprecated) {
emit(
node.startOffset,
"APIs annotated with @PlannedRemoval must also be annotated with @Deprecated",
false,
)
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ 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 StyleRuleSetProvider : RuleSetProviderV3(RuleSetId("aws-kotlin-repo-tools-rules")) {
internal const val RULE_SET = "aws-kotlin-repo-tools-rules"

class StyleRuleSetProvider : RuleSetProviderV3(RuleSetId(RULE_SET)) {
override fun getRuleProviders() = setOf(
RuleProvider { CopyrightHeaderRule() },
RuleProvider { ExpressionBodyRule() },
RuleProvider { MultilineIfElseBlockRule() },
RuleProvider { PlannedRemovalRule() },
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*
* 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.RuleProvider
import kotlin.test.Test
import kotlin.test.assertEquals

class PlannedRemovalRuleTest {
val ruleEngine = KtLintRuleEngine(
setOf(
RuleProvider { PlannedRemovalRule() },
),
)

private fun hasLintingErrors(codeSnippet: String): Boolean {
val code = Code.fromSnippet(codeSnippet)
var hasErrors = false
ruleEngine.lint(code) {
// Error callback function
hasErrors = true
}
return hasErrors
}

@Test
fun testRule() {
assertEquals(
false,
hasLintingErrors(
"""
@PlannedRemoval(1, 2)
@Deprecated
class Foo {}
""".trimIndent(),
),
)

assertEquals(
false,
hasLintingErrors(
"""
@Deprecated
@PlannedRemoval(1, 2)
class Foo {}
""".trimIndent(),
),
)

assertEquals(
false,
hasLintingErrors(
"""
@Deprecated
class Foo {}
""".trimIndent(),
),
)

assertEquals(
true,
hasLintingErrors(
"""
@PlannedRemoval(1, 2)
class Foo {}
""".trimIndent(),
),
)

assertEquals(
true,
hasLintingErrors(
"""
@PlannedRemoval(1, 2)
class Foo {}

@Deprecated
class Bar {}
""".trimIndent(),
),
)

assertEquals(
true,
hasLintingErrors(
"""
@PlannedRemoval(1, 2)
class Foo {}

@PlannedRemoval(1, 2)
class Bar {}
""".trimIndent(),
),
)
}
}
Loading