Skip to content

Commit 547332e

Browse files
yigityigit
committed
Fix constructor related issues:
This PR fixes a bunch of issues / inconsistencies in constuctor delcarations. * changed KSFunctionDeclarationImpl and KSFunctionDeclarationJavaImpl to return <init> from name if it is constructor for consistency * added KSFunctionDeclaration.isConstructor extension function for convenience * changed getConstructors to simply use declaredFunctions.filter { it.kind == CONSTURCTOR } for consistency * updated KSClassDeclarationDescriptorImpl.kt declarations to include * added synthetic constructors for non-interface classes that do not have a constructor (including java annotations which do get a constructor when they are in .class files) * fixed KSConstructorSyntheticImpl to implement returnType Fixes: google#273 Fixes: google#114 Fixes: google#113 Test: constructorDeclarations.kt
1 parent f9c5c76 commit 547332e

File tree

12 files changed

+468
-24
lines changed

12 files changed

+468
-24
lines changed

‎api/src/main/kotlin/com/google/devtools/ksp/utils.kt

+9-7
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ fun Resolver.getClassDeclarationByName(name: String): KSClassDeclaration? = getC
4747
/**
4848
* Get functions directly declared inside the class declaration.
4949
*
50-
* What are included: member functions, extension functions declared inside it, etc.
50+
* What are included: member functions, constructors, extension functions declared inside it, etc.
5151
* What are NOT included: inherited functions, extension functions declared outside it.
5252
*/
5353
fun KSClassDeclaration.getDeclaredFunctions(): List<KSFunctionDeclaration> {
@@ -65,11 +65,9 @@ fun KSClassDeclaration.getDeclaredProperties(): List<KSPropertyDeclaration> {
6565
}
6666

6767
fun KSClassDeclaration.getConstructors(): List<KSFunctionDeclaration> {
68-
val functions = if (this.origin == Origin.JAVA) this.getAllFunctions() else this.getDeclaredFunctions()
69-
return functions.filter { it.simpleName.asString() == this.simpleName.asString() || it.simpleName.asString() == "<init>"}
70-
.let { constructors ->
71-
this.primaryConstructor?.let { constructors.plus(it) } ?: constructors
72-
}
68+
return getDeclaredFunctions().filter {
69+
it.isConstructor()
70+
}
7371
}
7472

7573
/**
@@ -241,7 +239,11 @@ fun KSDeclaration.isVisibleFrom(other: KSDeclaration): Boolean {
241239
} ?: false
242240
else -> false
243241
}
244-
245242
}
246243

244+
/**
245+
* Returns `true` if this is a constructor function.
246+
*/
247+
fun KSFunctionDeclaration.isConstructor() = this.simpleName.asString() == "<init>"
248+
247249
const val ExceptionMessage = "please file a bug at https://github.com/google/ksp/issues/new"

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/ConstructorDeclarationsProcessor.kt

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2020 Google LLC
3+
* Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
19+
package com.google.devtools.ksp.processor
20+
21+
import com.google.devtools.ksp.getClassDeclarationByName
22+
import com.google.devtools.ksp.getConstructors
23+
import com.google.devtools.ksp.processing.Resolver
24+
import com.google.devtools.ksp.symbol.*
25+
26+
class ConstructorDeclarationsProcessor : AbstractTestProcessor() {
27+
val visitor = ConstructorsVisitor()
28+
29+
override fun toResult(): List<String> {
30+
return visitor.toResult()
31+
}
32+
33+
override fun process(resolver: Resolver) {
34+
resolver.getAllFiles().map { it.accept(visitor, Unit) }
35+
val classNames = visitor.classNames().toList() // copy
36+
// each class has a cousin in the lib package, visit them as well, make sure
37+
// we report the same structure when they are compiled code as well
38+
classNames.forEach {
39+
resolver
40+
.getClassDeclarationByName("lib.${it.simpleName.asString()}")
41+
?.accept(visitor, Unit)
42+
}
43+
}
44+
45+
class ConstructorsVisitor : KSVisitorVoid() {
46+
private val declarationsByClass = LinkedHashMap<KSClassDeclaration, MutableList<String>>()
47+
fun classNames() = declarationsByClass.keys
48+
fun toResult() : List<String> {
49+
return declarationsByClass.entries
50+
.sortedBy {
51+
// sort by simple name to get cousin classes next to each-other
52+
// since we traverse the lib after main, lib will be the second one
53+
// because sortedBy is stable sort
54+
it.key.simpleName.asString()
55+
}.flatMap {
56+
listOf("class: " + it.key.qualifiedName!!.asString()) + it.value
57+
}
58+
}
59+
fun KSFunctionDeclaration.toSignature(): String {
60+
return this.simpleName.asString() +
61+
"(${this.parameters.map {
62+
buildString {
63+
append(it.type.resolve().declaration.qualifiedName?.asString())
64+
if (it.hasDefault) {
65+
append("(hasDefault)")
66+
}
67+
}
68+
}.joinToString(",")})" +
69+
": ${this.returnType?.resolve()?.declaration?.qualifiedName?.asString()
70+
?: "<no-return>"}"
71+
}
72+
73+
override fun visitClassDeclaration(classDeclaration: KSClassDeclaration, data: Unit) {
74+
val declarations = mutableListOf<String>()
75+
declarations.addAll(
76+
classDeclaration.getConstructors().map {
77+
it.toSignature()
78+
}.sorted()
79+
)
80+
// TODO add some assertions that if we go through he path of getDeclarations
81+
// we still find the same constructors
82+
declarationsByClass[classDeclaration] = declarations
83+
}
84+
85+
override fun visitFile(file: KSFile, data: Unit) {
86+
file.declarations.map { it.accept(this, Unit) }
87+
}
88+
}
89+
}

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassDeclarationDescriptorImpl.kt

