Disallow more conflicts with class discriminator on encoding:

- from JsonNamingStrategy
- from ALL_JSON_OBJECTS
- from default polymorphic serializer

Changed the type of thrown exception to SerializationException because it is our contract for .encodeToString.

Stop recommending array polymorphism because it is outdated.

Author: sandwwraith

formats/json-tests/commonTest/src/kotlinx/serialization/features/DefaultPolymorphicSerializerTest.kt

@@ -6,6 +6,7 @@ package kotlinx.serialization.features
 import kotlinx.serialization.*
 import kotlinx.serialization.json.*
 import kotlinx.serialization.modules.*
+import kotlinx.serialization.test.assertFailsWithMessage
 import kotlin.test.*
 
 class DefaultPolymorphicSerializerTest : JsonTestBase() {
@@ -33,4 +34,17 @@ class DefaultPolymorphicSerializerTest : JsonTestBase() {
             json.decodeFromString<Project>(""" {"type":"unknown","name":"example"}""", it))
     }
 
+    @Test
+    fun defaultSerializerConflictWithDiscriminatorNotAllowed() {
+        @Suppress("UNCHECKED_CAST") val module = SerializersModule {
+            polymorphicDefaultSerializer(Project::class) {
+                DefaultProject.serializer() as KSerializer<Project>
+            }
+        }
+        val j = Json { serializersModule = module }
+        assertFailsWithMessage<SerializationException>("Class 'kotlinx.serialization.features.DefaultPolymorphicSerializerTest.DefaultProject' cannot be serialized as base class 'kotlinx.serialization.Polymorphic<Project>' because it has property name that conflicts with JSON class discriminator 'type'.") {
+            j.encodeToString<Project>(DefaultProject("example", "custom"))
+        }
+    }
+
 }

formats/json-tests/commonTest/src/kotlinx/serialization/features/JsonNamingStrategyTest.kt

@@ -214,6 +214,7 @@ class JsonNamingStrategyTest : JsonTestBase() {
     }
 
     @Serializable
+    @SerialName("SealedBase")
     sealed interface SealedBase {
         @Serializable
         @JsonClassDiscriminator("typeSub")
@@ -244,4 +245,28 @@ class JsonNamingStrategyTest : JsonTestBase() {
             json
         )
     }
+
+    @Test
+    fun testClashWithDiscriminator() {
+        val correctJson = Json(jsonWithNaming) {
+            classDiscriminator = "test_base"
+        }
+        val holder = Holder(SealedBase.SealedSub2(), SealedBase.SealedMid.SealedSub1)
+
+        // Should pass because same name is only on different levels
+        assertJsonFormAndRestored(
+            Holder.serializer(),
+            holder,
+            """{"test_base":{"test_base":"SealedSub2","test_case":0},"test_mid":{"typeSub":"SealedSub1"}}""",
+            correctJson
+        )
+
+        val incorrectJson = Json(jsonWithNaming) {
+            classDiscriminator = "test_case"
+        }
+
+        assertFailsWithMessage<SerializationException>("Class 'SealedSub2' cannot be serialized as base class 'SealedBase' because it has property name that conflicts with JSON class discriminator 'test_case'.") {
+            incorrectJson.encodeToString<Holder>(holder)
+        }
+    }
 }

formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeBaseTest.kt

