From 20ac2f983207dfb575d76c2ab7040c41df97604e Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Fri, 24 Jan 2025 11:27:53 -0800 Subject: [PATCH 1/6] KSP2: fix type expansion --- .../kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt | 2 +- .../google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt | 8 ++------ .../com/google/devtools/ksp/impl/symbol/kotlin/util.kt | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt index a4c18d035d..e926c3abb7 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt @@ -556,7 +556,7 @@ class ResolverAAImpl( fun checkAnnotated(annotated: KSAnnotated): Boolean { return annotated.annotations.any { val kaType = (it.annotationType.resolve() as? KSTypeImpl)?.type ?: return@any false - kaType.toAbbreviatedType().symbol?.classId?.asFqNameString() == realAnnotationName + kaType.fullyExpand().symbol?.classId?.asFqNameString() == realAnnotationName } } diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt index 02aca77f80..84b985354d 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/KSTypeImpl.kt @@ -162,7 +162,7 @@ class KSTypeImpl private constructor(internal val type: KaType) : KSType { get() = type is KaFunctionType && type.isSuspend override fun hashCode(): Int { - return type.toAbbreviatedType().hashCode() + return type.fullyExpand().hashCode() } override fun equals(other: Any?): Boolean { @@ -179,8 +179,4 @@ class KSTypeImpl private constructor(internal val type: KaType) : KSType { } } -internal fun KaType.toAbbreviatedType(): KaType = - when (val symbol = this.classifierSymbol()) { - is KaTypeAliasSymbol -> symbol.expandedType.toAbbreviatedType() - else -> this - } +internal fun KaType.fullyExpand(): KaType = analyze { fullyExpandedType } diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt index 41cb654b05..99a0dcaee4 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt @@ -158,7 +158,7 @@ internal fun KaType.render(inFunctionType: Boolean = false): String { if (!inFunctionType) { append("[typealias ${symbol.name.asString()}]") } else { - append(this@render.toAbbreviatedType().render(inFunctionType = true)) + append(this@render.fullyExpand().render(inFunctionType = true)) } } else { append(this@render.symbol.name?.asString()) From e439c392f5eb4754f35ee33867555668e922f7fb Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Thu, 23 Jan 2025 11:02:23 -0800 Subject: [PATCH 2/6] KSP2: expand types in getJavaWildcard which aligns with KSP1. --- .../kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt | 2 +- .../kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt index e926c3abb7..eda6de6047 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/ResolverAAImpl.kt @@ -367,7 +367,7 @@ class ResolverAAImpl( if (type.isError) return reference val position = findRefPosition(ref) - val ktType = (type as KSTypeImpl).type + val ktType = (type as KSTypeImpl).type.fullyExpand() // cast to FIR internal needed due to missing support in AA for type mapping mode // and corresponding type mapping APIs. val coneType = (ktType as KaFirType).coneType diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt index 99a0dcaee4..bfdf281895 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt @@ -739,13 +739,13 @@ internal fun getVarianceForWildcard( @OptIn(KaExperimentalApi::class) internal fun KaType.toWildcard(mode: TypeMappingMode): KaType { - val parameters = this.classifierSymbol()?.typeParameters ?: emptyList() val args = this.typeArguments() return analyze { when (this@toWildcard) { is KaClassType -> { // TODO: missing annotations from original type. - buildClassType(this@toWildcard.expandedSymbol!!.tryResolveToTypePhase()) { + buildClassType(symbol.tryResolveToTypePhase()) { + val parameters = symbol.typeParameters parameters.zip(args).map { (param, arg) -> val argMode = mode.updateFromAnnotations(arg.type) val variance = getVarianceForWildcard(param, arg, argMode) From b0381509ecaafd174df1fa7e55f5c02e181afc42 Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Fri, 24 Jan 2025 14:45:21 -0800 Subject: [PATCH 3/6] KSP2: fix/implement most precise covariant check. Some out variance was incorrectly optimized away. --- .../devtools/ksp/impl/symbol/kotlin/util.kt | 43 ++++++++++++++++--- .../testData/javaWildcards2.kt | 2 +- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt index bfdf281895..32525440c2 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt @@ -42,7 +42,6 @@ import org.jetbrains.kotlin.analysis.api.fir.evaluate.FirAnnotationValueConverte import org.jetbrains.kotlin.analysis.api.fir.symbols.KaFirSymbol import org.jetbrains.kotlin.analysis.api.fir.symbols.KaFirValueParameterSymbol import org.jetbrains.kotlin.analysis.api.fir.types.KaFirFunctionType -import org.jetbrains.kotlin.analysis.api.fir.types.KaFirType import org.jetbrains.kotlin.analysis.api.impl.base.types.KaBaseStarTypeProjection import org.jetbrains.kotlin.analysis.api.impl.base.types.KaBaseTypeArgumentWithVariance import org.jetbrains.kotlin.analysis.api.platform.lifetime.KotlinAlwaysAccessibleLifetimeToken @@ -723,12 +722,11 @@ internal fun getVarianceForWildcard( } if (projectionKind == Variance.INVARIANT || projectionKind == parameterVariance) { if (mode.skipDeclarationSiteWildcardsIfPossible && projection !is KaStarTypeProjection) { - val coneType = (projection.type as KaFirType).coneType - // TODO: fix most precise covariant argument case. - if (parameterVariance == Variance.OUT_VARIANCE) { + val type = projection.type ?: return parameterVariance + if (parameterVariance == Variance.OUT_VARIANCE && type.isMostPreciseCovariantArgument()) { return Variance.INVARIANT } - if (parameterVariance == Variance.IN_VARIANCE && coneType.isAny) { + if (parameterVariance == Variance.IN_VARIANCE && type.isMostPreciseContravariantArgument()) { return Variance.INVARIANT } } @@ -737,6 +735,41 @@ internal fun getVarianceForWildcard( return Variance.OUT_VARIANCE } +internal fun KaType.isMostPreciseContravariantArgument(): Boolean = analyze { isAnyType } + +internal fun KaType.isMostPreciseCovariantArgument() = !canHaveSubtypesIgnoreNullability() + +@OptIn(KaExperimentalApi::class) +private fun KaType.canHaveSubtypesIgnoreNullability(): Boolean = + analyze { + val type = fullyExpandedType + val symbol = expandedSymbol ?: return@analyze true + if (symbol.classKind == KaClassKind.ENUM_CLASS || symbol.isExpect) { + return@analyze true + } + if (symbol.modality != KaSymbolModality.FINAL) { + return@analyze true + } + + symbol.typeParameters.forEachIndexed { idx, param -> + val projection = type.typeArguments().get(idx) + + if (projection !is KaTypeArgumentWithVariance) { + return@analyze true + } + + val type = projection.type + val effectiveVariance = getEffectiveVariance(param.variance, projection.variance) + if (effectiveVariance == Variance.OUT_VARIANCE && !type.isMostPreciseCovariantArgument()) { + return@analyze true + } + if (effectiveVariance == Variance.IN_VARIANCE && !type.isMostPreciseContravariantArgument()) { + return@analyze true + } + } + false + } + @OptIn(KaExperimentalApi::class) internal fun KaType.toWildcard(mode: TypeMappingMode): KaType { val args = this.typeArguments() diff --git a/kotlin-analysis-api/testData/javaWildcards2.kt b/kotlin-analysis-api/testData/javaWildcards2.kt index 0f0949efed..1cc0015a87 100644 --- a/kotlin-analysis-api/testData/javaWildcards2.kt +++ b/kotlin-analysis-api/testData/javaWildcards2.kt @@ -71,7 +71,7 @@ // enumList : List // jvmWildcard : List // suppressJvmWildcard : List -// : VarianceSubjectSuppressed +// : VarianceSubjectSuppressed // R : Any? // END From 932c23ddf0cb160dcaf2f10d7dd8effaad80bbb6 Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Fri, 24 Jan 2025 16:05:49 -0800 Subject: [PATCH 4/6] KSP2: star-projected invariance should become covariant --- .../kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt index 32525440c2..cf45dfd07e 100644 --- a/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt +++ b/kotlin-analysis-api/src/main/kotlin/com/google/devtools/ksp/impl/symbol/kotlin/util.kt @@ -711,7 +711,7 @@ internal fun getVarianceForWildcard( val projectionKind = if (projection is KaTypeArgumentWithVariance) { projection.variance } else { - Variance.INVARIANT + Variance.OUT_VARIANCE } val parameterVariance = parameter.variance if (parameterVariance == Variance.INVARIANT) { From 260dc5f98b4d4deaab7002ad9748559c6643ce08 Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Fri, 24 Jan 2025 16:02:15 -0800 Subject: [PATCH 5/6] KSP2: Enable remaining wildcard tests --- .../com/google/devtools/ksp/test/KSPAATest.kt | 3 +- .../testData/equivalentJavaWildcards.kt | 208 ++++++++++++++++++ 2 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 kotlin-analysis-api/testData/equivalentJavaWildcards.kt diff --git a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPAATest.kt b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPAATest.kt index bf7d873a59..ab5e85660c 100644 --- a/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPAATest.kt +++ b/kotlin-analysis-api/src/test/kotlin/com/google/devtools/ksp/test/KSPAATest.kt @@ -256,11 +256,10 @@ class KSPAATest : AbstractKSPAATest() { runTest("../test-utils/testData/api/equals.kt") } - @Disabled @TestMetadata("equivalentJavaWildcards.kt") @Test fun testEquivalentJavaWildcards() { - runTest("../test-utils/testData/api/equivalentJavaWildcards.kt") + runTest("../kotlin-analysis-api/testData/equivalentJavaWildcards.kt") } @TestMetadata("errorTypes.kt") diff --git a/kotlin-analysis-api/testData/equivalentJavaWildcards.kt b/kotlin-analysis-api/testData/equivalentJavaWildcards.kt new file mode 100644 index 0000000000..793e423850 --- /dev/null +++ b/kotlin-analysis-api/testData/equivalentJavaWildcards.kt @@ -0,0 +1,208 @@ +/* + * 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. + */ + +// WITH_RUNTIME +// TEST PROCESSOR: EquivalentJavaWildcardProcessor +// EXPECTED: +// bar1 : [@JvmSuppressWildcards(true)] A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// - @JvmSuppressWildcards : JvmSuppressWildcards -> JvmSuppressWildcards +// bar2 : [@JvmSuppressWildcards(false)] A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// - @JvmSuppressWildcards : JvmSuppressWildcards -> JvmSuppressWildcards +// bar3 : [@JvmWildcard] A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// - @JvmWildcard : JvmWildcard -> JvmWildcard +// p1 : A -> A +// - CONTRAVARIANT X : X -> X +// - COVARIANT X : X -> X +// p1.getter() : A -> A +// - CONTRAVARIANT X : X -> X +// - COVARIANT X : X -> X +// p2 : A -> A +// - INVARIANT Any : Any -> Any +// - INVARIANT Y : Y -> Y +// p2.getter() : A -> A +// - INVARIANT Any : Any -> Any +// - INVARIANT Y : Y -> Y +// p3 : A<*, *> -> A +// p3.getter() : A<*, *> -> A +// p4 : B -> B +// - INVARIANT X : X -> X +// p4.getter() : B -> B +// - INVARIANT X : X -> X +// p5 : B -> B +// - CONTRAVARIANT X : X -> X +// p5.getter() : B -> B +// - CONTRAVARIANT X : X -> X +// p6 : B -> B +// - COVARIANT X : X -> X +// p6.getter() : B -> B +// - COVARIANT X : X -> X +// p7 : B<*> -> B +// p7.getter() : B<*> -> B +// p8 : B> -> B> +// - INVARIANT A : A -> A +// - - INVARIANT X : X -> X +// - - COVARIANT Y : Y -> Y +// p8.getter() : B> -> B> +// - INVARIANT A : A -> A +// - - INVARIANT X : X -> X +// - - COVARIANT Y : Y -> Y +// v1 : A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// v2 : A -> A +// - INVARIANT Any : Any -> Any +// - INVARIANT Y : Y -> Y +// v3 : A<*, *> -> A +// v4 : B -> B +// - INVARIANT X : X -> X +// v5 : B -> B +// - CONTRAVARIANT X : X -> X +// v6 : B -> B +// - COVARIANT X : X -> X +// v7 : B<*> -> B +// v8 : B> -> B> +// - INVARIANT A : A -> A +// - - INVARIANT X : X -> X +// - - COVARIANT Y : Y -> Y +// foo : Unit -> Unit +// r1 : A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// r2 : A -> A +// - INVARIANT Any : Any -> Any +// - INVARIANT Y : Y -> Y +// r3 : A<*, *> -> A +// r4 : B -> B +// - INVARIANT X : X -> X +// r5 : B -> B +// - CONTRAVARIANT X : X -> X +// r6 : B -> B +// - COVARIANT X : X -> X +// r7 : B<*> -> B +// r8 : B> -> B> +// - INVARIANT A : A -> A +// - - INVARIANT X : X -> X +// - - COVARIANT Y : Y -> Y +// X : Any -> Any +// : X -> X +// Y : X -> X +// : Y -> Y +// A : Any -> Any +// T1 : Any? -> Any? +// T2 : Any? -> Any? +// : A<*, *> -> A +// T1 : Any? -> Any? +// T2 : Any? -> Any? +// B : Any -> Any +// T : Any? -> Any? +// synthetic constructor for B : B<*> -> B +// T : Any? -> Any? +// C1 : A -> A +// - INVARIANT X : X -> X +// - INVARIANT X : X -> X +// : C1 -> C1 +// C2 : A -> A +// - INVARIANT Any : Any -> Any +// - INVARIANT Y : Y -> Y +// : C2 -> C2 +// C3 : B -> B +// - INVARIANT X : X -> X +// : C3 -> C3 +// C4 : B> -> B> +// - INVARIANT A : A -> A +// - - INVARIANT X : X -> X +// - - COVARIANT Y : Y -> Y +// : C4 -> C4 +// END + +open class X() +final class Y() : X() + +open class A() +open class B + +// FIXME: should this annotation propagate to the return type? +// @JvmSuppressWildcards(false) +// fun bar(): A = TODO() + +// A +fun bar1(): @JvmSuppressWildcards(true) A = TODO() +// A +fun bar2(): @JvmSuppressWildcards(false) A = TODO() +// A +fun bar3(): @JvmWildcard A = TODO() + +// A +val p1: A = TODO() +// A +val p2: A = TODO() +// A +val p3: A<*, *> = TODO() +// B +val p4: B = TODO() +// B +val p5: B = TODO() +// B +val p6: B = TODO() +// B +val p7: B<*> = TODO() +// B> +val p8: B> + +// void foo(A, A, A, B, B, B, B, B>); +fun foo( + v1: A, + v2: A, + v3: A<*, *>, + v4: B, + v5: B, + v6: B, + v7: B<*>, + v8: B>, +): Unit = Unit + +// A +fun r1(): A = TODO() +// A +fun r2(): A = TODO() +// A +fun r3(): A<*, *> = TODO() +// B +fun r4(): B = TODO() +// B +fun r5(): B = TODO() +// B +fun r6(): B = TODO() +// B +fun r7(): B<*> = TODO() +// B> +fun r8(): B> = TODO() + +// extends A +class C1() : A() +// A +class C2() : A() +// B +class C3() : B() +// B> +class C4() : B>() From 6c4a8c602dda49b1b5aa09f8936c831fd881542e Mon Sep 17 00:00:00 2001 From: Ting-Yuan Huang Date: Fri, 24 Jan 2025 16:06:18 -0800 Subject: [PATCH 6/6] KSP1: fix KSTypeArgument.parent --- .../symbol/impl/binary/KSClassifierReferenceDescriptorImpl.kt | 2 +- test-utils/testData/api/equivalentJavaWildcards.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassifierReferenceDescriptorImpl.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassifierReferenceDescriptorImpl.kt index 4d83abb41d..ca6c88a3b5 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassifierReferenceDescriptorImpl.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassifierReferenceDescriptorImpl.kt @@ -68,7 +68,7 @@ class KSClassifierReferenceDescriptorImpl private constructor( } override val typeArguments: List by lazy { - arguments.map { KSTypeArgumentDescriptorImpl.getCached(it, origin, this.parent) } + arguments.map { KSTypeArgumentDescriptorImpl.getCached(it, origin, this) } } override fun referencedName(): String { diff --git a/test-utils/testData/api/equivalentJavaWildcards.kt b/test-utils/testData/api/equivalentJavaWildcards.kt index c4f3ecf092..7282cf40f1 100644 --- a/test-utils/testData/api/equivalentJavaWildcards.kt +++ b/test-utils/testData/api/equivalentJavaWildcards.kt @@ -77,7 +77,7 @@ // - - INVARIANT X : X -> X // - - COVARIANT Y : Y -> Y // p8.getter() : B> -> B> -// - INVARIANT A : A -> A +// - INVARIANT A : A -> A // - - INVARIANT X : X -> X // - - COVARIANT Y : Y -> Y // v1 : A -> A