Skip to content

Commit

Permalink
Fix constructor related issues:
Browse files Browse the repository at this point in the history
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: #273
Fixes: #114
Fixes: #113
Test: constructorDeclarations.kt
  • Loading branch information
yigit authored and neetopia committed Jan 30, 2021
1 parent f9c5c76 commit 03ad88a
Show file tree
Hide file tree
Showing 12 changed files with 468 additions and 24 deletions.
16 changes: 9 additions & 7 deletions api/src/main/kotlin/com/google/devtools/ksp/utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fun Resolver.getClassDeclarationByName(name: String): KSClassDeclaration? = getC
/**
* Get functions directly declared inside the class declaration.
*
* What are included: member functions, extension functions declared inside it, etc.
* What are included: member functions, constructors, extension functions declared inside it, etc.
* What are NOT included: inherited functions, extension functions declared outside it.
*/
fun KSClassDeclaration.getDeclaredFunctions(): List<KSFunctionDeclaration> {
Expand All @@ -65,11 +65,9 @@ fun KSClassDeclaration.getDeclaredProperties(): List<KSPropertyDeclaration> {
}

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

/**
Expand Down Expand Up @@ -241,7 +239,11 @@ fun KSDeclaration.isVisibleFrom(other: KSDeclaration): Boolean {
} ?: false
else -> false
}

}

/**
* Returns `true` if this is a constructor function.
*/
fun KSFunctionDeclaration.isConstructor() = this.simpleName.asString() == "<init>"

const val ExceptionMessage = "please file a bug at https://github.com/google/ksp/issues/new"
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*
* Copyright 2020 Google LLC
* Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


package com.google.devtools.ksp.processor

import com.google.devtools.ksp.getClassDeclarationByName
import com.google.devtools.ksp.getConstructors
import com.google.devtools.ksp.processing.Resolver
import com.google.devtools.ksp.symbol.*

class ConstructorDeclarationsProcessor : AbstractTestProcessor() {
val visitor = ConstructorsVisitor()

override fun toResult(): List<String> {
return visitor.toResult()
}

override fun process(resolver: Resolver) {
resolver.getAllFiles().map { it.accept(visitor, Unit) }
val classNames = visitor.classNames().toList() // copy
// each class has a cousin in the lib package, visit them as well, make sure
// we report the same structure when they are compiled code as well
classNames.forEach {
resolver
.getClassDeclarationByName("lib.${it.simpleName.asString()}")
?.accept(visitor, Unit)
}
}

class ConstructorsVisitor : KSVisitorVoid() {
private val declarationsByClass = LinkedHashMap<KSClassDeclaration, MutableList<String>>()
fun classNames() = declarationsByClass.keys
fun toResult() : List<String> {
return declarationsByClass.entries
.sortedBy {
// sort by simple name to get cousin classes next to each-other
// since we traverse the lib after main, lib will be the second one
// because sortedBy is stable sort
it.key.simpleName.asString()
}.flatMap {
listOf("class: " + it.key.qualifiedName!!.asString()) + it.value
}
}
fun KSFunctionDeclaration.toSignature(): String {
return this.simpleName.asString() +
"(${this.parameters.map {
buildString {
append(it.type.resolve().declaration.qualifiedName?.asString())
if (it.hasDefault) {
append("(hasDefault)")
}
}
}.joinToString(",")})" +
": ${this.returnType?.resolve()?.declaration?.qualifiedName?.asString()
?: "<no-return>"}"
}

override fun visitClassDeclaration(classDeclaration: KSClassDeclaration, data: Unit) {
val declarations = mutableListOf<String>()
declarations.addAll(
classDeclaration.getConstructors().map {
it.toSignature()
}.sorted()
)
// TODO add some assertions that if we go through he path of getDeclarations
// we still find the same constructors
declarationsByClass[classDeclaration] = declarations
}

override fun visitFile(file: KSFile, data: Unit) {
file.declarations.map { it.accept(this, Unit) }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
}

