Add packed support to the ProtoBufSchemaGenerator.

5 files changed, 80 insertions, 5 deletions

diff --git a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt
index df57c9bf..b91ddab2 100644
--- a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt
+++ b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt
@@ -6,6 +6,7 @@ import kotlinx.serialization.*
 import kotlinx.serialization.builtins.*
 import kotlinx.serialization.descriptors.*
 import kotlinx.serialization.protobuf.*
+import kotlinx.serialization.protobuf.internal.*
 
 /**
  * Experimental generator of ProtoBuf schema that is compatible with [serializable][Serializable] Kotlin classes
@@ -175,9 +176,11 @@ public object ProtoBufSchemaGenerator {
 
             val fieldDescriptor = messageDescriptor.getElementDescriptor(index)
 
+            val isList = fieldDescriptor.isProtobufRepeated
+
             nestedTypes += when {
                 fieldDescriptor.isProtobufNamedType -> generateNamedType(messageType, index)
-                fieldDescriptor.isProtobufRepeated -> generateListType(messageType, index)
+                isList -> generateListType(messageType, index)
                 fieldDescriptor.isProtobufMap -> generateMapType(messageType, index)
                 else -> throw IllegalStateException(
                     "Unprocessed message field type with serial name " +
@@ -192,7 +195,16 @@ public object ProtoBufSchemaGenerator {
                 throw IllegalArgumentException("Field number $number is repeated in the class with serial name ${messageDescriptor.serialName}")
             }
 
-            append(' ').append(fieldName).append(" = ").append(number).appendLine(';')
+            append(' ').append(fieldName).append(" = ").append(number)
+
+            val isPackRequested = annotations.filterIsInstance<ProtoPacked>().singleOrNull() != null
+
+            when {
+                !isPackRequested -> appendLine(';')
+                !isList -> throw IllegalArgumentException("ProtoPacked annotation provided for ${messageDescriptor.getElementName(index)}: $fieldDescriptor, but packing is only valid on repeated fields (lists)")
+                !fieldDescriptor.getElementDescriptor(0).isPackable -> throw IllegalArgumentException("ProtoPacked annotation provided for ${messageDescriptor.getElementName(index)}: $fieldDescriptor, but packed can only be applied to primitive numeric types")
+                else -> appendLine(" [packed=true];")
+            }
         }
         appendLine('}')
 
diff --git a/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt b/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt
index 4517fb9e..81858df2 100644
--- a/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt
+++ b/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt
@@ -1,8 +1,9 @@
 package kotlinx.serialization.protobuf.schema
 
 import kotlinx.serialization.*
-import kotlinx.serialization.protobuf.ProtoNumber
+import kotlinx.serialization.protobuf.*
 import kotlin.test.Test
+import kotlin.test.assertContains
 import kotlin.test.assertFailsWith
 
 class SchemaValidationsTest {
@@ -38,6 +39,12 @@ class SchemaValidationsTest {
         SECOND
     }
 
+    @Serializable
+    class NonListPackedField(@ProtoPacked val i: Int)
+
+    @Serializable
+    class ListOfNonNumberPackedField(@ProtoPacked val s: List<String>)
+
 
     @Test
     fun testInvalidEnumElementSerialName() {
@@ -76,6 +83,20 @@ class SchemaValidationsTest {
     }
 
     @Test
+    fun testNonListPackedField() {
+        val descriptor = NonListPackedField.serializer().descriptor
+        val e = assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptor) }
+        assertContains(e.message ?: "", "only valid on repeated fields (lists)")
+    }
+
+    @Test
+    fun testNonNumberPackedField() {
+        val descriptor = ListOfNonNumberPackedField.serializer().descriptor
+        val e = assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptor) }
+        assertContains(e.message ?: "", "packed can only be applied to primitive numeric types")
+    }
+
+    @Test
     fun testInvalidOptionName() {
         val descriptors = listOf(ValidClass.serializer().descriptor)
         assertFailsWith(IllegalArgumentException::class) {
diff --git a/formats/protobuf/jvmTest/resources/PackedListClass.proto b/formats/protobuf/jvmTest/resources/PackedListClass.proto
new file mode 100644
index 00000000..bf096a16
--- /dev/null
+++ b/formats/protobuf/jvmTest/resources/PackedListClass.proto
@@ -0,0 +1,23 @@
+syntax = "proto2";
+
+package kotlinx.serialization.protobuf.schema.generator;
+
+// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.PackedListClass'
+message PackedListClass {
+  repeated int32 intList = 1 [packed=true];
+  repeated int32 intArray = 2 [packed=true];
+  // WARNING: nullable elements of collections can not be represented in protobuf
+  repeated int32 boxedIntArray = 3 [packed=true];
+  repeated OptionsClass messageList = 4;
+  repeated OverriddenEnumName enumList = 5;
+}
+
+// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.OptionsClass'
+message OptionsClass {
+  required int32 i = 1;
+}
+
+enum OverriddenEnumName {
+  FIRST = 0;
+  OverriddenElementName = 1;
+}
diff --git a/formats/protobuf/jvmTest/resources/common/schema.proto b/formats/protobuf/jvmTest/resources/common/schema.proto
index 1a5bc8d3..e8a0b4cd 100644
--- a/formats/protobuf/jvmTest/resources/common/schema.proto
+++ b/formats/protobuf/jvmTest/resources/common/schema.proto
@@ -42,6 +42,16 @@ message ListClass {
   repeated OverriddenEnumName enumList = 5;
 }
 
+// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.PackedListClass'
+message PackedListClass {
+  repeated int32 intList = 1 [packed=true];
+  repeated int32 intArray = 2 [packed=true];
+  // WARNING: nullable elements of collections can not be represented in protobuf
+  repeated int32 boxedIntArray = 3 [packed=true];
+  repeated OptionsClass messageList = 4;
+  repeated OverriddenEnumName enumList = 5;
+}
+
 // serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.MapClass'
 message MapClass {
   map<int32, float> scalarMap = 1;
diff --git a/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt b/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt
index a8ffc31c..7b075a4e 100644
--- a/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt
+++ b/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt
@@ -3,8 +3,7 @@ package kotlinx.serialization.protobuf.schema
 import kotlinx.serialization.*
 import kotlinx.serialization.descriptors.SerialDescriptor
 import kotlinx.serialization.protobuf.ProtoIntegerType
-import kotlinx.serialization.protobuf.ProtoNumber
-import kotlinx.serialization.protobuf.ProtoType
+import kotlinx.serialization.protobuf.*
 import kotlin.reflect.KClass
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -16,6 +15,7 @@ internal val commonClasses = listOf(
     GenerationTest.FieldNumberClass::class,
     GenerationTest.SerialNameClass::class,
     GenerationTest.ListClass::class,
+    GenerationTest.PackedListClass::class,
     GenerationTest.MapClass::class,
     GenerationTest.OptionalClass::class,
     GenerationTest.ContextualHolder::class,
@@ -93,6 +93,15 @@ class GenerationTest {
     )
 
     @Serializable
+    class PackedListClass(
+        @ProtoPacked val intList: List<Int>,
+        @ProtoPacked val intArray: IntArray,
+        @ProtoPacked val boxedIntArray: Array<Int?>,
+        val messageList: List<OptionsClass>,
+        val enumList: List<SerialNameEnum>
+    )
+
+    @Serializable
     class MapClass(
         val scalarMap: Map<Int, Float>,
         val bytesMap: Map<Int, List<Byte>>,