Use sealed interfaces for forward-compatible enums

Resolves: #467

protocol-generator/cdp-kotlin-generator/src/main/kotlin/org/hildan/chrome/devtools/protocol/generator/DomainTypesGenerator.kt

@@ -2,8 +2,6 @@ package org.hildan.chrome.devtools.protocol.generator
 
 import com.squareup.kotlinpoet.*
 import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
-import kotlinx.serialization.json.JsonElement
-import kotlinx.serialization.json.JsonTransformingSerializer
 import org.hildan.chrome.devtools.protocol.model.ChromeDPDomain
 import org.hildan.chrome.devtools.protocol.model.ChromeDPType
 import org.hildan.chrome.devtools.protocol.model.DomainTypeDeclaration
@@ -20,7 +18,7 @@ fun ChromeDPDomain.createDomainTypesFileSpec(): FileSpec =
 private fun FileSpec.Builder.addDomainType(typeDeclaration: DomainTypeDeclaration, experimentalDomain: Boolean) {
     when (val type = typeDeclaration.type) {
         is ChromeDPType.Object -> addType(typeDeclaration.toDataClassTypeSpec(type))
-        is ChromeDPType.Enum -> addType(typeDeclaration.toEnumTypeSpec(type, experimentalDomain))
+        is ChromeDPType.Enum -> addTypes(typeDeclaration.toEnumAndSerializerTypeSpecs(type, experimentalDomain))
         is ChromeDPType.NamedRef -> addTypeAlias(typeDeclaration.toTypeAliasSpec(type))
     }
 }
@@ -33,80 +31,106 @@ private fun DomainTypeDeclaration.toDataClassTypeSpec(type: ChromeDPType.Object)
         addPrimaryConstructorProps(type.properties)
     }.build()
 
