Compatibility with Kotlin 2.x

[Hyunk3l]

hei team

I'm trying to use rewriteRunhttps://github.com/Hyunk3l/ecommerce in this public project I've (kotlin 2.1, java 17, etc.)

rewrite {
    activeRecipe(
        "org.openrewrite.java.OrderImports",
    )
}

I'm trying with this recipe and this is the error I get

'org.jetbrains.kotlin.fir.session.environment.AbstractProjectFileSearchScope org.jetbrains.kotlin.cli.jvm.compiler.VfsBasedProjectEnvironment.getSearchScopeByPsiFiles(java.lang.Iterable, boolean)'

I suspect it's because of Kotlin 2.1.. so I downgraded it to 2.0.20 but

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':shopping:rewriteRun'.
> 'org.jetbrains.kotlin.fir.FirSession org.jetbrains.kotlin.fir.session.FirSessionFactoryHelper.createSessionWithDependencies(org.jetbrains.kotlin.name.Name, org.jetbrains.kotlin.platform.TargetPlatform, org.jetbrains.kotlin.resolve.PlatformDependentAnalyzerServices, org.jetbrains.kotlin.fir.java.FirProjectSessionProvider, org.jetbrains.kotlin.fir.session.environment.AbstractProjectEnvironment, org.jetbrains.kotlin.config.LanguageVersionSettings, org.jetbrains.kotlin.fir.session.environment.AbstractProjectFileSearchScope, org.jetbrains.kotlin.fir.session.environment.AbstractProjectFileSearchScope, org.jetbrains.kotlin.incremental.components.LookupTracker, org.jetbrains.kotlin.incremental.components.EnumWhenTracker, org.jetbrains.kotlin.incremental.components.ImportTracker, org.jetbrains.kotlin.fir.session.IncrementalCompilationContext, java.util.List, boolean, kotlin.jvm.functions.Function1, kotlin.jvm.functions.Function1)'

Slack Message

[Hyunk3l]

BTW, I'm using Kotlin 1.9.22 now and it's working correctly

[timtebeek]

Thanks for logging the issue and posting the workaround! As also indicated via Slack definitely something to look into, with help appreciated. I imagine we'll want to support both K1 and K2, so that's be interesting.

[pettermahlen]

I'm running into similar issues when using Kotlin 1.9.24 - unfortunately, it's not realistic to couple the repo's Kotlin version to the one that openrewrite-kotlin is using. I think that, probably, openrewrite-kotlin could shade its version of the embeddable Kotlin compiler, so that it can use a different one to parse Kotlin code than the one that is used when building. The concrete issue I'm seeing is

'org.jetbrains.kotlin.fir.FirSession org.jetbrains.kotlin.fir.session.FirSessionFactoryHelper.createSessionWithDependencies(org.jetbrains.kotlin.name.Name, org.jetbrains.kotlin.platform.TargetPlatform, org.jetbrains.kotlin.resolve.PlatformDependentAnalyzerServices, org.jetbrains.kotlin.fir.java.FirProjectSessionProvider, org.jetbrains.kotlin.fir.session.environment.AbstractProjectEnvironment, org.jetbrains.kotlin.config.LanguageVersionSettings, org.jetbrains.kotlin.fir.session.environment.AbstractProjectFileSearchScope, org.jetbrains.kotlin.fir.session.environment.AbstractProjectFileSearchScope, org.jetbrains.kotlin.incremental.components.LookupTracker, org.jetbrains.kotlin.incremental.components.EnumWhenTracker, org.jetbrains.kotlin.incremental.components.ImportTracker, org.jetbrains.kotlin.fir.session.IncrementalCompilationContext, java.util.List, boolean, kotlin.jvm.functions.Function1, kotlin.jvm.functions.Function1)'
java.lang.NoSuchMethodError: 
	at org.openrewrite.kotlin.KotlinParser.parse(KotlinParser.java:461)
	at org.openrewrite.kotlin.KotlinParser.parseInputs(KotlinParser.java:171)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:282)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:132)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:127)

[timtebeek]

