fix: refinements

packages/governance/test/unitTests/test-paramGovernance.js

@@ -216,7 +216,7 @@ test('multiple params bad change', async t => {
       ),
     {
       message:
-        'In "getAmountOf" method of (Zoe Invitation issuer) arg 0: bigint "[13n]" - Must be a remotable',
+        'In "getAmountOf" method of (Zoe Invitation issuer) arg 0: bigint "[13n]" - Must be a remotable (Payment)',
     },
   );
 });

packages/store/src/keys/checkKey.js

@@ -161,7 +161,7 @@ export const checkCopySet = (s, check = x => x) => {
       X`Not a copySet: ${s}`,
     ) &&
     checkElements(s.payload, check) &&
-    checkKey(s.payload);
+    checkKey(s.payload, check);
   if (result) {
     copySetMemo.add(s);
   }

packages/store/src/patterns/patternMatchers.js

@@ -1,3 +1,4 @@
+/* eslint-disable no-use-before-define */
 // @ts-check
 
 import {
@@ -24,7 +25,6 @@ import {
   checkScalarKey,
   isScalarKey,
   checkCopySet,
-  checkCopyBag,
   checkCopyMap,
   copyMapKeySet,
 } from '../keys/checkKey.js';
@@ -33,7 +33,6 @@ import {
 
 const { quote: q, details: X } = assert;
 const { entries, values } = Object;
-const { ownKeys } = Reflect;
 
 /** @type WeakSet<Pattern> */
 const patternMemo = new WeakSet();
@@ -104,35 +103,16 @@ const makePatternKit = () => {
           return matchHelper.checkIsMatcherPayload(patt.payload, check);
         }
         switch (tag) {
-          case 'copySet': {
-            if (!checkCopySet(patt, check)) {
-              return false;
-            }
-            // For a copySet to be a pattern, all its elements must be patterns.
-            // If all the elements are keys, then the copySet pattern is also
-            // a key and is already taken of. For the case where some elements
-            // are non-key patterns, general support is both hard and not
-            // currently needed. Currently, we only need a copySet of a single
-            // non-key pattern element.
+          case 'copySet':
+          case 'copyBag': {
             assert(
-              patt.payload.length === 1,
-              X`Non-singleton copySets with matcher not yet implemented: ${patt}`,
+              !isKey(patt),
+              X`internal: The key case should have been dealt with earlier: ${patt}`,
+            );
+            return check(
+              false,
+              X`A ${q(tag)} must be a Key but was not: ${patt}`,
             );
-            return checkPattern(patt.payload[0], check);
-          }
-          case 'copyBag': {
-            if (!checkCopyBag(patt, check)) {
-              return false;
-            }
-            // If it is a CopyBag, then it must also be a key and we
-            // should never get here.
-            if (isKey(patt)) {
-              assert.fail(
-                X`internal: The key case should have been dealt with earlier: ${patt}`,
-              );
-            } else {
-              assert.fail(X`A CopyMap must be a Key but was not: ${patt}`);
-            }
           }
           case 'copyMap': {
             return (
@@ -337,37 +317,26 @@ const makePatternKit = () => {
         if (matchHelper) {
           return matchHelper.checkMatches(specimen, patt.payload, check);
         }
-        const msg = X`${specimen} - Only a ${q(pattTag)} matches a ${q(
-          pattTag,
-        )} pattern: ${patt}`;
-        if (specStyle !== 'tagged') {
-          return check(false, msg);
-        }
-        const specTag = getTag(specimen);
-        if (pattTag !== specTag) {
-          return check(false, msg);
+        if (specStyle !== 'tagged' || getTag(specimen) !== pattTag) {
+          return check(
+            false,
+            X`${specimen} - Only a ${q(pattTag)} matches a ${q(
+              pattTag,
+            )} pattern: ${patt}`,
+          );
         }
         const { payload: pattPayload } = patt;
         const { payload: specPayload } = specimen;
         switch (pattTag) {
-          case 'copySet': {
-            if (!checkCopySet(specimen, check)) {
-              return false;
-            }
-            if (pattPayload.length !== specPayload.length) {
-              return check(
-                false,
-                X`copySet ${specimen} - Must have as many elements as copySet pattern: ${patt}`,
-              );
-            }
-            // Should already be validated by checkPattern. But because this
-            // is a check that may loosen over time, we also assert everywhere
-            // we still rely on the restriction.
+          case 'copySet':
+          case 'copyBag': {
             assert(
-              patt.payload.length === 1,
-              X`Non-singleton copySets with matcher not yet implemented: ${patt}`,
+              !isKey(patt),
+              X`internal: The key case should have been dealt with earlier: ${patt}`,
+            );
+            assert.fail(
+              X`internal: A ${q(pattTag)} must be a Key but was not: ${patt}`,
             );
-            return checkMatches(specPayload[0], pattPayload[0], check, 0);
           }
           case 'copyMap': {
             if (!checkCopySet(specimen, check)) {
@@ -568,7 +537,7 @@ const makePatternKit = () => {
   // /////////////////////// Match Helpers /////////////////////////////////////
 
   /** @type {MatchHelper} */
-  const matchAnyHelper = Far('M.any helper', {
+  const matchAnyHelper = Far('match:any helper', {
     checkMatches: (_specimen, _matcherPayload, _check = x => x) => true,
 
     checkIsMatcherPayload: (matcherPayload, check = x => x) =>
@@ -737,7 +706,7 @@ const makePatternKit = () => {
   });
 
   /** @type {MatchHelper} */
-  const matchRemotableHelper = Far('M.remotable helper', {
+  const matchRemotableHelper = Far('match:remotable helper', {
     checkMatches: (specimen, remotableDesc, check = x => x) => {
       const specimenKind = passStyleOf(specimen);
       if (specimenKind === 'remotable') {
@@ -753,17 +722,11 @@ const makePatternKit = () => {
     },
 
     checkIsMatcherPayload: (allegedRemotableDesc, check = x => x) =>
-      check(
-        passStyleOf(allegedRemotableDesc) === 'copyRecord',
-        X`A remotableDesc must be a copyRecord: ${allegedRemotableDesc}`,
-      ) &&
-      check(
-        typeof allegedRemotableDesc.label === 'string',
-        X`A remotableDesc must have a string label: ${allegedRemotableDesc}`,
-      ) &&
-      check(
-        ownKeys(allegedRemotableDesc).length === 1,
-        X`Additional properties on remotableDesc not yet recognized: ${allegedRemotableDesc}`,
+      checkMatches(
+        allegedRemotableDesc,
+        harden({ label: M.string() }),
+        check,
+        'match:remotable payload',
       ),
 
     getRankCover: (_remotableDesc, _encodePassable) =>
@@ -886,10 +849,12 @@ const makePatternKit = () => {
       ),
 
     checkIsMatcherPayload: (entryPatt, check = x => x) =>
-      check(
-        passStyleOf(entryPatt) === 'copyArray' && entryPatt.length === 2,
-        X`${entryPatt} - Must be an pair of patterns`,
-      ) && checkPattern(entryPatt, check),
+      checkMatches(
+        entryPatt,
+        harden([M.pattern(), M.pattern()]),
+        check,
+        'match:recordOf payload',
+      ),
 
     getRankCover: _entryPatt => getPassStyleCover('copyRecord'),
 
@@ -928,10 +893,12 @@ const makePatternKit = () => {
       ),
 
     checkIsMatcherPayload: (entryPatt, check = x => x) =>
-      check(
-        passStyleOf(entryPatt) === 'copyArray' && entryPatt.length === 2,
-        X`${entryPatt} - Must be an pair of patterns`,
-      ) && checkPattern(entryPatt, check),
+      checkMatches(
+        entryPatt,
+        harden([M.pattern(), M.pattern()]),
+        check,
+        'match:bagOf payload',
+      ),
 
     getRankCover: () => getPassStyleCover('tagged'),
 
@@ -954,10 +921,12 @@ const makePatternKit = () => {
       ),
 
     checkIsMatcherPayload: (entryPatt, check = x => x) =>
-      check(
-        passStyleOf(entryPatt) === 'copyArray' && entryPatt.length === 2,
-        X`${entryPatt} - Must be an pair of patterns`,
-      ) && checkPattern(entryPatt, check),
+      checkMatches(
+        entryPatt,
+        harden([M.pattern(), M.pattern()]),
+        check,
+        'match:mapOf payload',
+      ),
 
     getRankCover: _entryPatt => getPassStyleCover('tagged'),
 
@@ -1149,6 +1118,11 @@ const makePatternKit = () => {
   const PromiseShape = makeKindMatcher('promise');
   const UndefinedShape = makeKindMatcher('undefined');
 
+  const makeRemotableMatcher = (label = undefined) =>
+    label === undefined
+      ? RemotableShape
+      : makeMatcher('match:remotable', harden({ label }));
+
   /**
    * @param {'sync'|'async'} callKind
    * @param {ArgGuard[]} argGuards
@@ -1224,10 +1198,7 @@ const makePatternKit = () => {
     set: () => SetShape,
     bag: () => BagShape,
     map: () => MapShape,
-    remotable: (label = undefined) =>
-      label === undefined
-        ? RemotableShape
-        : makeMatcher('match:remotable', harden({ label })),
+    remotable: makeRemotableMatcher,
     error: () => ErrorShape,
     promise: () => PromiseShape,
     undefined: () => UndefinedShape,

packages/store/src/types.js

@@ -500,8 +500,10 @@
  * @property {() => Matcher} set A CopySet
  * @property {() => Matcher} bag A CopyBag
  * @property {() => Matcher} map A CopyMap
- * @property {(interfaceName?: string) => Matcher} remotable
- * A far object or its remote presence
+ * @property {(label?: string) => Matcher} remotable
+ * A far object or its remote presence. The optional `label` is purely for
+ * diagnostic purpose. It does not enforce any constraint beyond the
+ * must-be-a-remotable constraint.
  * @property {() => Matcher} error
  * Error objects are passable, but are neither keys nor symbols.
  * They do not have a useful identity.

packages/store/test/test-patterns.js

@@ -2,6 +2,7 @@
 
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';
+import { makeTagged } from '@endo/marshal';
 import { makeCopyBag, makeCopyMap, makeCopySet } from '../src/keys/checkKey.js';
 import { fit, matches, M } from '../src/patterns/patternMatchers.js';
 import '../src/types.js';
@@ -263,6 +264,71 @@ const matchTests = harden([
       ],
     ],
   },
+  {
+    specimen: makeTagged('mysteryTag', 88),
+    yesPatterns: [M.any(), M.not(M.pattern())],
+    noPatterns: [
+      [
+        M.pattern(),
+        'A passable tagged "mysteryTag" is not a pattern: "[mysteryTag]"',
+      ],
+    ],
+  },
+  {
+    specimen: makeTagged('match:any', undefined),
+    yesPatterns: [M.any(), M.pattern()],
+    noPatterns: [
+      [M.key(), 'A passable tagged "match:any" is not a key: "[match:any]"'],
+    ],
+  },
+  {
+    specimen: makeTagged('match:any', 88),
+    yesPatterns: [M.any(), M.not(M.pattern())],
+    noPatterns: [[M.pattern(), 'Payload must be undefined: 88']],
+  },
+  {
+    specimen: makeTagged('match:remotable', 88),
+    yesPatterns: [M.any(), M.not(M.pattern())],
+    noPatterns: [
+      [
+        M.pattern(),
+        'match:remotable payload: 88 - Must be a copyRecord to match a copyRecord pattern: {"label":"[match:kind]"}',
+      ],
+    ],
+  },
+  {
+    specimen: makeTagged('match:remotable', harden({ label: 88 })),
+    yesPatterns: [M.any(), M.not(M.pattern())],
+    noPatterns: [
+      [
+        M.pattern(),
+        'match:remotable payload: label: number 88 - Must be a string',
+      ],
+    ],
+  },
+  {
+    specimen: makeTagged('match:recordOf', harden([M.string(), M.nat()])),
+    yesPatterns: [M.pattern()],
+    noPatterns: [
+      [
+        M.key(),
+        'A passable tagged "match:recordOf" is not a key: "[match:recordOf]"',
+      ],
+    ],
+  },
+  {
+    specimen: makeTagged(
+      'match:recordOf',
+      harden([M.string(), Promise.resolve(null)]),
+    ),
+    yesPatterns: [M.any(), M.not(M.pattern())],
+    noPatterns: [
+      [
+        M.pattern(),
+        'match:recordOf payload: [1]: A "promise" cannot be a pattern',
+      ],
+    ],
+  },
 ]);
 
 test('test simple matches', t => {
@@ -281,3 +347,10 @@ test('test simple matches', t => {
     }
   }
 });
+
+test('well formed patterns', t => {
+  // @ts-expect-error purposeful type violation for testing
+  t.throws(() => M.remotable(88), {
+    message: 'match:remotable payload: label: number 88 - Must be a string',
+  });
+});