-private fun DomainTypeDeclaration.toEnumTypeSpec(type: ChromeDPType.Enum, experimentalDomain: Boolean): TypeSpec =
+private fun DomainTypeDeclaration.toEnumAndSerializerTypeSpecs(type: ChromeDPType.Enum, experimentalDomain: Boolean): List<TypeSpec> =
+    if (experimental || experimentalDomain) {
+        val serializerTypeSpec = serializerForFCEnum(names.className, type.enumValues)
+        val serializerClass = ClassName(names.packageName, serializerTypeSpec.name!!)
+        listOf(serializerTypeSpec, toFCEnumTypeSpec(type, serializerClass))
+    } else {
+        listOf(toStableEnumTypeSpec(type))
+    }
+
+private fun DomainTypeDeclaration.toStableEnumTypeSpec(type: ChromeDPType.Enum): TypeSpec =
     TypeSpec.enumBuilder(names.className).apply {
-        addKDocAndStabilityAnnotations(element = this@toEnumTypeSpec)
+        addKDocAndStabilityAnnotations(element = this@toStableEnumTypeSpec)
         type.enumValues.forEach {
             addEnumConstant(
-                name = it.dashesToCamelCase(),
+                name = protocolEnumEntryNameToKotlinName(it),
                 typeSpec = TypeSpec.anonymousClassBuilder().addAnnotation(Annotations.serialName(it)).build()
             )
         }
-        if (experimental || experimentalDomain) {
-            require(type.enumValues.none { it.equals(UndefinedEnumEntryName, ignoreCase = true) }) {
-                "Cannot synthesize the '$UndefinedEnumEntryName' value for experimental enum " +
-                    "${names.declaredName} (of domain ${names.domain.domainName}) because it clashes with an " +
-                    "existing value (case-insensitive). Values:\n - ${type.enumValues.joinToString("\n - ")}"
-            }
-            addEnumConstant(
-                name = UndefinedEnumEntryName,
-                typeSpec = TypeSpec.anonymousClassBuilder()
-                    .addKdoc("This extra enum entry represents values returned by Chrome that were not defined in " +
-                                 "the protocol (for instance new values that were added later).")
-                    .build(),
-            )
+        addAnnotation(Annotations.serializable)
+    }.build()
 
-            // see https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/json.md#extending-the-behavior-of-the-plugin-generated-serializer
-            val enumSerializer = serializerForEnumWithUnknown(names.className, type.enumValues)
-            addType(enumSerializer)
+private fun DomainTypeDeclaration.toFCEnumTypeSpec(type: ChromeDPType.Enum, serializerClass: ClassName): TypeSpec =
+    TypeSpec.interfaceBuilder(names.className).apply {
+        addModifiers(KModifier.SEALED)
+        addKDocAndStabilityAnnotations(element = this@toFCEnumTypeSpec)
+        addAnnotation(Annotations.serializableWith(serializerClass))
 
-            val serializerClass = names.className.nestedClass(enumSerializer.name!!)
-            addAnnotation(Annotations.serializableWith(serializerClass))
-            addAnnotation(Annotations.keepGeneratedSerializer)
-        } else {
-            addAnnotation(Annotations.serializable)
+        type.enumValues.forEach {
+            addType(TypeSpec.objectBuilder(protocolEnumEntryNameToKotlinName(it)).apply {
+                addModifiers(KModifier.DATA)
+                addSuperinterface(names.className)
+
+                // For calls to serializers made directly with this sub-object instead of the FC enum's interface.
+                // Example: Json.encodeToString(AXPropertyName.url)
+                // (and not Json.encodeToString<AXPropertyName>(AXPropertyName.url))
+                addAnnotation(Annotations.serializableWith(serializerClass))
+            }.build())
+        }
+        require(type.enumValues.none { it.equals(UndefinedEnumEntryName, ignoreCase = true) }) {
+            "Cannot synthesize the '$UndefinedEnumEntryName' value for experimental enum " +
+                "${names.declaredName} (of domain ${names.domain.domainName}) because it clashes with an " +
+                "existing value (case-insensitive). Values:\n - ${type.enumValues.joinToString("\n - ")}"
         }
+        addType(notInProtocolClassTypeSpec(serializerClass))
     }.build()
 
-private fun String.dashesToCamelCase(): String = replace(Regex("""-(\w)""")) { it.groupValues[1].uppercase() }
+private fun DomainTypeDeclaration.notInProtocolClassTypeSpec(serializerClass: ClassName) =
+    TypeSpec.classBuilder(UndefinedEnumEntryName).apply {
+        addModifiers(KModifier.DATA)
+        addSuperinterface(names.className)
+
+        // For calls to serializers made directly with this sub-object instead of the FC enum's interface.
+        // Example: Json.encodeToString(AXPropertyName.NotDefinedInProtocol("notRendered"))
+        // (and not Json.encodeToString<AXPropertyName>(AXPropertyName.NotDefinedInProtocol("notRendered"))
+        addAnnotation(Annotations.serializableWith(serializerClass))
 
-private fun serializerForEnumWithUnknown(enumClass: ClassName, enumValues: List<String>): TypeSpec =
-    TypeSpec.objectBuilder("SerializerWithUnknown").apply {
-        addModifiers(KModifier.INTERNAL)
-        superclass(JsonTransformingSerializer::class.asClassName().parameterizedBy(enumClass))
-        addSuperclassConstructorParameter("%T.generatedSerializer()", enumClass)
-        addProperty(knownValuesProperty(enumValues))
-        addFunction(FunSpec.builder("transformDeserialize").apply {
+        addKdoc(
+            "This extra enum entry represents values returned by Chrome that were not defined in " +
+                "the protocol (for instance new values that were added later)."
+        )
+        primaryConstructor(FunSpec.constructorBuilder().apply {
+            addParameter("value", String::class)
+        }.build())
+        addProperty(PropertySpec.builder("value", String::class).initializer("value").build())
+    }.build()
+
+private fun serializerForFCEnum(fcEnumClass: ClassName, enumValues: List<String>): TypeSpec =
+    TypeSpec.objectBuilder("${fcEnumClass.simpleName}Serializer").apply {
+        addModifiers(KModifier.PRIVATE)
+        superclass(ExtDeclarations.fcEnumSerializer.parameterizedBy(fcEnumClass))
+        addSuperclassConstructorParameter("%T::class", fcEnumClass)
+
+        addFunction(FunSpec.builder("fromCode").apply {
             addModifiers(KModifier.OVERRIDE)
-            addParameter("element", JsonElement::class)
-            returns(JsonElement::class)
-            addStatement("val jsonValue = element.%M.content", ExtDeclarations.Serialization.jsonPrimitiveExtension)
-            addStatement(
-                format = "return if (jsonValue in knownValues) element else %M(%S)",
-                ExtDeclarations.Serialization.JsonPrimitiveFactory,
-                UndefinedEnumEntryName,
-            )
+            addParameter("code", String::class)
+            returns(fcEnumClass)
+            beginControlFlow("return when (code)")
+            enumValues.forEach {
+                addCode("%S -> %T\n", it, fcEnumClass.nestedClass(protocolEnumEntryNameToKotlinName(it)))
+            }
+            addCode("else -> %T(code)", fcEnumClass.nestedClass(UndefinedEnumEntryName))
+            endControlFlow()
         }.build())
-        addFunction(FunSpec.builder("transformSerialize").apply {
+
+        addFunction(FunSpec.builder("codeOf").apply {
             addModifiers(KModifier.OVERRIDE)
-            addParameter("element", JsonElement::class)
-            returns(JsonElement::class)
-            addStatement("val jsonValue = element.%M.content", ExtDeclarations.Serialization.jsonPrimitiveExtension)
-            addStatement(
-                format = "require(jsonValue in knownValues) { %S }",
-                "Cannot serialize the '$UndefinedEnumEntryName' enum value placeholder for $enumClass. " +
-                    "Please use use one of the following valid values instead: $enumValues",
-            )
-            addStatement("return element")
+            addParameter("value", fcEnumClass)
+            returns(String::class)
+            beginControlFlow("return when (value)")
+            enumValues.forEach {
+                addCode("is %T -> %S\n", fcEnumClass.nestedClass(protocolEnumEntryNameToKotlinName(it)), it)
+            }
+            addCode("is %T -> value.value", fcEnumClass.nestedClass(UndefinedEnumEntryName))
+            endControlFlow()
         }.build())
     }.build()
 
-private fun knownValuesProperty(values: List<String>): PropertySpec =
-    PropertySpec.builder("knownValues", SET.parameterizedBy(String::class.asTypeName())).apply {
-        addModifiers(KModifier.INTERNAL)
-        val format = List(values.size) { "%S" }.joinToString(separator = ", ", prefix = "setOf(", postfix = ")")
-        initializer(format = format, *values.toTypedArray())
-    }.build()
+private fun protocolEnumEntryNameToKotlinName(protocolName: String) = protocolName.dashesToCamelCase()
+
+private fun String.dashesToCamelCase(): String = replace(Regex("""-(\w)""")) { it.groupValues[1].uppercase() }
 
 private fun DomainTypeDeclaration.toTypeAliasSpec(type: ChromeDPType.NamedRef): TypeAliasSpec =
     TypeAliasSpec.builder(names.declaredName, type.typeName).apply {

protocol-generator/cdp-kotlin-generator/src/main/kotlin/org/hildan/chrome/devtools/protocol/names/ExternalDeclarations.kt

@@ -27,22 +27,19 @@ object ExtDeclarations {
 
     val experimentalChromeApi = ClassName(protocolPackage, "ExperimentalChromeApi")
 
+    val fcEnumSerializer = ClassName(protocolPackage, "FCEnumSerializer")
+
     val allDomainsTargetInterface = ClassName(targetsPackage, "AllDomainsTarget")
     val allDomainsTargetImplementation = ClassName(targetsPackage, "UberTarget")
 
-    val sessionsFileName = "ChildSessions"
-    val sessionAdaptersFileName = "ChildSessionAdapters"
+    const val sessionsFileName = "ChildSessions"
+    const val sessionAdaptersFileName = "ChildSessionAdapters"
     val childSessionInterface = ClassName(sessionsPackage, "ChildSession")
     val childSessionUnsafeFun = childSessionInterface.member("unsafe")
 
     fun targetInterface(target: TargetType): ClassName = ClassName(targetsPackage, "${target.kotlinName}Target")
     fun sessionInterface(target: TargetType): ClassName = ClassName(sessionsPackage, "${target.kotlinName}Session")
     fun sessionAdapter(target: TargetType): ClassName = ClassName(sessionsPackage, "${target.kotlinName}SessionAdapter")
-
-    object Serialization {
-        val jsonPrimitiveExtension = MemberName("kotlinx.serialization.json", "jsonPrimitive")
-        val JsonPrimitiveFactory = MemberName("kotlinx.serialization.json", "JsonPrimitive")
-    }
 }
 
 object Annotations {
@@ -55,9 +52,6 @@ object Annotations {
         .addMember("with = %T::class", serializerClass)
         .build()
 
-    @OptIn(ExperimentalSerializationApi::class)
-    val keepGeneratedSerializer = AnnotationSpec.builder(KeepGeneratedSerializer::class).build()
-
     val jvmOverloads = AnnotationSpec.builder(JvmOverloads::class).build()
 
     val deprecatedChromeApi = AnnotationSpec.builder(Deprecated::class)
@@ -80,6 +74,12 @@ object Annotations {
         // annotating the relevant property/constructor-arg with experimental annotation. The whole class/constructor
         // would need to be annotated as experimental, which is not desirable
         "OPT_IN_USAGE",
+        // we add @SerializableWith on each sub-object in forward-compatible enum interfaces to avoid issues when using
+        // the serializers of the subtypes directly. The serialization plugin complains because we're using the parent
+        // interface serializer on each subtype instead of a KSerializer<Subtype>, which is technically not safe in
+        // general. We accept the tradeoff in our case, which is that this will throw ClassCaseException:
+        // val value: AXPropertyName.level = Json.decodeFromString<AXPropertyName.level>("\"url\"")
+        "SERIALIZER_TYPE_INCOMPATIBLE",
     )
 
     @Suppress("SameParameterValue")

src/commonMain/kotlin/org/hildan/chrome/devtools/protocol/FCEnumSerializer.kt

@@ -0,0 +1,23 @@
+package org.hildan.chrome.devtools.protocol
+
+import kotlinx.serialization.*
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
+import kotlin.reflect.*
+
+abstract class FCEnumSerializer<FC : Any>(fcClass: KClass<FC>) : KSerializer<FC> {
+
+    override val descriptor = PrimitiveSerialDescriptor(
+        serialName = fcClass.simpleName ?: error("Cannot create serializer for anonymous class"),
+        kind = PrimitiveKind.STRING,
+    )
+
+    override fun deserialize(decoder: Decoder): FC = fromCode(decoder.decodeString())
+
+    override fun serialize(encoder: Encoder, value: FC) {
+        encoder.encodeString(codeOf(value))
+    }
+
+    abstract fun fromCode(code: String): FC
+    abstract fun codeOf(value: FC): String
+}

src/commonTest/kotlin/org/hildan/chrome/devtools/protocol/FCEnumSerializerTest.kt

@@ -0,0 +1,58 @@
+package org.hildan.chrome.devtools.protocol
+
+import kotlinx.serialization.*
+import kotlinx.serialization.json.*
+import org.hildan.chrome.devtools.domains.accessibility.*
+import org.hildan.chrome.devtools.domains.bluetoothemulation.*
+import kotlin.test.*
+
+class FCEnumSerializerTest {
+
+    @Test
+    fun deserializesKnownValues() {
+        assertEquals(AXPropertyName.url, Json.decodeFromString<AXPropertyName>("\"url\""))
+        assertEquals(AXPropertyName.level, Json.decodeFromString<AXPropertyName>("\"level\""))
+        assertEquals(AXPropertyName.hiddenRoot, Json.decodeFromString<AXPropertyName>("\"hiddenRoot\""))
+    }
+
+    @Test
+    fun deserializesKnownValues_withDashes() {
+        assertEquals(CentralState.poweredOn, Json.decodeFromString<CentralState>("\"powered-on\""))
+        assertEquals(CentralState.poweredOff, Json.decodeFromString<CentralState>("\"powered-off\""))
+    }
+
+    @Test
+    fun deserializesUnknownValues() {
+        assertEquals(AXPropertyName.NotDefinedInProtocol("notRendered"), Json.decodeFromString<AXPropertyName>("\"notRendered\""))
+        assertEquals(AXPropertyName.NotDefinedInProtocol("uninteresting"), Json.decodeFromString<AXPropertyName>("\"uninteresting\""))
+    }
+
+    @Test
+    fun serializesKnownValues() {
+        assertEquals("\"url\"", Json.encodeToString(AXPropertyName.url))
+        assertEquals("\"level\"", Json.encodeToString(AXPropertyName.level))
+        assertEquals("\"hiddenRoot\"", Json.encodeToString(AXPropertyName.hiddenRoot))
+    }
+
+    @Test
+    fun serializesKnownValues_withDashes() {
+        assertEquals("\"powered-on\"", Json.encodeToString(CentralState.poweredOn))
+        assertEquals("\"powered-off\"", Json.encodeToString(CentralState.poweredOff))
+    }
+
+    @Test
+    fun serializesUnknownValues() {
+        assertEquals("\"notRendered\"", Json.encodeToString<AXPropertyName>(AXPropertyName.NotDefinedInProtocol("notRendered")))
+        assertEquals("\"uninteresting\"", Json.encodeToString(AXPropertyName.NotDefinedInProtocol("uninteresting")))
+        assertEquals("\"totallyInexistentStuff\"", Json.encodeToString(AXPropertyName.NotDefinedInProtocol("totallyInexistentStuff")))
+    }
+
+    @Test
+    fun failsOnWeirdTypeUsage() {
+        assertFailsWith<ClassCastException> {
+            // not unused: the explicit type is necessary to throw the exception
+            @Suppress("UNUSED_VARIABLE")
+            val value: AXPropertyName.level = Json.decodeFromString<AXPropertyName.level>("\"url\"")
+        }
+    }
+}

src/jvmTest/kotlin/IntegrationTests.kt

@@ -148,13 +148,21 @@ class IntegrationTests {
                             n.properties.anyUndefinedName() || n.ignoredReasons.anyUndefinedName()
                         }
                     }
+                    tree.nodes.forEach {
+                        it.properties?.forEach { prop ->
+                            println(prop.name)
+                        }
+                        it.ignoredReasons?.forEach { prop ->
+                            println(prop.name)
+                        }
+                    }
                 }
             }
         }
     }
 
     private fun List<AXProperty>?.anyUndefinedName(): Boolean =
-        this != null && this.any { it.name == AXPropertyName.NotDefinedInProtocol }
+        this != null && this.any { it.name is AXPropertyName.NotDefinedInProtocol }
 
     @OptIn(ExperimentalChromeApi::class)
     @Test