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.

Sign up for GitHub

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

Jump to bottom

wrapping method psi to JavaMethodImpl causes exception #265

Closed
yigit opened this issue Jan 23, 2021 · 0 comments · Fixed by #266
Closed

wrapping method psi to JavaMethodImpl causes exception #265

yigit opened this issue Jan 23, 2021 · 0 comments · Fixed by #266
Assignees
@neetopia
Labels
bug Something isn't working

Comments

@yigit
Copy link
Collaborator

yigit commented Jan 23, 2021

Wehn resolveJavaDeclaration is called with a psi that represents a constructor, seems like we need to wrap it into a constructor impl. I guess this also happens in other places where we automatically wrap a PsiMethod into JavaMethodImpl

Caused by: java.lang.AssertionError: PsiMethod which is a constructor should be wrapped in JavaConstructorImpl: Child2
	at org.jetbrains.kotlin.load.java.structure.impl.JavaMethodImpl.<init>(JavaMethodImpl.java:37)
	at com.google.devtools.ksp.processing.impl.ResolverImpl.resolveJavaDeclaration(ResolverImpl.kt:331)
	at com.google.devtools.ksp.processing.impl.ResolverImpl.resolveFunctionDeclaration(ResolverImpl.kt:363)
	at com.google.devtools.ksp.processing.impl.ResolverImpl.computeAsMemberOf(ResolverImpl.kt:609)
	at com.google.devtools.ksp.processing.impl.ResolverImpl.asMemberOf(ResolverImpl.kt:598)
	at androidx.room.compiler.processing.ksp.KSAsMemberOfKt.typeAsMemberOf(KSAsMemberOf.kt:64)
@neetopia neetopia self-assigned this Jan 23, 2021
@neetopia neetopia added the bug Something isn't working label Jan 23, 2021
yigit added a commit to yigit/ksp that referenced this issue Jan 24, 2021
@yigit
Properly wrap PSIMethods that are constructors
5bd5342 
This PR fixes a bug where we were wrapping all PsiMethods to
JavaMethodImpl without checking if they are constructor.

Both cases, we were wrapping them to find the containing class,
so I've added an extension function to ModuleClassResolver
to find containing descriptor via PsiMethod which abstracts
this resolution.

Test: extended resolveJavaType and signatureMapper tests to trigger
the crash
Fixes: google#265
yigit added a commit to yigit/ksp that referenced this issue Jan 24, 2021
@yigit
Properly wrap PSIMethods that are constructors
740e306 
This PR fixes a bug where we were wrapping all PsiMethods to
JavaMethodImpl without checking if they are constructor.

Both cases, we were wrapping them to find the containing class,
so I've added an extension function to ModuleClassResolver
to find containing descriptor via PsiMethod which abstracts
this resolution.

Also fixes a bug where ClassDescriptor.findEnclosedDescriptor was
not checking constructors (also convered in the updated tests).

Test: extended resolveJavaType and signatureMapper tests to trigger
the crash
Fixes: google#265
yigit added a commit to yigit/ksp that referenced this issue Jan 24, 2021
@yigit
Properly wrap PSIMethods that are constructors
ce9dd1a 
This PR fixes a bug where we were wrapping all PsiMethods to
JavaMethodImpl without checking if they are constructor.

Both cases, we were wrapping them to find the containing class,
so I've added an extension function to ModuleClassResolver
to find containing descriptor via PsiMethod which abstracts
this resolution.

Also fixes a bug where ClassDescriptor.findEnclosedDescriptor was
not checking constructors (also convered in the updated tests).

Test: extended resolveJavaType and signatureMapper tests to trigger
the crash
Fixes: google#265
@yigit yigit mentioned this issue Jan 24, 2021
Merged
neetopia pushed a commit that referenced this issue Jan 25, 2021
@yigit @neetopia
Properly wrap PSIMethods that are constructors
11a9ae6 
This PR fixes a bug where we were wrapping all PsiMethods to
JavaMethodImpl without checking if they are constructor.

Both cases, we were wrapping them to find the containing class,
so I've added an extension function to ModuleClassResolver
to find containing descriptor via PsiMethod which abstracts
this resolution.

Also fixes a bug where ClassDescriptor.findEnclosedDescriptor was
not checking constructors (also convered in the updated tests).

Test: extended resolveJavaType and signatureMapper tests to trigger
the crash
Fixes: #265
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@neetopia neetopia

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Properly wrap PSIMethods that are constructors yigit/ksp
2 participants
@yigit @neetopia