smithy-lang / smithy-rs

Code generation for the AWS SDK for Rust, as well as server and generic smithy client generation.
Apache License 2.0
489 stars 185 forks source link

Ktlint precommit is not compatible with JDK 17 #1516

Open crisidev opened 2 years ago

crisidev commented 2 years ago

We use https://github.com/macisamuele/language-formatters-pre-commit-hooks with pre-commit to autoformat code during commit.

Our config is using revision v1.6.1 of this hook, which is failing with JDK 17 with the error

Exception in thread "main" java.util.concurrent.ExecutionException: java.lang.NoClassDefFoundError: Could not initialize class org.jetbrains.kotlin.com.intellij.openapi.util.objectTree.ThrowableInterner

After updating macisamuele/language-formatters-pre-commit-hooks to available version (v2.3.0), the error become more clear:

openjdk version "17.0.3" 2022-04-19 LTS
OpenJDK Runtime Environment Corretto-17.0.3.6.1 (build 17.0.3+6-LTS)
OpenJDK 64-Bit Server VM Corretto-17.0.3.6.1 (build 17.0.3+6-LTS, mixed mode, sharing)

Traceback (most recent call last):
...
language_formatters_pre_commit_hooks.pre_conditions.ToolNotInstalled: JRE: version < 16.0 is required to run this pre-commit hook.
Make sure that you have it installed and available on your path.
Download/Install URL: https://www.java.com/en/download/
rcoh commented 2 years ago

can we manually set the env of the pre-commit hook to be something like JAVA_HOME=<java 11> <run hook>?

jdisanti commented 2 years ago

Ktlint (version 0.46 at time of writing) technically works on JDK 16+ according to its own changelog, but it looks like it requires finicky JVM args in order to do so. For example, the Ktlint CLI automatically adds --add-opens java.base/java.lang=ALL-UNNAMED if it detects Java 16+.

The pre-commit hook has no way of adding JVM args, and even explicitly prohibits JDK 16+ in the most recent versions. I decided to play around with making pre-commit call into Gradle to run Ktlint instead of using the hook:

.pre-commit.config.yaml:

- repo: local
  hooks:
  # ...
  - id: ktlint
    name: Ktlint
    entry: ./.pre-commit-hooks/ktlint.py
    language: python
    files: ^.*\.kt$

.pre-commit-hooks/ktlint.py:

import os
import sys
import subprocess

# Runs a shell command
def run(command):
    subprocess.run(command, stdout=sys.stderr, stderr=sys.stderr, shell=False, check=True)

# Returns the output from a shell command. Bails if the command failed
def get_cmd_output(command):
    result = subprocess.run(command, capture_output=True, shell=False, check=True)
    return result.stdout.decode("utf-8").strip()

def main():
    repo_root = get_cmd_output(["git", "rev-parse", "--show-toplevel"])
    os.chdir(repo_root)

    file_list = ':'.join(sys.argv[1:])
    run(["./gradlew", "ktlintPreCommit", f"-Pktlint.filelist={file_list}"])

if __name__ == "__main__":
    main()

build.gradle.kts:

tasks.register<JavaExec>("ktlintPreCommit") {
    description = "Run by pre-commit to check files"
    group = "formatting"
    classpath = configurations.getByName("ktlint")
    main = "com.pinterest.ktlint.Main"
    // Fixes Ktlint for Java 16+: https://github.com/pinterest/ktlint/issues/1195#issuecomment-1009027802
    jvmArgs = listOf("--add-opens=java.base/java.lang=ALL-UNNAMED")
    args = listOf("-F") + project.property("ktlint.filelist")!!.toString().split(":")
}

This got me passed the NoClassDefFoundError, but ultimately I encountered something new:

Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private transient java.lang.Object java.lang.Throwable.backtrace accessible: module java.base does not "opens java.lang" to unnamed module @d310a75

It seems Ktlint still doesn't actually work on JDK 17, even with the JVM args hack. When that gets fixed, then we can revisit this approach if the pre-commit hook is still not updated.

can we manually set the env of the pre-commit hook to be something like JAVA_HOME=<java 11> <run hook>?

The pre-commit hook documents how to work with multiple JDK versions to work around this, but I don't like the additional setup that is required for it. There's also a way to run it in Docker, but then installing Docker is required to run Ktlint.