override val declarations: List<KSDeclaration> by lazy {
listOf(descriptor.unsubstitutedMemberScope.getDescriptorsFiltered(), descriptor.staticScope.getDescriptorsFiltered()).flatten()
listOf(
descriptor.unsubstitutedMemberScope.getDescriptorsFiltered(),
descriptor.staticScope.getDescriptorsFiltered(),
descriptor.constructors
).flatten()
.filter {
it is MemberDescriptor
&& it.visibility != DescriptorVisibilities.INHERITED
Expand Down Expand Up @@ -125,12 +129,11 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
}
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import com.google.devtools.ksp.symbol.impl.findClosestOverridee
import com.google.devtools.ksp.symbol.impl.toFunctionKSModifiers
import com.google.devtools.ksp.symbol.impl.toKSFunctionDeclaration
import com.google.devtools.ksp.symbol.impl.toKSModifiers
import org.jetbrains.kotlin.descriptors.ClassConstructorDescriptor
import org.jetbrains.kotlin.load.java.isFromJava
import org.jetbrains.kotlin.resolve.OverridingUtil
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
Expand Down Expand Up @@ -62,8 +63,12 @@ class KSFunctionDeclarationDescriptorImpl private constructor(val descriptor: Fu
}

override val functionKind: FunctionKind by lazy {

when {
descriptor.dispatchReceiverParameter == null -> if (descriptor.isFromJava) FunctionKind.STATIC else FunctionKind.TOP_LEVEL
descriptor.dispatchReceiverParameter == null -> when {
descriptor.isFromJava -> FunctionKind.STATIC
else -> FunctionKind.TOP_LEVEL
}
!descriptor.name.isSpecial && !descriptor.name.asString().isEmpty() -> FunctionKind.MEMBER
descriptor is AnonymousFunctionDescriptor -> FunctionKind.ANONYMOUS
else -> throw IllegalStateException("Unable to resolve FunctionKind for ${descriptor.fqNameSafe}, $ExceptionMessage")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class KSClassDeclarationJavaEnumEntryImpl private constructor(val psi: PsiEnumCo
}

override fun getAllFunctions(): List<KSFunctionDeclaration> =
descriptor?.getAllFunctions(true) ?: emptyList()
descriptor?.getAllFunctions() ?: emptyList()

override fun getAllProperties(): List<KSPropertyDeclaration> =
descriptor?.getAllProperties() ?: emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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

import com.google.devtools.ksp.isConstructor
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiJavaFile
import org.jetbrains.kotlin.descriptors.ClassDescriptor
Expand All @@ -32,6 +33,7 @@ import com.google.devtools.ksp.symbol.impl.kotlin.KSExpectActualNoImpl
import com.google.devtools.ksp.symbol.impl.kotlin.KSNameImpl
import com.google.devtools.ksp.symbol.impl.kotlin.getKSTypeCached
import com.google.devtools.ksp.symbol.impl.replaceTypeArguments
import com.google.devtools.ksp.symbol.impl.synthetic.KSConstructorSyntheticImpl
import com.google.devtools.ksp.symbol.impl.toKSFunctionDeclaration
import com.intellij.psi.PsiEnumConstant
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
Expand Down Expand Up @@ -78,13 +80,13 @@ class KSClassDeclarationJavaImpl private constructor(val psi: PsiClass) : KSClas
}

override fun getAllFunctions(): List<KSFunctionDeclaration> =
descriptor?.getAllFunctions(true) ?: emptyList()
descriptor?.getAllFunctions() ?: emptyList()

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

override val declarations: List<KSDeclaration> by lazy {
(psi.fields.map {
val allDeclarations = (psi.fields.map {
when (it) {
is PsiEnumConstant -> KSClassDeclarationJavaEnumEntryImpl.getCached(it)
else -> KSPropertyDeclarationJavaImpl.getCached(it)
Expand All @@ -93,6 +95,20 @@ class KSClassDeclarationJavaImpl private constructor(val psi: PsiClass) : KSClas
psi.constructors.map { KSFunctionDeclarationJavaImpl.getCached(it) } +
psi.methods.map { KSFunctionDeclarationJavaImpl.getCached(it) })
.distinct()
// java annotation classes are interface. they get a constructor in .class
// hence they should get one here.
if (classKind == ClassKind.ANNOTATION_CLASS || !psi.isInterface) {
val hasConstructor = allDeclarations.any {
it is KSFunctionDeclaration && it.isConstructor()
}
if (hasConstructor) {
allDeclarations
} else {
allDeclarations + KSConstructorSyntheticImpl(this)
}
} else {
allDeclarations
}
}

override val modifiers: Set<Modifier> by lazy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.google.devtools.ksp.symbol.impl.kotlin.KSNameImpl
import com.intellij.lang.jvm.JvmModifier
import com.intellij.psi.PsiJavaFile
import com.intellij.psi.PsiMethod
import com.intellij.psi.impl.source.PsiClassReferenceType

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

override val extensionReceiver: KSTypeReference? = null

override val functionKind: FunctionKind = if (psi.hasModifier(JvmModifier.STATIC)) FunctionKind.STATIC else FunctionKind.MEMBER
override val functionKind: FunctionKind = when {
psi.hasModifier(JvmModifier.STATIC) -> FunctionKind.STATIC
else -> FunctionKind.MEMBER
}

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

override val returnType: KSTypeReference? by lazy {
if (psi.returnType != null) {
KSTypeReferenceJavaImpl.getCached(psi.returnType!!)
} else {
null
when {
psi.returnType != null -> {
KSTypeReferenceJavaImpl.getCached(psi.returnType!!)
}
psi.isConstructor -> {
psi.containingClass?.let { containingClass ->
KSTypeReferenceLiteJavaImpl.getCached(
KSClassDeclarationJavaImpl.getCached(containingClass).asStarProjectedType()
)
}
}
else -> {
null
}
}
}

override val simpleName: KSName by lazy {
KSNameImpl.getCached(psi.name)
if (psi.isConstructor) {
KSNameImpl.getCached("<init>")
} else {
KSNameImpl.getCached(psi.name)
}
}

override val typeParameters: List<KSTypeParameter> by lazy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

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

import com.google.devtools.ksp.isConstructor
import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.Visibilities
Expand Down Expand Up @@ -61,7 +62,19 @@ class KSClassDeclarationImpl private constructor(val ktClassOrObject: KtClassOrO
?.map { KSPropertyDeclarationParameterImpl.getCached((it as KSValueParameterImpl).ktParameter) } ?: emptyList()
val result = ktClassOrObject.declarations.getKSDeclarations().toMutableList()
result.addAll(propertiesFromConstructor)
result
if (classKind != ClassKind.INTERFACE) {
// check if we need to add a synthetic constructor
val hasConstructor = result.any {
it is KSFunctionDeclaration && it.isConstructor()
}
if (hasConstructor) {
result
} else {
result + KSConstructorSyntheticImpl(this)
}
} else {
result
}
}

override val primaryConstructor: KSFunctionDeclaration? by lazy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ class KSFunctionDeclarationImpl private constructor(val ktFunction: KtFunction)
return descriptor?.findClosestOverridee()?.toKSFunctionDeclaration()
}

override val simpleName: KSName by lazy {
if (ktFunction is KtConstructor<*>) {
KSNameImpl.getCached("<init>")
} else {
KSNameImpl.getCached(ktFunction.name!!)
}
}

override val declarations: List<KSDeclaration> by lazy {
if (!ktFunction.hasBlockBody()) {
emptyList()
Expand All @@ -64,7 +72,7 @@ class KSFunctionDeclarationImpl private constructor(val ktFunction: KtFunction)
FunctionKind.TOP_LEVEL
} else {
when (ktFunction) {
is KtNamedFunction, is KtPrimaryConstructor, is KtSecondaryConstructor -> FunctionKind.MEMBER
is KtNamedFunction, is KtConstructor<*> -> FunctionKind.MEMBER
is KtFunctionLiteral -> if (ktFunction.node.findChildByType(KtTokens.FUN_KEYWORD) != null) FunctionKind.ANONYMOUS else FunctionKind.LAMBDA
else -> throw IllegalStateException("Unexpected psi type ${ktFunction.javaClass}, $ExceptionMessage")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class KSConstructorSyntheticImpl(val ksClassDeclaration: KSClassDeclaration) : K
ksClassDeclaration
}

override val returnType: KSTypeReference? = null
override val returnType: KSTypeReference = KSTypeReferenceSyntheticImpl(
ksClassDeclaration.asStarProjectedType()
)

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ public void testCompanion() throws Exception {
runTest("testData/api/companion.kt");
}

@TestMetadata("constructorDeclarations.kt")
public void testConstructorDeclarations() throws Exception {
runTest("testData/api/constructorDeclarations.kt");
}

@TestMetadata("crossModuleTypeAlias.kt")
public void testCrossModuleTypeAlias() throws Exception {
runTest("testData/api/crossModuleTypeAlias.kt");
Expand Down
Loading

0 comments on commit 03ad88a

Please sign in to comment.