fix: Patterns and Keys (#4210)

packages/marshal/index.js

@@ -4,6 +4,7 @@ export {
   isObject,
   assertChecker,
   getTag,
+  hasOwnPropertyOf,
 } from './src/helpers/passStyle-helpers.js';
 
 export { getErrorConstructor, toPassableError } from './src/helpers/error.js';
@@ -17,12 +18,7 @@ export {
   passableSymbolForName,
 } from './src/helpers/symbol.js';
 
-export {
-  passStyleOf,
-  assertPassable,
-  everyPassableChild,
-  somePassableChild,
-} from './src/passStyleOf.js';
+export { passStyleOf, assertPassable } from './src/passStyleOf.js';
 
 export { pureCopy, sameValueZero } from './src/pureCopy.js';
 export { deeplyFulfilled } from './src/deeplyFulfilled.js';

packages/marshal/src/assertPassStyleOf.js

@@ -4,19 +4,11 @@ import { passStyleOf } from './passStyleOf.js';
 
 const { details: X, quote: q } = assert;
 
-/**
- * @typedef {Array} CopyArray
- */
-
-/**
- * @typedef {Record<string, Passable>} CopyRecord
- */
-
 /**
  * Check whether the argument is a pass-by-copy array, AKA a "copyArray"
  * in @agoric/marshal terms
  *
- * @param {CopyArray} array
+ * @param {CopyArray<any>} array
  * @returns {boolean}
  */
 const isCopyArray = array => passStyleOf(array) === 'copyArray';
@@ -26,7 +18,7 @@ harden(isCopyArray);
  * Check whether the argument is a pass-by-copy record, AKA a
  * "copyRecord" in @agoric/marshal terms
  *
- * @param {CopyRecord} record
+ * @param {CopyRecord<any>} record
  * @returns {boolean}
  */
 const isRecord = record => passStyleOf(record) === 'copyRecord';
@@ -45,7 +37,7 @@ harden(isRemotable);
  * Assert that the argument is a pass-by-copy array, AKA a "copyArray"
  * in @agoric/marshal terms
  *
- * @param {CopyArray} array
+ * @param {CopyArray<any>} array
  * @param {string=} optNameOfArray
  * @returns {void}
  */
@@ -64,7 +56,7 @@ harden(assertCopyArray);
  * Assert that the argument is a pass-by-copy record, or a
  * "copyRecord" in @agoric/marshal terms
  *
- * @param {CopyRecord} record
+ * @param {CopyRecord<any>} record
  * @param {string=} optNameOfRecord
  * @returns {void}
  */

packages/marshal/src/helpers/copyArray.js

@@ -44,11 +44,6 @@ export const CopyArrayHelper = harden({
       TypeError,
     );
     // Recursively validate that each member is passable.
-    CopyArrayHelper.every(candidate, v => !!passStyleOfRecur(v));
+    candidate.every(v => !!passStyleOfRecur(v));
   },
-
-  every: (passable, fn) =>
-    // Note that we explicitly call `fn` with only the arguments we want
-    // to provide.
-    passable.every((v, i) => fn(v, i)),
 });

packages/marshal/src/helpers/copyRecord.js

@@ -18,7 +18,6 @@ const { ownKeys } = Reflect;
 const {
   getPrototypeOf,
   getOwnPropertyDescriptors,
-  entries,
   prototype: objectPrototype,
 } = Object;
 
@@ -62,11 +61,6 @@ export const CopyRecordHelper = harden({
       checkNormalProperty(candidate, name, 'string', true, assertChecker);
     }
     // Recursively validate that each member is passable.
-    CopyRecordHelper.every(candidate, v => !!passStyleOfRecur(v));
+    Object.values(candidate).every(v => !!passStyleOfRecur(v));
   },
-
-  every: (passable, fn) =>
-    // Note that we explicitly call `fn` with only the arguments we want
-    // to provide.
-    entries(passable).every(([k, v]) => fn(v, k)),
 });

packages/marshal/src/helpers/error.js

@@ -95,8 +95,6 @@ export const ErrorHelper = harden({
   assertValid: candidate => {
     ErrorHelper.canBeValid(candidate, assertChecker);
   },
-
-  every: (_passable, _fn) => true,
 });
 
 /**

packages/marshal/src/helpers/internal-types.js

@@ -27,10 +27,4 @@
  * @property {(candidate: any,
  *             passStyleOfRecur: PassStyleOf
  *            ) => void} assertValid
- *
- * @property {(passable: Passable,
- *             fn: (passable: Passable, index: any) => boolean
- *           ) => boolean} every
- * For recuring through the nested passable structure. Like
- * `Array.prototype.every`, return `false` to stop early.
  */

