-
Notifications
You must be signed in to change notification settings - Fork 68
DRAFT: Very rough in-progress work on Kotlin support in the Gazelle plugin #313
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
base: main
Are you sure you want to change the base?
Conversation
This has a lot of in-progress stuff in the KtParser for trying figure out how to properly handle fully-qualified types all over Kotlin code
I should note: I also haven't yet added support for generating test rules for Kotlin |
I think this has been in limbo long enough. Let's push ahead with this, and we'll see what happens :) That okay with you @MorganR? |
I would definitely want to tidy some in progress pieces before merging it, but happy to move forwards if you like the overall approach! I can start sending some small & tidy PRs over if that works for you? |
Smaller changes are far easier to review, so please do feel free to break this up any way you choose to. |
@MorganR we're strongly considering donating the implementation from aspect-cli to this repo. Would you like to discuss joining efforts? Maybe on Bazel slack? |
Do not merge this! It's for discussion. This has soo many caveats right now. It is ugly, is messy, is incomplete, has debug statements everywhere, has commented out code, etc.
I'm putting this up anyway so that people can take an early look at the overall approach, and give any general comments or guidance.
The overall approach:
jvm_kotlin_enabled
, which defaults tofalse
, to optionally enable Kotlin processing. Use this flag to optionally look for Kotlin files in addition to Java files within packages. If a Kotlin file is present in a package, then we use thekt_jvm_library
rule instead ofjava_library
rule. Most remaining Go logic is unchanged.KtParser
to theJavaParser
gRPC server. This uses the Kotlin compiler's parser to process Kotlin files, and extract the same data that is extracted by the Java-orientedClasspathParser
.Questions for reviewers
java
. By adding Kotlin, I'm somewhat implicitly changing the interpretation of this to something more likejvm
. Any thoughts yay or nay on name changes? That's not an all or nothing thing. For example, maybe we rename the GazelleLanguage
tojvm
, but keep the existing directive names to avoid breaking users. Or, we add newjvm
directives, but continue to support the oldjava
ones as well. Lots of other possibilities too.FAQ
Why do this here when there is another Kotlin plugin?
A few reasons:
Why add the
KtParser
to the same gRPC request handler, instead of a separate one for Kotlin?I'd be happy to change this if others feel strongly, but we're ultimately trying to produce the exact same format of output data for Java and Kotlin, and merge them into a single set of data per package. We could either make two separate gRPC calls from Go land and then merge the data there, or make a single call and merge the data in Java land. That seemed like less boilerplate (defining new RPC requests) and less overhead (fewer network calls, even if they're to localhost).
Why add the
KtParser
to the same Java parsing server, instead of a separate server entirely?See above, but also this would be even more resource overhead. No need to kick off multiple JVM processes if we can do it all in one.