DRAFT: Very rough in-progress work on Kotlin support in the Gazelle plugin
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:
- Add a new Gazelle directive
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_libraryrule instead ofjava_libraryrule. Most remaining Go logic is unchanged. - Add a
KtParserto theJavaParsergRPC server. This uses the Kotlin compiler's parser to process Kotlin files, and extract the same data that is extracted by the Java-orientedClasspathParser.- This file currently handles detecting a main function, imported types and packages, and class declarations. It has partial coverage for exported types, and has a bunch of commented out code where I'm trying to figure out the best way to tackle detecting fully-qualified types and packages throughout the file. I also haven't yet added support for any annotation handling.
Questions for reviewers
- Overall, WDYT? If people think this is worth adding, I'll make a series of smaller, cleaner PRs to actually get this done.
- The plugin, its directives, packages, and classes often use the name
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 GazelleLanguagetojvm, but keep the existing directive names to avoid breaking users. Or, we add newjvmdirectives, but continue to support the oldjavaones as well. Lots of other possibilities too.
FAQ
Why do this here when there is another Kotlin plugin?
A few reasons:
- If you're using Kotlin + Java, and exclusively using Kotlin with the JVM, then I'm not sure that it actually makes sense to treat Kotlin as a separate language from Gazelle's perspective. That makes sense for two languages like protobuf and Java, which have a strictly one-way dependency graph. However, in my experience, Java and Kotlin code frequently depend on each other, sometimes from separate packages, and sometimes within the same package. If dependencies are only between packages, then I can see separate Gazelle plugins potentially working with Gazelle's cross-resolve feature, though this seems more intended for entirely separate resolution methods, not inter-dependent language plugins. Additionally, I don't see a good way to handle intra-package Java & Kotlin dependencies. If the Java plugin knows nothing about Kotlin, it will try to produce a java_library rule for the Java files, which will lack some Kotlin-specific information. A Kotlin plugin would then need to replace this rule with a Kotlin one, and extend it to handle the extra information from the Kotlin files. This seems a bit hacky and error prone. Conversely, adding Kotlin support directly to the Java plugin is turning out to be relatively painless (minus me trying to learn how to use the Kotlin parser).
- There is an open support request for it.
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.
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?
Hey folks, I apologies but I had to set this down for a while. I can get back into it now. @alexeagle, I've messaged you on Slack.