Skip to content

fix: custom ktlint rules #130

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 5 commits into from
Sep 11, 2025
Merged

fix: custom ktlint rules #130

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
695802f
misc: fix custom ktlint rules
0marperez Sep 10, 2025
7b1831c
feedback
0marperez Sep 10, 2025
e48515e
self review
0marperez Sep 10, 2025
fbbd4a9
feedback and self review
0marperez Sep 11, 2025
34452da
invent and simplify
0marperez Sep 11, 2025
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: 2 additions & 1 deletion 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 @@ -15,7 +15,7 @@ import org.gradle.language.base.plugins.LifecycleBasePlugin
* Configure lint rules for the project
* @param lintPaths list of paths relative to the project root to lint (or not lint).
*/
fun Project.configureLinting(lintPaths: List<String>) {
fun Project.configureLinting(lintPaths: List<String>, repoToolsVersion: String) {
Copy link
Member

@lauzadis lauzadis Sep 10, 2025

Choose a reason for hiding this comment

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

I'm not a fan of having to keep track of aws-kotlin-repo-tools version in a second place. We have access to the Project, can't we dive in and find the aws-kotlin-repo-tools version?

Copy link
Member

@lauzadis lauzadis Sep 10, 2025

Choose a reason for hiding this comment

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

From the caller side, if you can get the aws-kotlin-repo-tools version from libs.versions.toml and pass it to configureLinting, that would work too

Copy link
Contributor Author

@0marperez 0marperez Sep 10, 2025

Choose a reason for hiding this comment

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

I'm not a fan of having to keep track of aws-kotlin-repo-tools version in a second place. We have access to the Project, can't we dive in and find the aws-kotlin-repo-tools version?

We don't keep track of the current version in repo tools like we do in the other repos so we can't do that unless we make bigger changes

From the caller side, if you can get the aws-kotlin-repo-tools version from libs.versions.toml and pass it to configureLinting, that would work too

That's what I'm planning to do, I made sure we can do so before I posted the PR

Copy link
Member

@lauzadis lauzadis Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The Project in this context is aws-sdk-kotlin, smithy-kotlin, aws-crt-kotlin which do declare a version of aws-kotlin-repo-tools in their libs.versions.toml file and should be accessible, but your other solution is fine too

That's what I'm planning to do

Ok 👍 , as long as we don't have to configure the aws-kotlin-repo-tools version in two places

Copy link
Contributor

@ianbotsf ianbotsf Sep 10, 2025

Choose a reason for hiding this comment

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

Can we get the version from inside configureLinting?

fun Project.configureLinting(lintPaths: List<String>) {
    val repoToolsVersion = project
        .configurations
        .filter { it.isCanBeResolved }
        .flatMap { it.resolvedConfiguration.resolvedArtifacts }
        .map { it.moduleVersion.id }
        .filter { it.group == "aws.sdk.kotlin.gradle" && it.name == "build-support" }
        .map { it.version }
        .distinct()
        .single()
    ...
}

Copy link
Contributor Author

@0marperez 0marperez Sep 11, 2025

Choose a reason for hiding this comment

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

We can't as far as I know, this code returns an empty list. The list is already empty by the flatMap call.

Copy link
Contributor Author

@0marperez 0marperez Sep 11, 2025

Choose a reason for hiding this comment

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

This code worked:

    val repoToolsVersion = extensions
        .getByType<VersionCatalogsExtension>()
        .named("libs")
        .findVersion("aws-kotlin-repo-tools-version")
        .get()
        .requiredVersion

verifyRootProject { "Kotlin SDK lint 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
Expand All @@ -32,6 +32,7 @@ fun Project.configureLinting(lintPaths: List<String>) {
attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling.SHADOWED))
}
}
ktlint("aws.sdk.kotlin.gradle:ktlint-rules:$repoToolsVersion")
Copy link
Contributor Author

@0marperez 0marperez Sep 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

See: "in case of custom ruleset"

}

// add the buildscript classpath which should pick up our custom ktlint-rules (via runtimeOnly dep on this plugin)
Expand Down
2 changes: 1 addition & 1 deletion ktlint-rules/src/main/kotlin/software/aws/ktlint/rules/CopyrightHeaderRule.kt
View file
Open in desktop
Copy link
Contributor Author

@0marperez 0marperez Sep 10, 2025

Choose a reason for hiding this comment

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

Rule ID changes are to comply with Ktlint regex. e.g. see: https://github.com/pinterest/ktlint/blob/cd39c67816dee8c26b19a076eb064c97a248f670/ktlint-ruleset-template/src/main/kotlin/yourpkgname/NoVarRule.kt#L12

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("copyright-header"), About()) {
class CopyrightHeaderRule : Rule(RuleId("repo-tools:copyright-header"), About()) {
companion object {
private val header = """
/*
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -8,7 +8,7 @@ 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 : RuleSetProviderV3(RuleSetId("custom-ktlint-rules")) {
class CustomRuleSetProvider : RuleSetProviderV3(RuleSetId("repo-tools")) {
override fun getRuleProviders() = setOf(
RuleProvider { CopyrightHeaderRule() },
RuleProvider { ExpressionBodyRule() },
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -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("expression-body"), About()) {
class ExpressionBodyRule : Rule(RuleId("repo-tools:expression-body"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -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("multiline-if-else-block"), About()) {
class MultilineIfElseBlockRule : Rule(RuleId("repo-tools:multiline-if-else-block"), About()) {
override fun beforeVisitChildNodes(
node: ASTNode,
autoCorrect: Boolean,
Expand Down
0 ...m.pinterest.ktlint.core.RuleSetProviderV3 → ...nt.cli.ruleset.core.api.RuleSetProviderV3
View file
Open in desktop
Copy link
Contributor Author

@0marperez 0marperez Sep 10, 2025

Choose a reason for hiding this comment

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

See: https://github.com/pinterest/ktlint/blob/master/ktlint-ruleset-template/src/main/resources/META-INF/services/com.pinterest.ktlint.cli.ruleset.core.api.RuleSetProviderV3

File renamed without changes.
Loading