feat: Add a way to reorder elements during encoding

[Chuckame]

Closes #2668

[pdvrieze]

A good starting point, but some issues, including a lack of proper support for nested structs (or a nested struct to be wrapped in a value class).

[pdvrieze]

This cache means you need to create an instance at least per structure descriptor, but more conveniently for every instance. In such case beginStructure can just become private val delegateEncoder: CompositeEncoder.

Although it may also make sense to have this retained (and used for subStructures, although in such case the element index would be an important parameter)

[Chuckame]

For the first part, I suppose you were talking about the supplier that should just be the encoder (no supplier), and I agree.

But for the second part, I don't understand, the buffer is just for the corresponding descriptor, that should not be shared with sub-structure: we are reordering one level of a given descriptor, but not the sub structures.

[pdvrieze]

This is incorrect. It is valid to have an inline/value class have a struct member. That would just result in a nested reordering encoder (delay-created with the result of calling beginStructure).

[Chuckame]

The beginStructure is called by the serializer from inside the encodeSerializableValue, so it should never be called directly AFAIK 🤔

[pdvrieze]

What I mean is that you have an encoder on which beginStructure is called. In the implementation it will create/call the reordering composite encoder (and return it), and perhaps also to the delegate encoder (to be passed to the reordering encoder).

[Chuckame]

Ok I got it. Is it such an expected standard use case to encode inlined structures like this encoder.encodeInlineElement(desc, idx).beginStructure(...) ? Shouldn't it be expected to be like the serializer plugin (encoder.encodeInlineElement(desc, idx).encodeSerializableValue(...)) where structures are always encoded through encodeSerializableValue or encodeNullableSerializableValue to allow interception and support nullability ?

[pdvrieze]

Note that the implementation of this should be equivalent to:
thisEncoder. encodeSerializableElement(MyData. serializer. descriptor, 0, MyInt. serializer(), myInt)

Including the support of nested structs (nested structs always go through encodeSerializableElement)

[Chuckame]

Yes, this is working like this after reordering, or maybe I misunderstood

[pdvrieze]

What about:

@Serializable data class Outer(val a: Middle, val b: Int)

@Serializable value class Middle(val i: Inner)

@Serializable value class Inner(val c: String, d: Boolean)

The way I read it, this at least can be implemented using encodeInlineElement(d, 0) on the composite encoder for outer. Which would then mean that the inner value is serialized by calling encodeSerializableValue() on that (BufferingInlineEncoder) encoder.

[Chuckame]

Test has been successful with your example, so I still don't get your point https://github.com/Kotlin/kotlinx.serialization/pull/2722/files#r1648164815

the inner value is serialized by calling encodeSerializableValue() on that (BufferingInlineEncoder) encoder

No, here are the steps:

• encodeInlineElement(d, 0) -> returns BufferingInlineEncoder with descriptor of Middle and index 0, but does not buffer anything for the moment (this is the role of BufferingInlineEncoder)
• BufferingInlineEncoder.encodeSerializableValue is called, so nothing more is done, no nested call is done even for Inner thanks to bufferEncoding buffering compositeEncoderDelegate.encodeInlineElement(descriptor, elementIndex).encodeSerializableValue(serializer, value)
• then when endStructure is called, replays the elements encoding, so the Inner will be reached out only at this moment

[Chuckame]

First, I did not cover everything as I wanted first to agree on the main idea of buffering the calls. It seems that it's ok for you 😄

Secondly, if it's ok for you, I'll focus on the buffering itself, and then refine the public API and namings. I've did the current work quickly to show you the idea.

To finish, thanks for the quick answer 🚀

I'll make some tests to test the different use cases

[pdvrieze]

@Chuckame The buffering is basically correct (I've implemented this for XML for quite some time now)

[Chuckame]

@pdvrieze I've tried with all the possible combinations, and value classes serializers are not directly using encodeInlineElement but encodeSerializableElement that is buffered, which I think it's a bug where encodeInlineElement is never called (see #2738 freshly created).

Let's imagine that this bug doesn't exist, and encodeInlineElement is called as expected, beginStructure call on the inline's encoder is explicitly forbidden or discouraged as said in the docs:

    /**
     * [...]
     *
     * Calling [Encoder.beginStructure] on returned instance leads to an unspecified behavior and, in general, is prohibited.
     */
    public fun encodeInline(descriptor: SerialDescriptor): Encoder

So I would say that this use case shouldn't be handled to follow the docs, as this beginStructure call should be done by the structure's serializer.