fix: review suggestions

packages/ERTP/src/typeGuards.js

@@ -114,15 +114,15 @@ harden(isCopyBagValue);
 // One GOOGOLth should be enough decimal places for anybody.
 export const MAX_ABSOLUTE_DECIMAL_PLACES = 100;
 
-export const AssetValueShape = M.or('nat', 'set', 'copySet', 'copyBag');
+export const AssetKindShape = M.or('nat', 'set', 'copySet', 'copyBag');
 
 export const DisplayInfoShape = M.partial(
   harden({
     decimalPlaces: M.and(
       M.gte(-MAX_ABSOLUTE_DECIMAL_PLACES),
       M.lte(MAX_ABSOLUTE_DECIMAL_PLACES),
     ),
-    assetKind: AssetValueShape,
+    assetKind: AssetKindShape,
   }),
   harden({
     // Including this empty `rest` ensures that there are no other
@@ -148,19 +148,19 @@ export const BrandI = M.interface('Brand', {
 });
 
 /**
- * @param {Pattern} [brand]
- * @param {Pattern} [assetKind]
+ * @param {Pattern} [brandShape]
+ * @param {Pattern} [assetKindShape]
  * @param {Pattern} [amountShape]
  */
 export const makeIssuerInterfaces = (
-  brand = BrandShape,
-  assetKind = AssetValueShape,
+  brandShape = BrandShape,
+  assetKindShape = AssetKindShape,
   amountShape = AmountShape,
 ) => {
   const IssuerI = M.interface('Issuer', {
-    getBrand: M.call().returns(brand),
+    getBrand: M.call().returns(brandShape),
     getAllegedName: M.call().returns(M.string()),
-    getAssetKind: M.call().returns(assetKind),
+    getAssetKind: M.call().returns(assetKindShape),
     getDisplayInfo: M.call().returns(DisplayInfoShape),
     makeEmptyPurse: M.call().returns(PurseShape),
 
@@ -190,11 +190,11 @@ export const makeIssuerInterfaces = (
   });
 
   const PaymentI = M.interface('Payment', {
-    getAllegedBrand: M.call().returns(brand),
+    getAllegedBrand: M.call().returns(brandShape),
   });
 
   const PurseI = M.interface('Purse', {
-    getAllegedBrand: M.call().returns(brand),
+    getAllegedBrand: M.call().returns(brandShape),
     getCurrentAmount: M.call().returns(amountShape),
     getCurrentAmountNotifier: M.call().returns(NotifierShape),
     // PurseI does *not* delay `deposit` until `srcPayment` is fulfulled.

packages/SwingSet/src/liveslots/virtualObjectManager.js

@@ -500,9 +500,8 @@ export function makeVirtualObjectManager(
    *
    * @param {DefineKindOptions<*>} options
    *    Additional options to configure the virtual object kind
-   *    being defined.  Currently the only supported option is `finish`, an
-   *    optional finisher function that can perform post-creation initialization
-   *    operations, such as inserting the new object in a cyclical object graph.
+   *    being defined. See the documentation of DefineKindOptions
+   *    for the meaning of each option.
    *
    * @param {boolean} isDurable  A flag indicating whether or not the newly defined
    *    kind should be a durable kind.

packages/store/src/patterns/interface-tools.js

@@ -63,6 +63,10 @@ const isAwaitArgGuard = argGuard =>
 
 const desync = methodGuard => {
   const { argGuards, optionalArgGuards = [], restArgGuard } = methodGuard;
+  assert(
+    !isAwaitArgGuard(restArgGuard),
+    X`Rest args may not be awaited: ${restArgGuard}`,
+  );
   const rawArgGuards = [...argGuards, ...optionalArgGuards];
 
   const awaitIndexes = [];

packages/store/src/patterns/patternMatchers.js

@@ -112,7 +112,7 @@ const makePatternKit = () => {
             );
             return check(
               false,
-              X`A ${q(tag)} must be a Key but was not: ${patt}`,
+              X`A ${q(tag)} - Must be a Key but was not: ${patt}`,
             );
           }
           case 'copyMap': {
@@ -262,11 +262,11 @@ const makePatternKit = () => {
       return check(keyEQ(specimen, patt), X`${specimen} - Must be: ${patt}`);
     }
     assertPattern(patt);
-    const specStyle = passStyleOf(specimen);
+    const specimenStyle = passStyleOf(specimen);
     const pattStyle = passStyleOf(patt);
     switch (pattStyle) {
       case 'copyArray': {
-        if (specStyle !== 'copyArray') {
+        if (specimenStyle !== 'copyArray') {
           return check(
             false,
             X`${specimen} - Must be a copyArray to match a copyArray pattern: ${patt}`,
@@ -282,34 +282,32 @@ const makePatternKit = () => {
         return patt.every((p, i) => checkMatches(specimen[i], p, check, i));
       }
       case 'copyRecord': {
-        if (specStyle !== 'copyRecord') {
+        if (specimenStyle !== 'copyRecord') {
           return check(
             false,
             X`${specimen} - Must be a copyRecord to match a copyRecord pattern: ${patt}`,
           );
         }
-        const [specNames, specValues] = recordParts(specimen);
+        const [specimenNames, specimenValues] = recordParts(specimen);
         const [pattNames, pattValues] = recordParts(patt);
-        {
-          const missing = listDifference(pattNames, specNames);
-          if (missing.length >= 1) {
-            return check(
-              false,
-              X`${specimen} - Must have missing properties ${q(missing)}`,
-            );
-          }
-          const unexpected = listDifference(specNames, pattNames);
-          if (unexpected.length >= 1) {
-            return check(
-              false,
-              X`${specimen} - Must not have unexpected properties: ${q(
-                unexpected,
-              )}`,
-            );
-          }
+        const missing = listDifference(pattNames, specimenNames);
+        if (missing.length >= 1) {
+          return check(
+            false,
+            X`${specimen} - Must have missing properties ${q(missing)}`,
+          );
+        }
+        const unexpected = listDifference(specimenNames, pattNames);
+        if (unexpected.length >= 1) {
+          return check(
+            false,
+            X`${specimen} - Must not have unexpected properties: ${q(
+              unexpected,
+            )}`,
+          );
         }
         return pattNames.every((label, i) =>
-          checkMatches(specValues[i], pattValues[i], check, label),
+          checkMatches(specimenValues[i], pattValues[i], check, label),
         );
       }
       case 'tagged': {
@@ -318,16 +316,16 @@ const makePatternKit = () => {
         if (matchHelper) {
           return matchHelper.checkMatches(specimen, patt.payload, check);
         }
-        if (specStyle !== 'tagged' || getTag(specimen) !== pattTag) {
+        const specimenTag =
+          specimenStyle === 'tagged' ? getTag(specimen) : undefined;
+        if (specimenStyle !== 'tagged' || specimenTag !== pattTag) {
           return check(
             false,
-            X`${specimen} - Only a ${q(pattTag)} matches a ${q(
-              pattTag,
-            )} pattern: ${patt}`,
+            X`${specimen} - Must be tagged as a ${pattTag}: ${specimenTag}`,
           );
         }
         const { payload: pattPayload } = patt;
-        const { payload: specPayload } = specimen;
+        const { payload: specimenPayload } = specimen;
         switch (pattTag) {
           case 'copySet':
           case 'copyBag': {
@@ -344,15 +342,15 @@ const makePatternKit = () => {
               return false;
             }
             const pattKeySet = copyMapKeySet(pattPayload);
-            const specKeySet = copyMapKeySet(specPayload);
+            const specimenKeySet = copyMapKeySet(specimenPayload);
             // Compare keys as copySets
-            if (checkMatches(specKeySet, pattKeySet, check)) {
+            if (checkMatches(specimenKeySet, pattKeySet, check)) {
               return false;
             }
             const pattValues = pattPayload.values;
-            const specValues = specPayload.values;
+            const specimenValues = specimenPayload.values;
             // compare values as copyArrays
-            return checkMatches(specValues, pattValues, check);
+            return checkMatches(specimenValues, pattValues, check);
           }
           default: {
             assert.fail(X`Unexpected tag ${q(pattTag)}`);
@@ -563,13 +561,13 @@ const makePatternKit = () => {
   // /////////////////////// Match Helpers /////////////////////////////////////
 
   /** @type {MatchHelper} */
-  const matchAnyHelper = Far('match.any helper', {
+  const matchAnyHelper = Far('match:any helper', {
     checkMatches: (_specimen, _matcherPayload, _check) => true,
 
     checkIsMatcherPayload: (matcherPayload, check) =>
       check(
         matcherPayload === undefined,
-        X`Payload must be undefined: ${matcherPayload}`,
+        X`match:any payload: ${matcherPayload} - Must be undefined`,
       ),
 
     getRankCover: (_matchPayload, _encodePassable) => ['', '{'],
@@ -639,7 +637,7 @@ const makePatternKit = () => {
       if (matches(specimen, patt)) {
         return check(
           false,
-          X`${specimen} - must fail negated pattern: ${patt}`,
+          X`${specimen} - Must fail negated pattern: ${patt}`,
         );
       } else {
         return true;
@@ -654,7 +652,7 @@ const makePatternKit = () => {
   });
 
   /** @type {MatchHelper} */
-  const matchScalarHelper = Far('M.scalar helper', {
+  const matchScalarHelper = Far('match:scalar helper', {
     checkMatches: (specimen, _matcherPayload, check) =>
       checkScalarKey(specimen, check),
 
@@ -666,7 +664,7 @@ const makePatternKit = () => {
   });
 
   /** @type {MatchHelper} */
-  const matchKeyHelper = Far('M.key helper', {
+  const matchKeyHelper = Far('match:key helper', {
     checkMatches: (specimen, _matcherPayload, check) =>
       checkKey(specimen, check),
 
@@ -678,7 +676,7 @@ const makePatternKit = () => {
   });
 
   /** @type {MatchHelper} */
-  const matchPatternHelper = Far('M.pattern helper', {
+  const matchPatternHelper = Far('match:pattern helper', {
     checkMatches: (specimen, _matcherPayload, check) =>
       checkPattern(specimen, check),
 
@@ -690,13 +688,13 @@ const makePatternKit = () => {
   });
 
   /** @type {MatchHelper} */
-  const matchKindHelper = Far('M.kind helper', {
+  const matchKindHelper = Far('match:kind helper', {
     checkMatches: checkKind,
 
     checkIsMatcherPayload: (allegedKeyKind, check) =>
       check(
         typeof allegedKeyKind === 'string',
-        X`A kind name must be a string: ${allegedKeyKind}`,
+        X`match:kind: payload: ${allegedKeyKind} - A kind name must be a string`,
       ),
 
     getRankCover: (kind, _encodePassable) => {
@@ -1025,10 +1023,10 @@ const makePatternKit = () => {
       let specR;
       let newBase;
       if (baseStyle === 'copyArray') {
-        const { length: specLen } = specimen;
+        const { length: specimenLen } = specimen;
         const { length: baseLen } = base;
-        if (specLen < baseLen) {
-          newBase = harden(base.slice(0, specLen));
+        if (specimenLen < baseLen) {
+          newBase = harden(base.slice(0, specimenLen));
           specB = specimen;
           // eslint-disable-next-line no-use-before-define
           specR = [];

packages/store/test/test-patterns.js

@@ -36,8 +36,8 @@ const matchTests = harden([
     ],
     noPatterns: [
       [4, '3 - Must be: 4'],
-      [M.not(3), '3 - must fail negated pattern: 3'],
-      [M.not(M.any()), '3 - must fail negated pattern: "[match:any]"'],
+      [M.not(3), '3 - Must fail negated pattern: 3'],
+      [M.not(M.any()), '3 - Must fail negated pattern: "[match:any]"'],
       [M.string(), 'number 3 - Must be a string'],
       [[3, 4], '3 - Must be: [3,4]'],
       [M.gte(7), '3 - Must be >= 7'],
@@ -284,7 +284,7 @@ const matchTests = harden([
   {
     specimen: makeTagged('match:any', 88),
     yesPatterns: [M.any(), M.not(M.pattern())],
-    noPatterns: [[M.pattern(), 'Payload must be undefined: 88']],
+    noPatterns: [[M.pattern(), 'match:any payload: 88 - Must be undefined']],
   },
   {
     specimen: makeTagged('match:remotable', 88),

packages/vat-data/src/far-class-utils.js

@@ -64,7 +64,7 @@ export const defineVirtualFarClassKit = (
     thisfulMethods: true,
     interfaceGuard: interfaceGuardKit,
   });
-harden(defineVirtualFarClass);
+harden(defineVirtualFarClassKit);
 
 /**
  * @template A,S,T

packages/vat-data/src/index.js

@@ -11,3 +11,5 @@ export {
   vivifyFarInstance,
   vivifySingleton,
 } from './far-class-utils.js';
+
+/** @template T @typedef {import('./types.js').DefineKindOptions<T>} DefineKindOptions */

packages/vat-data/src/kind-utils.js

@@ -5,11 +5,7 @@ import {
   makeKindHandle,
 } from './vat-data-bindings.js';
 
-/** @template L,R @typedef {import('@endo/eventual-send').RemotableBrand<L, R>} RemotableBrand */
 /** @typedef {import('./types.js').Baggage} Baggage */
-/** @template T @typedef {import('./types.js').DefineKindOptions<T>} DefineKindOptions */
-/** @template T @typedef {import('./types.js').KindFacet<T>} KindFacet */
-/** @template T @typedef {import('./types.js').KindFacets<T>} KindFacets */
 /** @typedef {import('./types.js').DurableKindHandle} DurableKindHandle */
 
 /**

packages/vat-data/src/types.d.ts

@@ -42,10 +42,54 @@ declare class DurableKindHandleClass {
 }
 export type DurableKindHandle = DurableKindHandleClass;
 
+/**
+ * Grab bag of options that can be provided to `defineDurableKind` and its
+ * siblings. Not all options are meaningful in all contexts. See the
+ * doc-comments on each option.
+ */
 type DefineKindOptions<C> = {
+  /**
+   * If provided, the `finish` function will be called after the instance is
+   * made and internally registered, but before it is returned. The finish
+   * function is to do any post-intantiation initialization that should be
+   * done before exposing the object to its clients.
+   */
   finish?: (context: C) => void;
+
+  /**
+   * Meaningful to `makeScalarBigMapStore` and its siblings. These maker
+   * fuctions will make either virtual or durable stores, depending on
+   * this flag. Defaults to off, making virtual but not durable collections.
+   *
+   * Generally, durable collections are provided with `provideDurableMapStore`
+   * and its sibling, which use this flag internally. If you do not make
+   * durable collections by other means, you can consider this as
+   * intended for internal use only.
+   */
   durable?: boolean;
+
+  /**
+   * Intended for internal use only.
+   * Should the raw methods receive their `context` argument as their first
+   * argument or as their `this` binding? For `defineDurableKind` and its
+   * siblings (including `vivifySingleton`), this defaults to off, meaning that
+   * their behavior methods receive `context` as their first argument.
+   * `vivifyFarClass` and its siblings (including `vivifyFarInstance`) use
+   * this flag internally to indicate that their methods receive `context`
+   * as their `this` binding.
+   */
   thisfulMethods?: boolean;
+
+  /**
+   * Intended for internal use only.
+   * If an `interfaceGuard` is provided, then the raw methods passed alongside
+   * it wrapped by a function that first checks that this method's guard
+   * pattern is satisfied before calling the raw method.
+   *
+   * In `defineDurableKind` and its siblings, this defaults to off.
+   * `vivifyFarClass` use this internally to protect their raw class methods
+   * using the provided interface.
+   */
   interfaceGuard?: object; // TODO type
 };