V2: Clarify map behavior with message_encoding = DELIMITED (#829)

Author: timostamm

packages/protobuf-test/extra/edition2023-map-encoding.proto

@@ -19,7 +19,10 @@ option features.message_encoding = DELIMITED;
 
 // Map fields are syntactic sugar for a repeated message field with field 1 for
 // key and field 2 for value. Despite that, the file feature message_encoding =
-// DELIMITED should NOT apply to this "synthetic" message.
+// DELIMITED should NOT apply to this "synthetic" message, and it should also
+// not apply to map message values.
 message Edition2023MapEncodingMessage {
-  map<int32, bool> map_field = 77;
+  map<int32, string> string_map = 77;
+  map<int32, Child> message_map = 88;
+  message Child {}
 }

packages/protobuf-test/src/edition2023-serialize.test.ts

@@ -24,7 +24,10 @@ import {
 import * as edition2023_ts from "./gen/ts/extra/edition2023_pb.js";
 import * as edition2023_proto2_ts from "./gen/ts/extra/edition2023-proto2_pb.js";
 import * as edition2023_proto3_ts from "./gen/ts/extra/edition2023-proto3_pb.js";
-import { Edition2023MapEncodingMessageDesc } from "./gen/ts/extra/edition2023-map-encoding_pb.js";
+import {
+  Edition2023MapEncodingMessage_ChildDesc,
+  Edition2023MapEncodingMessageDesc,
+} from "./gen/ts/extra/edition2023-map-encoding_pb.js";
 import { BinaryReader, BinaryWriter, WireType } from "@bufbuild/protobuf/wire";
 
 describe("edition2023 serialization", () => {
@@ -213,26 +216,40 @@ describe("edition2023 serialization", () => {
   describe("message_encoding DELIMITED with maps", () => {
     test("should round-trip", () => {
       const a = create(Edition2023MapEncodingMessageDesc);
-      a.mapField[123] = true;
+      a.stringMap[123] = "abc";
       const bytes = toBinary(Edition2023MapEncodingMessageDesc, a);
       const b = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
       expect(b).toStrictEqual(a);
     });
-    test("should expect LENGTH_PREFIXED", () => {
+    test("should expect LENGTH_PREFIXED map entry", () => {
       const w = new BinaryWriter();
       w.tag(77, WireType.LengthDelimited);
-      w.uint32(4);
+      w.fork();
+      w.tag(1, WireType.Varint).int32(123);
+      w.tag(2, WireType.Varint).string("abc");
+      w.join();
+      const bytes = w.finish();
+      const msg = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
+      expect(msg.stringMap).toStrictEqual({
+        123: "abc",
+      });
+    });
+    test("should expect LENGTH_PREFIXED map value message", () => {
+      const w = new BinaryWriter();
+      w.tag(88, WireType.LengthDelimited);
+      w.fork();
       w.tag(1, WireType.Varint).int32(123);
-      w.tag(2, WireType.Varint).bool(true);
+      w.tag(2, WireType.LengthDelimited).fork().join();
+      w.join();
       const bytes = w.finish();
       const msg = fromBinary(Edition2023MapEncodingMessageDesc, bytes);
-      expect(msg.mapField).toStrictEqual({
-        123: true,
+      expect(msg.messageMap).toStrictEqual({
+        123: create(Edition2023MapEncodingMessage_ChildDesc),
       });
     });
-    test("should serialize LENGTH_PREFIXED", () => {
+    test("should serialize map entry LENGTH_PREFIXED", () => {
       const msg = create(Edition2023MapEncodingMessageDesc);
-      msg.mapField[123] = true;
+      msg.stringMap[123] = "abc";
       const bytes = toBinary(Edition2023MapEncodingMessageDesc, msg);
       const r = new BinaryReader(bytes);
       {
@@ -242,17 +259,43 @@ describe("edition2023 serialization", () => {
         const length = r.uint32();
         expect(length).toBe(r.len - r.pos);
       }
+      {
+        const [number] = r.tag();
+        expect(number).toBe(1);
+        expect(r.int32()).toBe(123);
+      }
+      {
+        const [number] = r.tag();
+        expect(number).toBe(2);
+        expect(r.string()).toBe("abc");
+      }
+      {
+        expect(r.pos).toBe(r.len);
+      }
+    });
+    test("should serialize map value message LENGTH_PREFIXED", () => {
+      const msg = create(Edition2023MapEncodingMessageDesc);
+      msg.messageMap[123] = create(Edition2023MapEncodingMessage_ChildDesc);
+      const bytes = toBinary(Edition2023MapEncodingMessageDesc, msg);
+      const r = new BinaryReader(bytes);
       {
         const [number, wireType] = r.tag();
+        expect(number).toBe(88);
+        expect(wireType).toBe(WireType.LengthDelimited);
+        const length = r.uint32();
+        expect(length).toBe(r.len - r.pos);
+      }
+      {
+        const [number] = r.tag();
         expect(number).toBe(1);
-        expect(wireType).toBe(WireType.Varint);
         expect(r.int32()).toBe(123);
       }
       {
         const [number, wireType] = r.tag();
         expect(number).toBe(2);
-        expect(wireType).toBe(WireType.Varint);
-        expect(r.bool()).toBe(true);
+        expect(wireType).toBe(WireType.LengthDelimited);
+        const length = r.uint32();
+        expect(length).toBe(0);
       }
       {
         expect(r.pos).toBe(r.len);

packages/protobuf-test/src/gen/js/extra/edition2023-map-encoding_pb.d.ts

@@ -500,7 +500,8 @@ type descFieldMapCommon<T extends ScalarType = ScalarType> = T extends Exclude<S
   readonly oneof: undefined;
   /**
    * Encode the map entry message delimited (a.k.a. proto2 group encoding),
-   * or length-prefixed? This is always false for map fields.
+   * or length-prefixed? As of Edition 2023, this is always false for map fields,
+   * and also applies to map values, if they are messages.
    */
   readonly delimitedEncoding: false;
 } : never;