Fix TaggedDecoder nullable decoding (#2456)

Make the TaggedDecoder.decodeNullableSerializableElement implementation consistent with AbstractDecoder,

so it is possible to differentiate nullable and non-nullable serializers.

Fixes #2455 

4 files changed, 71 insertions, 14 deletions

diff --git a/core/commonMain/src/kotlinx/serialization/encoding/AbstractDecoder.kt b/core/commonMain/src/kotlinx/serialization/encoding/AbstractDecoder.kt
index fad7ef87..ffe6dd37 100644
--- a/core/commonMain/src/kotlinx/serialization/encoding/AbstractDecoder.kt
+++ b/core/commonMain/src/kotlinx/serialization/encoding/AbstractDecoder.kt
@@ -74,8 +74,7 @@ public abstract class AbstractDecoder : Decoder, CompositeDecoder {
         index: Int,
         deserializer: DeserializationStrategy<T?>,
         previousValue: T?
-    ): T? {
-        val isNullabilitySupported = deserializer.descriptor.isNullable
-        return if (isNullabilitySupported || decodeNotNullMark()) decodeSerializableValue(deserializer, previousValue) else decodeNull()
+    ): T? = decodeIfNullable(deserializer) {
+        decodeSerializableValue(deserializer, previousValue)
     }
 }
diff --git a/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt b/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt
index f29c8057..dc4aa2ab 100644
--- a/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt
+++ b/core/commonMain/src/kotlinx/serialization/encoding/Decoding.kt
@@ -260,12 +260,17 @@ public interface Decoder {
      * Decodes the nullable value of type [T] by delegating the decoding process to the given [deserializer].
      */
     @ExperimentalSerializationApi
-    public fun <T : Any> decodeNullableSerializableValue(deserializer: DeserializationStrategy<T?>): T? {
-        val isNullabilitySupported = deserializer.descriptor.isNullable
-        return if (isNullabilitySupported || decodeNotNullMark()) decodeSerializableValue(deserializer) else decodeNull()
+    public fun <T : Any> decodeNullableSerializableValue(deserializer: DeserializationStrategy<T?>): T? = decodeIfNullable(deserializer) {
+        decodeSerializableValue(deserializer)
     }
 }
 
+@OptIn(ExperimentalSerializationApi::class)
+internal inline fun <T : Any> Decoder.decodeIfNullable(deserializer: DeserializationStrategy<T?>, block: () -> T?): T? {
+    val isNullabilitySupported = deserializer.descriptor.isNullable
+    return if (isNullabilitySupported || decodeNotNullMark()) block() else decodeNull()
+}
+
 /**
  * [CompositeDecoder] is a part of decoding process that is bound to a particular structured part of
  * the serialized form, described by the serial descriptor passed to [Decoder.beginStructure].
diff --git a/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt b/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt
index 3af5d351..cf71388c 100644
--- a/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt
+++ b/core/commonMain/src/kotlinx/serialization/internal/Tagged.kt
@@ -206,7 +206,6 @@ public abstract class TaggedDecoder<Tag : Any?> : Decoder, CompositeDecoder {
     protected open fun <T : Any?> decodeSerializableValue(deserializer: DeserializationStrategy<T>, previousValue: T?): T =
         decodeSerializableValue(deserializer)
 
-
     // ---- Implementation of low-level API ----
 
     override fun decodeInline(descriptor: SerialDescriptor): Decoder =
@@ -284,13 +283,11 @@ public abstract class TaggedDecoder<Tag : Any?> : Decoder, CompositeDecoder {
         index: Int,
         deserializer: DeserializationStrategy<T?>,
         previousValue: T?
-    ): T? =
-        tagBlock(descriptor.getTag(index)) {
-            if (decodeNotNullMark()) decodeSerializableValue(
-                deserializer,
-                previousValue
-            ) else decodeNull()
+    ): T? = tagBlock(descriptor.getTag(index)) {
+        decodeIfNullable(deserializer) {
+            decodeSerializableValue(deserializer, previousValue)
         }
+    }
 
     private fun <E> tagBlock(tag: Tag, block: () -> E): E {
         pushTag(tag)
diff --git a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonElementDecodingTest.kt b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonElementDecodingTest.kt
index d0b105a0..3cdfa082 100644
--- a/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonElementDecodingTest.kt
+++ b/formats/json-tests/commonTest/src/kotlinx/serialization/json/JsonElementDecodingTest.kt
@@ -1,6 +1,8 @@
 package kotlinx.serialization.json
 
-import kotlinx.serialization.Serializable
+import kotlinx.serialization.*
+import kotlinx.serialization.descriptors.*
+import kotlinx.serialization.encoding.*
 import kotlin.test.*
 
 class JsonElementDecodingTest : JsonTestBase() {
@@ -51,4 +53,58 @@ class JsonElementDecodingTest : JsonTestBase() {
         json = json.replace("%", "0")
         Json.parseToJsonElement(json)
     }
+
+    private open class NullAsElementSerializer<T : Any>(private val serializer: KSerializer<T>, val nullElement: T) : KSerializer<T?> {
+        final override val descriptor: SerialDescriptor = serializer.descriptor.nullable
+
+        final override fun serialize(encoder: Encoder, value: T?) {
+            serializer.serialize(encoder, value ?: nullElement)
+        }
+
+        final override fun deserialize(decoder: Decoder): T = serializer.deserialize(decoder)
+    }
+
+    private object NullAsJsonNullJsonElementSerializer : NullAsElementSerializer<JsonElement>(JsonElement.serializer(), JsonNull)
+    private object NullAsJsonNullJsonPrimitiveSerializer : NullAsElementSerializer<JsonPrimitive>(JsonPrimitive.serializer(), JsonNull)
+    private object NullAsJsonNullJsonNullSerializer : NullAsElementSerializer<JsonNull>(JsonNull.serializer(), JsonNull)
+    private val noExplicitNullsOrDefaultsJson = Json {
+        explicitNulls = false
+        encodeDefaults = false
+    }
+
+    @Test
+    fun testNullableJsonElementDecoding() {
+        @Serializable
+        data class Wrapper(
+            @Serializable(NullAsJsonNullJsonElementSerializer::class)
+            val value: JsonElement? = null,
+        )
+
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = JsonNull), """{"value":null}""", noExplicitNullsOrDefaultsJson)
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = null), """{}""", noExplicitNullsOrDefaultsJson)
+    }
+
+    @Test
+    fun testNullableJsonPrimitiveDecoding() {
+        @Serializable
+        data class Wrapper(
+            @Serializable(NullAsJsonNullJsonPrimitiveSerializer::class)
+            val value: JsonPrimitive? = null,
+        )
+
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = JsonNull), """{"value":null}""", noExplicitNullsOrDefaultsJson)
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = null), """{}""", noExplicitNullsOrDefaultsJson)
+    }
+
+    @Test
+    fun testNullableJsonNullDecoding() {
+        @Serializable
+        data class Wrapper(
+            @Serializable(NullAsJsonNullJsonNullSerializer::class)
+            val value: JsonNull? = null,
+        )
+
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = JsonNull), """{"value":null}""", noExplicitNullsOrDefaultsJson)
+        assertJsonFormAndRestored(Wrapper.serializer(), Wrapper(value = null), """{}""", noExplicitNullsOrDefaultsJson)
+    }
 }