fwcd / fwcd/kotlin-language-server

[Deps] Investigate ways of integrating `ktfmt`

Open
#338 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

formatting
Dominant language
Kotlin
Stars
2k
Forks
252
PR merge metrics
No merged PRs in 30d

Description

We're vendoring `ktfmt` ([here](https://github.com/fwcd/ktfmt/tree/kls)) since it uses the embedded Kotlin compiler rather than the standard one, which we use. The embedded Kotlin compiler uses other package paths (e.g. `org.jetbrains.kotlin.com.intellij` instead of `com.intellij`), which causes weird clashes at runtime, for example:

```
java.lang.NoSuchMethodError: 'org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment$Companion.createForProduction(org.jetbrains.kotlin.com.intellij.openapi.Disposable, org.jetbrains.kotlin.config.CompilerConfiguration, org.jetbrains.kotlin.cli.jvm.compiler.EnvironmentConfigFiles)'
at com.facebook.ktfmt.format.Parser.parse(Parser.kt:45)
at com.facebook.ktfmt.format.Formatter.sortedAndDistinctImports(Formatter.kt:141)
at com.facebook.ktfmt.format.Formatter.format(Formatter.kt:84)
at org.javacs.kt.formatting.FormatterKt.formatKotlinCode(Formatter.kt:10)
at org.javacs.kt.KotlinTextDocumentService$formatting$1.invoke(KotlinTextDocumentService.kt:204)
at org.javacs.kt.KotlinTextDocumentService$formatting$1.invoke(KotlinTextDocumentService.kt:199)
at org.javacs.kt.util.AsyncExecutor.compute$lambda-2(AsyncExecutor.kt:19)
at java.base/java.util.concurrent.CompletableFuture$AsyncSupply.run(CompletableFuture.java:1700)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:829)
```

While this is non-ideal, it's currently the best solution I could find that works for our setup. A better solution would be to automate this as part of our Gradle build (i.e. rewrite the package paths and replace the transitive `kotlin-compiler-embeddable` dependency with `kotlin-compiler` - basically do [this](https://github.com/facebookincubator/ktfmt/compare/main...fwcd:kls) automatically), but I couldn't figure out a clean way of doing so yet.

_Originally posted by @fwcd in https://github.com/fwcd/kotlin-language-server/issues/330#issuecomment-1072499406_

---

... there were other dependencies which needed the non-embedded compiler and didn't have an embedded version themselves (I think it was the `kotlin-scripting-compiler` or perhaps the IDEA plugin, which we no longer depend on though, can't remember).

_Originally posted by @fwcd in https://github.com/fwcd/kotlin-language-server/issues/330#issuecomment-1073071615_

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Gradle build and the linked ktfmt fork comparison; trace Parser.kt and Formatter.kt where the runtime clash appears. Investigate replacing kotlin-compiler-embeddable with kotlin-compiler and the package-path rewrite, including the unresolved non-embedded dependencies. Done means the integration avoids the reported NoSuchMethodError.

Written by the indexing model from the issue text.

Assessment

Tech stack
kotlin
Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.