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

Implement Jdeps using K2 APIs #1166

Merged
merged 7 commits into from
Aug 20, 2024
Merged

Conversation

jbarr21
Copy link
Contributor

@jbarr21 jbarr21 commented Apr 26, 2024

This PR moves the kotlin_rules to Kotlin 2.0.0 & 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
  • Add JdepsGenExtension2 which leverages FirAdditionalCheckers plugin extension to collect info from the FIR tree
  • We use the ClassUsageRecorder 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
  • Override the Kotlin language version to 1.9 for KSP task since KSP does not support 2.0
  • Update example apps to work with K2

@restingbull
Copy link
Collaborator

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.

@Bencodes
Copy link
Collaborator

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

@jbarr21
Copy link
Contributor Author

jbarr21 commented May 27, 2024

@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
Copy link
Contributor Author

jbarr21 commented Jul 18, 2024

@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

class ClassUsageRecorder(
internal val explicitClassesCanonicalPaths: MutableSet<String> = mutableSetOf(),
internal val implicitClassesCanonicalPaths: MutableSet<String> = mutableSetOf(),
private val seen: MutableSet<ClassId> = mutableSetOf(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be unused, is that intentional?

@Bencodes
Copy link
Collaborator

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 Bencodes merged commit f8e5c77 into bazelbuild:master Aug 20, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants