Change priority for PolymorphicSerializer inside serializer() function

Now, we look up serializers for non-sealed interfaces in SerializersModule first,
and only then unconditionally return PolymorphicSerializer.
This is done because with old behavior, it was impossible to override interface serializer with context, as interfaces were treated as always having PolymorphicSerializer.

This is a behavioral change, although impact should be minimal, as for most usecases (without modules), serializer will be the same and special workarounds are performed so cache would also work as expected.

Note that KClass.serializer() function will no longer return PolymorphicSerializer for interfaces at all and return null instead. It is OK, because this function is marked with @InternalSerializationApi, and we can change its behavior.

Intrinsics in plugin with changed functionality are expected to be merged in 2.0.0-RC.

Fixes #2060

Author: sandwwraith

core/api/kotlinx-serialization-core.api

@@ -120,6 +120,8 @@ public abstract interface annotation class kotlinx/serialization/Serializer : ja
 }
 
 public final class kotlinx/serialization/SerializersKt {
+	public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
+	public static final fun moduleThenPolymorphic (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;
 	public static final fun noCompiledSerializer (Ljava/lang/String;)Lkotlinx/serialization/KSerializer;
 	public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;)Lkotlinx/serialization/KSerializer;
 	public static final fun noCompiledSerializer (Lkotlinx/serialization/modules/SerializersModule;Lkotlin/reflect/KClass;[Lkotlinx/serialization/KSerializer;)Lkotlinx/serialization/KSerializer;

core/commonMain/src/kotlinx/serialization/Serializers.kt

@@ -189,24 +189,38 @@ private fun SerializersModule.serializerByKTypeImpl(
     val isNullable = type.isMarkedNullable
     val typeArguments = type.arguments.map(KTypeProjection::typeOrThrow)
 
-    val cachedSerializer = if (typeArguments.isEmpty()) {
+    val cachedSerializer = if (this.hasInterfaceContextualSerializers) {
+        null
+    } else if (typeArguments.isEmpty()) {
         findCachedSerializer(rootClass, isNullable)
     } else {
-        findParametrizedCachedSerializer(rootClass, typeArguments, isNullable).getOrNull()
+        findParametrizedCachedSerializer(
+            rootClass,
+            typeArguments,
+            isNullable
+        ).getOrNull()
+    }
+    if (cachedSerializer != null) return cachedSerializer
+    var polymorphicFallback: KSerializer<Any?>? = null
+    if (rootClass.isInterface()) {
+        // Note that there is no check if interface is sealed, because for sealed interfaces we always have their serializers in cache
+        // except for one rare case with named companion (see SerializersLookupNamedCompanionTest.SealedInterfaceWithExplicitAnnotation).
+        // That case always returned incorrect PolymorphicSerializer instead of SealedClassSerializer, even before 1.9.20.
+        polymorphicFallback = PolymorphicSerializer(rootClass).cast()
     }
-    cachedSerializer?.let { return it }
 
     // slow path to find contextual serializers in serializers module
     val contextualSerializer: KSerializer<out Any?>? = if (typeArguments.isEmpty()) {
-        getContextual(rootClass)
+        rootClass.serializerOrNull()
+            ?: getContextual(rootClass)
+            ?: polymorphicFallback
     } else {
         val serializers = serializersForParameters(typeArguments, failOnMissingTypeArgSerializer) ?: return null
         // first, we look among the built-in serializers, because the parameter could be contextual
         rootClass.parametrizedSerializerOrNull(serializers) { typeArguments[0].classifier }
-            ?: getContextual(
-                rootClass,
-                serializers
-            )
+            ?: getContextual(rootClass, serializers)
+            // PolymorphicSerializer always is returned even for Interface<T>, although it rarely works as expected.
+            ?: polymorphicFallback
     }
     return contextualSerializer?.cast<Any>()?.nullable(isNullable)
 }
@@ -376,3 +390,24 @@ internal fun noCompiledSerializer(
 ): KSerializer<*> {
     return module.getContextual(kClass, argSerializers.asList()) ?: kClass.serializerNotRegistered()
 }
+
+/**
+ * Overloads of [moduleThenPolymorphic] should never be called directly.
+ * Instead, compiler inserts calls to them when intrinsifying [serializer] function.
+ *
+ * If no request KClass is an interface, plugin performs call to [moduleThenPolymorphic] to achieve special behavior for interface serializers.
+ * (They are only serializers that have module priority over default [PolymorphicSerializer]).
+ */
+@OptIn(ExperimentalSerializationApi::class)
+@Suppress("unused")
+@PublishedApi
+internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>): KSerializer<*> {
+    return module.getContextual(kClass) ?: PolymorphicSerializer(kClass)
+}
+
+@OptIn(ExperimentalSerializationApi::class)
+@Suppress("unused")
+@PublishedApi
+internal fun moduleThenPolymorphic(module: SerializersModule, kClass: KClass<*>, argSerializers: Array<KSerializer<*>>): KSerializer<*> {
+    return module.getContextual(kClass, argSerializers.asList()) ?: PolymorphicSerializer(kClass)
+}

core/commonMain/src/kotlinx/serialization/SerializersCache.kt

@@ -5,6 +5,7 @@
 package kotlinx.serialization
 
 import kotlinx.serialization.builtins.nullable
+import kotlinx.serialization.internal.*
 import kotlinx.serialization.internal.cast
 import kotlinx.serialization.internal.createCache
 import kotlinx.serialization.internal.createParametrizedCache
@@ -18,13 +19,13 @@ import kotlin.reflect.KType
  * Cache for non-null non-parametrized and non-contextual serializers.
  */
 @ThreadLocal
-private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() }
+private val SERIALIZERS_CACHE = createCache { it.serializerOrNull() ?: it.polymorphicIfInterface() }
 
 /**
  * Cache for nullable non-parametrized and non-contextual serializers.
  */
 @ThreadLocal
-private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { it.serializerOrNull()?.nullable?.cast() }
+private val SERIALIZERS_CACHE_NULLABLE = createCache<Any?> { (it.serializerOrNull() ?: it.polymorphicIfInterface())?.nullable?.cast() }
 
 /**
  * Cache for non-null parametrized and non-contextual serializers.
@@ -72,3 +73,5 @@ internal fun findParametrizedCachedSerializer(
         PARAMETRIZED_SERIALIZERS_CACHE_NULLABLE.get(clazz, types)
     }
 }
+
+internal fun KClass<*>.polymorphicIfInterface() = if (this.isInterface()) PolymorphicSerializer(this) else null

core/commonMain/src/kotlinx/serialization/internal/Platform.common.kt

@@ -141,6 +141,8 @@ internal expect fun BooleanArray.getChecked(index: Int): Boolean
 
 internal expect fun <T : Any> KClass<T>.compiledSerializerImpl(): KSerializer<T>?
 
+internal expect fun <T: Any> KClass<T>.isInterface(): Boolean
+
 /**
  * Create serializers cache for non-parametrized and non-contextual serializers.
  * The activity and type of cache is determined for a specific platform and a specific environment.

core/commonMain/src/kotlinx/serialization/modules/SerializersModule.kt

@@ -67,6 +67,9 @@ public sealed class SerializersModule {
      */
     @ExperimentalSerializationApi
     public abstract fun dumpTo(collector: SerializersModuleCollector)
+
+    @InternalSerializationApi
+    internal abstract val hasInterfaceContextualSerializers: Boolean
 }
 
 /**
@@ -76,7 +79,14 @@ public sealed class SerializersModule {
     level = DeprecationLevel.WARNING,
     replaceWith = ReplaceWith("EmptySerializersModule()"))
 @JsName("EmptySerializersModuleLegacyJs") // Compatibility with JS
-public val EmptySerializersModule: SerializersModule = SerialModuleImpl(emptyMap(), emptyMap(), emptyMap(), emptyMap(), emptyMap())
+public val EmptySerializersModule: SerializersModule = SerialModuleImpl(
+    emptyMap(),
+    emptyMap(),
+    emptyMap(),
+    emptyMap(),
+    emptyMap(),
+    false
+)
 
 /**
  * Returns a combination of two serial modules
@@ -147,7 +157,8 @@ internal class SerialModuleImpl(
     @JvmField val polyBase2Serializers: Map<KClass<*>, Map<KClass<*>, KSerializer<*>>>,
     private val polyBase2DefaultSerializerProvider: Map<KClass<*>, PolymorphicSerializerProvider<*>>,
     private val polyBase2NamedSerializers: Map<KClass<*>, Map<String, KSerializer<*>>>,
-    private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>
+    private val polyBase2DefaultDeserializerProvider: Map<KClass<*>, PolymorphicDeserializerProvider<*>>,
+    internal override val hasInterfaceContextualSerializers: Boolean
 ) : SerializersModule() {
 
     override fun <T : Any> getPolymorphic(baseClass: KClass<in T>, value: T): SerializationStrategy<T>? {

core/commonMain/src/kotlinx/serialization/modules/SerializersModuleBuilders.kt

@@ -49,6 +49,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
     private val polyBase2DefaultSerializerProvider: MutableMap<KClass<*>, PolymorphicSerializerProvider<*>> = hashMapOf()
     private val polyBase2NamedSerializers: MutableMap<KClass<*>, MutableMap<String, KSerializer<*>>> = hashMapOf()
     private val polyBase2DefaultDeserializerProvider: MutableMap<KClass<*>, PolymorphicDeserializerProvider<*>> = hashMapOf()
+    private var hasInterfaceContextualSerializers: Boolean = false
 
     /**
      * Adds [serializer] associated with given [kClass] for contextual serialization.
@@ -155,6 +156,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
             }
         }
         class2ContextualProvider[forClass] = provider
+        if (forClass.isInterface()) hasInterfaceContextualSerializers = true
     }
 
     @JvmName("registerDefaultPolymorphicSerializer") // Don't mangle method name for prettier stack traces
@@ -229,7 +231,7 @@ public class SerializersModuleBuilder @PublishedApi internal constructor() : Ser
 
     @PublishedApi
     internal fun build(): SerializersModule =
-        SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider)
+        SerialModuleImpl(class2ContextualProvider, polyBase2Serializers, polyBase2DefaultSerializerProvider, polyBase2NamedSerializers, polyBase2DefaultDeserializerProvider, hasInterfaceContextualSerializers)
 }
 
 /**

core/commonTest/src/kotlinx/serialization/InterfaceContextualSerializerTest.kt

@@ -0,0 +1,182 @@
+/*
+ * Copyright 2017-2023 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.serialization
+
+import kotlinx.serialization.builtins.*
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
+import kotlinx.serialization.modules.*
+import kotlinx.serialization.test.*
+import kotlin.reflect.*
+import kotlin.test.*
+
+// Imagine this is a 3rd party interface
+interface IApiError {
+    val code: Int
+}
+
+@Serializable(CustomSer::class)
+interface HasCustom
+
+
+object CustomSer: KSerializer<HasCustom> {
+    override val descriptor: SerialDescriptor
+        get() = TODO("Not yet implemented")
+
+    override fun serialize(encoder: Encoder, value: HasCustom) {
+        TODO("Not yet implemented")
+    }
+
+    override fun deserialize(decoder: Decoder): HasCustom {
+        TODO("Not yet implemented")
+    }
+}
+
+@Suppress("UNCHECKED_CAST")
+class InterfaceContextualSerializerTest {
+
+    object MyApiErrorSerializer : KSerializer<IApiError> {
+        override val descriptor: SerialDescriptor = PrimitiveSerialDescriptor("IApiError", PrimitiveKind.INT)
+
+        override fun serialize(encoder: Encoder, value: IApiError) {
+            encoder.encodeInt(value.code)
+        }
+
+        override fun deserialize(decoder: Decoder): IApiError {
+            val code = decoder.decodeInt()
+            return object : IApiError {
+                override val code: Int = code
+            }
+        }
+    }
+
+    private inline fun <reified T> SerializersModule.doTest(block: (KSerializer<T>) -> Unit) {
+        block(this.serializer<T>())
+        block(this.serializer(typeOf<T>()) as KSerializer<T>)
+    }
+
+    // Native, WASM - can't retrieve serializer (no .isInterface)
+    @Test
+    fun testDefault() {
+        if (isNative() || isWasm()) return
+        assertEquals(PolymorphicKind.OPEN, serializer<IApiError>().descriptor.kind)
+        assertEquals(PolymorphicKind.OPEN, serializer(typeOf<IApiError>()).descriptor.kind)
+    }
+
+    @Test
+    fun testCustom() {
+        assertSame(CustomSer, serializer<HasCustom>())
+        assertSame(CustomSer, serializer(typeOf<HasCustom>()) as KSerializer<HasCustom>)
+    }
+
+    // JVM - intrinsics kick in
+    @Test
+    fun testContextual() {
+        val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
+        assertSame(MyApiErrorSerializer, module.serializer(typeOf<IApiError>()) as KSerializer<IApiError>)
+        shouldFail<AssertionError>(beforeKotlin = "2.0.0", onJvm = true, onWasm = false, onNative = false, onJs = false ) {
+            assertSame(MyApiErrorSerializer, module.serializer<IApiError>())
+        }
+    }
+
+    // JVM - intrinsics kick in
+    @Test
+    fun testInsideList() {
+        val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
+        assertEquals(MyApiErrorSerializer.descriptor, module.serializer(typeOf<List<IApiError>>()).descriptor.elementDescriptors.first())
+        shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
+            assertEquals(
+                MyApiErrorSerializer.descriptor,
+                module.serializer<List<IApiError>>().descriptor.elementDescriptors.first()
+            )
+        }
+    }
+
+    @Test
+    fun testWithNullability() {
+        val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
+        assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer(typeOf<IApiError?>()).descriptor)
+        shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
+            assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer<IApiError?>().descriptor)
+        }
+    }
+
+    @Test
+    fun testWithNullabilityInsideList() {
+        val module = serializersModuleOf(IApiError::class, MyApiErrorSerializer)
+        assertEquals(MyApiErrorSerializer.nullable.descriptor, module.serializer(typeOf<List<IApiError?>>()).descriptor.elementDescriptors.first())
+        shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
+            assertEquals(
+                MyApiErrorSerializer.nullable.descriptor,
+                module.serializer<List<IApiError?>>().descriptor.elementDescriptors.first()
+            )
+        }
+    }
+
+    class Unrelated
+
+    object UnrelatedSerializer: KSerializer<Unrelated> {
+        override val descriptor: SerialDescriptor
+            get() = TODO("Not yet implemented")
+
+        override fun serialize(encoder: Encoder, value: Unrelated) {
+            TODO("Not yet implemented")
+        }
+
+        override fun deserialize(decoder: Decoder): Unrelated {
+            TODO("Not yet implemented")
+        }
+    }
+
+    @Test
+    fun interfaceSerializersAreCachedInsideIfModuleIsNotFilledWithInterface() = jvmOnly {
+        // Caches are implemented on JVM
+        val module = serializersModuleOf(Unrelated::class, UnrelatedSerializer)
+        val p1 = module.serializer(typeOf<List<IApiError>>())
+        assertEquals(PolymorphicKind.OPEN, p1.descriptor.elementDescriptors.first().kind)
+        val p2 = module.serializer(typeOf<List<IApiError>>())
+        assertSame(p1, p2)
+    }
+
+    @Test
+    fun interfaceSerializersAreCachedTopLevelIfModuleIsNotFilledWithInterface() = jvmOnly {
+        val module = serializersModuleOf(Unrelated::class, UnrelatedSerializer)
+        val p1 = module.serializer(typeOf<IApiError>())
+        assertEquals(PolymorphicKind.OPEN, p1.descriptor.kind)
+        val p2 = module.serializer(typeOf<IApiError>())
+        assertSame(p1, p2)
+    }
+
+    interface Parametrized<T> {
+        val param: List<T>
+    }
+
+    class PSer<T>(val tSer: KSerializer<T>): KSerializer<Parametrized<T>> {
+        override val descriptor: SerialDescriptor
+            get() = buildClassSerialDescriptor("PSer<${tSer.descriptor.serialName}>")
+
+        override fun serialize(encoder: Encoder, value: Parametrized<T>) {
+            TODO("Not yet implemented")
+        }
+
+        override fun deserialize(decoder: Decoder): Parametrized<T> {
+            TODO("Not yet implemented")
+        }
+    }
+
+    @Test
+    fun testParametrizedInterface() {
+        if (!isNative() && !isWasm()) {
+            assertEquals(PolymorphicKind.OPEN, serializer(typeOf<Parametrized<String>>()).descriptor.kind)
+        }
+        val md = SerializersModule {
+            contextual(Parametrized::class) { PSer(it[0]) }
+        }
+        assertEquals("PSer<kotlin.String>", md.serializer(typeOf<Parametrized<String>>()).descriptor.serialName)
+        shouldFail<AssertionError>(beforeKotlin = "2.0.0", onWasm = false, onNative = false, onJs = false ) {
+            assertEquals("PSer<kotlin.String>", md.serializer<Parametrized<String>>().descriptor.serialName)
+        }
+    }
+}

core/commonTest/src/kotlinx/serialization/SerializersLookupNamedCompanionTest.kt

@@ -89,12 +89,15 @@ class SerializersLookupNamedCompanionTest {
             )
         }
 
-        // should fail because annotation @NamedCompanion will be placed again by the compilation plugin
+        // Will return incorrect `PolymorphicSerializer` on JVM as before 1.9.20
+        // because annotation @NamedCompanion will be placed again by the compilation plugin
         // and they both will be placed into @Container annotation - thus they will be invisible to the runtime
-        shouldFail<SerializationException>(sinceKotlin = "1.9.20", onJs = false, onNative = false, onWasm = false) {
-            serializer(typeOf<SealedInterfaceWithExplicitAnnotation>())
-        }
+        // On other platforms, AssociatedObject is working as expected.
+        assertEquals(
+            if (isJvm()) PolymorphicKind.OPEN else PolymorphicKind.SEALED,
+            serializer(typeOf<SealedInterfaceWithExplicitAnnotation>()).descriptor.kind
+        )
     }
 
 
-}
+}