some cleaning, added a really quick test

Author: Chuckame

core/commonMain/src/kotlinx/serialization/encoding/ReorderingCompositeEncoder.kt

@@ -15,89 +15,88 @@ import kotlinx.serialization.modules.*
  * This encoder will replicate the behavior of a standard encoding, but calling the `encode*Element` methods in
  * the order defined by [mapElementIndex]. It first buffers each `encode*Element` calls in an array following
  * the given indexes using [mapElementIndex], then when [endStructure] is called, it encodes the buffered calls
- * in the expected order by replaying the previous calls on a concrete [CompositeEncoder] provided
- * by [beginStructure].
+ * in the expected order by replaying the previous calls on the given [compositeEncoderDelegate].
  *
- * @param structureDescriptor descriptor of the structure being encoded
- * @param beginStructure provides the [CompositeEncoder] to be used to encode the given descriptor's elements in the expected order.
- * @param mapElementIndex maps the element index to the new reordered index (zero-based). If this mapper provides the same index for multiple elements, only the last one will be encoded as the previous ones will be overridden.
+ * @param compositeEncoderDelegate the [CompositeEncoder] to be used to encode the given descriptor's elements in the expected order.
+ * @param structureDescriptor descriptor of the structure being encoded and reordered
+ * @param mapElementIndex maps the element index to a new positional zero-based index. If this mapper provides the same index for multiple elements, only the last one will be encoded as the previous ones will be overridden. The mapped index just helps to reorder the elements, but the reordered `encode*Element` method calls will still pass the original element index.
  */
 @ExperimentalSerializationApi
 internal class ReorderingCompositeEncoder(
     structureDescriptor: SerialDescriptor,
-    private val beginStructure: (SerialDescriptor) -> CompositeEncoder,
+    private val compositeEncoderDelegate: CompositeEncoder,
     private val mapElementIndex: (SerialDescriptor, Int) -> Int,
 ) : CompositeEncoder {
-    // Each time we encode a field, if the next expected schema field index is not the good one, it is buffered until it's the time to encode it
     private var bufferedCalls = Array<BufferedCall?>(structureDescriptor.elementsCount) { null }
-    private var callShouldEncodeElementDefault = false
 
     override val serializersModule: SerializersModule
         // No need to return a serializers module as it's not used during buffering
         get() = EmptySerializersModule()
 
     private data class BufferedCall(
         val originalElementIndex: Int,
-        val encoder: CompositeEncoder.() -> Unit,
+        val encoder: () -> Unit,
     )
 
-    private inline fun bufferEncoding(
+    private fun bufferEncoding(
         descriptor: SerialDescriptor,
         index: Int,
-        crossinline encoder: CompositeEncoder.() -> Unit
+        encoder: () -> Unit
     ) {
-        if (callShouldEncodeElementDefault) {
-            bufferedCalls[mapElementIndex(descriptor, index)] = BufferedCall(index) {
-                if (shouldEncodeElementDefault(descriptor, index)) {
-                    encoder()
-                }
-            }
-        } else {
-            bufferedCalls[mapElementIndex(descriptor, index)] = BufferedCall(index) {
-                encoder()
-            }
-        }
-        callShouldEncodeElementDefault = false
+        bufferedCalls[mapElementIndex(descriptor, index)] = BufferedCall(index, encoder)
     }
 
     override fun endStructure(descriptor: SerialDescriptor) {
         encodeBufferedFields(descriptor)
     }
 
     private fun encodeBufferedFields(descriptor: SerialDescriptor) {
-        val classEncoder = beginStructure(descriptor)
-        bufferedCalls.forEach { fieldToEncode ->
-            fieldToEncode?.encoder?.invoke(classEncoder)
+        bufferedCalls.filterNotNull().forEach { fieldToEncode ->
+            fieldToEncode.encoder()
         }
-        classEncoder.endStructure(descriptor)
+        compositeEncoderDelegate.endStructure(descriptor)
     }
 
     override fun encodeBooleanElement(descriptor: SerialDescriptor, index: Int, value: Boolean) {
-        bufferEncoding(descriptor, index) { encodeBooleanElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeBooleanElement(descriptor, index, value)
+        }
     }
 
     override fun encodeByteElement(descriptor: SerialDescriptor, index: Int, value: Byte) {
-        bufferEncoding(descriptor, index) { encodeByteElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeByteElement(descriptor, index, value)
+        }
     }
 
     override fun encodeCharElement(descriptor: SerialDescriptor, index: Int, value: Char) {
-        bufferEncoding(descriptor, index) { encodeCharElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeCharElement(descriptor, index, value)
+        }
     }
 
     override fun encodeDoubleElement(descriptor: SerialDescriptor, index: Int, value: Double) {
-        bufferEncoding(descriptor, index) { encodeDoubleElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeDoubleElement(descriptor, index, value)
+        }
     }
 
     override fun encodeFloatElement(descriptor: SerialDescriptor, index: Int, value: Float) {
-        bufferEncoding(descriptor, index) { encodeFloatElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeFloatElement(descriptor, index, value)
+        }
     }
 
     override fun encodeIntElement(descriptor: SerialDescriptor, index: Int, value: Int) {
-        bufferEncoding(descriptor, index) { encodeIntElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeIntElement(descriptor, index, value)
+        }
     }
 
     override fun encodeLongElement(descriptor: SerialDescriptor, index: Int, value: Long) {
-        bufferEncoding(descriptor, index) { encodeLongElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeLongElement(descriptor, index, value)
+        }
     }
 
     override fun <T : Any> encodeNullableSerializableElement(
@@ -106,7 +105,9 @@ internal class ReorderingCompositeEncoder(
         serializer: SerializationStrategy<T>,
         value: T?
     ) {
-        bufferEncoding(descriptor, index) { encodeNullableSerializableElement(descriptor, index, serializer, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeNullableSerializableElement(descriptor, index, serializer, value)
+        }
     }
 
     override fun <T> encodeSerializableElement(
@@ -115,39 +116,44 @@ internal class ReorderingCompositeEncoder(
         serializer: SerializationStrategy<T>,
         value: T
     ) {
-        bufferEncoding(descriptor, index) { encodeSerializableElement(descriptor, index, serializer, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeSerializableElement(descriptor, index, serializer, value)
+        }
     }
 
     override fun encodeShortElement(descriptor: SerialDescriptor, index: Int, value: Short) {
-        bufferEncoding(descriptor, index) { encodeShortElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeShortElement(descriptor, index, value)
+        }
     }
 
     override fun encodeStringElement(descriptor: SerialDescriptor, index: Int, value: String) {
-        bufferEncoding(descriptor, index) { encodeStringElement(descriptor, index, value) }
+        bufferEncoding(descriptor, index) {
+            compositeEncoderDelegate.encodeStringElement(descriptor, index, value)
+        }
     }
 
     override fun encodeInlineElement(descriptor: SerialDescriptor, index: Int): Encoder {
         return BufferingInlineEncoder(descriptor, index)
     }
 
     override fun shouldEncodeElementDefault(descriptor: SerialDescriptor, index: Int): Boolean {
-        callShouldEncodeElementDefault = true
-        return true
+        return compositeEncoderDelegate.shouldEncodeElementDefault(descriptor, index)
     }
 
     private inner class BufferingInlineEncoder(
         private val descriptor: SerialDescriptor,
         private val elementIndex: Int,
     ) : Encoder {
-        private var nullable = false
+        private var encodeNotNullMarkCalled = false
 
         override val serializersModule: SerializersModule
             get() = this@ReorderingCompositeEncoder.serializersModule
 
         private fun bufferEncoding(encoder: Encoder.() -> Unit) {
             bufferEncoding(descriptor, elementIndex) {
                 encodeInlineElement(descriptor, elementIndex).apply {
-                    if (nullable) {
+                    if (encodeNotNullMarkCalled) {
                         encodeNotNullMark()
                     }
                     encoder()
@@ -156,7 +162,7 @@ internal class ReorderingCompositeEncoder(
         }
 
         override fun encodeNotNullMark() {
-            nullable = true
+            encodeNotNullMarkCalled = true
         }
 
         override fun <T : Any> encodeNullableSerializableValue(serializer: SerializationStrategy<T>, value: T?) {
@@ -231,10 +237,10 @@ internal class ReorderingCompositeEncoder(
  * Encodes the [structureDescriptor] elements in a specific order provided by [elementIndexMapper].
  *
  * @param structureDescriptor descriptor of the structure being encoded and reordered
- * @param elementIndexMapper maps the element index to the new reordered index (zero-based). If this mapper provides the same index for multiple elements, only the last one will be encoded as the previous ones will be overridden.
+ * @param elementIndexMapper maps the element index to a new positional zero-based index. If this mapper provides the same index for multiple elements, only the last one will be encoded as the previous ones will be overridden. The mapped index just helps to reorder the elements, but the reordered `encode*Element` method calls will still pass the original element index.
  */
 @ExperimentalSerializationApi
-public fun CompositeEncoder.reorderElements(
+public fun CompositeEncoder.encodeReorderingElements(
     structureDescriptor: SerialDescriptor,
     elementIndexMapper: (SerialDescriptor, Int) -> Int,
-): CompositeEncoder = ReorderingCompositeEncoder(structureDescriptor, { this }, elementIndexMapper)
+): CompositeEncoder = ReorderingCompositeEncoder(structureDescriptor, this, elementIndexMapper)

core/commonTest/src/kotlinx/serialization/encoding/ReorderingCompositeEncoderTest.kt

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2017-2024 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+package kotlinx.serialization.encoding
+
+import kotlinx.serialization.*
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.modules.*
+import kotlin.jvm.*
+import kotlin.test.*
+
+class ReorderingCompositeEncoderTest {
+    @Test
+    fun shouldReorderWell() {
+        val first = 42
+        val second = "string"
+        val third = true
+        val value = OuterToBeReordered(Middle(Inner(second, third)), first)
+
+        val encoder = ReorderingEncoder()
+
+        OuterToBeReordered.serializer().serialize(encoder, value)
+
+        assertContentEquals(actual = encoder.encodedValues, expected = listOf(first, second, third))
+    }
+
+    private class ReorderingEncoder(
+        val encodedValues: MutableList<Any?> = mutableListOf()
+    ) : AbstractEncoder() {
+        override val serializersModule: SerializersModule
+            get() = EmptySerializersModule()
+
+        override fun encodeValue(value: Any) {
+            encodedValues.add(value)
+        }
+
+        override fun encodeNull() {
+            encodedValues.add(null)
+        }
+
+        override fun beginStructure(descriptor: SerialDescriptor): CompositeEncoder {
+            return BasicCompositeEncoder().encodeReorderingElements(descriptor) { _, index ->
+                // invert fields
+                if (index == 0) 1 else 0
+            }
+        }
+
+        private inner class BasicCompositeEncoder : AbstractEncoder() {
+            override val serializersModule: SerializersModule
+                get() = EmptySerializersModule()
+
+            override fun encodeValue(value: Any) {
+                encodedValues.add(value)
+            }
+
+            override fun encodeNull() {
+                encodedValues.add(null)
+            }
+        }
+    }
+
+    @Serializable
+    private data class OuterToBeReordered(val a: Middle, val b: Int)
+
+    @JvmInline
+    @Serializable
+    private value class Middle(val i: Inner)
+
+    @Serializable
+    private data class Inner(val c: String, val d: Boolean)
+}