From 44b7245d2904e5b0a06f97bf00fa26d2f794839b Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Mon, 31 Jan 2022 15:47:11 +0300 Subject: [PATCH 1/3] Use EnumSerializer for explicitly serializable enum instead of auto-generated Resolves Kotlin/kotlinx.serialization#683 Kotlin/kotlinx.serialization#1372 --- .../kotlinx/serialization/internal/Enums.kt | 49 ++++++++++++++--- .../serialization/CachedSerializersTest.kt | 25 +++++++++ .../SerializersLookupEnumTest.kt | 52 ++++++++++++++++++- .../serialization/EnumSerializationTest.kt | 2 +- 4 files changed, 119 insertions(+), 9 deletions(-) diff --git a/core/commonMain/src/kotlinx/serialization/internal/Enums.kt b/core/commonMain/src/kotlinx/serialization/internal/Enums.kt index 1e6201546..2feace988 100644 --- a/core/commonMain/src/kotlinx/serialization/internal/Enums.kt +++ b/core/commonMain/src/kotlinx/serialization/internal/Enums.kt @@ -7,6 +7,7 @@ package kotlinx.serialization.internal import kotlinx.serialization.* import kotlinx.serialization.descriptors.* import kotlinx.serialization.encoding.* +import kotlin.jvm.Volatile /* * Descriptor used for explicitly serializable enums by the plugin. @@ -49,20 +50,54 @@ internal class EnumDescriptor( } } -// Used for enums that are not explicitly serializable by the plugin +@OptIn(ExperimentalSerializationApi::class) +@InternalSerializationApi +internal fun > createSimpleEnumSerializer(serialName: String, values: Array): KSerializer { + return EnumSerializer(serialName, values) +} + +@OptIn(ExperimentalSerializationApi::class) +@InternalSerializationApi +internal fun > createMarkedEnumSerializer( + serialName: String, + values: Array, + names: Array, + annotations: Array?> +): KSerializer { + val descriptor = EnumDescriptor(serialName, values.size) + values.forEachIndexed { i, v -> + val elementName = names.getOrNull(i) ?: v.name + descriptor.addElement(elementName) + annotations.getOrNull(i)?.forEach { + descriptor.pushAnnotation(it) + } + } + + return EnumSerializer(serialName, values, descriptor) +} + +// TODO we can create another class for factories, it may be a copy-paste of this class or a subclass for some `AbstractEnumSerializer` with common `serialize`, `deserialize` and `toString` functions @PublishedApi @OptIn(ExperimentalSerializationApi::class) internal class EnumSerializer>( serialName: String, private val values: Array ) : KSerializer { + @Volatile + private var overriddenDescriptor: SerialDescriptor? = null - override val descriptor: SerialDescriptor = buildSerialDescriptor(serialName, SerialKind.ENUM) { - values.forEach { - val fqn = "$serialName.${it.name}" - val enumMemberDescriptor = buildSerialDescriptor(fqn, StructureKind.OBJECT) - element(it.name, enumMemberDescriptor) - } + internal constructor(serialName: String, values: Array, descriptor: SerialDescriptor) : this(serialName, values) { + overriddenDescriptor = descriptor + } + + override val descriptor: SerialDescriptor by lazy { + overriddenDescriptor ?: createUnmarkedDescriptor(serialName) + } + + private fun createUnmarkedDescriptor(serialName: String): SerialDescriptor { + val d = EnumDescriptor(serialName, values.size) + values.forEach { d.addElement(it.name) } + return d } override fun serialize(encoder: Encoder, value: T) { diff --git a/core/commonTest/src/kotlinx/serialization/CachedSerializersTest.kt b/core/commonTest/src/kotlinx/serialization/CachedSerializersTest.kt index 77fa593e9..e1e7d7ff3 100644 --- a/core/commonTest/src/kotlinx/serialization/CachedSerializersTest.kt +++ b/core/commonTest/src/kotlinx/serialization/CachedSerializersTest.kt @@ -22,11 +22,36 @@ class CachedSerializersTest { @Serializable abstract class Abstract + @Serializable + enum class SerializableEnum {A, B} + + @SerialInfo + annotation class MyAnnotation(val x: Int) + + @Serializable + enum class SerializableMarkedEnum { + @SerialName("first") + @MyAnnotation(1) + C, + @MyAnnotation(2) + D + } + @Test fun testObjectSerializersAreSame() = noJsLegacy { assertSame(Object.serializer(), Object.serializer()) } + @Test + fun testSerializableEnumSerializersAreSame() = noJsLegacy { + assertSame(SerializableEnum.serializer(), SerializableEnum.serializer()) + } + + @Test + fun testSerializableMarkedEnumSerializersAreSame() = noJsLegacy { + assertSame(SerializableMarkedEnum.serializer(), SerializableMarkedEnum.serializer()) + } + @Test fun testSealedSerializersAreSame() = noJsLegacy { assertSame(SealedParent.serializer(), SealedParent.serializer()) diff --git a/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt b/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt index 47433ef7d..c3e3f8c83 100644 --- a/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt +++ b/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt @@ -6,6 +6,7 @@ package kotlinx.serialization import kotlinx.serialization.descriptors.* import kotlinx.serialization.encoding.* +import kotlinx.serialization.internal.EnumSerializer import kotlinx.serialization.test.* import kotlin.test.* @@ -47,9 +48,45 @@ class SerializersLookupEnumTest { @Serializable enum class PlainEnum + @Serializable + enum class SerializableEnum { C, D } + + @Serializable + enum class SerializableMarkedEnum { C, @SerialName("NotD") D } + @Test fun testPlainEnum() { - assertEquals(PlainEnum.serializer(), serializer()) + if (isJsLegacy()) { + assertSame(PlainEnum.serializer()::class, serializer()::class) + } else { + assertSame(PlainEnum.serializer(), serializer()) + } + + if (!isJs()) { + assertIs>(serializer()) + } + } + + @Test + fun testSerializableEnumSerializer() { + assertIs>(SerializableEnum.serializer()) + + if (isJsLegacy()) { + assertSame(SerializableEnum.serializer()::class, serializer()::class) + } else { + assertSame(SerializableEnum.serializer(), serializer()) + } + } + + @Test + fun testSerializableMarkedEnumSerializer() { + assertIs>(SerializableMarkedEnum.serializer()) + + if (isJsLegacy()) { + assertSame(SerializableMarkedEnum.serializer()::class, serializer()::class) + } else { + assertSame(SerializableMarkedEnum.serializer(), serializer()) + } } @Test @@ -63,4 +100,17 @@ class SerializersLookupEnumTest { assertIs(EnumExternalClass.serializer()) assertIs(serializer()) } + + @Test + fun testEnumPolymorphic() { + if (isJvm()) { + assertEquals( + PolymorphicSerializer(EnumPolymorphic::class).descriptor, + serializer().descriptor + ) + } else { + // FIXME serializer is broken for K/JS and K/Native. Remove `assertFails` after fix + assertFails { serializer() } + } + } } diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/EnumSerializationTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/EnumSerializationTest.kt index c7350ceec..a57d452b2 100644 --- a/formats/json-tests/commonTest/src/kotlinx/serialization/EnumSerializationTest.kt +++ b/formats/json-tests/commonTest/src/kotlinx/serialization/EnumSerializationTest.kt @@ -127,7 +127,7 @@ class EnumSerializationTest : JsonTestBase() { fun testStructurallyEqualDescriptors() { val libraryGenerated = Wrapper.serializer().descriptor.getElementDescriptor(0) val codeGenerated = MyEnum2.serializer().descriptor - assertNotEquals(libraryGenerated::class, codeGenerated::class) + assertEquals(libraryGenerated::class, codeGenerated::class) libraryGenerated.assertDescriptorEqualsTo(codeGenerated) } } From 9d7804e4f1bbe2aab9999047e6d4b84a0b98db16 Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Wed, 7 Sep 2022 20:17:46 +0200 Subject: [PATCH 2/3] ~review fixes --- core/commonMain/src/kotlinx/serialization/internal/Enums.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/core/commonMain/src/kotlinx/serialization/internal/Enums.kt b/core/commonMain/src/kotlinx/serialization/internal/Enums.kt index 2feace988..7e940769f 100644 --- a/core/commonMain/src/kotlinx/serialization/internal/Enums.kt +++ b/core/commonMain/src/kotlinx/serialization/internal/Enums.kt @@ -76,14 +76,12 @@ internal fun > createMarkedEnumSerializer( return EnumSerializer(serialName, values, descriptor) } -// TODO we can create another class for factories, it may be a copy-paste of this class or a subclass for some `AbstractEnumSerializer` with common `serialize`, `deserialize` and `toString` functions @PublishedApi @OptIn(ExperimentalSerializationApi::class) internal class EnumSerializer>( serialName: String, private val values: Array ) : KSerializer { - @Volatile private var overriddenDescriptor: SerialDescriptor? = null internal constructor(serialName: String, values: Array, descriptor: SerialDescriptor) : this(serialName, values) { From 62225f4872c82c71374ddb293dfe1830a63635a7 Mon Sep 17 00:00:00 2001 From: "Sergey.Shanshin" Date: Thu, 22 Sep 2022 19:36:27 +0200 Subject: [PATCH 3/3] fixes --- .../serialization/SerializersLookupEnumTest.kt | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt b/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt index c3e3f8c83..8c0f65677 100644 --- a/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt +++ b/core/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt @@ -100,17 +100,4 @@ class SerializersLookupEnumTest { assertIs(EnumExternalClass.serializer()) assertIs(serializer()) } - - @Test - fun testEnumPolymorphic() { - if (isJvm()) { - assertEquals( - PolymorphicSerializer(EnumPolymorphic::class).descriptor, - serializer().descriptor - ) - } else { - // FIXME serializer is broken for K/JS and K/Native. Remove `assertFails` after fix - assertFails { serializer() } - } - } }