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

[Kotlin] Migrate kapt to ksp #3284

Open
choweiyuan opened this issue Jun 10, 2022 · 27 comments
Open

[Kotlin] Migrate kapt to ksp #3284

choweiyuan opened this issue Jun 10, 2022 · 27 comments

Comments

@choweiyuan
Copy link

Why the new feature should be added

kapt is in maintenance mode, should move away from deprecated plugin - https://kotlinlang.org/docs/kapt.html

kapt is in maintenance mode. We are keeping it up-to-date with recent Kotlin and Java releases but have no plans to implement new features. Please use the [Kotlin Symbol Processing API (KSP)](https://kotlinlang.org/docs/ksp-overview.html) for annotation processing. [See the list of libraries supported by KSP](https://kotlinlang.org/docs/ksp-overview.html#supported-libraries).

Comparison/Benefits:
https://kotlinlang.org/docs/ksp-why-ksp.html#comparison-to-kapt

How the new feature should work

queryDSL kotlin is currently using kapt to generate the code, this feature will utilise KSP to generate code instead. Might have performance gain there too.

@IceBlizz6
Copy link

tschuchortdev/kotlin-compile-testing#302 (comment)

Kapt is not compatible with K2 and I'm not sure it ever will be

For those that want to use the new Kotlin compiler it seems like we may be out of luck until this is solved.

@choweiyuan
Copy link
Author

tschuchortdev/kotlin-compile-testing#302 (comment)

Kapt is not compatible with K2 and I'm not sure it ever will be

For those that want to use the new Kotlin compiler it seems like we may be out of luck until this is solved.

https://blog.jetbrains.com/kotlin/2023/02/k2-kotlin-2-0/ looks like K2 will be stable in Kotlin 2.0! We'll have a problem when Kotlin 2.0 is released?

@spyro2000
Copy link

The current state is very worrying indeed. KASP was 'just' very slow and throw constant warnings with QueryDSL, but with K2 it will appearently stop working alltogether. The problem might be, that the QueryDSL team is not willing to make any Kotlin-specific changes, with is perfectly understandable. But what is the strategy for dealing with this situation? Is there any?

@IceBlizz6
Copy link

IceBlizz6 commented Jun 23, 2023

I can mentioned that i did end up writing a very short and simple QueryDSL symbol processor.
It was just to cover my needs so it only supports jakarta annotations and there are some differences like it doesn't output separate super classes.
I probably won't publish that, but writing it didn't seem very challenging.

https://github.com/querydsl/querydsl/tree/master/querydsl-kotlin-codegen/src/main/kotlin/com/querydsl/kotlin/codegen
The QueryDSL codebase already have kotlin specific things like this code generator for kotlin Q-classes.
So them adding a symbol processor doesn't seem very far fetched.

I wanted to see if i could try to contribute with my KSP skills.
after looking over and tinkering with QueryDSL codebase a few times i eventually gave up.
The codebase is absolutely massive and very difficult for me to wrap my head around.

I don't know if i am the right person to write a feature-complete processor for QueryDSL,
my knowledge about this project is not good enough to write something feature complete at the moment.
But at first glance the task doesn't seem out of reach.

@spyro2000
Copy link

Ok, then we have a real problem here and should eventually really move away from QueryDSL. Looks like a dead-end. Was hoping, that KAPT -> KSP would be straight forward but if its that hard, than that's basically it. :(

@IceBlizz6
Copy link

@jwgmeligmeyling
What's your take on this?

Do you have anyone that could write a kotlin symbol processor?

My vacation is coming up so i will have some time to spare, If i were to make an attempt at this then is there any place where i could get a full overview over all the features that i would need to support?

@jwgmeligmeyling
Copy link
Member

Ok, then we have a real problem here and should eventually really move away from QueryDSL. Looks like a dead-end.

You actually don’t need Q classes to use Querydsl (for example use query aliases) nor you have to rely on APT to generate Q classes for you.

Do you have anyone that could write a kotlin symbol processor?

No, haven’t had the need for it yet. I don’t use Kotlin in my data layers personally.

I don't know if i am the right person to write a feature-complete processor for QueryDSL

Codegen is well unit tested. I think you can just migrate the example domain types and expected generated code to kotlin and develop this in a TDD way. I am confident you’ll be very complete by the end of it 😄

Just having access to the classname, attribute names and types should get you pretty far. Its only with things like custom annotations (like QueryDelegate or QueryEntities) where things get a bit interesting. Mostly because here configuration from other source files affects the output of conversion from other source files, meaning special attention is needed for parallelization and partial compilation.

Copy link

stale bot commented Jun 25, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 25, 2024
@xlaussel
Copy link

Hi,
I also use QueryDsl with Kapt for QClasses generation with Kotlin 2.0.0.
It is still workinq because Kapt is in maintenance mode ( https://kotlinlang.org/docs/kapt.html ) but it would be better to migrate du Ksp for long term.

@stale stale bot removed the wontfix label Jun 26, 2024
@IceBlizz6
Copy link

Hey
I actually did end up writing a KSP module for this, we have been using it for some time already.
But it's mostly written to fit into our current project.
If other people are interested in this then maybe i could try to write a version that would be used by the wider public.

@xlaussel
Copy link

Hey I actually did end up writing a KSP module for this, we have been using it for some time already. But it's mostly written to fit into our current project. If other people are interested in this then maybe i could try to write a version that would be used by the wider public.

What is the estimated amount of work?

@IceBlizz6
Copy link

IceBlizz6 commented Jun 28, 2024

What is the estimated amount of work?

What i currently have is a KSP project that finds all classes annotated with Entity and writes a Q class.
Simply iterates through properties, if the property is an int then it writes out a NumberPath property in the Q class.
This worked really well but it's obviously a simplification because it doesn't try to use all the existing configuration that is available in QueryDSL project. It also only supports jakarta.

I think that for something that will be used by more people i will need to support the existing available configurations in QueryDSL.

I think the most viable approach here may be to copy querydsl-apt module and write something as close as possible to this implementation with ksp.
Basically just creating a querydsl-ksp module. Trying to keep everything as similar as possible to querydsl-apt.

If that becomes very time consuming or a lot more challenging that expected then i may opt for a simplified version that i write from scratch with more limited support and has its own set of configurations similar to what i'm using today.

I'm spending the next few days to look into this now.
I'll write an update here once i know more.

@IceBlizz6
Copy link

Reached the first milestone now.
I have a working symbol processor, the code is trying to replicate the functionality of querydsl-apt.
It uses CodegenModule like APT so it picks up configurations from querydsl-kotlin-codegen dependency.
It may crash if other extensions try to render the code, the CodegenModule does not contain things like ProcessingEnvironment and RoundEnvironment as these are only available from APT.

I made an attempt to directly translate that code into Kotlin and replace APT stuff with KSP stuff, That attempt was abandoned for 3 reasons.

  1. A lot of the code in querydsl-apt depends on Element classes from javax which is only available when using APT
  2. Some configuration options does not seem to make much sense for Kotlin, like "useFields" and "useGetters"
  3. Some of the code seemed very obtuse and needlessly complicated

So i wrote big chunks of the logic in querydsl-apt from scratch.
There are a few more things to fix before this could be released. Currently it only supports about half of the configuration options so i still need to add support for the rest.

@jwgmeligmeyling
I hoped you would be so kind as to answer a few questions here.

Could this be submitted as a pull request?
It would essentially be a new module next to querydsl-apt likely named querydsl-ksp that would give users an alternative to apt.
If yes then there are a few follow up questions:

  1. How to deal with javax vs jakarta? Does it need to support both?
  2. Any requirements for test coverage?
  3. Users will probably need a guide on how to configure this, in what file would i put that?
  4. com.querydsl.codegen.utils.model.Types has some variations that are suffixed with "_P", do these make any difference to the generated code?
  5. How does extension types work? what is their purpose?
    (found in com.querydsl.apt.Context field named"extensionTypes")

@alzamon
Copy link

alzamon commented Sep 15, 2024

Is there any movement in this issue? Or is there another issue or alternative solution?

I'm on a project that is delaying Kotlin k2 upgrade because of this.

Using querydsl with spring-data-rest for automatic dynamic filter queries.

@IceBlizz6
Copy link

Is there any movement in this issue? Or is there another issue or alternative solution?

I'm on a project that is delaying Kotlin k2 upgrade because of this.

Using querydsl with spring-data-rest for automatic dynamic filter queries.

Hey, thanks for the message.
It's good to know that people are still interested.
Admittedly my project got a bit buried after i didn't get any response to my last message from any of the maintainers and i got some other things i had to look into.

I unburied the project now so i will take some time next week to try to fix it up into a state where it could be published.
I am not getting response from any of the maintainers here so i think i'll probably publish this on my own personal github account.
I will link that here when it's ready.

As i mentioned in my last message there is some challenges when it comes to porting this.
The supported settings will likely be limited and i'm not sure if i can guarantee that it will behave exactly like how QueryDSL is generating code today.

Luckily my project will only deal with generating code so it should be easy to spot and report issues.

If you are using settings from QueryDSL for the purpose of generating code then please list them here.
I will try to ensure that they are available (if applicable to Kotlin code).

@xlaussel
Copy link

Hey, thanks for the message. It's good to know that people are still interested. Admittedly my project got a bit buried after i didn't get any response to my last message from any of the maintainers and i got some other things i had to look into.

Hello,
@IceBlizz6, I am still interested to, thank you for the work.

@Almazon, I use the actual querydsl-apt with kotlin k2
I have the following message : Kapt currently doesn't support language version 2.0+. Falling back to 1.9.
But it works for the moment, I suppose that it is because there are no notable changes in the language grammar himself.

@spyro2000
Copy link

Stupid question maybe, but if I generate Java Q-classes instead, are they any expected issues to use them from a Kotlin project? Does QueryDSL use any Kotlin enhancement (like direct support of lambdas etc.?)

@cswan-log
Copy link

@IceBlizz6 Nice to see someone tackling the problem!

Since this project is kind of inactive (or at least slow in movement), you should try to integrate the KSP handling to the OpenFeign fork instead. The development there is more than active.

@IceBlizz6
Copy link

Since this project is kind of inactive (or at least slow in movement), you should try to integrate the KSP handling to the OpenFeign fork instead. The development there is more than active.

Thank you for the suggestion.

i found that project here:
https://github.com/OpenFeign/querydsl

I can probably start by creating KSP for the fork, then if someone needs it for this original querydsl project then i can probably do that too.
I suspect the code will be more or less identical unless OpenFeign has done some radical changes.

@IceBlizz6
Copy link

@spyro2000
You may have a valid question, but i'm not entirely sure if and how it is related to this issue.
I will only be creating an alternative to generating Q-classes in Kotlin so any other QueryDSL code will be called as usual.

@IceBlizz6
Copy link

https://github.com/IceBlizz6/querydsl-ksp

I got it working and have uploaded it to github.
I'm currently struggling a bit with figuring out how to upload the artifact to maven central.
I have written a readme with setup and and available options, but i'm holding on to it until i can upload this to maven central as the guide needs to include the dependency path.

I made 2 separate attempts at this, the first attempt tried to use and apply all of the existing querydsl configurations like picking up serializers from the classpath, the second one was more of a implementation from scratch based on observation of how the kotlin serializer worked.
The second attempt won out in the end, but i may make another attempt at the first one at some point later.

It supports options for including/excluding classes and packages, prefixing and suffixing classes/packages.
It currently only checks for jakarta annotations, i wasn't sure if support for the old deprecated javax annotations would be necessary but if someone requests it then i think they can easily be added.

I will post here again when i get the upload working and have published the readme.

@IceBlizz6
Copy link

Should be good now.
Readme added with instructions.

I tested it on my own big project with hundreds of entities and a few other projects and it seems fine but there may be cases that aren't covered so please test it and tell me if things aren't working.

@xlaussel
Copy link

Should be good now. Readme added with instructions.

I tested it on my own big project with hundreds of entities and a few other projects and it seems fine but there may be cases that aren't covered so please test it and tell me if things aren't working.

Hi, thank you.

I found some issues. I added it on github.

@JYC11
Copy link

JYC11 commented Oct 17, 2024

@IceBlizz6 hi, would you be able to add your ksp annotation processing to https://github.com/OpenFeign/querydsl? This is a fork of querydsl that is being actively maintained

@IceBlizz6
Copy link

Hey @JYC11
I'm testing a bit now, it seems like my project may be working for the OpenFeign fork aswell without modifications because the fork kept the same package names.

Could you check if my latest version works for you?

@IceBlizz6
Copy link

On reading this again i realize i may have misunderstood, did you mean to add a pull request to add my code into the fork and cooperate with that author?
Or did you mean that you wanted my project to be compatible with the fork?

@JYC11
Copy link

JYC11 commented Oct 18, 2024

@IceBlizz6 I'll check now if your project works with the fork! Yes I initially meant to ask if it's possible for you to add a pull request to the fork and cooperate with the author. Thanks for your contribution either way

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

No branches or pull requests

8 participants