From ffcd57cb317f801ec9cf8a8db1e405edb9807b62 Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Wed, 2 Jun 2021 17:35:13 +0200 Subject: [PATCH] [Commonizer] TypeCommonizer: Support commonizing typeAlias and their underlying types --- .../kotlin/commonizer/core/Commonizer.kt | 6 ++ .../commonizer/core/TypeAliasCommonizer.kt | 3 - .../kotlin/commonizer/core/TypeCommonizer.kt | 50 +++++++++-- .../commonized/common/package_root.kt | 4 + .../commonizer/core/AbstractCommonizerTest.kt | 3 +- .../commonizer/core/TypeCommonizerTest.kt | 20 ++--- ...hicalClassAndTypeAliasCommonizationTest.kt | 29 +++++++ .../HierarchicalPropertyCommonizationTest.kt | 85 +++++++++++++++++++ 8 files changed, 177 insertions(+), 23 deletions(-) diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/Commonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/Commonizer.kt index c548980530a..0f3048b1f28 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/Commonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/Commonizer.kt @@ -29,6 +29,12 @@ abstract class AbstractStandardCommonizer : Commonizer { State.IN_PROGRESS -> commonizationResult() } + val resultOrNull: R? + get() = when (state) { + State.EMPTY, State.ERROR -> null + State.IN_PROGRESS -> commonizationResult() + } + final override fun commonizeWith(next: T): Boolean { val result = when (state) { State.ERROR -> return false diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeAliasCommonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeAliasCommonizer.kt index 3f854eb80c4..2e3ba614970 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeAliasCommonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeAliasCommonizer.kt @@ -52,9 +52,6 @@ private class TypeAliasShortCircuitingCommonizer( expandedType = expandedType.result as CirClassType ) - val resultOrNull: CirTypeAlias? - get() = if (hasResult) commonizationResult() else null - override fun initialize(first: CirTypeAlias) { name = first.name } diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeCommonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeCommonizer.kt index fafa1fa2e52..4a6e07823e1 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeCommonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/TypeCommonizer.kt @@ -14,25 +14,57 @@ import org.jetbrains.kotlin.commonizer.utils.isUnderKotlinNativeSyntheticPackage import org.jetbrains.kotlin.descriptors.Visibility class TypeCommonizer(private val classifiers: CirKnownClassifiers) : AbstractStandardCommonizer() { - private lateinit var wrapped: Commonizer<*, CirType> + private var typeCommonizer: AbstractStandardCommonizer<*, CirType>? = null + private var underlyingTypeCommonizer: TypeCommonizer? = null - override fun commonizationResult() = wrapped.result + override fun commonizationResult() = typeCommonizer?.resultOrNull + ?: underlyingTypeCommonizer?.result + ?: typeCommonizer?.result + ?: failInErrorState() override fun initialize(first: CirType) { @Suppress("UNCHECKED_CAST") - wrapped = when (first) { + typeCommonizer = when (first) { is CirClassType -> ClassTypeCommonizer(classifiers) is CirTypeAliasType -> TypeAliasTypeCommonizer(classifiers) is CirTypeParameterType -> TypeParameterTypeCommonizer() is CirFlexibleType -> FlexibleTypeCommonizer(classifiers) - } as Commonizer<*, CirType> + } as AbstractStandardCommonizer<*, CirType> + + if (first is CirTypeAliasType) { + underlyingTypeCommonizer = TypeCommonizer(classifiers) + } } - override fun doCommonizeWith(next: CirType) = when (next) { - is CirClassType -> (wrapped as? ClassTypeCommonizer)?.commonizeWith(next) == true - is CirTypeAliasType -> (wrapped as? TypeAliasTypeCommonizer)?.commonizeWith(next) == true - is CirTypeParameterType -> (wrapped as? TypeParameterTypeCommonizer)?.commonizeWith(next) == true - is CirFlexibleType -> (wrapped as? FlexibleTypeCommonizer)?.commonizeWith(next) == true + override fun doCommonizeWith(next: CirType): Boolean { + return when (next) { + is CirClassType -> doCommonizeWithClassType(next) + is CirTypeAliasType -> doCommonizeWithTypeAliasType(next) + is CirTypeParameterType -> (typeCommonizer as? TypeParameterTypeCommonizer)?.commonizeWith(next) == true + is CirFlexibleType -> (typeCommonizer as? FlexibleTypeCommonizer)?.commonizeWith(next) == true + } + } + + private fun doCommonizeWithClassType(next: CirClassType): Boolean { + if ((typeCommonizer as? ClassTypeCommonizer) == null) { + typeCommonizer = null + } + + val firstAnswer = (typeCommonizer as? ClassTypeCommonizer)?.commonizeWith(next) == true + val secondAnswer = underlyingTypeCommonizer?.commonizeWith(next) == true + return firstAnswer || secondAnswer + } + + private fun doCommonizeWithTypeAliasType(next: CirTypeAliasType): Boolean { + if (typeCommonizer != null && underlyingTypeCommonizer == null) { + underlyingTypeCommonizer = TypeCommonizer(classifiers) + underlyingTypeCommonizer?.commonizeWith(typeCommonizer?.resultOrNull ?: return false) + typeCommonizer = null + } + + val firstAnswer = (typeCommonizer as? TypeAliasTypeCommonizer)?.commonizeWith(next) == true + val secondAnswer = underlyingTypeCommonizer?.commonizeWith(next.underlyingType) == true + return firstAnswer || secondAnswer } } diff --git a/native/commonizer/testData/callableMemberCommonization/returnTypes/commonized/common/package_root.kt b/native/commonizer/testData/callableMemberCommonization/returnTypes/commonized/common/package_root.kt index 762073da803..d5b5e346246 100644 --- a/native/commonizer/testData/callableMemberCommonization/returnTypes/commonized/common/package_root.kt +++ b/native/commonizer/testData/callableMemberCommonization/returnTypes/commonized/common/package_root.kt @@ -14,12 +14,16 @@ typealias C = Planet expect val property1: Int expect val property2: String expect val property3: Planet +expect val property4: Planet +expect val property5: Planet expect val property6: Planet expect val property7: C expect fun function1(): Int expect fun function2(): String expect fun function3(): Planet +expect fun function4(): Planet +expect fun function5(): Planet expect fun function6(): Planet expect fun function7(): C diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/AbstractCommonizerTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/AbstractCommonizerTest.kt index 14e4a1b4669..0db39cf1f38 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/AbstractCommonizerTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/AbstractCommonizerTest.kt @@ -48,7 +48,8 @@ abstract class AbstractCommonizerTest { val commonized = createCommonizer().apply { variants.forEachIndexed { index, variant -> val result = commonizeWith(variant) - if (index >= failureIndex) assertFalse(result) else assertTrue(result) + if (index >= failureIndex) assertFalse(result, "Expected to fail at index $index") + else assertTrue(result, "Expected to not fail at index $index") } } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt index f85ad7cbdbc..4728ce83681 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/TypeCommonizerTest.kt @@ -189,8 +189,9 @@ class TypeCommonizerTest : AbstractCommonizerTest() { mockTAType("kotlin/sequences/SequenceBuilder") { mockClassType("kotlin/sequences/SequenceScope") } ) - @Test(expected = IllegalCommonizerStateException::class) - fun taTypesInKotlinPackageWithDifferentNames() = doTestFailure( + @Test + fun taTypesInKotlinPackageWithDifferentNames() = doTestSuccess( + expected = mockClassType("kotlin/sequences/SequenceScope"), mockTAType("kotlin/sequences/SequenceBuilder") { mockClassType("kotlin/sequences/SequenceScope") }, mockTAType("kotlin/sequences/SequenceBuilder") { mockClassType("kotlin/sequences/SequenceScope") }, mockTAType("kotlin/sequences/FictitiousTypeAlias") { mockClassType("kotlin/sequences/SequenceScope") } @@ -214,8 +215,9 @@ class TypeCommonizerTest : AbstractCommonizerTest() { mockTAType("kotlinx/cinterop/CArrayPointer") { mockClassType("kotlinx/cinterop/CPointer") } ) - @Test(expected = IllegalCommonizerStateException::class) - fun taTypesInKotlinxPackageWithDifferentNames() = doTestFailure( + @Test() + fun taTypesInKotlinxPackageWithDifferentNames() = doTestSuccess( + expected = mockClassType("kotlinx/cinterop/CPointer"), mockTAType("kotlinx/cinterop/CArrayPointer") { mockClassType("kotlinx/cinterop/CPointer") }, mockTAType("kotlinx/cinterop/CArrayPointer") { mockClassType("kotlinx/cinterop/CPointer") }, mockTAType("kotlinx/cinterop/FictitiousTypeAlias") { mockClassType("kotlinx/cinterop/CPointer") } @@ -265,11 +267,11 @@ class TypeCommonizerTest : AbstractCommonizerTest() { mockTAType("org/sample/FooAlias") { mockClassType("org/sample/Foo") } ) - @Test(expected = IllegalCommonizerStateException::class) - fun taTypesInUserPackageWithDifferentNames() = doTestFailure( + @Test + fun taTypesInUserPackageWithDifferentNames() = doTestSuccess( + expected = mockClassType("org/sample/Foo"), mockTAType("org/sample/FooAlias") { mockClassType("org/sample/Foo") }, mockTAType("org/sample/BarAlias") { mockClassType("org/sample/Foo") }, - shouldFailOnFirstVariant = true ) @Test @@ -402,8 +404,7 @@ class TypeCommonizerTest : AbstractCommonizerTest() { fun taTypesInUserPackageWithDifferentNullability1() = doTestFailure( mockTAType("org/sample/FooAlias", nullable = false) { mockClassType("org/sample/Foo") }, mockTAType("org/sample/FooAlias", nullable = false) { mockClassType("org/sample/Foo") }, - mockTAType("org/sample/FooAlias", nullable = true) { mockClassType("org/sample/Foo") }, - shouldFailOnFirstVariant = true + mockTAType("org/sample/FooAlias", nullable = true) { mockClassType("org/sample/Foo") } ) @Test(expected = IllegalCommonizerStateException::class) @@ -411,7 +412,6 @@ class TypeCommonizerTest : AbstractCommonizerTest() { mockTAType("org/sample/FooAlias", nullable = true) { mockClassType("org/sample/Foo") }, mockTAType("org/sample/FooAlias", nullable = true) { mockClassType("org/sample/Foo") }, mockTAType("org/sample/FooAlias", nullable = false) { mockClassType("org/sample/Foo") }, - shouldFailOnFirstVariant = true ) private fun prepareCache(variants: Array) { diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt index 9a08fd6a14a..08d4b420117 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalClassAndTypeAliasCommonizationTest.kt @@ -241,4 +241,33 @@ class HierarchicalClassAndTypeAliasCommonizationTest : AbstractInlineSourcesComm """.trimIndent() ) } + + fun `test typeAlias chain with nullability 2`() { + val result = commonize { + outputTarget("(a, b)") + simpleSingleSourceTarget( + "a", """ + class AB + typealias V = AB + typealias Y = V? + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class AB + typealias V = AB + typealias Y = V + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class AB expect constructor() + expect typealias V = AB + expect class Y + """.trimIndent() + ) + } } diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt index fb7b0987ea6..20121ac60e3 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalPropertyCommonizationTest.kt @@ -106,4 +106,89 @@ class HierarchicalPropertyCommonizationTest : AbstractInlineSourcesCommonization """.trimIndent() ) } + + fun `test typeAliased property and class typed property`() { + val result = commonize { + outputTarget("(a, b)") + simpleSingleSourceTarget( + "a", """ + class AB + typealias TA = AB + val x: TA = TA() + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class AB + val x: AB = AB() + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class AB expect constructor() + expect val x: AB + """.trimIndent() + ) + } + + fun `test class typed property and typeAliased property`() { + val result = commonize { + outputTarget("(a, b)") + simpleSingleSourceTarget( + "a", """ + class AB + val x: AB = AB() + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class AB + typealias TA = AB + val x: TA = TA + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class AB expect constructor() + expect val x: AB + """.trimIndent() + ) + } + + + fun `test single typeAliased property and double typeAliased property`() { + val result = commonize { + outputTarget("(a, b)") + simpleSingleSourceTarget( + "a", """ + class AB + typealias TA_AB = AB + val x: TA_AB = TA_AB() + """.trimIndent() + ) + + simpleSingleSourceTarget( + "b", """ + class AB + typealias TA_AB = AB + typealias TA_B = TA_AB + val x: TA_B = TA_B() + """.trimIndent() + ) + } + + result.assertCommonized( + "(a, b)", """ + expect class AB expect constructor() + expect typealias TA_AB = AB + expect val x: AB + """.trimIndent() + ) + } }