Serializer lookup rework (#958)

    * Make @serializable and @polymorphic retain in runtime
    * Do not construct default serializer for explicitly annotated enum
    * Properly lookup serializer for interfaces
   * Lookup builtin serializer before contextual to provide consistent experience

Fixes #928

Author: qwwdfsad

Diff for: runtime/api/kotlinx-serialization-core.api

@@ -97,6 +97,7 @@ public final class kotlinx/serialization/PolymorphicSerializer : kotlinx/seriali
 	public fun <init> (Lkotlin/reflect/KClass;)V
 	public fun getBaseClass ()Lkotlin/reflect/KClass;
 	public fun getDescriptor ()Lkotlinx/serialization/descriptors/SerialDescriptor;
+	public fun toString ()Ljava/lang/String;
 }
 
 public final class kotlinx/serialization/PolymorphicSerializerKt {

Diff for: runtime/commonMain/src/kotlinx/serialization/Annotations.kt

@@ -68,7 +68,7 @@ import kotlin.reflect.*
  * @see Serializer
  */
 @Target(AnnotationTarget.PROPERTY, AnnotationTarget.CLASS, AnnotationTarget.TYPE)
-@Retention(AnnotationRetention.BINARY)
+@Retention(AnnotationRetention.RUNTIME)
 public annotation class Serializable(
     val with: KClass<out KSerializer<*>> = KSerializer::class // Default value indicates that auto-generated serializer is used
 )
@@ -202,7 +202,7 @@ public annotation class UseSerializers(vararg val serializerClasses: KClass<out
  * with special compiler plugin support which would be added later.
  */
 @Target(AnnotationTarget.PROPERTY, AnnotationTarget.TYPE, AnnotationTarget.CLASS)
-@Retention(AnnotationRetention.BINARY)
+@Retention(AnnotationRetention.RUNTIME)
 public annotation class Polymorphic
 
 /**

Diff for: runtime/commonMain/src/kotlinx/serialization/PolymorphicSerializer.kt

@@ -74,6 +74,10 @@ public class PolymorphicSerializer<T : Any>(override val baseClass: KClass<T>) :
                 buildSerialDescriptor("kotlinx.serialization.Polymorphic<${baseClass.simpleName}>", SerialKind.CONTEXTUAL)
             )
         }.withContext(baseClass)
+
+    override fun toString(): String {
+        return "kotlinx.serialization.PolymorphicSerializer(baseClass: $baseClass)"
+    }
 }
 
 public fun <T : Any> AbstractPolymorphicSerializer<T>.findPolymorphicSerializer(

Diff for: runtime/commonMain/src/kotlinx/serialization/Serializers.kt

@@ -8,7 +8,6 @@
 package kotlinx.serialization
 
 import kotlinx.serialization.builtins.*
-import kotlinx.serialization.builtins.nullable
 import kotlinx.serialization.internal.*
 import kotlinx.serialization.modules.*
 import kotlin.jvm.*
@@ -35,36 +34,40 @@ public inline fun <reified T> SerializersModule.serializer(): KSerializer<T> {
  * [type] argument can be obtained with experimental [typeOf] method.
  */
 public fun serializer(type: KType): KSerializer<Any?> {
-    val result = EmptySerializersModule.serializerByKTypeImpl(type)
+    val result = EmptySerializersModule.serializerByKTypeImpl(type) ?: type.kclass().serializerNotRegistered()
     return result.nullable(type.isMarkedNullable)
 }
 
 /**
- * Retrieves serializer for the given [type] from the current [SerializersModule] and,
- * if not found, fallbacks to plain [serializer] method.
+ * Attempts to create a serializer for the given [type] and fallbacks to [contextual][SerializersModule.getContextual]
+ * lookup for non-serializable types.
  * [type] argument can be obtained with experimental [typeOf] method.
  */
 public fun SerializersModule.serializer(type: KType): KSerializer<Any?> {
     val kclass = type.kclass()
     val isNullable = type.isMarkedNullable
-    getContextual(kclass)?.nullable(isNullable)?.let { return it.cast() }
-    return serializerByKTypeImpl(type).nullable(isNullable).cast()
+    val builtin = serializerByKTypeImpl(type)
+    if (builtin != null) {
+        return builtin.nullable(isNullable).cast()
+    }
+
+    return getContextual(kclass)?.nullable(isNullable)?.cast() ?: type.kclass().serializerNotRegistered()
 }
 
-private fun SerializersModule.serializerByKTypeImpl(type: KType): KSerializer<Any> {
+private fun SerializersModule.serializerByKTypeImpl(type: KType): KSerializer<Any>? {
     val rootClass = type.kclass()
     val typeArguments = type.arguments
         .map { requireNotNull(it.type) { "Star projections in type arguments are not allowed, but had $type" } }
     return when {
-        typeArguments.isEmpty() -> getContextual(rootClass) ?: rootClass.serializer()
-        else -> builtinSerializer(typeArguments, rootClass)
-    }.cast()
+        typeArguments.isEmpty() -> rootClass.serializerOrNull() ?: getContextual(rootClass)
+        else -> builtinSerializerOrNull(typeArguments, rootClass)
+    }?.cast()
 }
 
-private fun SerializersModule.builtinSerializer(
+private fun SerializersModule.builtinSerializerOrNull(
     typeArguments: List<KType>,
     rootClass: KClass<Any>
-): KSerializer<out Any> {
+): KSerializer<out Any>? {
     val serializers = typeArguments
         .map(::serializer)
     // Array is not supported, see KT-32839

Diff for: runtime/commonMain/src/kotlinx/serialization/descriptors/ContextAware.kt

@@ -98,4 +98,8 @@ private class ContextDescriptor(
         result = 31 * result + serialName.hashCode()
         return result
     }
+
+    override fun toString(): String {
+        return "ContextDescriptor(kClass: $kClass, original: $original)"
+    }
 }

Diff for: runtime/commonMain/src/kotlinx/serialization/internal/PluginHelperInterfaces.kt

@@ -31,7 +31,6 @@ public interface GeneratedSerializer<T> : KSerializer<T> {
  * Should not be used in any user code. Please use generated `.serializer(kSerializer1, kSerializer2, ...)`
  * method on a companion or top-level `serializer(KType)` function.
  */
-@InternalSerializationApi
 @Deprecated("Inserted into generated code and should not be used directly", level = DeprecationLevel.HIDDEN)
 public interface SerializerFactory {
     public fun serializer(vararg typeParamsSerializers: KSerializer<*>): KSerializer<*>

Diff for: runtime/commonTest/src/kotlinx/serialization/SerializersLookupEnumTest.kt

@@ -0,0 +1,78 @@
+/*
+ * Copyright 2017-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.serialization
+
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
+import kotlinx.serialization.json.*
+import kotlinx.serialization.test.*
+import kotlin.test.*
+
+// This is unimplemented functionality that should be
+@Suppress("RemoveExplicitTypeArguments") // This is exactly what's being tested
+class SerializersLookupEnumTest : JsonTestBase() {
+    @Serializable(with = EnumExternalObjectSerializer::class)
+    enum class EnumExternalObject
+
+    @Serializer(forClass = EnumExternalObject::class)
+    object EnumExternalObjectSerializer {
+        override val descriptor: SerialDescriptor = buildSerialDescriptor("tmp", SerialKind.ENUM)
+
+        override fun serialize(encoder: Encoder, value: EnumExternalObject) {
+            TODO()
+        }
+
+        override fun deserialize(decoder: Decoder): EnumExternalObject {
+            TODO()
+        }
+    }
+
+    @Serializable(with = EnumExternalClassSerializer::class)
+    enum class EnumExternalClass
+
+    @Serializer(forClass = EnumExternalObject::class)
+    class EnumExternalClassSerializer {
+        override val descriptor: SerialDescriptor = buildSerialDescriptor("tmp", SerialKind.ENUM)
+
+        override fun serialize(encoder: Encoder, value: EnumExternalObject) {
+            TODO()
+        }
+
+        override fun deserialize(decoder: Decoder): EnumExternalObject {
+            TODO()
+        }
+    }
+
+    @Polymorphic
+    enum class EnumPolymorphic
+
+    @Serializable
+    enum class PlainEnum
+
+    @Test
+    fun testPlainEnum() {
+        assertEquals(PlainEnum.serializer(), serializer<PlainEnum>())
+    }
+
+    @Test
+    fun testEnumExternalObject() {
+        assertFailsWith<SerializationException> { (serializer<EnumExternalObject>()) }
+    }
+
+    @Test
+    fun testEnumExternalClass() {
+        assertFailsWith<SerializationException> { serializer<EnumExternalClass>() }
+    }
+
+    @Test
+    fun testEnumPolymorphic() {
+        jvmOnly {
+            assertEquals(
+                PolymorphicSerializer(EnumPolymorphic::class).descriptor,
+                serializer<EnumPolymorphic>().descriptor
+            )
+        }
+    }
+}

Diff for: runtime/commonTest/src/kotlinx/serialization/TypeOfSerializerLookupTest.kt renamed to runtime/commonTest/src/kotlinx/serialization/SerializersLookupTest.kt

@@ -16,7 +16,7 @@ import kotlin.reflect.*
 import kotlin.test.*
 
 @Suppress("RemoveExplicitTypeArguments") // This is exactly what's being tested
-class TypeOfSerializerLookupTest : JsonTestBase() {
+class SerializersLookupTest : JsonTestBase() {
 
     @Test
     fun testPrimitive() {
@@ -135,45 +135,64 @@ class TypeOfSerializerLookupTest : JsonTestBase() {
         assertSerializedWithType("{}", SampleObject)
     }
 
+    class IntBox(val i: Int)
 
-    class CustomIntSerializer(isNullable: Boolean) : KSerializer<Int?> {
+    class CustomIntSerializer(isNullable: Boolean) : KSerializer<IntBox?> {
         override val descriptor: SerialDescriptor
 
         init {
             val d = PrimitiveSerialDescriptor("CIS", PrimitiveKind.INT)
             descriptor = if (isNullable) d.nullable else d
         }
 
-        override fun serialize(encoder: Encoder, value: Int?) {
+        override fun serialize(encoder: Encoder, value: IntBox?) {
             if (value == null) encoder.encodeInt(41)
             else encoder.encodeInt(42)
         }
 
-        override fun deserialize(decoder: Decoder): Int? {
+        override fun deserialize(decoder: Decoder): IntBox? {
             TODO()
         }
     }
 
     @Test
     fun testContextualLookup() {
-        val module = SerializersModule { contextual(CustomIntSerializer(false).cast<Int>()) }
+        val module = SerializersModule { contextual(CustomIntSerializer(false).cast<IntBox>()) }
         val json = Json { serializersModule = module }
-        val data = listOf(listOf(1))
+        val data = listOf(listOf(IntBox(1)))
         assertEquals("[[42]]", json.encodeToString(data))
     }
 
     @Test
     fun testContextualLookupNullable() {
-        val module = SerializersModule { contextual(CustomIntSerializer(true).cast<Int>()) }
-        val serializer = module.serializer<List<List<Int?>>>()
-        assertEquals("[[41]]", Json.encodeToString(serializer, listOf(listOf<Int?>(null))))
+        val module = SerializersModule { contextual(CustomIntSerializer(true).cast<IntBox>()) }
+        val serializer = module.serializer<List<List<IntBox?>>>()
+        assertEquals("[[41]]", Json.encodeToString(serializer, listOf(listOf<IntBox?>(null))))
     }
 
     @Test
     fun testContextualLookupNonNullable() {
-        val module = SerializersModule { contextual(CustomIntSerializer(false).cast<Int>()) }
-        val serializer = module.serializer<List<List<Int?>>>()
-        assertEquals("[[null]]", Json.encodeToString(serializer, listOf(listOf<Int?>(null))))
+        val module = SerializersModule { contextual(CustomIntSerializer(false).cast<IntBox>()) }
+        val serializer = module.serializer<List<List<IntBox?>>>()
+        assertEquals("[[null]]", Json.encodeToString(serializer, listOf(listOf<IntBox?>(null))))
+    }
+
+    @Test
+    fun testCompiledWinsOverContextual() {
+        val contextual = object : KSerializer<Int> {
+            override val descriptor: SerialDescriptor = Int.serializer().descriptor
+
+            override fun serialize(encoder: Encoder, value: Int) {
+                fail()
+            }
+
+            override fun deserialize(decoder: Decoder): Int {
+                fail()
+            }
+        }
+        val json = Json { serializersModule = SerializersModule { contextual(contextual) } }
+        assertEquals("[[1]]", json.encodeToString(listOf(listOf<Int>(1))))
+        assertEquals("42", json.encodeToString(42))
     }
 
     // Tests with [constructSerializerForGivenTypeArgs] are unsupported on legacy Kotlin/JS

Diff for: runtime/jvmMain/src/kotlinx/serialization/SerializersJvm.kt

@@ -25,14 +25,13 @@ public fun serializer(type: Type): KSerializer<Any> = EmptySerializersModule.ser
 
 /**
  * Retrieves serializer for the given reflective Java [type] using
- * [contextual][SerializersModule.getContextual] lookup and fallback to reflective construction.
+ * reflective construction and [contextual][SerializersModule.getContextual] lookup for non-serializable types.
  *
  * [serializer] is intended to be used as an interoperability layer for libraries like GSON and Retrofit,
  * that operate with reflective Java [Type] and cannot use [typeOf].
- * Serializers is looked up in the contextual serializers of the module and then constructed reflectively.
  *
  * For application-level serialization, it is recommended to use `serializer<T>()` instead as it is aware of
- * Kotlin-specific type information, such as nullability, sealed classes and object.
+ * Kotlin-specific type information, such as nullability, sealed classes and object singletons.
  */
 public fun SerializersModule.serializer(type: Type): KSerializer<Any> = when (type) {
     is GenericArrayType -> {
@@ -59,7 +58,7 @@ public fun SerializersModule.serializer(type: Type): KSerializer<Any> = when (ty
                 // since it uses Java TypeToken, not Kotlin one
                 val varargs = args.map { serializer(it) as KSerializer<Any?> }.toTypedArray()
                 (rootClass.kotlin.constructSerializerForGivenTypeArgs(*varargs) as? KSerializer<Any>)
-                    ?: contextualOrReflective(rootClass.kotlin as KClass<Any>)
+                    ?: reflectiveOrContextual(rootClass.kotlin as KClass<Any>)
             }
         }
     }
@@ -69,7 +68,7 @@ public fun SerializersModule.serializer(type: Type): KSerializer<Any> = when (ty
 
 private fun SerializersModule.typeSerializer(type: Class<*>): KSerializer<Any> {
     return if (!type.isArray) {
-        contextualOrReflective(type.kotlin as KClass<Any>)
+        reflectiveOrContextual(type.kotlin as KClass<Any>)
     } else {
         val eType: Class<*> = type.componentType
         val s = serializer(eType)
@@ -94,8 +93,8 @@ private fun SerializersModule.genericArraySerializer(type: GenericArrayType): KS
     return ArraySerializer(kclass, serializer) as KSerializer<Any>
 }
 
-private fun <T: Any> SerializersModule.contextualOrReflective(kClass: KClass<T>): KSerializer<T> {
-    return getContextual(kClass) ?: kClass.serializer()
+private fun <T: Any> SerializersModule.reflectiveOrContextual(kClass: KClass<T>): KSerializer<T> {
+    return kClass.serializerOrNull() ?: getContextual(kClass) ?: kClass.serializerNotRegistered()
 }
 
 @Deprecated("Deprecated during serialization 1.0 API stabilization", ReplaceWith("serializer(type)"), level = DeprecationLevel.ERROR)