Report INNER modifier for class descriptors #232
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR fixes a bug where the inner modifier would be lost if the kotlin sources is in dependencies (when wrapped in a descriptor). Unfortunately, for descriptors, there is no easy way to distinguish between JAVA_STATIC and INNER. That is, if a java nested class that is annotated with STATIC is compiled, descriptor API will no longer report any modifiers for it. Similarly, if a Java inner class (that has no static annotation) is compiled, we'll start reporting INNER for it. Even though this is slightly confusing, it is at least consistent from a kotlin perspective. Also fixed a bug in multi-module testing setup where we were NOT compiling java sources in test modules. Fixes: google#231 Test: javaModifiers more test cases report inner for descriptors
ting-yuan
approved these changes
Jan 6, 2021
copybara-service bot
pushed a commit
to androidx/androidx
that referenced
this pull request
Jan 6, 2021
Nested classes in kotlin are static by default. This CL updates KspHasModifiers to add the static modifier when it is a nested kotlin class without the inner modifier. A possible alternative solution is to add `isInner` to XTypeElement because Room is using isStatic as a proxy to discover inner classes. I've not implemented that because removing isStatic is not very feasible from XHasModifiers but we can do a followup CL to divide XHasModifiers and remove isStatic from there. Right now, KSP does not report INNER modifier for .class files which is why I couldn't add a test with compiled code. google/ksp#232 I've also renamed KspTypeElementTest to XTypeElementTest since it runs all but 1 test w/ both processors. Bug: 160322705 Test: XTypeElementTest Change-Id: Ie07382a8744b26eda3b4454d45ea74bfae71fd70
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug where the inner modifier would be lost if the
kotlin sources is in dependencies (when wrapped in a descriptor).
Unfortunately, for descriptors, there is no easy way to distinguish
between JAVA_STATIC and INNER. That is, if a java nested class that is
annotated with STATIC is compiled, descriptor API will no longer report
any modifiers for it. Similarly, if a Java inner class (that has no
static annotation) is compiled, we'll start reporting INNER for it.
Even though this is slightly confusing, it is at least consistent from a
kotlin perspective.
Also fixed a bug in multi-module testing setup where we were NOT
compiling java sources in test modules.
Fixes: #231
Test: javaModifiers
more test cases
report inner for descriptors