Kotlin / kotlinx.serialization

Kotlin multiplatform / multi-format serialization
Apache License 2.0
5.41k stars 620 forks source link

Auto-generated SerialDescriptors for collections should include the annotations on generic type arguments #2237

Open berlix opened 1 year ago

berlix commented 1 year ago

Use case

I am building a new BinaryFormat implementation for Protobuf 3. It contains a @ProtoIntegerType annotation that determines the encoding to use for integer values:

@SerialInfo
@Target(AnnotationTarget.PROPERTY, AnnotationTarget.TYPE)
annotation class ProtoIntegerType(val type: IntegerType)

enum class IntegerType {
    Default,
    Unsigned,
    Signed,
    Fixed,
    SignedFixed
}

In order to cover the Protobuf spec as well as possible, it is necessary to be able to specify the encodings for integers that are in collections (e.g. in a List<Int> or a Map<Int, Int>). The natural way to do this seems to be by applying the @ProtoIntegerType annotation to the generic type arguments, like in this example:

@Serializable
data class Example(
    val list: List<@ProtoIntegerType(IntegerType.Unsigned Int>,
    val map: Map<@ProtoIntegerType(IntegerType.Fixed) Int, @ProtoIntegerType(IntegerType.Signed) Int>
)

Problem

Unfortunately (and contrary to what one might expect), the auto-generated ListLikeDescriptor and MapLikeDescriptor instances do not return these annotations from their getElementAnnotations(index: Int) methods, so the encoder has no way of determining the desired integer encodings.

Feature request

The feature request is that auto-generated SerialDescriptor instances of collections do include any annotations on the type arguments in the return values of getElementAnnotations.

sandwwraith commented 1 year ago

Interesting feature. However, I'm not sure that getElementAnnotations is a proper place to store them — e.g. in some complex cases of @MyInfo(1) val list: List<@MyInfo(2) Map<T, @MyInfo(3) E>> it will turn into a complete mess. This feature requires designing a whole new complex API.

berlix commented 1 year ago

Thank you for your response, and apologies for the delay in mine.

I don't see how this would require a new API. To expand on your example:

@Serializable
data class Example(
    @MyInfo(1) val list: List<@MyInfo(2) Map<@MyInfo(3) T, @MyInfo(4) E>>
)

Given val exampleDescriptor = Example.serializer().descriptor,

This would match the way descriptors of StructureKind.LIST and StructureKind.MAP are already structured.

Does this make sense, or am I missing something?

sandwwraith commented 1 year ago

I thought you've suggested to place everything under exampleDescriptor.getElementAnnotations(0), thanks for clarification. This makes sense for Lists and Maps. However, there are even more complex cases:

@Serializable
data class Example(
   val box: Box<@MyInfo(1) T>
)

@Serializable
data class Box<T>(@MyInfo(2) val listed: List<@MyInfo(3) T>)

In that case, Example.serializer().descriptor.getElementDescriptor(0) refers to Box descriptor, and for Box descriptor, getElementAnnotations(0) should return annotations on the declaration site of val listed, not the use site of T. Moreover, type of listed is not T, but List<T>. Generally, getElementAnnotations(i) returns annotations for elements — which are properties — and not generic arguments.

The general solution would be to introduce some Type notion, as in reflection: classes have properties with types and type parameters, type can have arguments and annotations, et cetera. However, the goal of this library is not to turn into compiler-based reflection at some point :)

berlix commented 1 year ago

It's true that my suggested solution only addresses the case of collections, since they do model their element types in getElementDescriptor(i) (which perhaps was a problematic design decision to begin with, but now here we are :-)).

I do think it's an important use case though - I can't think of any other clean way of modelling the case I outlined initially.

Your example is still interesting: I agree that SerialDescriptor should not attempt to include full descriptions of any Kotlin types. Not sure it's necessary though: It may just boil down to a design decision as to whether the @MyInfo(1) annotation should be "inherited" and show up in exampleDescriptor.getElementDescriptor(0).getElementDescriptor(0).getElementAnnotations(0) alongside @MyInfo(3) or not. Am I getting this right, or am I missing an aspect of the example, or are there even more cases that would cause complications?

In case I am getting it right: I am not familiar enough with Kotlin's type philosophy to know what considerations would have to go into making that design decision - though as far as my use case is concerned, I think either option would be viable.