SerializersModule overwriteWith throws when used twice

[gpunto]

Describe the bug

When overwriteWith is used twice, the second call throws an exception complaining that there are duplicated serializers. However, the duplicates should've been removed by the first overwriteWith call.

For the snippet below, the error message is:

Multiple polymorphic serializers for base class 'interface example.Parent (Kotlin reflection is not available)' have the same serial name 'CHILD': 'class example.ChildA (Kotlin reflection is not available)' and 'class example.ChildB (Kotlin reflection is not available)=example.ChildB$$serializer@4d95d2a2'

To Reproduce

interface Parent

@Serializable
@SerialName("CHILD")
data class ChildA(val a: String) : Parent

@Serializable
@SerialName("CHILD")
data class ChildB(val b: String) : Parent

fun main() {
    val moduleA = SerializersModule {
        polymorphic(Parent::class) {
            subclass(ChildA::class)
        }
    }
    val moduleB = SerializersModule {
        polymorphic(Parent::class) {
            subclass(ChildB::class)
        }
    }
    val firstMerge = moduleA overwriteWith moduleB
    firstMerge overwriteWith SerializersModule { } // <- throws
}

Also note that while this throws:

firstMerge overwriteWith anotherModule

this doesn't:

anotherModule overwriteWith firstMerge

However, we can't replace the first with the second, because they are not equivalent.

Expected behavior

I expect overwriteWith not to throw because of duplicated serializers. We should be able to do:

module1 overwriteWith module2 overwriteWith module3 …

And get a combined "union" module.

Environment

• Kotlin version: 2.0.20
• Library version: 1.7.2
• Kotlin platforms: JVM
• Gradle version: 8.10.1

[sandwwraith]

Hm, this issue is quite nuanced because you have two different classes ChildA and ChildB, yet they share the same serial name CHILD. overwriteWith was originally intended to overwrite serializers mapping where both class and serial name match (e.g. replace ChildBSerializer with ChildBAlternativeSerializer). Thus, moduleA overwriteWith moduleB creates broken firstMerge module: it contains two class mappings ChildA::class -> ChildASerializer; ChildB::class -> ChildBSerializer and yet only one serial name mapping "CHILD" -> ChildBSerializer.

I have to ask, what is your use case here and what behavior from moduleA overwriteWith moduleB you expect. I see here two options:

1. Throw exception anyway, since we have serial name conflict for two different classes and we can overwrite serializer only when class and serial name match.
2. Remove mapping for ChildA::class completely, since we overwriting for CHILD name. Therefore, firstMerge would be identical to moduleB.

[gpunto]

Hey @sandwwraith, thanks for the context!

We have an interface that is implemented by many concrete classes. Then, we have a common SerializersModule which contains most of the polymorphic serializers for these implementors. This is shared across the code. However, in some features, we have serial name "collisions", meaning that some classes in the features have the same type discriminator used by deserializers in the common module, but they are different classes. What we wanted is what you explain in 2: have a kind of set union where we keep all feature-specific serializers + the non-conflicting ones coming from the common module.

Example in pseudocode, assuming C and C1 have the same serial name:

module { A, B, C } overwriteWith module { C1, D } -> module { A, B, C1, D }

While trying this, we ended up in a situation with two overwriteWith (because some poor choices led us to have more than 2 conflicting SerializersModules) and we experienced the crash.