Move JvmOverloads workaround for constructors to Psi model
Previously, the workaround for the Psi issue
https://youtrack.jetbrains.com/issue/KT-57537 was to emulate the
behavior of `@JvmOverloads` by creating overloads of the constructors,
one for each parameter with a default value. That was added to
`SignatureWriter` but that meant that it would not work with snapshots
as it relied on having a `PsiConstructorItem` when generating the
signature which is not true when using snapshots. It was also the wrong
place because a problem with the Psi model should be fixed in the Psi
model, and not leak through the model API.
This change moves the emulation of `@JvmOverloads` from
`SignatureWriter` to the Psi model where it belongs. As that is done
before any snapshot is taken that ensures that it will work properly
when using snapshots. That allowed the `shouldExpandOverloads()` to be
removed from the `CallableItem` and moved to be a private method in
`PsiCodebaseAssembler`.
Bug: 367623741
Test: ./gradlew
(cherry picked from https://android-review.googlesource.com/q/commit:e7edec3f7664cfc2c8a5bf3975081e9c4b9cb08a)
Merged-In: I73053d79ee1a8c12f1967d2349df6a09fffcaacc
Change-Id: I73053d79ee1a8c12f1967d2349df6a09fffcaacc
diff --git a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCallableItem.kt b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCallableItem.kt
index 4fd88ff..a70551b 100644
--- a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCallableItem.kt
+++ b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCallableItem.kt
@@ -21,8 +21,6 @@
import com.android.tools.metalava.model.type.MethodFingerprint
import com.intellij.psi.PsiMethod
import com.intellij.psi.PsiParameter
-import org.jetbrains.kotlin.name.JvmStandardClassIds
-import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.uast.UMethod
internal interface PsiCallableItem : CallableItem, PsiItem {
@@ -31,16 +29,6 @@
val psiMethod: PsiMethod
- override fun shouldExpandOverloads(): Boolean {
- val ktFunction = (psiMethod as? UMethod)?.sourcePsi as? KtFunction ?: return false
- return modifiers.isActual() &&
- psiMethod.hasAnnotation(JvmStandardClassIds.JVM_OVERLOADS_FQ_NAME.asString()) &&
- // It is /technically/ invalid to have actual functions with default values, but
- // some places suppress the compiler error, so we should handle it here too.
- ktFunction.valueParameters.none { it.hasDefaultValue() } &&
- parameters().any { it.hasDefaultValue() }
- }
-
companion object {
/**
* Create a list of [PsiParameterItem]s.
@@ -55,8 +43,8 @@
psiMethod: PsiMethod,
containingCallable: PsiCallableItem,
enclosingTypeItemFactory: PsiTypeItemFactory,
+ psiParameters: List<PsiParameter> = psiMethod.psiParameters,
): List<PsiParameterItem> {
- val psiParameters = psiMethod.psiParameters
val fingerprint = MethodFingerprint(containingCallable.name(), psiParameters.size)
return psiParameters.mapIndexed { index, parameter ->
PsiParameterItem.create(
diff --git a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCodebaseAssembler.kt b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCodebaseAssembler.kt
index af778f7..ca2fb9e 100644
--- a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCodebaseAssembler.kt
+++ b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiCodebaseAssembler.kt
@@ -27,6 +27,7 @@
import com.android.tools.metalava.model.ClassKind
import com.android.tools.metalava.model.ClassOrigin
import com.android.tools.metalava.model.ClassTypeItem
+import com.android.tools.metalava.model.ConstructorItem
import com.android.tools.metalava.model.Item
import com.android.tools.metalava.model.JAVA_PACKAGE_INFO
import com.android.tools.metalava.model.PackageItem
@@ -75,6 +76,8 @@
import org.jetbrains.kotlin.analysis.api.types.KaTypeNullability
import org.jetbrains.kotlin.asJava.classes.KtLightClassForFacade
import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.JvmStandardClassIds
+import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtParameter
import org.jetbrains.kotlin.psi.KtProperty
import org.jetbrains.kotlin.psi.KtPropertyAccessor
@@ -82,6 +85,7 @@
import org.jetbrains.kotlin.psi.psiUtil.isPropertyParameter
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UFile
+import org.jetbrains.uast.UMethod
import org.jetbrains.uast.UParameter
import org.jetbrains.uast.UastFacade
import org.jetbrains.uast.kotlin.BaseKotlinUastResolveProviderService
@@ -243,6 +247,11 @@
psiMethod,
classTypeItemFactory,
)
+ addOverloadedKotlinConstructorsIfNecessary(
+ classItem,
+ classTypeItemFactory,
+ constructor
+ )
classItem.addConstructor(constructor)
} else {
val method =
@@ -452,6 +461,73 @@
!psiClass.isEnum
}
+ /**
+ * Returns true if overloads of this constructor should be checked separately when checking the
+ * signature of this constructor.
+ *
+ * This works around the issue of actual callable not generating overloads for @JvmOverloads
+ * annotation when the default is specified on expect side
+ * (https://youtrack.jetbrains.com/issue/KT-57537).
+ */
+ private fun PsiConstructorItem.shouldExpandOverloads(): Boolean {
+ val ktFunction = (psiMethod as? UMethod)?.sourcePsi as? KtFunction ?: return false
+ return modifiers.isActual() &&
+ psiMethod.hasAnnotation(JvmStandardClassIds.JVM_OVERLOADS_FQ_NAME.asString()) &&
+ // It is /technically/ invalid to have actual functions with default values, but
+ // some places suppress the compiler error, so we should handle it here too.
+ ktFunction.valueParameters.none { it.hasDefaultValue() } &&
+ parameters().any { it.hasDefaultValue() }
+ }
+
+ /**
+ * Add overloads of [constructor] if necessary.
+ *
+ * Workaround for https://youtrack.jetbrains.com/issue/KT-57537.
+ *
+ * For each parameter with a default value in [constructor] this adds a [ConstructorItem] that
+ * excludes that parameter and all following parameters with default values.
+ */
+ private fun addOverloadedKotlinConstructorsIfNecessary(
+ classItem: PsiClassItem,
+ enclosingClassTypeItemFactory: PsiTypeItemFactory,
+ constructor: PsiConstructorItem,
+ ) {
+ if (!constructor.shouldExpandOverloads()) {
+ return
+ }
+
+ val parameters = constructor.parameters()
+
+ // Create an overload of the constructor for each parameter that has a default value. The
+ // constructor will exclude that parameter and all following parameters that have default
+ // values.
+ for (currentParameterIndex in parameters.indices) {
+ val currentParameter = parameters[currentParameterIndex]
+ // There is no need to create an overload if the parameter does not have default value.
+ if (!currentParameter.hasDefaultValue()) continue
+
+ // Create an overloaded constructor.
+ val overloadConstructor =
+ PsiConstructorItem.create(
+ codebase,
+ classItem,
+ constructor.psiMethod,
+ enclosingClassTypeItemFactory,
+ psiParametersGetter = {
+ parameters.mapIndexedNotNull { index, parameterItem ->
+ // Ignore the current parameter as well as any following parameters
+ // with default values.
+ if (index >= currentParameterIndex && parameterItem.hasDefaultValue())
+ null
+ else (parameterItem as PsiParameterItem).psiParameter
+ }
+ },
+ )
+
+ classItem.addConstructor(overloadConstructor)
+ }
+ }
+
private fun findOrCreateClass(qualifiedName: String): ClassItem? {
// Check to see if the class has already been seen and if so return it immediately.
codebase.findClass(qualifiedName)?.let {
diff --git a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiConstructorItem.kt b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiConstructorItem.kt
index fdcd1b8..0d2e3df 100644
--- a/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiConstructorItem.kt
+++ b/metalava-model-psi/src/main/java/com/android/tools/metalava/model/psi/PsiConstructorItem.kt
@@ -34,6 +34,7 @@
import com.intellij.psi.JavaPsiFacade
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiParameter
import org.jetbrains.kotlin.psi.KtClassOrObject
import org.jetbrains.kotlin.psi.KtPrimaryConstructor
import org.jetbrains.uast.UMethod
@@ -78,6 +79,7 @@
containingClass: ClassItem,
psiMethod: PsiMethod,
enclosingClassTypeItemFactory: PsiTypeItemFactory,
+ psiParametersGetter: (PsiMethod) -> List<PsiParameter> = { it.psiParameters },
): PsiConstructorItem {
assert(psiMethod.isConstructor)
val name = psiMethod.name
@@ -119,6 +121,7 @@
psiMethod,
containingCallable as PsiCallableItem,
constructorTypeItemFactory,
+ psiParametersGetter(psiMethod),
)
},
returnType = containingClass.type(),
diff --git a/metalava-model/src/main/java/com/android/tools/metalava/model/CallableItem.kt b/metalava-model/src/main/java/com/android/tools/metalava/model/CallableItem.kt
index 54d66a2..ac6345f 100644
--- a/metalava-model/src/main/java/com/android/tools/metalava/model/CallableItem.kt
+++ b/metalava-model/src/main/java/com/android/tools/metalava/model/CallableItem.kt
@@ -108,16 +108,6 @@
containingClass().qualifiedName()}.${name()}(${parameters().joinToString { it.type().toSimpleType() }})"
}
- /**
- * Returns true if overloads of this callable should be checked separately when checking the
- * signature of this callable.
- *
- * This works around the issue of actual callable not generating overloads for @JvmOverloads
- * annotation when the default is specified on expect side
- * (https://youtrack.jetbrains.com/issue/KT-57537).
- */
- fun shouldExpandOverloads(): Boolean = false
-
override fun equalsToItem(other: Any?): Boolean {
if (this === other) return true
if (other !is CallableItem) return false
diff --git a/metalava/src/main/java/com/android/tools/metalava/SignatureWriter.kt b/metalava/src/main/java/com/android/tools/metalava/SignatureWriter.kt
index 48ab0da..274380d 100644
--- a/metalava/src/main/java/com/android/tools/metalava/SignatureWriter.kt
+++ b/metalava/src/main/java/com/android/tools/metalava/SignatureWriter.kt
@@ -34,7 +34,6 @@
import com.android.tools.metalava.model.visitors.ApiVisitor
import com.android.tools.metalava.model.visitors.FilteringApiVisitor
import java.io.PrintWriter
-import java.util.BitSet
class SignatureWriter(
private val writer: PrintWriter,
@@ -77,33 +76,13 @@
}
override fun visitConstructor(constructor: ConstructorItem) {
- fun writeConstructor(skipMask: BitSet? = null) {
- write(" ctor ")
- writeModifiers(constructor)
- writeTypeParameterList(constructor.typeParameterList, addSpace = true)
- write(constructor.containingClass().fullName())
- writeParameterList(constructor, skipMask)
- writeThrowsList(constructor)
- write(";\n")
- }
-
- // Workaround for https://youtrack.jetbrains.com/issue/KT-57537
- if (constructor.shouldExpandOverloads()) {
- val parameters = constructor.parameters()
- val defaultMask = BitSet(parameters.size)
-
- // fill the bitmask for all parameters
- parameters.forEachIndexed { i, item -> defaultMask.set(i, item.hasDefaultValue()) }
-
- // expand overloads ordered by number of parameters, skipping last parameters first
- for (i in parameters.indices) {
- if (!defaultMask.get(i)) continue
- writeConstructor(defaultMask)
- defaultMask.clear(i)
- }
- }
-
- writeConstructor()
+ write(" ctor ")
+ writeModifiers(constructor)
+ writeTypeParameterList(constructor.typeParameterList, addSpace = true)
+ write(constructor.containingClass().fullName())
+ writeParameterList(constructor)
+ writeThrowsList(constructor)
+ write(";\n")
}
override fun visitField(field: FieldItem) {
@@ -262,13 +241,10 @@
}
}
- private fun writeParameterList(callable: CallableItem, skipMask: BitSet? = null) {
+ private fun writeParameterList(callable: CallableItem) {
write("(")
var writtenParams = 0
- callable.parameters().asSequence().forEachIndexed { i, parameter ->
- // skip over defaults when generating @JvmOverloads permutations
- if (skipMask != null && skipMask.get(i)) return@forEachIndexed
-
+ callable.parameters().asSequence().forEach { parameter ->
if (writtenParams > 0) {
write(", ")
}