packages/marshal/src/helpers/tagged.js

@@ -51,8 +51,6 @@ export const TaggedHelper = harden({
     checkNormalProperty(candidate, 'payload', 'string', true, assertChecker);
 
     // Recursively validate that each member is passable.
-    TaggedHelper.every(candidate, v => !!passStyleOfRecur(v));
+    !!passStyleOfRecur(candidate.payload);
   },
-
-  every: (passable, fn) => fn(passable.payload, 'payload'),
 });

packages/marshal/src/passStyleOf.js

@@ -28,9 +28,9 @@ const { isFrozen } = Object;
  * does what it is supposed to do. `makePassStyleOf` is not trying to defend
  * itself against malicious helpers, though it does defend against some
  * accidents.
- * @returns {{passStyleOf: PassStyleOf, HelperTable: any}}
+ * @returns {PassStyleOf}
  */
-const makePassStyleOfKit = passStyleHelpers => {
+const makePassStyleOf = passStyleHelpers => {
   const HelperTable = {
     __proto__: null,
     copyArray: undefined,
@@ -179,35 +179,18 @@ const makePassStyleOfKit = passStyleHelpers => {
 
     return passStyleOfRecur(passable);
   };
-  return harden({ passStyleOf, HelperTable });
+  return harden(passStyleOf);
 };
 
-const { passStyleOf, HelperTable } = makePassStyleOfKit([
+export const passStyleOf = makePassStyleOf([
   CopyArrayHelper,
   CopyRecordHelper,
   TaggedHelper,
   RemotableHelper,
   ErrorHelper,
 ]);
-export { passStyleOf };
 
 export const assertPassable = val => {
   passStyleOf(val); // throws if val is not a passable
 };
 harden(assertPassable);
-
-export const everyPassableChild = (passable, fn) => {
-  const passStyle = passStyleOf(passable);
-  const helper = HelperTable[passStyle];
-  if (helper) {
-    // everyPassable guards .every so that each helper only gets a
-    // genuine passable of its own flavor.
-    return helper.every(passable, fn);
-  }
-  return true;
-};
-harden(everyPassableChild);
-
-export const somePassableChild = (passable, fn) =>
-  !everyPassableChild(passable, (v, i) => !fn(v, i));
-harden(somePassableChild);

packages/marshal/src/types.js

@@ -97,6 +97,16 @@
  * The leaves of a Passable's pass-by-copy superstructure.
  */
 
+/**
+ * @template T
+ * @typedef {T[]} CopyArray
+ */
+
+/**
+ * @template T
+ * @typedef {Record<string, T>} CopyRecord
+ */
+
 /**
  * @typedef {{
  *   [PASS_STYLE]: 'tagged',

packages/store/src/index.js

@@ -3,8 +3,14 @@
 export { isKey, assertKey } from './keys/checkKey.js';
 export { keyLT, keyLTE, keyEQ, keyGTE, keyGT } from './keys/compareKeys.js';
 
-export { makePatternKit } from './patterns/patternMatchers.js';
-export { compareRank } from './patterns/rankOrder.js';
+export {
+  M,
+  isPattern,
+  assertPattern,
+  matches,
+  fit,
+} from './patterns/patternMatchers.js';
+// export { compareRank } from './patterns/rankOrder.js';
 
 export { makeScalarWeakSetStore } from './stores/scalarWeakSetStore.js';
 export { makeScalarSetStore } from './stores/scalarSetStore.js';

packages/store/src/keys/checkKey.js

@@ -6,7 +6,6 @@
 import {
   assertChecker,
   assertPassable,
-  everyPassableChild,
   getTag,
   isObject,
   passStyleOf,
@@ -94,10 +93,13 @@ const checkKeyInternal = (val, check = x => x) => {
 
   const passStyle = passStyleOf(val);
   switch (passStyle) {
-    case 'copyRecord':
+    case 'copyRecord': {
+      // A copyRecord is a key iff all its children are keys
+      return Object.values(val).every(checkIt);
+    }
     case 'copyArray': {
-      // A copyRecord or copyArray is a key iff all its children are keys
-      return everyPassableChild(val, checkIt);
+      // A copyArray is a key iff all its children are keys
+      return val.every(checkIt);
     }
     case 'tagged': {
       const tag = getTag(val);

packages/store/src/keys/copyMap.js

@@ -6,7 +6,7 @@ import {
   makeTagged,
   passStyleOf,
 } from '@agoric/marshal';
-import { compareRank, sortByRank } from '../patterns/rankOrder.js';
+import { compareAntiRank, sortByRank } from '../patterns/rankOrder.js';
 import { checkCopySetKeys } from './copySet.js';
 
 // eslint-disable-next-line spaced-comment
@@ -15,7 +15,7 @@ import { checkCopySetKeys } from './copySet.js';
 const { details: X } = assert;
 const { ownKeys } = Reflect;
 
-/** @type WeakSet<CopyMap> */
+/** @type WeakSet<CopyMap<any,any>> */
 const copyMapMemo = new WeakSet();
 
 /**
@@ -63,49 +63,105 @@ export const assertCopyMap = m => checkCopyMap(m, assertChecker);
 harden(assertCopyMap);
 
 /**
- * @param {CopyMap} m
- * @param {(v: Passable, i: number) => boolean} fn
- * @returns {boolean}
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @returns {K[]}
  */
-export const everyCopyMapKey = (m, fn) => {
+export const getCopyMapKeys = m => {
   assertCopyMap(m);
-  return m.payload.keys.every((v, i) => fn(v, i));
+  return m.payload.keys;
 };
-harden(everyCopyMapKey);
+harden(getCopyMapKeys);
 
 /**
- * @param {CopyMap} m
- * @param {(v: Passable, i: number) => boolean} fn
- * @returns {boolean}
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @returns {V[]}
  */
-export const everyCopyMapValue = (m, fn) => {
+export const getCopyMapValues = m => {
   assertCopyMap(m);
-  return m.payload.values.every((v, i) => fn(v, i));
+  return m.payload.values;
 };
+harden(getCopyMapValues);
+
+/**
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @returns {Iterable<[K,V]>}
+ */
+export const getCopyMapEntries = m => {
+  assertCopyMap(m);
+  const {
+    payload: { keys, values },
+  } = m;
+  const { length } = keys;
+  return harden({
+    [Symbol.iterator]: () => {
+      let i = 0;
+      return harden({
+        next: () => {
+          /** @type {IteratorResult<[K,V],void>} */
+          let result;
+          if (i < length) {
+            result = harden({ done: false, value: [keys[i], values[i]] });
+            i += 1;
+            return result;
+          } else {
+            result = harden({ done: true, value: undefined });
+          }
+          return result;
+        },
+      });
+    },
+  });
+};
+harden(getCopyMapEntries);
+
+/**
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @param {(key: K, index: number) => boolean} fn
+ * @returns {boolean}
+ */
+export const everyCopyMapKey = (m, fn) =>
+  getCopyMapKeys(m).every((key, index) => fn(key, index));
+harden(everyCopyMapKey);
+
+/**
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @param {(value: V, index: number) => boolean} fn
+ * @returns {boolean}
+ */
+export const everyCopyMapValue = (m, fn) =>
+  getCopyMapValues(m).every((value, index) => fn(value, index));
 harden(everyCopyMapValue);
 
 /**
- * @param {CopyMap} m
- * @returns {CopySet}
+ * @template K,V
+ * @param {CopyMap<K,V>} m
+ * @returns {CopySet<K>}
  */
 export const copyMapKeySet = m =>
   // A copyMap's keys are already in the internal form used by copySets.
   makeTagged('copySet', m.payload.keys);
 harden(copyMapKeySet);
 
 /**
- * @param {Iterable<[Passable, Passable]>} entries
- * @returns {CopyMap}
+ * @template K,V
+ * @param {Iterable<[K, V]>} entries
+ * @returns {CopyMap<K,V>}
  */
 export const makeCopyMap = entries => {
   // This is weird, but reverse rank sorting the entries is a good first step
   // for getting the rank sorted keys together with the values
   // organized by those keys. Also, among values associated with
-  // keys in the same equivalence class, those are rank sorted. This
+  // keys in the same equivalence class, those are rank sorted.
+  // TODO This
   // could solve the copyMap cover issue explained in patternMatchers.js.
   // But only if we include this criteria in our validation of copyMaps,
   // which we currently do not.
-  const sortedEntries = [...sortByRank(entries, compareRank)].reverse();
+  const sortedEntries = sortByRank(entries, compareAntiRank);
   const keys = sortedEntries.map(([k, _v]) => k);
   const values = sortedEntries.map(([_k, v]) => v);
   return makeTagged('copyMap', { keys, values });