@@ -80,7 +80,7 @@ abstract class JsonClassDiscriminatorModeBaseTest(
     @SerialName("mixed")
     data class MixedPolyAndRegular(val sb: SealedBase, val sc: SealedContainer, val i: Inner)
 
-    private inline fun <reified T> doTest(expected: String, obj: T) {
+    internal inline fun <reified T> doTest(expected: String, obj: T) {
         parametrizedTest { mode ->
             val serialized = json.encodeToString(serializer<T>(), obj, mode)
             assertEquals(expected, serialized, "Failed with mode = $mode")
@@ -150,4 +150,8 @@ abstract class JsonClassDiscriminatorModeBaseTest(
         val nm = NullableMixed(null, null)
        doTest(expected, nm)
     }
+
+    @Serializable
+    @SerialName("Conflict")
+    class Conflict(val type: String)
 }

formats/json-tests/commonTest/src/kotlinx/serialization/json/polymorphic/JsonClassDiscriminatorModeTest.kt

@@ -7,6 +7,7 @@ package kotlinx.serialization.json.polymorphic
 import kotlinx.serialization.*
 import kotlinx.serialization.json.*
 import kotlinx.serialization.modules.*
+import kotlinx.serialization.test.assertFailsWithMessage
 import kotlin.test.*
 
 class ClassDiscriminatorModeAllObjectsTest :
@@ -46,6 +47,13 @@ class ClassDiscriminatorModeAllObjectsTest :
     @Test
     fun testNullable() = testNullable("""{"type":"NullableMixed","sb":null,"sc":null}""")
 
+    @Test
+    fun testConflictWithDiscriminator() {
+        assertFailsWithMessage<SerializationException>("Class 'Conflict' cannot be serialized in ALL_JSON_OBJECTS class discriminator mode because it has property name that conflicts with JSON class discriminator 'type'") {
+            json.encodeToString(Conflict("foo"))
+        }
+    }
+
 }
 
 class ClassDiscriminatorModeNoneTest :
@@ -83,6 +91,11 @@ class ClassDiscriminatorModeNoneTest :
     @Test
     fun testNullable() = testNullable("""{"sb":null,"sc":null}""")
 
+    @Test
+    fun testConflictWithDiscriminator() {
+        doTest("""{"type":"foo"}""", Conflict("foo"))
+    }
+
     interface CommandType
 
     @Serializable // For Kotlin/JS

formats/json-tests/commonTest/src/kotlinx/serialization/modules/SerialNameCollisionInSealedClassesTest.kt

@@ -24,8 +24,8 @@ class SerialNameCollisionInSealedClassesTest {
 
     @Test
     fun testCollisionWithDiscriminator() {
-        assertFailsWith<IllegalStateException> { Json("type").encodeToString(Base.serializer(), Base.Child("a")) }
-        assertFailsWith<IllegalStateException> { Json("type2").encodeToString(Base.serializer(), Base.Child("a")) }
+        assertFailsWith<SerializationException> { Json("type").encodeToString(Base.serializer(), Base.Child("a")) }
+        assertFailsWith<SerializationException> { Json("type2").encodeToString(Base.serializer(), Base.Child("a")) }
         Json("f").encodeToString(Base.serializer(), Base.Child("a"))
     }
 

formats/json/commonMain/src/kotlinx/serialization/json/internal/JsonNamesMap.kt

@@ -8,8 +8,8 @@ package kotlinx.serialization.json.internal
 import kotlinx.serialization.*
 import kotlinx.serialization.descriptors.*
 import kotlinx.serialization.encoding.*
+import kotlinx.serialization.internal.jsonCachedSerialNames
 import kotlinx.serialization.json.*
-import kotlin.native.concurrent.*
 
 internal val JsonDeserializationNamesKey = DescriptorSchemaCache.Key<Map<String, Int>>()
 
@@ -19,7 +19,7 @@ private fun SerialDescriptor.buildDeserializationNamesMap(json: Json): Map<Strin
     fun MutableMap<String, Int>.putOrThrow(name: String, index: Int) {
         val entity = if (kind == SerialKind.ENUM) "enum value" else "property"
         if (name in this) {
-            throw JsonException(
+            throw JsonDecodingException(
                 "The suggested name '$name' for $entity ${getElementName(index)} is already one of the names for $entity " +
                         "${getElementName(getValue(name))} in ${this@buildDeserializationNamesMap}"
             )
@@ -72,6 +72,12 @@ internal fun SerialDescriptor.getJsonElementName(json: Json, index: Int): String
     return if (strategy == null) getElementName(index) else serializationNamesIndices(json, strategy)[index]
 }
 
+// Emits only names used for encoding, i.e. from naming strategy, but not from @JsonNames
+internal fun SerialDescriptor.getJsonEncodedNames(json: Json): Set<String> {
+    val strategy = namingStrategy(json)
+    return if (strategy == null) jsonCachedSerialNames() else serializationNamesIndices(json, strategy).toSet()
+}
+
 internal fun SerialDescriptor.namingStrategy(json: Json) =
     if (kind == StructureKind.CLASS) json.configuration.namingStrategy else null
 
@@ -85,7 +91,6 @@ private fun Json.decodeCaseInsensitive(descriptor: SerialDescriptor) =
  * Serves same purpose as [SerialDescriptor.getElementIndex] but respects [JsonNames] annotation
  * and [JsonConfiguration] settings.
  */
-@OptIn(ExperimentalSerializationApi::class)
 internal fun SerialDescriptor.getJsonNameIndex(json: Json, name: String): Int {
     if (json.decodeCaseInsensitive(this)) {
         return getJsonNameIndexSlowPath(json, name.lowercase())

formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt

@@ -9,8 +9,6 @@ import kotlinx.serialization.*
 import kotlinx.serialization.descriptors.*
 import kotlinx.serialization.internal.*
 import kotlinx.serialization.json.*
-import kotlinx.serialization.modules.*
-import kotlin.jvm.*
 
 @Suppress("UNCHECKED_CAST")
 internal inline fun <T> JsonEncoder.encodePolymorphically(
@@ -37,32 +35,35 @@ internal inline fun <T> JsonEncoder.encodePolymorphically(
         val casted = serializer as AbstractPolymorphicSerializer<Any>
         requireNotNull(value) { "Value for serializer ${serializer.descriptor} should always be non-null. Please report issue to the kotlinx.serialization tracker." }
         val actual = casted.findPolymorphicSerializer(this, value)
-        if (baseClassDiscriminator != null) {
-            validateIfSealed(serializer, actual, baseClassDiscriminator)
-            checkKind(actual.descriptor.kind)
-        }
         actual as SerializationStrategy<T>
     } else serializer
 
-    if (baseClassDiscriminator != null) ifPolymorphic(baseClassDiscriminator, actualSerializer.descriptor.serialName)
+    if (baseClassDiscriminator != null) {
+        json.checkEncodingConflicts(serializer, actualSerializer, baseClassDiscriminator)
+        checkKind(actualSerializer.descriptor.kind)
+        ifPolymorphic(baseClassDiscriminator, actualSerializer.descriptor.serialName)
+    }
     actualSerializer.serialize(this, value)
 }
 
-private fun validateIfSealed(
+private fun Json.checkEncodingConflicts(
     serializer: SerializationStrategy<*>,
     actualSerializer: SerializationStrategy<*>,
     classDiscriminator: String
 ) {
-    if (serializer !is SealedClassSerializer<*>) return
-    @Suppress("DEPRECATION_ERROR")
-    if (classDiscriminator in actualSerializer.descriptor.jsonCachedSerialNames()) {
+    if (classDiscriminator in actualSerializer.descriptor.getJsonEncodedNames(this)) {
         val baseName = serializer.descriptor.serialName
         val actualName = actualSerializer.descriptor.serialName
-        error(
-            "Sealed class '$actualName' cannot be serialized as base class '$baseName' because" +
+
+        val text = when {
+            configuration.classDiscriminatorMode == ClassDiscriminatorMode.ALL_JSON_OBJECTS && baseName == actualName -> "in ALL_JSON_OBJECTS class discriminator mode"
+            else -> "as base class '$baseName'"
+        }
+        throw JsonEncodingException(
+            "Class '$actualName' cannot be serialized $text because" +
                     " it has property name that conflicts with JSON class discriminator '$classDiscriminator'. " +
-                    "You can either change class discriminator in JsonConfiguration, " +
-                    "rename property with @SerialName annotation or fall back to array polymorphism"
+                    "You can either change class discriminator in JsonConfiguration, or " +
+                    "rename property with @SerialName annotation."
         )
     }
 }