Implemented encoding null in key and value of a map in Protobuf

Also improved `null` encoding error messages.

Resolves #2760

Author: shanshin

formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/internal/ProtobufTaggedEncoder.kt

@@ -14,6 +14,7 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
         ACCEPTABLE,
         OPTIONAL,
         COLLECTION,
+        LIST_ELEMENT,
         NOT_NULL
     }
     private var nullableMode: NullableMode = NullableMode.NOT_NULL
@@ -37,7 +38,8 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
         if (nullableMode != NullableMode.ACCEPTABLE) {
             val message = when (nullableMode) {
                 NullableMode.OPTIONAL -> "'null' is not supported for optional properties in ProtoBuf"
-                NullableMode.COLLECTION -> "'null' is not supported for collection types in ProtoBuf"
+                NullableMode.COLLECTION -> "'null' is not supported as the value of collection types in ProtoBuf"
+                NullableMode.LIST_ELEMENT -> "'null' is not supported as the value of a list element in ProtoBuf"
                 NullableMode.NOT_NULL -> "'null' is not allowed for not-null properties"
                 else -> "'null' is not supported in ProtoBuf";
             }
@@ -137,12 +139,12 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
                 NullableMode.OPTIONAL
             else {
                 val elementDescriptor = descriptor.getElementDescriptor(index)
-                if (elementDescriptor.kind.isMapOrList())
-                    NullableMode.COLLECTION
-                else if (!descriptor.kind.isMapOrList() && elementDescriptor.isNullable) // or: `serializer.descriptor`
-                    NullableMode.ACCEPTABLE
-                else
-                    NullableMode.NOT_NULL
+                when {
+                    !elementDescriptor.isNullable -> NullableMode.NOT_NULL
+                    elementDescriptor.kind.isMapOrList() -> NullableMode.COLLECTION
+                    descriptor.kind == StructureKind.LIST -> NullableMode.LIST_ELEMENT
+                    else -> NullableMode.ACCEPTABLE
+                }
             }
 
         pushTag(descriptor.getTag(index))
@@ -157,10 +159,15 @@ internal abstract class ProtobufTaggedEncoder : ProtobufTaggedBase(), Encoder, C
     ) {
         nullableMode = if (descriptor.isElementOptional(index))
             NullableMode.OPTIONAL
-        else if (descriptor.getElementDescriptor(index).kind.isMapOrList())
-            NullableMode.COLLECTION
-        else
-            NullableMode.ACCEPTABLE
+        else  {
+            // we don't check nullability of element because this function called always for nullable `value`
+            val elementDescriptor = descriptor.getElementDescriptor(index)
+            when {
+                elementDescriptor.kind.isMapOrList() -> NullableMode.COLLECTION
+                descriptor.kind == StructureKind.LIST -> NullableMode.LIST_ELEMENT
+                else -> NullableMode.ACCEPTABLE
+            }
+        }
 
         pushTag(descriptor.getTag(index))
         encodeNullableSerializableValue(serializer, value)

formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/ProtobufCollectionsTest.kt

@@ -16,25 +16,23 @@ class ProtobufCollectionsTest {
     @Serializable
     data class MapWithNullableNestedLists(val m: Map<List<Int>?, List<Int>?>)
 
+    @Serializable
+    data class MapWithNullableNestedMaps(val m: Map<Map<String, Int>?, Map<String, Int>?>)
+
     @Serializable
     data class NullableListElement(val l: List<Int?>)
 
     @Serializable
     data class NullableMapKey(val m: Map<Int?, Int>)
 
     @Serializable
-    data class NullableMapValue(val m: Map<Int, Int?>)
+    data class NullableMap(val m: Map<Int?, Int?>)
 
     @Test
     fun testEncodeNullAsListElement() {
         assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableListElement(listOf(null))) }
     }
 
-    @Test
-    fun testEncodeNullAsMapKey() {
-        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableMapKey(mapOf(null to 42))) }
-    }
-
     @Test
     fun  testEmptyListIsNotListOfEmpty() {
         val emptyListBytes = ProtoBuf.encodeToByteArray(ListWithNestedList(emptyList()))
@@ -54,15 +52,46 @@ class ProtobufCollectionsTest {
     }
 
     @Test
-    fun testEncodeNullAsMapValue() {
-        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(NullableMapValue(mapOf(42 to null))) }
+    fun testNullMap() {
+        val keyNull = NullableMap(mapOf(null to 42))
+        val valueNull = NullableMap(mapOf(42 to null))
+        val bothNull = NullableMap(mapOf(null to null))
+
+        val encodedKeyNull = ProtoBuf.encodeToHexString(keyNull)
+        val encodedValueNull = ProtoBuf.encodeToHexString(valueNull)
+        val encodedBothNull = ProtoBuf.encodeToHexString(bothNull)
+        assertEquals(encodedKeyNull, "0a02102a")
+        assertEquals(encodedValueNull, "0a02082a")
+        assertEquals(encodedBothNull, "0a00")
+
+        val decodedKeyNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedKeyNull)
+        val decodedValueNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedValueNull)
+        val decodedBothNull = ProtoBuf.decodeFromHexString<NullableMap>(encodedBothNull)
+        assertEquals(decodedKeyNull, keyNull)
+        assertEquals(decodedValueNull, valueNull)
+        assertEquals(decodedBothNull, bothNull)
+    }
+
+    @Test
+    fun testRepeatNullKeyInMap() {
+        // there are two entries in message: (null to 42) and (null to 43), the last one is used
+        val decoded = ProtoBuf.decodeFromHexString<NullableMap>("0a04102a102b")
+        assertEquals(NullableMap(mapOf(null to 43)), decoded)
+    }
+
+    @Test
+    fun testCollectionsInNullableMap() {
+        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(null to listOf(42))) ) }
+        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedLists(mapOf(listOf(42) to null)) ) }
+        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedMaps(mapOf(null to mapOf("key" to 42))) ) }
+        assertFailsWith(SerializationException::class) { ProtoBuf.encodeToByteArray(MapWithNullableNestedMaps(mapOf(mapOf("key" to 42) to null)) ) }
     }
 
     @Test
     fun testEncodeMapWithNullableValue() {
-        val map = NullableMapValue(mapOf(42 to 43))
+        val map = NullableMap(mapOf(42 to 43))
         val bytes = ProtoBuf.encodeToByteArray(map)
-        val decoded = ProtoBuf.decodeFromByteArray<NullableMapValue>(bytes)
+        val decoded = ProtoBuf.decodeFromByteArray<NullableMap>(bytes)
         assertEquals(map, decoded)
     }
 

formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/ProtobufTypeParameterTest.kt

@@ -73,7 +73,7 @@ class ProtobufTypeParameterTest {
                 fail()
             } catch (e: SerializationException) {
                 assertEquals(
-                    "'null' is not supported for collection types in ProtoBuf", e.message
+                    "'null' is not supported as the value of collection types in ProtoBuf", e.message
                 )
             }
         }