Properly wrap PSIMethods that are constructors

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 · Jan 24, 2021 · 740e306 · 740e306
1 parent d3dcef1
commit 740e306
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 10 deletions.
diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processing/impl/ResolverImpl.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processing/impl/ResolverImpl.kt
@@ -27,6 +27,7 @@ import com.google.devtools.ksp.symbol.impl.binary.*
 import com.google.devtools.ksp.symbol.impl.findPsi
 import com.google.devtools.ksp.symbol.impl.java.*
 import com.google.devtools.ksp.symbol.impl.kotlin.*
+import com.google.devtools.ksp.symbol.impl.resolveContainingClass
 import com.google.devtools.ksp.symbol.impl.synthetic.KSConstructorSyntheticImpl
 import com.google.devtools.ksp.symbol.impl.synthetic.KSPropertyGetterSyntheticImpl
 import com.google.devtools.ksp.symbol.impl.synthetic.KSPropertySetterSyntheticImpl
@@ -320,15 +321,15 @@ class ResolverImpl(
                 // TODO: get rid of hardcoded check if possible.
                 val property = if (psi.name.startsWith("set") || psi.name.startsWith("get")) {
                     moduleClassResolver
-                        .resolveClass(JavaMethodImpl(psi).containingClass)
+                        .resolveContainingClass(psi)
                         ?.findEnclosedDescriptor(
                             kindFilter = DescriptorKindFilter.CALLABLES
                         ) {
                             (it as? PropertyDescriptor)?.getter?.findPsi() == psi || (it as? PropertyDescriptor)?.setter?.findPsi() == psi
                         }
                 } else null
                 property ?: moduleClassResolver
-                    .resolveClass(JavaMethodImpl(psi).containingClass)
+                    .resolveContainingClass(psi)
                     ?.findEnclosedDescriptor(
                         kindFilter = DescriptorKindFilter.FUNCTIONS,
                         filter = { it.findPsi() == psi }
@@ -457,9 +458,8 @@ class ResolverImpl(
                         check(owner is PsiMethod) {
                             "unexpected owner type: $owner / ${owner?.javaClass}"
                         }
-                        moduleClassResolver.resolveClass(
-                            JavaMethodImpl(owner).containingClass
-                        )?.findEnclosedDescriptor(
+                        moduleClassResolver.resolveContainingClass(owner)
+                        ?.findEnclosedDescriptor(
                             kindFilter = DescriptorKindFilter.FUNCTIONS,
                             filter = { it.findPsi() == owner }
                         ) as FunctionDescriptor
@@ -767,5 +767,7 @@ private inline fun ClassDescriptor.findEnclosedDescriptor(
     ) ?: this.staticScope.findEnclosedDescriptor(
         kindFilter = kindFilter,
         filter = filter
-    )
+    ) ?: constructors.firstOrNull {
+        kindFilter.accepts(it) && filter(it)
+    }
 }
diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/MapSignatureProcessor.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/MapSignatureProcessor.kt
@@ -31,7 +31,7 @@ class MapSignatureProcessor : AbstractTestProcessor() {
     }
 
     override fun process(resolver: Resolver) {
-        listOf("Cls", "JavaIntefaceWithVoid")
+        listOf("Cls", "JavaIntefaceWithVoid", "JavaClass")
             .map { className ->
                 resolver.getClassDeclarationByName(className)!!
             }.forEach { subject ->

diff --git a/...iler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/ResolveJavaTypeProcessor.kt b/...iler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/ResolveJavaTypeProcessor.kt
@@ -45,8 +45,8 @@ class ResolveJavaTypeProcessor : AbstractTestProcessor() {
         }
 
         override fun visitFunctionDeclaration(function: KSFunctionDeclaration, data: Unit) {
-            if (function.simpleName.asString() == "wildcardParam") {
-                function.parameters[0].type!!.accept(this, Unit)
+            function.parameters.forEach {
+                it.type.accept(this, Unit)
             }
             function.returnType?.accept(this, Unit)
         }

diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/utils.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/utils.kt
@@ -37,13 +37,17 @@ import com.intellij.psi.impl.source.PsiClassImpl
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.load.java.JavaDescriptorVisibilities
 import org.jetbrains.kotlin.load.java.descriptors.JavaClassConstructorDescriptor
+import org.jetbrains.kotlin.load.java.structure.JavaMember
+import org.jetbrains.kotlin.load.java.structure.impl.JavaConstructorImpl
+import org.jetbrains.kotlin.load.java.structure.impl.JavaMethodImpl
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.resolve.descriptorUtil.getOwnerForEffectiveDispatchReceiverParameter
 import org.jetbrains.kotlin.resolve.source.getPsi
 import org.jetbrains.kotlin.types.KotlinType
 import org.jetbrains.kotlin.types.StarProjectionImpl
 import org.jetbrains.kotlin.types.TypeProjectionImpl
 import org.jetbrains.kotlin.types.replace
+import org.jetbrains.kotlin.load.java.lazy.ModuleClassResolver
 
 val jvmModifierMap = mapOf(
     JvmModifier.PUBLIC to Modifier.PUBLIC,
@@ -323,3 +327,11 @@ internal inline fun <reified T : CallableMemberDescriptor> T.findClosestOverride
     }
     return null
 }
+
+fun ModuleClassResolver.resolveContainingClass(psiMethod: PsiMethod): ClassDescriptor? {
+    return if (psiMethod.isConstructor) {
+        resolveClass(JavaConstructorImpl(psiMethod).containingClass)
+    } else {
+        resolveClass(JavaMethodImpl(psiMethod).containingClass)
+    }
+}
diff --git a/compiler-plugin/testData/api/resolveJavaType.kt b/compiler-plugin/testData/api/resolveJavaType.kt
@@ -17,6 +17,7 @@
 
 // TEST PROCESSOR: ResolveJavaTypeProcessor
 // EXPECTED:
+// C.<init>.X?
 // kotlin.Int
 // kotlin.String?
 // kotlin.collections.MutableSet<out kotlin.Any?>?
@@ -32,7 +33,7 @@
 // FILE: a.kt
 annotation class Test
 @Test
-class Foo<P>: C<P>() {
+class Foo<P>(p:P): C<P>(p) {
 
 }
 
@@ -43,6 +44,9 @@ import java.util.List;
 import java.util.Set;
 
 public class C<T> {
+    public C() {}
+    // to reproduce the case where type reference is owned by a constructor
+    public <X> C(X x) {}
     public int intFun() {}
 
     public String strFun() {}

diff --git a/compiler-plugin/testData/api/signatureMapper.kt b/compiler-plugin/testData/api/signatureMapper.kt
@@ -23,6 +23,8 @@
 // ()Ljava/lang/String;
 // LJavaIntefaceWithVoid;
 // ()Ljava/lang/Void;
+// LJavaClass;
+// ()V
 // END
 
 // FILE: Cls.kt
@@ -36,3 +38,8 @@ class Cls {
 interface JavaIntefaceWithVoid {
     Void getVoid();
 }
+
+// FILE: JavaClass.java
+class JavaClass {
+    JavaClass() {}
+}