Skip to content

Compatibility with Kotlin 2.x #5128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Hyunk3l opened this issue Dec 6, 2024 — with Slack · 20 comments
Open

Compatibility with Kotlin 2.x #5128

Hyunk3l opened this issue Dec 6, 2024 — with Slack · 20 comments
Labels
bug Something isn't working dependencies enhancement New feature or request parser-kotlin

Comments

Copy link

Hyunk3l commented Dec 6, 2024

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
Copy link
Author

Hyunk3l commented Dec 6, 2024

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

@timtebeek timtebeek added bug Something isn't working enhancement New feature or request dependencies labels Dec 7, 2024
@timtebeek timtebeek moved this to Backlog in OpenRewrite Dec 7, 2024
@timtebeek
Copy link
Member

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
Copy link

pettermahlen commented Dec 12, 2024

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
Copy link
Member

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
Copy link

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
Copy link
Member

timtebeek commented Dec 13, 2024

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.

timtebeek referenced this issue in openrewrite/rewrite-kotlin Dec 13, 2024
As discussed on #618
timtebeek referenced this issue in openrewrite/rewrite-kotlin Dec 13, 2024
As discussed on #618
@barbulescu
Copy link
Contributor

barbulescu commented Jan 18, 2025

Useful links to use for supporting Kotlin 2.x:

@barbulescu
Copy link
Contributor

barbulescu commented Jan 18, 2025

Started working on openrewrite/rewrite-kotlin#627 - draft, still with compile errors

@knutwannheden knutwannheden transferred this issue from openrewrite/rewrite-kotlin Mar 5, 2025
@idanakav
Copy link

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

@timtebeek
Copy link
Member

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

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

@timtebeek timtebeek moved this to Backlog in OpenRewrite Mar 13, 2025
@barbulescu
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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

@greg-at-moderne
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

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:

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
Copy link
Contributor

I am working on main branch of my fork.

@timtebeek
Copy link
Member

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. 🤔

public J visitSimpleNameExpression(KtSimpleNameExpression expression, ExecutionContext data) {
// The correct type cannot consistently be associated to the expression due to the relationship between the PSI and FIR.
// The parent tree should use the associated FIR to fix mis-mapped types.
// I.E. MethodInvocations, MethodDeclarations, VariableNames should be set manually through `createIdentifier(psi, type)`
return createIdentifier(expression, type(expression));

@barbulescu
Copy link
Contributor

barbulescu commented Jun 8, 2025

@barbulescu
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies enhancement New feature or request parser-kotlin
Projects
Status: Backlog
Development

No branches or pull requests

6 participants