Skip to content
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

Move from KtFile to FirFile #7404

Open
BraisGabin opened this issue Jun 26, 2024 · 4 comments
Open

Move from KtFile to FirFile #7404

BraisGabin opened this issue Jun 26, 2024 · 4 comments

Comments

@BraisGabin
Copy link
Member

This change is going to be painful. The new compiler K2 comes with a new frontend: FIR. 2.0 still supports PSI but we should start thinking about moving from PSI to FIR.

We should find a way to transition from one to another. I see two main chunks of work:

  1. The core, all the detekt engine that make the rules work.
  2. The rules.

It will be great if we can change all the core without touching the rules, and it seems that it can be possible. FirElement has a property called psi that should help us with this. Later, once we have the core stable would be the time to move the rules from PSI to FIR. That seems like A LOT of work so probably we will need to ask help to the community to do it. For that reason I think that we should have our core kind of ready and the tests in place so a contributor can focus on the translation of the rules.

I didn't check too much but I think that we are going to be lucky and we will not need to change our rule tests that much.

I'm working on this but any help is more than welcome.

@3flex
Copy link
Member

3flex commented Jun 26, 2024

Take a look at the Kotlin analysis API if you haven't yet - this might be an option, but the analysis API looks like the path forward with the most longevity.

It should support both the old and new front-end from what I could gather - which is also important to make sure we support backwards compatibility with older language versions.

@3flex
Copy link
Member

3flex commented Jun 27, 2024

Analysis API readme: https://github.com/JetBrains/kotlin/blob/master/docs/analysis/analysis-api/analysis-api.md

There's an implementation for both the old and new compiler front ends.

We'd likely integrate with the "standalone" module for the CLI (I believe this is what KSP is doing for KSP2).

Also important to note is that the Kotlin plugin for IDEA is using this API. This should make IDEA inspections simple to port if we use this, and also guarantees that we can offer analysis capabilities that are equal to what the Kotlin IDEA plugin offers, i.e. fully comprehensive and taking care of things that are a bit tricky or hacky in detekt at the moment like dealing with smart casts.

And the analyze function that's the entry point actually accepts a KtElement, so moving away from PSI would make that harder to use.

@3flex
Copy link
Member

3flex commented Sep 4, 2024

https://kotlin.github.io/analysis-api/migrating-from-k1.html

I do wonder if bindingContext will become unavailable to detekt in future Kotlin releases... I'm actually a bit surprised everything still seems to work!

@3flex
Copy link
Member

3flex commented Sep 20, 2024

@nebffa has expressed interest in uplifting compiler plugin support for K2. Before any work starts we should discuss the approach a bit further.

Analysis API has a key limitation: it does not support compiler plugins at this stage. I'm not sure if it ever will. Based on the definitions of different components used by Analysis API, this is where the different components fit:

  • Environment: CLI
  • Platform: Current platforms are IntelliJ Kotlin plugin and the Standalone Analysis API. This is where support for compiler plugin is lacking - there's no platform available for compiler plugins, at least as far as I can tell. detekt will need to use the Standalone Analysis API for core support via CLI & Gradle plugin. The detekt IntelliJ plugin can probably go a different direction and start integrating directly with the IntelliJ Kotlin platform once rules have been updated to use the Analysis API.
  • Engine: provided by the platform
  • Platform interface: defined in Kotlin's analysis-api-platform-interface
  • User: detekt, utilising the API provided by Kotlin's analysis-api module.

There's a good document with background on adding K2 support to KSP in KSP2: https://github.com/google/ksp/blob/2.0.20-1.0.25/docs/ksp2.md.

Key points for KSP2:

  • it uses Standalone Analysis API. It's no longer a compiler plugin.
  • Because it uses the existing compiler plugin, KSP1 will not support Kotlin 2.1. We're in the same position currently as KSP1 - our compiler plugin should work when using language version 1.9 on Kotlin 2.0 code because Kotlin 2.0 didn't introduce new language features. But that is changing quickly. I'm also not even sure if the rest of detekt will work with Kotlin 2.1 code... keep an eye on Kotlin 2.1.0-RC #7649! We might have a large problem to quickly solve :D

The alternative is perhaps using FIR as originally proposed. @BraisGabin has done some work on this already. JetBrains is pushing the Analysis API for the detekt use case so I still believe it's the right path forward but it does have limitations that we'll need to accept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants