Reducing boilerplate in sealed classes for closed polymorphism.

[bartvanheukelom]

Serializers for sealed classes use fully qualified names as discriminators by default. This can introduce instability, as these names are susceptible to changes through refactorings, potentially affecting the serialized form. Additionally, in the context of closed polymorphism, there's no added value in fully qualifying the discriminators. (Also noted in #1950). To mitigate this, @SerialName annotations can be used to provide stable, short discriminators, but this introduces significant boilerplate code:

package biz.unstable.packagename

@Serializable sealed class Aminals {
    @Serializable @SerialName("Bird") object Bird : Aminals()
    @Serializable @SerialName("Cat") object Cat : Aminals()
    @Serializable @SerialName("Dog") object Dog : Aminals()
}

I propose adding a new annotation or option to the base class to automatically use short, locally scoped names as default discriminators. This feature would eliminate the need for manual @SerialName annotations in subclasses, simplifying the code and keeping discriminators stable against refactorings. Additionally, it would be great if this or another annotation could be used to specify that all subclasses of a sealed class should be @Serializable by default. The resulting code would look a lot cleaner:

package biz.unstable.packagename

@Serializable
@AllChildrenSerializable
@ShortDiscriminators
sealed class Aminals {
    object Bird : Aminals()
    object Cat : Aminals()
    object Dog : Aminals()
}

For deeper hierarchies, the discriminator would reflect the nesting level, relative to the specific base class used.

@Serializable ... sealed class Aminals {
    object Bird : Aminals()
    sealed class Cats : Aminals() {
        object Tabby : Cats()
        object Torty : Cats()
    }
    object Dog : Aminals()
}

So KSerializer<Aminals> would use discriminators Bird, Cats.Tabby, etc. while KSerializer<Cats> would simply use Tabby, Torty.

[sandwwraith]

Two notes:

1. Given that sealed inheritors can be declared anywhere in the file, it is not clear whether this annotation should affect all inheritors or only nested ones. For sealed interfaces, inheritors can be declared anywhere in the program.
2. There's a potential name clash when two sealed classes have inheritors with the same names and, let's say, a common super interface. With explicit @SerialName, it is much easier to spot.

[bartvanheukelom]

1. It would be great if all those cases could be made easier, but I'd already be happy if it only worked for this specific way of structuring such a type.
2. I'm not sure what you mean by inheritors with the same names. Like, one is top-level and another is nested somewhere, for example? Either way, I'd be fine if the compiler plugin rejected the @ShortDiscriminators annotation at the slighest indication of something being "different".

[sandwwraith]

For 2, I meant something like this:

sealed interface I

sealed class One: I {
  class A: One()
  class B: One()
}

sealed class Two: I {
  class A: Two()
  class B: Two()
}

[natanfudge]

@sandwwraith For the case you showed, it is simple to check at compile time that two class names conflict. A simple error would handle such extreme cases:

@ShortDiscriminators does not allow duplicate class names in the sealed hierarchy, but this [One.A] class name is the same as Two.A which is also a subtype of I.

I am +1 for @ShortDiscriminators, and would be willing to contribute the feature myself.

I would go as far as to say this should have been the default behavior , and fully qualified type discriminators should be opt-in. It is simpler, more efficient, and makes more sense when talking to non-kotlin services that don't have a concept of "package names".

Obviously changing this would break a ton of code so I advocate for adding a project-wide flag for enabling it. I'm not aware of any way of configuring the compiler plugin right now so that aspect might be problematic right now.

Elizarov also suggested this feature in another issue.

[ryanholden8]

Would love to see @AllChildrenSerializable added as well. We built assertAllConcreteSubclassesOfSealedClassHaveSerializable for our unit tests to catch issues where we forgot to put a @Serializable on a sealed interface class implementation because there is no compile time warning, just a runtime crash in production 😮 .

Unfortunately, we still have to remember to manually add a unit test for each one.

// Test

class SealedInterfaceTest {
    /**
     * Ensures all models are marked with @Serializable.
     *
     * This helps catch any new classes that are added, that they have @Serializable.
     */
    @Test
    fun `all subclasses have Serializable annotation`() {
        assertAllConcreteSubclassesOfSealedClassHaveSerializable<SealInterfaceType>()
    }
}

// Helper Methods

/**
 * Recursively asserts [Serializable] annotation on all concrete classes defined for a Kotlin
 * sealed class.
 *
 * "All subclasses of a sealed class must be explicitly marked as `@Serializable`."
 * https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/polymorphism.md#sealed-classes
 */
inline fun <reified T : Any> assertAllConcreteSubclassesOfSealedClassHaveSerializable() =
    T::class.allConcreteSubclassesOfSealedClass().forEach {
        assert(
            value = it.hasAnnotation<Required>(),
            lazyMessage = { "`Serializable` annotation missing from `$it`" },
        )
    }

/**
 * Recursively gets all the concrete classes defined for a Kotlin sealed class.
 */
fun <R : Any> KClass<R>.allConcreteSubclassesOfSealedClass(): List<KClass<out R>> {
    val classes = sealedSubclasses
    val subClasses = mutableListOf<KClass<out R>>()

    // In case there is more than 1 hierarchy, recursively add sub classes of sub classes
    classes.forEach {
        subClasses.addAll(it.allConcreteSubclassesOfSealedClass())
    }

    return (classes + subClasses).filter { !it.isSealed && !it.isAbstract }
}

[bartvanheukelom]

@ryanholden8 good idea to write such unit tests. I might steal this idea but implement is as a custom annotation with processor. Easier to (remember to) add it to each class and impossible to forget executing (I often build and deploy without running tests to speed up the development iteration).