Add support for ktlint fix with -F

[smocherla-brex]

This is a follow-up to ktlint support with an action to patch/autocorrect violations where supported in rules using https://pinterest.github.io/ktlint/1.0.1/install/cli/#format-autocorrect and leveraging existing semantics with rules_lint

---

Changes are visible to end-users: yes

• Searched for relevant documentation and updated as needed: yes

Test plan

• Covered by existing test cases
• Also verified with ./lint.sh --fix --dry-run that hello.ktwould be updated

From bazel-out/k8-fastbuild/bin/src/ktlint.hello_kt.aspect_rules_lint.patch:
--- a/src/hello.kt
+++ b/src/hello.kt
@@ -1,6 +1,6 @@
-import javax.security.*
 import java.util.*
+import javax.security.*
 
 fun main() {
-  println("Hello, world!")
+    println("Hello, world!")
 }

[alexeagle]

This is confusing, they've shipped a format feature in the lint tool. We already have ktfmt in this repo. Is a user meant to choose one or the other?

[smocherla-brex]

ktlint acts both as a linter and a formatter (formatting is subset of the standard lint rules). This --format flag is slightly confusing as it goes beyond pure pretty-printing/formatting. For example, custom rules can use this https://github.com/pinterest/ktlint/blob/9b38b37d95fcb0ad90d07d64d581e6059ea8496e/ktlint-api-consumer/src/main/kotlin/com/example/ktlint/api/consumer/rules/NoVarRule.kt#L15 (as an e.g) with -F mapped to autoCorrect in their implementation to suggest custom fixes for the lint rules that'll show up in the diff, so these fixes can also be semantic suggestions/enforcements. That said, I haven't used ktfmt before so if you feel this is redundant, I can close the PR.

[alexeagle]

I don't want to close the PR - we DO want to have auto-fixes for linters that understand them.
It's a pity that the Kotlin ecosystem got into this confused state - Javascript (eslint) and Python (flake8/ruff) were careful to have linting not fight with formatting. It seems like ktlint authors have freely conflated the two.
We could use ktlint for both purposes, but I don't see a flag that allows you to only format and not lint.

I think we should raise a discussion on the ktlint repo to see if there's any recommended solution (and not make it Bazel specific - we just want to follow what other users do)

[sachnk]

I recently enabled both ktfmt and ktlint in my project. I can say that out-of-the-box, they will conflict with each other. Here's a simple example:

After running ktfmt:

Runtime.getRuntime()
    .addShutdownHook(
        Thread {
            println("Shutting down gRPC server...")
            stop()
            println("Server shut down.")
        }  // <-- no trailing comma
    )

After running ktlint:

Runtime.getRuntime()
    .addShutdownHook(
        Thread {
            println("Shutting down gRPC server...")
            stop()
            println("Server shut down.")
        },  // <-- trailing comma required here in order to satisfy linter
    )

I assume I can configure ktlint to match ktfmt for trailing comma related issues, but this quickly devolves into a rabbit-hole of guaranteeing these two tools remain mutually exclusive.

In fact, the trailing comma issue is the second conflict I hit. The first conflict I hit was with ktfmt defaulting to 2-space idents, while ktlint defaulting to 4-spaces. You can change ktfmt to 4-space indents by passing in --kotlinlang-style, which requires you to roll your own bash wrapper for it to work with format_multirun.

I don't know what other landmines I might hit as my project grows.

Ultimately, I think ktlint is fundamentally incompatible with any formatter, because of @alexeagle's earlier comment regarding conflating linting with formatting, i.e. ktlint has the potential to conflict with any formatter if you can't disable ktlint's formatting behavior. I've only started to work with the kotlin ecosystem, so I'm happy to be wrong here.

[sachnk]

I recently enabled both ktfmt and ktlint in my project. I can say that out-of-the-box, they will conflict with each other. Here's a simple example:

After running ktfmt:

Runtime.getRuntime()
    .addShutdownHook(
        Thread {
            println("Shutting down gRPC server...")
            stop()
            println("Server shut down.")
        }  // <-- no trailing comma
    )

After running ktlint:

Runtime.getRuntime()
    .addShutdownHook(
        Thread {
            println("Shutting down gRPC server...")
            stop()
            println("Server shut down.")
        },  // <-- trailing comma required here in order to satisfy linter
    )

I assume I can configure ktlint to match ktfmt for trailing comma related issues, but this quickly devolves into a rabbit-hole of guaranteeing these two tools remain mutually exclusive.

In fact, the trailing comma issue is the second conflict I hit. The first conflict I hit was with ktfmt defaulting to 2-space idents, while ktlint defaulting to 4-spaces. You can change ktfmt to 4-space indents by passing in --kotlinlang-style, which requires you to roll your own bash wrapper for it to work with format_multirun.

I don't know what other landmines I might hit as my project grows.

Ultimately, I think ktlint is fundamentally incompatible with any formatter, because of @alexeagle's earlier comment regarding conflating linting with formatting, i.e. ktlint has the potential to conflict with any formatter if you can't disable ktlint's formatting behavior. I've only started to work with the kotlin ecosystem, so I'm happy to be wrong here.

FWIW, I was finally able to get both tools to not trample each other. In case it's useful for others who are struggling with this, here's what I did.

1. Make ktfmt use 4-space indents by passing --kotlinglang-style to ktfmt. You could admittedly change ktlint to use 2-spaces, but for my repo, 4-spaces is more idiomatic.

java_binary(
    name = "ktfmt",
    args = [
        "--kotlinlang-style",
    ],
    main_class = "com.facebook.ktfmt.cli.Main",
    runtime_deps = [
        "@maven//:com_facebook_ktfmt",
    ],
)

sh_binary(
    name = "ktfmt_wrapper",
    srcs = ["ktfmt.sh"],
    data = [
        ":ktfmt",
        "@bazel_tools//tools/jdk:current_java_runtime",
    ],
    toolchains = ["@bazel_tools//tools/jdk:current_java_runtime"],
)

format_multirun(
  # ...
  kotlin = ":ktfmt_wrapper"
)

The ktfmt.sh above just looks like this:

#!/usr/bin/env bash
$(dirname $0)/ktfmt --kotlinlang-style "$@"

2. Updated my .editorconfig to force ktlint to match ktfmt:

[*.{kt,kts}]
ktlint_code_style = intellij_idea
ktlint_standard_import-ordering = disabled
ktlint_standard_trailing-comma-on-call-site = disabled
ktlint_standard_trailing-comma-on-declaration-site = enabled

For my current code, this seems to keep both tools happy.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}