fix: repeated uint64 fields do not encode properly with bigint option (…

…#751) * Fix: repeated uint64 fields do not encode properly with --forceLong=bigint * Add regression test. * Fix tests. Co-authored-by: Stephen Haberman <stephen.haberman@gmail.com>
stephenh · Jan 9, 2023 · dcdd7e2 · dcdd7e2
1 parent 024cac5
commit dcdd7e2
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 23 deletions.
diff --git a/integration/simple-long-bigint/numbers-long-string-test.ts b/integration/simple-long-bigint/numbers-long-string-test.ts
@@ -1,14 +1,14 @@
-import { Reader } from 'protobufjs';
-import * as Long from 'long';
-import { Numbers } from './simple';
-import { simple as pbjs, google } from './pbjs';
+import { Reader } from "protobufjs";
+import * as Long from "long";
+import { Numbers } from "./simple";
+import { simple as pbjs, google } from "./pbjs";
 import INumbers = pbjs.INumbers;
 import PbNumbers = pbjs.Numbers;
 import UInt64Value = google.protobuf.UInt64Value;
 import PbTimestamp = google.protobuf.Timestamp;
 
-describe('number', () => {
-  it('generates types correctly', () => {
+describe("number", () => {
+  it("generates types correctly", () => {
     const simple: Numbers = {
       double: 1,
       float: 2,
@@ -23,15 +23,16 @@ describe('number', () => {
       sfixed32: 11,
       sfixed64: BigInt("12"),
       guint64: BigInt("13"),
-      timestamp: new Date('1970-02-03T04:05:06.0071Z')
+      timestamp: new Date("1970-02-03T04:05:06.0071Z"),
+      uint64s: [BigInt("14"), BigInt("15"), BigInt("16")],
     };
     expect(simple.int64).toEqual(BigInt("4"));
     expect(simple.uint64).toEqual(BigInt("6"));
     expect(simple.guint64).toEqual(BigInt("13"));
-    expect(simple.timestamp).toEqual(new Date('1970-02-03T04:05:06.0071Z'))
+    expect(simple.timestamp).toEqual(new Date("1970-02-03T04:05:06.0071Z"));
   });
 
-  it('can decode', () => {
+  it("can decode", () => {
     const s1: INumbers = {
       double: 1,
       float: 2,
@@ -48,8 +49,9 @@ describe('number', () => {
       guint64: new UInt64Value({ value: Long.fromNumber(13, true) }),
       timestamp: new PbTimestamp({
         nanos: 7000001,
-        seconds: Long.fromNumber(1)
-      })
+        seconds: Long.fromNumber(1),
+      }),
+      uint64s: [Long.fromNumber(14), Long.fromNumber(15), Long.fromNumber(16)],
     };
     const expected: Numbers = {
       double: 1,
@@ -65,13 +67,14 @@ describe('number', () => {
       sfixed32: 11,
       sfixed64: BigInt("12"),
       guint64: BigInt("13"),
-      timestamp: new Date('1970-01-01T00:00:01.007000001Z')
+      timestamp: new Date("1970-01-01T00:00:01.007000001Z"),
+      uint64s: [BigInt("14"), BigInt("15"), BigInt("16")],
     };
     const s2 = Numbers.decode(Reader.create(PbNumbers.encode(PbNumbers.fromObject(s1)).finish()));
     expect(s2).toEqual(expected);
   });
 
-  it('can encode', () => {
+  it("can encode", () => {
     const s1: Numbers = {
       double: 1,
       float: 2,
@@ -86,7 +89,8 @@ describe('number', () => {
       sfixed32: 11,
       sfixed64: BigInt("12"),
       guint64: BigInt("13"),
-      timestamp: new Date('1980-01-01T00:00:01.123Z')
+      timestamp: new Date("1980-01-01T00:00:01.123Z"),
+      uint64s: [BigInt("14"), BigInt("15"), BigInt("16")],
     };
     const expected: INumbers = {
       double: 1,
@@ -103,17 +107,18 @@ describe('number', () => {
       sfixed64: Long.fromNumber(12),
       guint64: new UInt64Value({ value: Long.fromNumber(13, true) }),
       timestamp: new PbTimestamp({
-        "nanos": 123000000,
-        "seconds": Long.fromNumber(315532801)
-      })
+        nanos: 123000000,
+        seconds: Long.fromNumber(315532801),
+      }),
+      uint64s: [Long.fromNumber(14, true), Long.fromNumber(15, true), Long.fromNumber(16, true)],
     };
     const s2 = PbNumbers.toObject(PbNumbers.decode(Numbers.encode(s1).finish()));
     expect(s2).toEqual({
-      ...expected
+      ...expected,
     });
   });
 
-  it('can decode and fallback to default values', () => {
+  it("can decode and fallback to default values", () => {
     const s1: INumbers = {};
     const s2 = Numbers.decode(Reader.create(PbNumbers.encode(PbNumbers.fromObject(s1)).finish()));
     expect(s2.double).toEqual(0);
@@ -128,10 +133,11 @@ describe('number', () => {
     expect(s2.fixed64).toEqual(BigInt("0"));
     expect(s2.sfixed32).toEqual(0);
     expect(s2.guint64).toEqual(undefined);
-    expect(s2.timestamp).toEqual(undefined)
+    expect(s2.timestamp).toEqual(undefined);
+    expect(s2.uint64s).toEqual([]);
   });
 
-  it('observes how pbjs handles null', () => {
+  it("observes how pbjs handles null", () => {
     // the types are in theory only useful for construction
     const s1 = PbNumbers.fromObject({ double: null, float: 1 });
     // as after construction, they return the empty string
@@ -140,7 +146,7 @@ describe('number', () => {
     expect(s2.double).toEqual(0);
   });
 
-  it('has fromPartial', () => {
+  it("has fromPartial", () => {
     const s1 = Numbers.fromPartial({});
     expect(s1).toMatchInlineSnapshot(`
   Object {
@@ -158,6 +164,7 @@ describe('number', () => {
     "timestamp": undefined,
     "uint32": 0,
     "uint64": 0n,
+    "uint64s": Array [],
   }
   `);
   });

diff --git a/integration/simple-long-bigint/simple.bin b/integration/simple-long-bigint/simple.bin
diff --git a/integration/simple-long-bigint/simple.proto b/integration/simple-long-bigint/simple.proto
@@ -19,4 +19,5 @@ message Numbers {
   sfixed64 sfixed64 = 12;
   google.protobuf.UInt64Value guint64 = 13;
   google.protobuf.Timestamp timestamp = 14;
+  repeated uint64 uint64s = 15;
 }
diff --git a/integration/simple-long-bigint/simple.ts b/integration/simple-long-bigint/simple.ts
@@ -21,6 +21,7 @@ export interface Numbers {
   sfixed64: bigint;
   guint64: bigint | undefined;
   timestamp: Date | undefined;
+  uint64s: bigint[];
 }
 
 function createBaseNumbers(): Numbers {
@@ -39,6 +40,7 @@ function createBaseNumbers(): Numbers {
     sfixed64: BigInt("0"),
     guint64: undefined,
     timestamp: undefined,
+    uint64s: [],
   };
 }
 
@@ -86,6 +88,11 @@ export const Numbers = {
     if (message.timestamp !== undefined) {
       Timestamp.encode(toTimestamp(message.timestamp), writer.uint32(114).fork()).ldelim();
     }
+    writer.uint32(122).fork();
+    for (const v of message.uint64s) {
+      writer.uint64(v.toString());
+    }
+    writer.ldelim();
     return writer;
   },
 
@@ -138,6 +145,16 @@ export const Numbers = {
         case 14:
           message.timestamp = fromTimestamp(Timestamp.decode(reader, reader.uint32()));
           break;
+        case 15:
+          if ((tag & 7) === 2) {
+            const end2 = reader.uint32() + reader.pos;
+            while (reader.pos < end2) {
+              message.uint64s.push(longToBigint(reader.uint64() as Long));
+            }
+          } else {
+            message.uint64s.push(longToBigint(reader.uint64() as Long));
+          }
+          break;
         default:
           reader.skipType(tag & 7);
           break;
@@ -162,6 +179,7 @@ export const Numbers = {
       sfixed64: isSet(object.sfixed64) ? BigInt(object.sfixed64) : BigInt("0"),
       guint64: isSet(object.guint64) ? BigInt(object.guint64) : undefined,
       timestamp: isSet(object.timestamp) ? fromJsonTimestamp(object.timestamp) : undefined,
+      uint64s: Array.isArray(object?.uint64s) ? object.uint64s.map((e: any) => BigInt(e)) : [],
     };
   },
 
@@ -181,6 +199,11 @@ export const Numbers = {
     message.sfixed64 !== undefined && (obj.sfixed64 = message.sfixed64.toString());
     message.guint64 !== undefined && (obj.guint64 = message.guint64);
     message.timestamp !== undefined && (obj.timestamp = message.timestamp.toISOString());
+    if (message.uint64s) {
+      obj.uint64s = message.uint64s.map((e) => e.toString());
+    } else {
+      obj.uint64s = [];
+    }
     return obj;
   },
 
@@ -200,6 +223,7 @@ export const Numbers = {
     message.sfixed64 = object.sfixed64 ?? BigInt("0");
     message.guint64 = object.guint64 ?? undefined;
     message.timestamp = object.timestamp ?? undefined;
+    message.uint64s = object.uint64s?.map((e) => e) || [];
     return message;
   },
 };

diff --git a/src/main.ts b/src/main.ts
@@ -1151,10 +1151,11 @@ function generateEncode(ctx: Context, fullName: string, messageDesc: DescriptorP
       } else {
         // Ideally we'd reuse `writeSnippet` but it has tagging embedded inside of it.
         const tag = ((field.number << 3) | 2) >>> 0;
+        const rhs = (x: string) => (isLong(field) && options.forceLong === LongOption.BIGINT ? `${x}.toString()` : x);
         const listWriteSnippet = code`
           writer.uint32(${tag}).fork();
           for (const v of message.${fieldName}) {
-            writer.${toReaderCall(field)}(v);
+            writer.${toReaderCall(field)}(${rhs("v")});
           }
           writer.ldelim();
         `;