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

getJvmName throws exception for static java methods #170

Closed
yigit opened this issue Nov 25, 2020 · 2 comments
Closed

getJvmName throws exception for static java methods #170

yigit opened this issue Nov 25, 2020 · 2 comments
Assignees
Labels
bug Something isn't working P1 major features or blocking bugs

Comments

@yigit
Copy link
Collaborator

yigit commented Nov 25, 2020

For the given source:

class Foo { 
    static void staticMethod() {}
}

calling getJvmName on staticMethod throws:

Cannot find descriptor for staticMethod
java.lang.IllegalStateException: Cannot find descriptor for staticMethod
	at com.google.devtools.ksp.processing.impl.ResolverImpl.getJvmName(ResolverImpl.kt:520)
@neetopia neetopia self-assigned this Nov 25, 2020
@neetopia neetopia added bug Something isn't working P1 major features or blocking bugs labels Nov 25, 2020
@yigit
Copy link
Collaborator Author

yigit commented Nov 25, 2020

Here is some more debugging information:

in resolveJavaDeclaration, when we resolve the class as:
moduleClassResolver.resolveClass(JavaMethodImpl(psi).containingClass)
the static methods / fields disappear.

I can see JavaMethodImpl(psi).containingClass.methods has the method.
Further debugging shows it is actually in staticScope instead of unsubstitutedMemberScope.

I don't fully understand what these scopes are but should we searching all scopes instead?

@neetopia
Copy link
Contributor

Yeah we should do that. That hasn't been an issue for kotlin since static stuff are treated as an inner class.

yigit added a commit to yigit/ksp that referenced this issue Nov 25, 2020
This PR fixes a bug where static java declarations were not being resolved
properly because we would only look at the main scope of the class declaration
but we also need to check the static scope.

I've also refactored resolveJavaDeclaration to move common code to helpers.

Fixes: google#170
copybara-service bot pushed a commit to androidx/androidx that referenced this issue Nov 26, 2020
This required a fix where method element return type was not being
returned as member (javac implementation does that).

Also hit a bug in KSP when calling jvmname for java statics, added
a workaround.
google/ksp#170

Bug: 160322705
Test: XElementTest
Change-Id: Id1ffe436a520bc68f76986328d7d0f794ab24088
@neetopia neetopia closed this as completed Dec 5, 2020
copybara-service bot pushed a commit to androidx/androidx that referenced this issue Dec 9, 2020
This version is important because it also moves KSP's
kotlin dependency to 1.4.20.

I had to specify more versions for testing because we were
getting kotlin compiler dependencies from kotlin compile
testing, which is still on 1.4.10. Now compiler-processing-testing
artifact explicitly specifies those version to match whatever
AndroidX is using.

I've also removed workarounds for the following issues
as they are fixed.

google/ksp#170
google/ksp#174

Bug: 160322705
Test: existing tests
Change-Id: If948bcb762d1fad4911cf833a85cda6a36e7c787
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1 major features or blocking bugs
Projects
None yet
Development

No branches or pull requests

2 participants