Simplify, fix, and add tests for timestamp and duration handling

packages/cel/src/duration.test.ts

@@ -21,20 +21,170 @@ import { DurationSchema } from "@bufbuild/protobuf/wkt";
 import { createDuration } from "./duration.js";
 
 void suite("duration", () => {
-  void test("createDuration()", () => {
-    let actual = createDuration(0n, -1);
-    assert.ok(isMessage(actual, DurationSchema));
-    assert.equal(actual.seconds, -1n);
-    assert.equal(actual.nanos, 999999999);
+  void suite("createDuration()", () => {
+    void test("0s, 0ns", () => {
+      let actual = createDuration(0n, 0);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("0s, 1ns", () => {
+      let actual = createDuration(0n, 1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("0s, -1ns", () => {
+      let actual = createDuration(0n, -1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, -1);
+    });
+    void test("0s, 999,999,999ns", () => {
+      let actual = createDuration(0n, 999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 999999999);
+    });
+    void test("0s, -999,999,999ns", () => {
+      let actual = createDuration(0n, -999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, -999999999);
+    });
+    void test("0s, 1,000,000,000ns", () => {
+      let actual = createDuration(0n, 1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 1n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("0s, -1,000,000,000ns", () => {
+      let actual = createDuration(0n, -1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -1n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("0s, 1,000,000,001ns", () => {
+      let actual = createDuration(0n, 1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 1n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("0s, -1,000,000,001ns", () => {
+      let actual = createDuration(0n, -1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -1n);
+      assert.equal(actual.nanos, -1);
+    });
 
-    actual = createDuration(0n, -999999999);
-    assert.ok(isMessage(actual, DurationSchema));
-    assert.equal(actual.seconds, -1n);
-    assert.equal(actual.nanos, 1);
+    void test("1s, 0ns", () => {
+      let actual = createDuration(1n, 0);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 1n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("1s, 1ns", () => {
+      let actual = createDuration(1n, 1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 1n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("1s, -1ns", () => {
+      let actual = createDuration(1n, -1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 999999999);
+    });
+    void test("1s, 999,999,999ns", () => {
+      let actual = createDuration(1n, 999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 1n);
+      assert.equal(actual.nanos, 999999999);
+    });
+    void test("1s, -999,999,999ns", () => {
+      let actual = createDuration(1n, -999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("1s, 1,000,000,000ns", () => {
+      let actual = createDuration(1n, 1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 2n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("1s, -1,000,000,000ns", () => {
+      let actual = createDuration(1n, -1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("1s, 1,000,000,001ns", () => {
+      let actual = createDuration(1n, 1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 2n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("1s, -1,000,000,001ns", () => {
+      let actual = createDuration(1n, -1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, -1);
+    });
 
-    actual = createDuration(0n, -1000000000);
-    assert.ok(isMessage(actual, DurationSchema));
-    assert.equal(actual.seconds, -1n);
-    assert.equal(actual.nanos, 0);
+    void test("-1s, 0ns", () => {
+      let actual = createDuration(-1n, 0);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -1n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("-1s, 1ns", () => {
+      let actual = createDuration(-1n, 1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, -999999999);
+    });
+    void test("-1s, -1ns", () => {
+      let actual = createDuration(-1n, -1);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -1n);
+      assert.equal(actual.nanos, -1);
+    });
+    void test("-1s, 999,999,999ns", () => {
+      let actual = createDuration(-1n, 999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, -1);
+    });
+    void test("-1s, -999,999,999ns", () => {
+      let actual = createDuration(-1n, -999999999);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -1n);
+      assert.equal(actual.nanos, -999999999);
+    });
+    void test("-1s, 1,000,000,000ns", () => {
+      let actual = createDuration(-1n, 1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("-1s, -1,000,000,000ns", () => {
+      let actual = createDuration(-1n, -1000000000);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -2n);
+      assert.equal(actual.nanos, 0);
+    });
+    void test("-1s, 1,000,000,001ns", () => {
+      let actual = createDuration(-1n, 1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, 0n);
+      assert.equal(actual.nanos, 1);
+    });
+    void test("-1s, -1,000,000,001ns", () => {
+      let actual = createDuration(-1n, -1000000001);
+      assert.ok(isMessage(actual, DurationSchema));
+      assert.equal(actual.seconds, -2n);
+      assert.equal(actual.nanos, -1);
+    });
   });
 });

packages/cel/src/duration.ts

@@ -15,26 +15,48 @@
 import { type Duration, DurationSchema } from "@bufbuild/protobuf/wkt";
 import { create } from "@bufbuild/protobuf";
 
+const ONE_SECOND = 1000000000n;
+const MAX_DURATION_NANOS = 9223372036854775807n;
+const MIN_DURATION_NANOS = -9223372036854775808n;
+
 /**
- * Create a new Duration, validating the fields are in range.
+ * Create a new Duration, canonicalizing the representation
  */
-export function createDuration(seconds: bigint, nanos: number): Duration {
-  if (nanos >= 1000000000) {
-    seconds += BigInt(nanos / 1000000000);
-    nanos = nanos % 1000000000;
-  } else if (nanos < 0) {
-    const negSeconds = Math.ceil(-nanos / 1000000000);
-    seconds -= BigInt(negSeconds);
-    nanos = nanos + negSeconds * 1000000000;
-  }
-  // Must fit in 64 bits of nanoseconds for compatibility with golang
-  const totalNanos = seconds * 1000000000n + BigInt(nanos);
-  if (totalNanos > 9223372036854775807n || totalNanos < -9223372036854775808n) {
+export function createDuration(
+  seconds = 0n,
+  ns: bigint | number = 0n,
+): Duration {
+  // resolves differing signs
+  const fullNanos = seconds * ONE_SECOND + BigInt(ns);
+
+  // `fullNanos` must fit in a 64-bit signed integer per the spec
+  if (fullNanos > MAX_DURATION_NANOS || fullNanos < MIN_DURATION_NANOS) {
     throw new Error("duration out of range");
   }
-  return create(DurationSchema, { seconds: seconds, nanos: nanos });
+
+  return create(DurationSchema, {
+    seconds: fullNanos / ONE_SECOND,
+    nanos: Number(fullNanos % ONE_SECOND), // preserves sign
+  });
 }
 
+// Not in the spec, but for safety, we want to make sure this is not insane.
+const DURATION_STRING_LENGTH_LIMIT = 128;
+const UNITS = Object.freeze({
+  ns: 1n,
+  us: 1000n,
+  µs: 1000n,
+  ms: 1000n * 1000n,
+  s: 1000n * 1000n * 1000n,
+  m: 1000n * 1000n * 1000n * 60n,
+  h: 1000n * 1000n * 1000n * 60n * 60n,
+});
+
+const INT_REGEXP = /^\d+/;
+const UNIT_REGEXP = new RegExp(`^(${Object.keys(UNITS).join("|")})`) as {
+  exec(target: string): [keyof typeof UNITS] | null;
+};
+
 /**
  * Parses a CEL duration string.
  *
@@ -44,48 +66,52 @@ export function createDuration(seconds: bigint, nanos: number): Duration {
  * Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
  */
 export function parseDuration(str: string): Duration {
-  // The regex grouping the number and the unit is:
-  const re = /([-+]?(?:\d+|\d+\.\d*|\d*\.\d+))(ns|us|µs|ms|s|m|h)/;
-  // Loop over the string matching the regex.
-  let seconds = 0n;
-  let nanos = 0;
-  let remaining = str;
-  while (remaining.length > 0) {
-    const match = re.exec(remaining);
-    if (match === null) {
-      throw badDurationStr("invalid syntax");
-    }
-    const [, numStr, unit] = match;
-    const num = Number(numStr);
-    if (Number.isNaN(num)) {
+  if (str.length > DURATION_STRING_LENGTH_LIMIT) {
+    throw badDurationStr(
+      `duration string exceeds ${DURATION_STRING_LENGTH_LIMIT} characters`,
+    );
+  }
+
+  // handle unitless-zero (which might have a sign)
+  if (/^[-+]?0$/.test(str)) return createDuration();
+
+  // extract the sign
+  const [sign, values] = /^[+-]/.test(str)
+    ? [str[0] === "+" ? 1n : -1n, str.slice(1)]
+    : [1n, str];
+
+  let fullNanos = 0n;
+  let remainder = values;
+  while (remainder.length > 0) {
+    // consume integer part
+    const int = INT_REGEXP.exec(remainder)?.[0];
+    remainder = remainder.slice(int?.length ?? 0);
+
+    // if it exists, consume decimal point
+    if (remainder[0] === ".") remainder = remainder.slice(1);
+
+    // consume fractional part — if we didn't consume a decimal point, this will
+    // not consume anything, since all digits will have already been consumed
+    const fraction = INT_REGEXP.exec(remainder)?.[0];
+    remainder = remainder.slice(fraction?.length ?? 0);
+
+    // consume unit
+    const unit = UNIT_REGEXP.exec(remainder)?.[0];
+    remainder = remainder.slice(unit?.length ?? 0);
+
+    // we must get a unit and either an integer part or fractional part
+    if ((int ?? fraction) === undefined || unit === undefined) {
       throw badDurationStr("invalid syntax");
     }
-    switch (unit) {
-      case "ns":
-        nanos += num;
-        break;
-      case "us":
-      case "µs":
-        nanos += num * 1000;
-        break;
-      case "ms":
-        nanos += num * 1000000;
-        break;
-      case "s":
-        seconds += BigInt(num);
-        break;
-      case "m":
-        seconds += BigInt(num * 60);
-        break;
-      case "h":
-        seconds += BigInt(num * 3600);
-        break;
-      default:
-        throw badDurationStr("invalid syntax");
-    }
-    remaining = remaining.slice(match[0].length);
+
+    const factor = UNITS[unit];
+
+    fullNanos += BigInt(int ?? 0) * factor;
+    fullNanos +=
+      (BigInt(fraction ?? 0) * factor) / 10n ** BigInt(fraction?.length ?? 0);
   }
-  return createDuration(seconds, nanos);
+
+  return createDuration(0n, sign * fullNanos);
 }
 
 function badDurationStr(e: string) {