+7-4
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
7878
}
7979

8080
override val declarations: List<KSDeclaration> by lazy {
81-
listOf(descriptor.unsubstitutedMemberScope.getDescriptorsFiltered(), descriptor.staticScope.getDescriptorsFiltered()).flatten()
81+
listOf(
82+
descriptor.unsubstitutedMemberScope.getDescriptorsFiltered(),
83+
descriptor.staticScope.getDescriptorsFiltered(),
84+
descriptor.constructors
85+
).flatten()
8286
.filter {
8387
it is MemberDescriptor
8488
&& it.visibility != DescriptorVisibilities.INHERITED
@@ -125,12 +129,11 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
125129
}
126130
}
127131

128-
internal fun ClassDescriptor.getAllFunctions(explicitConstructor: Boolean = false): List<KSFunctionDeclaration> {
132+
internal fun ClassDescriptor.getAllFunctions(): List<KSFunctionDeclaration> {
129133
ResolverImpl.instance.incrementalContext.recordLookupForGetAllFunctions(this)
130134
val functionDescriptors = unsubstitutedMemberScope.getDescriptorsFiltered(DescriptorKindFilter.FUNCTIONS).toList()
131135
.filter { (it as FunctionDescriptor).visibility != DescriptorVisibilities.INVISIBLE_FAKE }.toMutableList()
132-
if (explicitConstructor)
133-
functionDescriptors += constructors
136+
functionDescriptors += constructors
134137
return functionDescriptors.map { KSFunctionDeclarationDescriptorImpl.getCached(it as FunctionDescriptor) }
135138
}
136139

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSFunctionDeclarationDescriptorImpl.kt

+6-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import com.google.devtools.ksp.symbol.impl.findClosestOverridee
3030
import com.google.devtools.ksp.symbol.impl.toFunctionKSModifiers
3131
import com.google.devtools.ksp.symbol.impl.toKSFunctionDeclaration
3232
import com.google.devtools.ksp.symbol.impl.toKSModifiers
33+
import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor
3334
import org.jetbrains.kotlin.load.java.isFromJava
3435
import org.jetbrains.kotlin.resolve.OverridingUtil
3536
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
@@ -62,8 +63,12 @@ class KSFunctionDeclarationDescriptorImpl private constructor(val descriptor: Fu
6263
}
6364

6465
override val functionKind: FunctionKind by lazy {
66+
6567
when {
66-
descriptor.dispatchReceiverParameter == null -> if (descriptor.isFromJava) FunctionKind.STATIC else FunctionKind.TOP_LEVEL
68+
descriptor.dispatchReceiverParameter == null -> when {
69+
descriptor.isFromJava -> FunctionKind.STATIC
70+
else -> FunctionKind.TOP_LEVEL
71+
}
6772
!descriptor.name.isSpecial && !descriptor.name.asString().isEmpty() -> FunctionKind.MEMBER
6873
descriptor is AnonymousFunctionDescriptor -> FunctionKind.ANONYMOUS
6974
else -> throw IllegalStateException("Unable to resolve FunctionKind for ${descriptor.fqNameSafe}, $ExceptionMessage")

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaEnumEntryImpl.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class KSClassDeclarationJavaEnumEntryImpl private constructor(val psi: PsiEnumCo
6565
}
6666

6767
override fun getAllFunctions(): List<KSFunctionDeclaration> =
68-
descriptor?.getAllFunctions(true) ?: emptyList()
68+
descriptor?.getAllFunctions() ?: emptyList()
6969

7070
override fun getAllProperties(): List<KSPropertyDeclaration> =
7171
descriptor?.getAllProperties() ?: emptyList()

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSClassDeclarationJavaImpl.kt

+18-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package com.google.devtools.ksp.symbol.impl.java
2020

21+
import com.google.devtools.ksp.isConstructor
2122
import com.intellij.psi.PsiClass
2223
import com.intellij.psi.PsiJavaFile
2324
import org.jetbrains.kotlin.descriptors.ClassDescriptor
@@ -32,6 +33,7 @@ import com.google.devtools.ksp.symbol.impl.kotlin.KSExpectActualNoImpl
3233
import com.google.devtools.ksp.symbol.impl.kotlin.KSNameImpl
3334
import com.google.devtools.ksp.symbol.impl.kotlin.getKSTypeCached
3435
import com.google.devtools.ksp.symbol.impl.replaceTypeArguments
36+
import com.google.devtools.ksp.symbol.impl.synthetic.KSConstructorSyntheticImpl
3537
import com.google.devtools.ksp.symbol.impl.toKSFunctionDeclaration
3638
import com.intellij.psi.PsiEnumConstant
3739
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
@@ -78,13 +80,13 @@ class KSClassDeclarationJavaImpl private constructor(val psi: PsiClass) : KSClas
7880
}
7981

8082
override fun getAllFunctions(): List<KSFunctionDeclaration> =
81-
descriptor?.getAllFunctions(true) ?: emptyList()
83+
descriptor?.getAllFunctions() ?: emptyList()
8284

8385
override fun getAllProperties(): List<KSPropertyDeclaration> =
8486
descriptor?.getAllProperties() ?: emptyList()
8587

8688
override val declarations: List<KSDeclaration> by lazy {
87-
(psi.fields.map {
89+
val allDeclarations = (psi.fields.map {
8890
when (it) {
8991
is PsiEnumConstant -> KSClassDeclarationJavaEnumEntryImpl.getCached(it)
9092
else -> KSPropertyDeclarationJavaImpl.getCached(it)
@@ -93,6 +95,20 @@ class KSClassDeclarationJavaImpl private constructor(val psi: PsiClass) : KSClas
9395
psi.constructors.map { KSFunctionDeclarationJavaImpl.getCached(it) } +
9496
psi.methods.map { KSFunctionDeclarationJavaImpl.getCached(it) })
9597
.distinct()
98+
// java annotation classes are interface. they get a constructor in .class
99+
// hence they should get one here.
100+
if (classKind == ClassKind.ANNOTATION_CLASS || !psi.isInterface) {
101+
val hasConstructor = allDeclarations.any {
102+
it is KSFunctionDeclaration && it.isConstructor()
103+
}
104+
if (hasConstructor) {
105+
allDeclarations
106+
} else {
107+
allDeclarations + KSConstructorSyntheticImpl(this)
108+
}
109+
} else {
110+
allDeclarations
111+
}
96112
}
97113

98114
override val modifiers: Set<Modifier> by lazy {

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/java/KSFunctionDeclarationJavaImpl.kt

+24-6
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.google.devtools.ksp.symbol.impl.kotlin.KSNameImpl
2626
import com.intellij.lang.jvm.JvmModifier
2727
import com.intellij.psi.PsiJavaFile
2828
import com.intellij.psi.PsiMethod
29+
import com.intellij.psi.impl.source.PsiClassReferenceType
2930

3031
class KSFunctionDeclarationJavaImpl private constructor(val psi: PsiMethod) : KSFunctionDeclaration, KSDeclarationJavaImpl(),
3132
KSExpectActual by KSExpectActualNoImpl() {
@@ -56,7 +57,10 @@ class KSFunctionDeclarationJavaImpl private constructor(val psi: PsiMethod) : KS
5657

5758
override val extensionReceiver: KSTypeReference? = null
5859

59-
override val functionKind: FunctionKind = if (psi.hasModifier(JvmModifier.STATIC)) FunctionKind.STATIC else FunctionKind.MEMBER
60+
override val functionKind: FunctionKind = when {
61+
psi.hasModifier(JvmModifier.STATIC) -> FunctionKind.STATIC
62+
else -> FunctionKind.MEMBER
63+
}
6064

6165
override val isAbstract: Boolean by lazy {
6266
this.modifiers.contains(Modifier.ABSTRACT) ||
@@ -81,15 +85,29 @@ class KSFunctionDeclarationJavaImpl private constructor(val psi: PsiMethod) : KS
8185
}
8286

8387
override val returnType: KSTypeReference? by lazy {
84-
if (psi.returnType != null) {
85-
KSTypeReferenceJavaImpl.getCached(psi.returnType!!)
86-
} else {
87-
null
88+
when {
89+
psi.returnType != null -> {
90+
KSTypeReferenceJavaImpl.getCached(psi.returnType!!)
91+
}
92+
psi.isConstructor -> {
93+
psi.containingClass?.let { containingClass ->
94+
KSTypeReferenceLiteJavaImpl.getCached(
95+
KSClassDeclarationJavaImpl.getCached(containingClass).asStarProjectedType()
96+
)
97+
}
98+
}
99+
else -> {
100+
null
101+
}
88102
}
89103
}
90104

91105
override val simpleName: KSName by lazy {
92-
KSNameImpl.getCached(psi.name)
106+
if (psi.isConstructor) {
107+
KSNameImpl.getCached("<init>")
108+
} else {
109+
KSNameImpl.getCached(psi.name)
110+
}
93111
}
94112

95113
override val typeParameters: List<KSTypeParameter> by lazy {

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSClassDeclarationImpl.kt

+14-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
package com.google.devtools.ksp.symbol.impl.kotlin
2020

21+
import com.google.devtools.ksp.isConstructor
2122
import org.jetbrains.kotlin.descriptors.ClassDescriptor
2223
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
2324
import org.jetbrains.kotlin.descriptors.Visibilities
@@ -61,7 +62,19 @@ class KSClassDeclarationImpl private constructor(val ktClassOrObject: KtClassOrO
6162
?.map { KSPropertyDeclarationParameterImpl.getCached((it as KSValueParameterImpl).ktParameter) } ?: emptyList()
6263
val result = ktClassOrObject.declarations.getKSDeclarations().toMutableList()
6364
result.addAll(propertiesFromConstructor)
64-
result
65+
if (classKind != ClassKind.INTERFACE) {
66+
// check if we need to add a synthetic constructor
67+
val hasConstructor = result.any {
68+
it is KSFunctionDeclaration && it.isConstructor()
69+
}
70+
if (hasConstructor) {
71+
result
72+
} else {
73+
result + KSConstructorSyntheticImpl(this)
74+
}
75+
} else {
76+
result
77+
}
6578
}
6679

6780
override val primaryConstructor: KSFunctionDeclaration? by lazy {

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/kotlin/KSFunctionDeclarationImpl.kt

+9-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ class KSFunctionDeclarationImpl private constructor(val ktFunction: KtFunction)
4343
return descriptor?.findClosestOverridee()?.toKSFunctionDeclaration()
4444
}
4545

46+
override val simpleName: KSName by lazy {
47+
if (ktFunction is KtConstructor<*>) {
48+
KSNameImpl.getCached("<init>")
49+
} else {
50+
KSNameImpl.getCached(ktFunction.name!!)
51+
}
52+
}
53+
4654
override val declarations: List<KSDeclaration> by lazy {
4755
if (!ktFunction.hasBlockBody()) {
4856
emptyList()
@@ -64,7 +72,7 @@ class KSFunctionDeclarationImpl private constructor(val ktFunction: KtFunction)
6472
FunctionKind.TOP_LEVEL
6573
} else {
6674
when (ktFunction) {
67-
is KtNamedFunction, is KtPrimaryConstructor, is KtSecondaryConstructor -> FunctionKind.MEMBER
75+
is KtNamedFunction, is KtConstructor<*> -> FunctionKind.MEMBER
6876
is KtFunctionLiteral -> if (ktFunction.node.findChildByType(KtTokens.FUN_KEYWORD) != null) FunctionKind.ANONYMOUS else FunctionKind.LAMBDA
6977
else -> throw IllegalStateException("Unexpected psi type ${ktFunction.javaClass}, $ExceptionMessage")
7078
}

‎compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/synthetic/KSConstructorSyntheticImpl.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ class KSConstructorSyntheticImpl(val ksClassDeclaration: KSClassDeclaration) : K
5454
ksClassDeclaration
5555
}
5656

57-
override val returnType: KSTypeReference? = null
57+
override val returnType: KSTypeReference = KSTypeReferenceSyntheticImpl(
58+
ksClassDeclaration.asStarProjectedType()
59+
)
5860

5961
override val annotations: List<KSAnnotation> = emptyList()
6062

‎compiler-plugin/src/test/java/com/google/devtools/ksp/test/KotlinKSPTestGenerated.java

+5
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ public void testCompanion() throws Exception {
8787
runTest("testData/api/companion.kt");
8888
}
8989

90+
@TestMetadata("constructorDeclarations.kt")
91+
public void testConstructorDeclarations() throws Exception {
92+
runTest("testData/api/constructorDeclarations.kt");
93+
}
94+
9095
@TestMetadata("crossModuleTypeAlias.kt")
9196
public void testCrossModuleTypeAlias() throws Exception {
9297
runTest("testData/api/crossModuleTypeAlias.kt");

0 commit comments

Comments
 (0)