Thanks for the report @pettermahlen ; Surprised to see that's even an issue with the v1.9.22 we're using: https://github.com/openrewrite/rewrite-kotlin/blob/16e576e78b795ebbace46b5a78a33a1158b4227a/build.gradle.kts#L16

Feel free to bump that up to 1.9.24 if that helps you there already; then we can tackle the v2 migration later.

[pettermahlen]

The main point I was trying to make is that it's not ideal for rewrite-kotlin to require users to use a specific version of Kotlin when building their source. Instead, since rewrite-kotlin uses internal APIs that change even with minor bumps, it should be shading those. That way, openrewrite-kotlin's upgrades of the compiler APIs it's using for parsing, and each of the projects upgrades of the version used to build with can be kept separate.

It seems like it would be very awkward if the latest version of kotlin-rewrite always only worked with the latest version of Kotlin.

[timtebeek]

Oh that I fully understand and agree with; my expectation though is that the 1.9 branch will not evolve much anymore, so for now ensuring we are compatible with the now-latest v1.9.25 would mean at least folks on 1.9 can continue to use rewrite-kotlin. v2 would be a larger effort which we can then tackle separately.

• https://github.com/openrewrite/rewrite-kotlin/pull/620

[barbulescu]

Useful links to use for supporting Kotlin 2.x:

• changelog
• FIR basics
• FirExpression Source code
• Kotlin Slack

[barbulescu]

Started working on https://github.com/openrewrite/rewrite-kotlin/pull/627 - draft, still with compile errors

• https://github.com/openrewrite/rewrite-kotlin/pull/627

[idanakav]

@barbulescu I'm curious to know if there are any remaining blockers to ship 2.1.x?

[timtebeek]

Thanks for your interest @idanakav ! As also indicated on that PR we've since moved rewrite-kotlin into this repository

• https://github.com/openrewrite/rewrite-kotlin/pull/627#issuecomment-2720704989

That means the same changes as seen there should now target this repository instead; any help there welcome! :)

[barbulescu]

There are still few issues, but I need more time to understand both Kotlin FIR and OpenRewrite.

It would be great if anyone with this know-how can help here.

If not, I will continue next weekend.

[barbulescu]

I am blocked at the part where FirSession is created in KotlinParser.

I got a suggestion, but it looks like there are a lot of changes on this area.

Can anybody help here?

[timtebeek]

@greg-at-moderne would you be able to help provide some guidance here? 🙏

[greg-at-moderne]

My knowledge of Kotlin is close to zero, but I am happy to help. I've just requested to join the Kotlin Lang Slack to read the context.

[barbulescu]

I had some progress and I am trying to fix WrappingAndBracesTest:blockLevelStatements test with this failure:

java.lang.IllegalStateException: LST contains missing or invalid type information
Identifier->Unary->Block->Block->ClassDeclaration->CompilationUnit
/*~~(Identifier type is missing or malformed)~~>*/n

	at org.openrewrite.kotlin.Assertions.assertValidTypes(Assertions.java:242)
	at org.openrewrite.kotlin.Assertions.validateTypes(Assertions.java:57)

I tried to debug but I get easily lost.

Is any strategy for finding the root cause of such an issue?

[timtebeek]

Hmm, quick answer from mobile seeing this now. The way I tend to go through is to first run the rest with coverage, and then from there set breakpoints on the paths I know are hit. In this case it looks like the type is missing on this ++ operator: https://github.com/openrewrite/rewrite/blob/2b9bc08784f7f7c0e265f268b783f55daba620fb/rewrite-java-test/src/test/java/org/openrewrite/java/format/WrappingAndBracesTest.java#L56

Is there any branch where we could maybe try to have a look as well? Not likely I'll get to it before Thursday with travel, but maybe Greg will have more ideas still.

[barbulescu]

I am working on main branch of my fork.

[timtebeek]

Thanks! Feel free to open up an early draft PR; that makes it easier for folks to be aware and check out your work already.

I had a brief look here, but can't see much more beyond confirming that when we look up the type for n here it's returned as null, despite being present in psiElementAssociations. 🤔 https://github.com/openrewrite/rewrite/blob/5b5b43d311c802f6524d0f2c80b941722ff9d077/rewrite-kotlin/src/main/java/org/openrewrite/kotlin/internal/KotlinTreeParserVisitor.java#L3088-L3092

