fix: fix ibids. test ibids and slots (#2625)

Author: erights

packages/marshal/src/marshal-justin.js

@@ -6,7 +6,7 @@
 import { Nat } from '@agoric/nat';
 import { assert, details as X, q } from '@agoric/assert';
 import { QCLASS, getErrorConstructor } from './marshal';
-// import { makeReviverIbidTable } from './ibidTables';
+import { makeReviverIbidTable } from './ibidTables';
 
 import './types';
 
@@ -31,16 +31,30 @@ const { stringify: quote } = JSON;
 const makeYesIndenter = () => {
   const strings = [];
   let level = 0;
-  const line = () => strings.push('\n', '  '.repeat(level));
+  let needSpace = false;
+  const line = () => {
+    needSpace = false;
+    return strings.push('\n', '  '.repeat(level));
+  };
   return harden({
     open: openBracket => {
-      assert(level >= 1);
       level += 1;
-      return strings.push(' ', openBracket);
+      if (needSpace) {
+        strings.push(' ');
+      }
+      needSpace = false;
+      return strings.push(openBracket);
     },
     line,
-    next: token => strings.push(' ', token),
+    next: token => {
+      if (needSpace && token !== ',') {
+        strings.push(' ');
+      }
+      needSpace = true;
+      return strings.push(token);
+    },
     close: closeBracket => {
+      assert(level >= 1);
       level -= 1;
       line();
       return strings.push(closeBracket);
@@ -96,60 +110,30 @@ const makeNoIndenter = () => {
   });
 };
 
-const registerIbid = _rawTree => {
-  // doesn't do anything yet.
-};
-
-const startIbid = _rawTree => {
-  // doesn't do anything yet.
-};
-
-const finishIbid = _rawTree => {
-  // doesn't do anything yet.
-};
-
 const identPattern = /^[a-zA-Z]\w*$/;
 
 /**
  * @param {Encoding} encoding
- * @param {CyclePolicy} _cyclePolicy
  * @param {boolean=} shouldIndent
  * @returns {string}
  */
-const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
-  // const ibidTable = makeReviverIbidTable(cyclePolicy);
-  const makeIndenter = shouldIndent ? makeYesIndenter : makeNoIndenter;
-  const out = makeIndenter();
-
-  const decodeProperty = (name, value) => {
-    out.line();
-    assert.typeof(name, 'string', X`Property name ${name} of must be a string`);
-    if (name === '__proto__') {
-      // JavaScript interprets `{__proto__: x, ...}`
-      // as making an object inheriting from `x`, whereas
-      // in JSON it is simply a property name. Preserve the
-      // JSON meaning.
-      out.next(`["__proto__"]:`);
-    } else if (identPattern.test(name)) {
-      out.next(`${name}:`);
-    } else {
-      out.next(`${quote(name)}:`);
-    }
-    // eslint-disable-next-line no-use-before-define
-    recur(value);
-    out.next(',');
-  };
+const decodeToJustin = (encoding, shouldIndent = false) => {
+  const ibidTable = makeReviverIbidTable('forbidCycles');
+  const ibidMap = new Map();
 
   /**
-   * Modeled after `fullRevive` in marshal.js
+   * The first pass populates ibidMap for use by `decode`.
+   * Since this is the first pass, it should do all input validation.
+   * Its control flow should mirror `recur` as closely as possible
+   * and the two should be maintained together. They must visit everything
+   * in the same order.
    *
    * @param {Encoding} rawTree
-   * @returns {number}
+   * @returns {void}
    */
-  const recur = rawTree => {
+  const prepare = rawTree => {
     if (Object(rawTree) !== rawTree) {
-      // primitives get quoted
-      return out.next(quote(rawTree));
+      return;
     }
     // Assertions of the above to narrow the type.
     assert.typeof(rawTree, 'object');
@@ -162,17 +146,12 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
         X`invalid qclass typeof ${q(typeof qclass)}`,
       );
       assert(!Array.isArray(rawTree));
-      // Switching on `encoded[QCLASS]` (or anything less direct, like
-      // `qclass`) does not discriminate rawTree in typescript@4.2.3 and
-      // earlier.
       switch (rawTree['@qclass']) {
-        // Encoding of primitives not handled by JSON
         case 'undefined':
         case 'NaN':
         case 'Infinity':
         case '-Infinity': {
-          // Their qclass is their expression source.
-          return out.next(qclass);
+          return;
         }
         case 'bigint': {
           const { digits } = rawTree;
@@ -181,19 +160,20 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
             'string',
             X`invalid digits typeof ${q(typeof digits)}`,
           );
-          return out.next(`${BigInt(digits)}n`);
+          return;
         }
         case '@@asyncIterator': {
-          return out.next('Symbol.asyncIterator');
+          return;
         }
-
         case 'ibid': {
           const { index } = rawTree;
-          return out.next(`getIbid(${index})`);
+          // ibidTable's `get` does some input validation.
+          const rawNode = ibidTable.get(index);
+          ibidMap.set(rawNode, index);
+          return;
         }
-
         case 'error': {
-          registerIbid(rawTree);
+          ibidTable.register(rawTree);
           const { name, message } = rawTree;
           assert.typeof(
             name,
@@ -209,27 +189,24 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
             'string',
             X`invalid error message typeof ${q(typeof message)}`,
           );
-          return out.next(`${name}(${quote(message)})`);
+          return;
         }
-
         case 'slot': {
-          registerIbid(rawTree);
-          let { index, iface } = rawTree;
-          index = Number(Nat(index));
+          ibidTable.register(rawTree);
+          const { index, iface } = rawTree;
+          assert.typeof(index, 'number');
+          Nat(index);
           assert.typeof(iface, 'string');
-          iface = quote(iface);
-          return out.next(`getSlotVal(${index},${iface})`);
+          return;
         }
-
         case 'hilbert': {
-          startIbid(rawTree);
+          ibidTable.start(rawTree);
           const { original, rest } = rawTree;
           assert(
             'original' in rawTree,
             X`Invalid Hilbert Hotel encoding ${rawTree}`,
           );
-          out.open('{');
-          decodeProperty(QCLASS, original);
+          prepare(original);
           if ('rest' in rawTree) {
             assert.typeof(
               rest,
@@ -245,14 +222,155 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
               !(QCLASS in rest),
               X`Rest encoding ${rest} must not contain ${q(QCLASS)}`,
             );
-            startIbid(rest);
+            ibidTable.start(rest);
+            const names = ownKeys(rest);
+            for (const name of names) {
+              assert.typeof(
+                name,
+                'string',
+                X`Property name ${name} of ${rawTree} must be a string`,
+              );
+              prepare(rest[name]);
+            }
+            ibidTable.finish(rest);
+          }
+          ibidTable.finish(rawTree);
+          return;
+        }
+
+        default: {
+          assert.fail(X`unrecognized ${q(QCLASS)} ${q(qclass)}`, TypeError);
+        }
+      }
+    } else if (Array.isArray(rawTree)) {
+      ibidTable.start(rawTree);
+      const { length } = rawTree;
+      for (let i = 0; i < length; i += 1) {
+        prepare(rawTree[i]);
+      }
+      ibidTable.finish(rawTree);
+    } else {
+      ibidTable.start(rawTree);
+      const names = ownKeys(rawTree);
+      for (const name of names) {
+        assert.typeof(
+          name,
+          'string',
+          X`Property name ${name} of ${rawTree} must be a string`,
+        );
+        prepare(rawTree[name]);
+      }
+      ibidTable.finish(rawTree);
+    }
+  };
+
+  const makeIndenter = shouldIndent ? makeYesIndenter : makeNoIndenter;
+  const out = makeIndenter();
+
+  /**
+   * This is the second pass recursion after the first pass `prepare`.
+   * The first pass initialized `ibidMap` and did input validation so
+   * here we can safely assume everything it validated.
+   *
+   * @param {Encoding} rawTree
+   * @returns {number}
+   */
+  const decode = rawTree => {
+    if (ibidMap.has(rawTree)) {
+      const index = ibidMap.get(rawTree);
+      out.next(`initIbid(${index},`);
+      // eslint-disable-next-line no-use-before-define
+      recur(rawTree);
+      return out.next(')');
+    }
+    // eslint-disable-next-line no-use-before-define
+    return recur(rawTree);
+  };
+
+  const decodeProperty = (name, value) => {
+    out.line();
+    if (name === '__proto__') {
+      // JavaScript interprets `{__proto__: x, ...}`
+      // as making an object inheriting from `x`, whereas
+      // in JSON it is simply a property name. Preserve the
+      // JSON meaning.
+      out.next(`["__proto__"]:`);
+    } else if (identPattern.test(name)) {
+      out.next(`${name}:`);
+    } else {
+      out.next(`${quote(name)}:`);
+    }
+    decode(value);
+    out.next(',');
+  };
+
+  /**
+   * Modeled after `fullRevive` in marshal.js
+   *
+   * @param {Encoding} rawTree
+   * @returns {number}
+   */
+  const recur = rawTree => {
+    if (Object(rawTree) !== rawTree) {
+      // primitives get quoted
+      return out.next(quote(rawTree));
+    }
+    // Assertions of the above to narrow the type.
+    assert.typeof(rawTree, 'object');
+    assert(rawTree !== null);
+    if (QCLASS in rawTree) {
+      const qclass = rawTree[QCLASS];
+      assert.typeof(qclass, 'string');
+      assert(!Array.isArray(rawTree));
+      // Switching on `encoded[QCLASS]` (or anything less direct, like
+      // `qclass`) does not discriminate rawTree in typescript@4.2.3 and
+      // earlier.
+      switch (rawTree['@qclass']) {
+        // Encoding of primitives not handled by JSON
+        case 'undefined':
+        case 'NaN':
+        case 'Infinity':
+        case '-Infinity': {
+          // Their qclass is their expression source.
+          return out.next(qclass);
+        }
+        case 'bigint': {
+          const { digits } = rawTree;
+          return out.next(`${BigInt(digits)}n`);
+        }
+        case '@@asyncIterator': {
+          return out.next('Symbol.asyncIterator');
+        }
+
+        case 'ibid': {
+          const { index } = rawTree;
+          return out.next(`getIbid(${index})`);
+        }
+
+        case 'error': {
+          const { name, message } = rawTree;
+          return out.next(`${name}(${quote(message)})`);
+        }
+
+        case 'slot': {
+          let { index, iface } = rawTree;
+          index = Number(Nat(index));
+          iface = quote(iface);
+          return out.next(`getSlotVal(${index},${iface})`);
+        }
+
+        case 'hilbert': {
+          const { original, rest } = rawTree;
+          out.open('{');
+          decodeProperty(QCLASS, original);
+          if ('rest' in rawTree) {
+            assert.typeof(rest, 'object');
+            assert(rest !== null);
             const names = ownKeys(rest);
             for (const name of names) {
               decodeProperty(name, rest[name]);
             }
-            finishIbid(rest);
           }
-          finishIbid(rawTree);
           return out.close('}');
         }
 
@@ -261,38 +379,33 @@ const decodeToJustin = (encoding, _cyclePolicy, shouldIndent = false) => {
         }
       }
     } else if (Array.isArray(rawTree)) {
-      startIbid(rawTree);
       const { length } = rawTree;
       if (length === 0) {
-        finishIbid(rawTree);
         return out.next('[]');
       } else {
         out.open('[');
         for (let i = 0; i < length; i += 1) {
           out.line();
-          recur(rawTree[i]);
+          decode(rawTree[i]);
           out.next(',');
         }
-        finishIbid(rawTree);
         return out.close(']');
       }
     } else {
-      startIbid(rawTree);
       const names = ownKeys(rawTree);
       if (names.length === 0) {
-        finishIbid(rawTree);
         return out.next('{}');
       } else {
         out.open('{');
         for (const name of names) {
           decodeProperty(name, rawTree[name]);
         }
-        finishIbid(rawTree);
         return out.close('}');
       }
     }
   };
-  recur(encoding);
+  prepare(encoding);
+  decode(encoding);
   return out.done();
 };
 harden(decodeToJustin);