K2 plugin migration #321

akozlova · 2024-10-01T19:40:04Z

Evident usages of K1 api (resolve) are migrated

sschuberth · 2024-10-07T14:54:42Z

build.gradle.kts

+//tasks.named<RunIdeTask>("runIde") {
+//   jvmArgumentProviders += CommandLineArgumentProvider {
+//      listOf("-Didea.kotlin.plugin.use.k2=true")
+//   }
+//}
+
+//tasks.test {
+//   jvmArgumentProviders += CommandLineArgumentProvider {
+//      listOf("-Didea.kotlin.plugin.use.k2=true")
+//   }
+//}
+


I believe we should not commit out-commented code.

Without vm options, tests and debug IDE would be run against K1 mode. It's ok for 242 and below but for the 243+, it should be changed to something else. It's better to separate migration of the plugin and migration of the test infra indeed. Hopefully, it would be easy for the maintainers to find corresponding options without the comments.

sschuberth · 2024-10-07T14:55:35Z

src/test/kotlin/io/kotest/plugin/intellij/psi/SpecTests.kt

@@ -21,51 +21,6 @@ class SpecTests : LightJavaCodeInsightFixtureTestCase() {
      return false
   }

-   fun testIsContainedInSpecFunSpec() {


Instead of merging `master´, rebase to keep the PR's history clean of merge commits for more convenient review.

- starting from 242, the plugin works over AnalysisAPI which hides all details about kotlin engine used inside kotlin plugin itself

sschuberth · 2024-10-17T06:17:09Z

@akozlova, could you please have a look at the test failures? There seems to be some Java version 17 vs 11 mismatch.

pschyska · 2024-10-17T14:27:13Z

Thanks for your work @akozlova!

i just wanted to report that I built the plugin locally (I commented everything but IC-242, because that's what I'm using, and also added kotlin.stdlib.default.dependency = false to gradle.properties because there was a warning about it. Not sure if that was necessary though).
It seems to work fine with IU-242.23339.11 in K2 Mode!
Incidentally, I got the same error about IC-243 as in the last pipeline run before I commented it, so this issue is probably real.

akozlova · 2024-10-17T14:39:21Z

I am sorry, which warning? Unfortunately I see compilation errors due to 2.1.0 used to compile new 243 builds and I can't fix it locally for now :/

pschyska · 2024-10-17T15:08:05Z

The warning was:

[org.jetbrains.intellij.platform] The following plugin configuration issues were found:
- The dependency on the Kotlin Standard Library (stdlib) is automatically added when using the Gradle Kotlin plugin and may conflict with the version provided with the IntelliJ Platform, see: https://jb.gg/intellij-platform-kotlin-stdlib

The given link mentions to add kotlin.stdlib.default.dependency = false to gradle.properties, which makes the warning go away.
It is unrelated to the 243 issue I believe.

pschyska · 2024-10-17T15:59:00Z

With the following changes I could build 242 and 243 locally:

From 9cb825461f8848c0820c8341c07b1e179ecaa347 Mon Sep 17 00:00:00 2001
From: Paul Schyska <[email protected]>
Date: Thu, 17 Oct 2024 17:52:25 +0200
Subject: [PATCH] Bump kotlin to 2.0.21, don't ignore kotlinx-coroutines
 dependencies

---
 build.gradle.kts          | 10 +++++-----
 gradle.properties         |  2 ++
 gradle/libs.versions.toml |  2 +-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/build.gradle.kts b/build.gradle.kts
index 19e28da..933ce47 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -172,11 +172,11 @@ dependencies {
 }

 configurations.all {
-   exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-core-jvm")
-   exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-core")
-   exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-jdk8")
-   exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-test")
-   exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-test-jvm")
+   // exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-core-jvm")
+   // exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-core")
+   // exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-jdk8")
+   // exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-test")
+   // exclude(group = "org.jetbrains.kotlinx", module = "kotlinx-coroutines-test-jvm")
 }

 sourceSets {
diff --git a/gradle.properties b/gradle.properties
index b571e4f..f9f6c6b 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -6,3 +6,5 @@ version=1.1.0

 org.gradle.caching=true
 sandbox = /tmp/
+
+kotlin.stdlib.default.dependency = false
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
index 550808d..d395689 100644
--- a/gradle/libs.versions.toml
+++ b/gradle/libs.versions.toml
@@ -1,5 +1,5 @@
 [versions]
-kotlin = "1.9.25"
+kotlin = "2.0.21"
 runtime-kotest = "4.2.0"
 # We separate these from the actual runtime dependencies
 test-kotest = "5.9.1"
--
2.46.1

This updates kotlin to 2.0.21, and un-excludes kotlinx-coroutines dependencies. I don't know why they were excluded in the first place though, so this might wrong.
I then ran:

PRODUCT_NAME=IC-243 JVM_TARGET=21 ./gradlew buildPlugin
PRODUCT_NAME=IC-242 JVM_TARGET=21 ./gradlew buildPlugin

and it built.

I don't have any intellij plugin experience, but maybe this helps you 😄

akozlova · 2024-10-17T19:11:08Z

Exclusion was done to avoid conflicts on runtime during running tests. 243 bundles patched version of coroutines and then it leads to exceptions like

ERROR: 'java.lang.Object kotlinx.coroutines.BuildersKt.runBlockingWithParallelismCompensation(kotlin.coroutines.CoroutineContext, kotlin.jvm.functions.Function2)'
java.lang.NoSuchMethodError: 'java.lang.Object kotlinx.coroutines.BuildersKt.runBlockingWithParallelismCompensation(kotlin.coroutines.CoroutineContext, kotlin.jvm.functions.Function2)'
	at kotlinx.coroutines.internal.intellij.IntellijCoroutines.runBlockingWithParallelismCompensation(intellij.kt:41)
	at com.intellij.openapi.progress.CoroutinesKt.runBlockingCancellable$lambda$0(coroutines.kt:146)
	at com.intellij.openapi.progress.ContextKt.prepareThreadContext(context.kt:85)
	at com.intellij.openapi.progress.CoroutinesKt.runBlockingCancellable(coroutines.kt:139)
	at com.intellij.openapi.progress.CoroutinesKt.runBlockingMaybeCancellable(coroutines.kt:175)
	at com.intellij.openapi.project.DumbServiceImpl.ensureInitialDumbTaskRequiredForSmartModeSubmitted(DumbServiceImpl.kt:702)
	at com.intellij.testFramework.IndexingTestUtil.shouldWait(IndexingTestUtil.kt:94)

But the idea is actually good, many thanks! I think, just excluding on runtime should do the trick.

sschuberth · 2024-10-25T08:21:58Z

@sksamuel or @Kantis could you please have a look as well? I'm not familiar with the IntelliJ plugin API.

sksamuel · 2024-10-25T22:39:43Z

gradle/libs.versions.toml

@@ -1,5 +1,5 @@
 [versions]
-kotlin = "1.9.25"
+kotlin = "2.0.21"


How does this impact users or does it not?

It's impossible to use 1.9.x version with kotlin jars from 243 platform anymore, because they were compiled with 2.1 already. Unfortunately, I think, I missed one version indeed: it seems, 223 was build with 1.8 and it won't be compatible with 2.0. Can we drop 223 or do I need to provide logic to make the version dependant on the platform version?

We can drop 223

Then there should be no problems with later versions

sksamuel · 2024-10-25T22:49:39Z

src/IC-243/kotlin/io/kotest/plugin/intellij/psi/superClasses.kt

+      .flatMap { ref ->
+         // SurroundSelectionWithFunctionIntention.isAvailable is called in EDT before the intention is applied
+         // unfortunately API to avoid this was introduced in 23.2 only
+         // this we need to move intentions to the facade or accept EDT here until 23.2- are still supported


can you clarify this comment, it doesn't quite make sense to me (about the intention)

It's the comment on allowing resolve on EDT which is prohibited, because it might be slow and lead to freezes of the whole IDE. It's possible to avoid this suppression if we would rewrite SurroundSelectionWithFunctionIntention but the API doesn't exist for older versions and we would need to have it in facade. So I decided to wait until you drop older versions and currently do nothing about it.

sksamuel · 2024-10-25T22:53:15Z

This updates kotlin to 2.0.21, and un-excludes kotlinx-coroutines dependencies. I don't know why they were excluded in the first place though, so this might wrong. I then ran:
PRODUCT_NAME=IC-243 JVM_TARGET=21 ./gradlew buildPlugin
PRODUCT_NAME=IC-242 JVM_TARGET=21 ./gradlew buildPlugin
and it built.

I don't have any intellij plugin experience, but maybe this helps you 😄

They were excluded because otherwise there's a clash (or was, maybe excluding kotlin itself avoids that).

akozlova mentioned this pull request Oct 7, 2024

K2 compiler compatibility #289

Open

sschuberth requested review from sksamuel and Kantis October 7, 2024 14:53

sschuberth reviewed Oct 7, 2024

View reviewed changes

akozlova closed this Oct 8, 2024

akozlova force-pushed the master branch from b7ef9f6 to 123d6d9 Compare October 8, 2024 06:06

akozlova added 2 commits October 8, 2024 08:06

update current version to 2024.2.2 to perform K2 migration

350daec

make plugin ready for K2 mode

624c387

- starting from 242, the plugin works over AnalysisAPI which hides all details about kotlin engine used inside kotlin plugin itself

akozlova reopened this Oct 8, 2024

fix 223 compatibility

bda4707

fix 243 compatibility

d74eec4

sksamuel reviewed Oct 25, 2024

View reviewed changes

sksamuel approved these changes Oct 25, 2024

View reviewed changes

Removed 223 build

2dde0d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

K2 plugin migration #321

K2 plugin migration #321

akozlova commented Oct 1, 2024 •

edited

Loading

sschuberth Oct 7, 2024

akozlova Oct 7, 2024

sschuberth Oct 7, 2024

sschuberth commented Oct 17, 2024

pschyska commented Oct 17, 2024

akozlova commented Oct 17, 2024

pschyska commented Oct 17, 2024

pschyska commented Oct 17, 2024

akozlova commented Oct 17, 2024 •

edited

Loading

sschuberth commented Oct 25, 2024

sksamuel Oct 25, 2024

akozlova Oct 28, 2024

sksamuel Oct 29, 2024

akozlova Oct 31, 2024

sksamuel Oct 25, 2024

akozlova Oct 28, 2024 •

edited

Loading

sksamuel commented Oct 25, 2024

K2 plugin migration #321

Are you sure you want to change the base?

K2 plugin migration #321

Conversation

akozlova commented Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sschuberth commented Oct 17, 2024

pschyska commented Oct 17, 2024

akozlova commented Oct 17, 2024

pschyska commented Oct 17, 2024

pschyska commented Oct 17, 2024

akozlova commented Oct 17, 2024 • edited Loading

sschuberth commented Oct 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akozlova Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

sksamuel commented Oct 25, 2024

akozlova commented Oct 1, 2024 •

edited

Loading

akozlova commented Oct 17, 2024 •

edited

Loading

akozlova Oct 28, 2024 •

edited

Loading