[barbulescu]

• https://github.com/openrewrite/rewrite/pull/5575

[barbulescu]

FirRegularClassSymbol seems to not be handled in KotlinTypeSignatureBuilder:signature, but not clear yet how to handle.

Cannot find ConeStubTypeForChainInference anymore so not sure how to handle this either.

[timtebeek]

Linking the no-login-or-slack-required link to the Kotlin lang Slack here as well: https://slack-chats.kotlinlang.org/t/28493795/what-is-the-recommended-way-to-migrate-firsessionfactoryhelp

What is the recommended way to migrate FirSessionFactoryHelper.INSTANCE.createSessionWithDependencies when moving to Kotlin +2.1? The usage is in Openrewrite https://github.com/openrewrite/rewrite/blob/main/rewrite-kotlin/src/main/java/org/openrewrite/kotlin/KotlinParser.java#L475

Most of production methods of session creation were migrated to the unified SessionConstructionUtils.prepareSessions utility. The same file contains methods like prepareJsSessions and prepareNativeSessions for specific platforms. Proper prepareJvmSessions is declared inside JvmFrontendPipelinePhase and I expect that utilities for other platforms would be eventually moved to similar phases too in the future

[barbulescu]

Solved the ++ operator handling and next is adding support for FirResolvedArgumentList.

29 more tests to fix.

[barbulescu]

I get many failures like this one:

org.opentest4j.AssertionFailedError: [When parsing and printing the source code back to text without modifications, the printed source didn't match the original source code. This means there is a bug in the parser implementation itself. Please open an issue to report this, providing a sample of the code that generated this error. "openRewriteFile.kt":
diff --git a/openRewriteFile.kt b/openRewriteFile.kt
index cbf907e..8ce565a 100644
--- a/openRewriteFile.kt
+++ b/openRewriteFile.kt
@@ -1,2 +1,2 @@ 
 val String .  extension   :   Any
-    get (  )  =  ""
\ No newline at end of file
+    get(  )  =  ""
\ No newline at end of file
] 
expected: 
  "val String .  extension   :   Any
      get (  )  =  """
 but was: 
  "val String .  extension   :   Any
      get(  )  =  """

Any idea where in parsing I should look for the issue?

[timtebeek]

Hi @barbulescu ; copying a link to the (likely) respective test here: https://github.com/openrewrite/rewrite/blob/4fa4491a4b030488c3f0aa7289a1219ac6c32614/rewrite-kotlin/src/test/java/org/openrewrite/kotlin/format/SpacesTest.java#L2748-L2762

If I change that test slightly I can jump in with the debugger to explore how that's defined on your branch:

    @Test
    void extensionProperty() {
        rewriteRun(
          kotlin(
            """
              val String .  extension   :   Any
                  get(  )  =  ""
              """,
            """
              val String.extension: Any
                  get() =  ""
              """,
            spec -> spec
              .beforeRecipe(cu -> {
                TreeVisitingPrinter.printTreeAll(cu); // Set a breakpoint here
              })
              .afterRecipe(cu -> {
                TreeVisitingPrinter.printTreeAll(cu);
            })
          )
        );
    }

Your branch

Main branch

Difference

If you compare the two closely, you'll find that the MethodDeclaration field parameters is a JContainer in both, but only on the main branch has a before for a single space, whereas on your branch that's empty. That then leads to the print inequality on the test input, and a failure to start running the associated test recipe. I hope that helps!

[barbulescu]

beforeRecipe is not called for me because assertContentEquals fails before that. Is any other way to see the tree before this assertion fails?

[timtebeek]

beforeRecipe is not called for me because assertContentEquals fails before that. Is any other way to see the tree before this assertion fails?

Hi @barbulescu ; that's a bit awkward indeed; while you could set a breakpoint and inspect the value in a debugger, perhaps this would be the more sympathetic change in the framework to allow you get inspect the value before it's rejected by validation:

• https://github.com/openrewrite/rewrite/pull/5888

Let's see if my colleagues agree, as I've had this desire in the past as well to inspect the LST elements, as opposed to just their text.