rules_kotlin icon indicating copy to clipboard operation
rules_kotlin copied to clipboard

Implement Jdeps using K2 APIs

Open jbarr21 opened this issue 1 year ago • 3 comments

This PR adds support for running Jdeps when compiling with K2 (#843).

High level list of changes:

  • Enable K2 tests in paramaterized test class
  • Compile rules_kotlin with Kotlin 2.0 since we require fixes that were made that aren't in K2 preview of v1.9.23
  • Comment out providing the JS standard lib since as of Kotlin 2.0, only a KLIB is shipped & there is no support (#808)
  • Add JdepsGenExtension2 which leverages FirAdditionalCheckers plugin extension to collect info from the FIR tree
  • Since we don't control instantiation of the checkers, we use the ClassUsageRecorder object to manage collection of AST info
  • The deps info in ClassUsageRecorder is init in the registrar & dumped to a proto on the ClassFileFactoryFinalizerExtension callback since there is no longer a onAnalysisCompleted callback in K2
  • Deps info is collected through various declaration & expression checkers using 3 main types FirClassSymbol, FirTypeRef, & ConeKotlinType which is then pushed into the ClassUsageRecorder so that supertypes & type args can also be collected, if necessary
  • Update the JdepsGenComponentRegistrar to invoke K1 or K2 impl based on language version set in the kotlinc configuration

Note to reviewers: Tests on CI will likely fail until the Kotlin JS std lib for 2.0 issue is properly sorted out Screenshot 2024-04-26 at 12 05 46 PM

jbarr21 avatar Apr 26 '24 20:04 jbarr21

This generally looks fine, but I would prefer to wait until the dependencies are out of RC.

Additionally, we have some concerns about backwards compatibility -- I am looking to setup several older version test for our CI.

restingbull avatar May 03 '24 20:05 restingbull

@jbarr21 Kotlin 2.0 is officially out, are you able to push this PR forward using the official release?

Bencodes avatar May 24 '24 20:05 Bencodes

@jbarr21 Kotlin 2.0 is officially out, are you able to push this PR forward using the official release?

I am able to, but won't until y'all decide what you're going to do about this

jbarr21 avatar May 27 '24 16:05 jbarr21

@Bencodes now that #1185 has been merged, I've rebased my PR onto the latest master, updated the example apps for K2, & have everything passing except for ktlint on files that I didn't modify

jbarr21 avatar Jul 18 '24 00:07 jbarr21

I was able to get an app building within our Android repo using this PR and the K2 Jdps implementation, so we should be good to merge this now.

Bencodes avatar Aug 16 '24 20:08 Bencodes