From fcc3bb4c1e013a173e18e66bfe8352e168f425ce Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 24 Aug 2022 16:02:39 -0700 Subject: [PATCH] fix: refinements --- .../test/unitTests/test-paramGovernance.js | 2 +- .../store/src/patterns/patternMatchers.js | 138 +++++++----------- packages/store/src/types.js | 6 +- packages/store/test/test-patterns.js | 72 +++++++++ packages/vat-data/src/far-class-utils.js | 8 - 5 files changed, 132 insertions(+), 94 deletions(-) diff --git a/packages/governance/test/unitTests/test-paramGovernance.js b/packages/governance/test/unitTests/test-paramGovernance.js index 9477f3bd68fe..7a0604f2089e 100644 --- a/packages/governance/test/unitTests/test-paramGovernance.js +++ b/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)', }, ); }); diff --git a/packages/store/src/patterns/patternMatchers.js b/packages/store/src/patterns/patternMatchers.js index 22c3543847b1..314e913c9931 100644 --- a/packages/store/src/patterns/patternMatchers.js +++ b/packages/store/src/patterns/patternMatchers.js @@ -1,3 +1,4 @@ +/* eslint-disable no-use-before-define */ // @ts-check import { @@ -24,16 +25,15 @@ import { checkScalarKey, isScalarKey, checkCopySet, - checkCopyBag, checkCopyMap, copyMapKeySet, + checkCopyBag, } from '../keys/checkKey.js'; /// const { quote: q, details: X } = assert; const { entries, values } = Object; -const { ownKeys } = Reflect; /** @type WeakSet */ const patternMemo = new WeakSet(); @@ -104,35 +104,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 +318,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)) { @@ -596,7 +566,7 @@ const makePatternKit = () => { // /////////////////////// Match Helpers ///////////////////////////////////// /** @type {MatchHelper} */ - const matchAnyHelper = Far('M.any helper', { + const matchAnyHelper = Far('match.any helper', { checkMatches: (_specimen, _matcherPayload, _check) => true, checkIsMatcherPayload: (matcherPayload, check) => @@ -765,7 +735,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') { @@ -781,17 +751,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) => @@ -914,10 +878,12 @@ const makePatternKit = () => { ), checkIsMatcherPayload: (entryPatt, check) => - 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'), @@ -956,10 +922,12 @@ const makePatternKit = () => { ), checkIsMatcherPayload: (entryPatt, check) => - 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'), @@ -982,10 +950,12 @@ const makePatternKit = () => { ), checkIsMatcherPayload: (entryPatt, check) => - 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'), @@ -1177,6 +1147,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 @@ -1252,10 +1227,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, diff --git a/packages/store/src/types.js b/packages/store/src/types.js index 7bd94f32d3a9..cf359c6bdf1c 100644 --- a/packages/store/src/types.js +++ b/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. diff --git a/packages/store/test/test-patterns.js b/packages/store/test/test-patterns.js index 647f26d8621a..527faf60c838 100644 --- a/packages/store/test/test-patterns.js +++ b/packages/store/test/test-patterns.js @@ -264,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 => { @@ -300,3 +365,10 @@ test('masking match failure', t => { message: 'A passable tagged "match:kind" is not a key: "[match:kind]"', }); }); + +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', + }); +}); diff --git a/packages/vat-data/src/far-class-utils.js b/packages/vat-data/src/far-class-utils.js index db079f9082d7..e562483d97dd 100644 --- a/packages/vat-data/src/far-class-utils.js +++ b/packages/vat-data/src/far-class-utils.js @@ -39,7 +39,6 @@ export const defineVirtualFarClass = ( thisfulMethods: true, interfaceGuard, }); -// @ts-expect-error TODO statically recognize harden harden(defineVirtualFarClass); /** @@ -65,7 +64,6 @@ export const defineVirtualFarClassKit = ( thisfulMethods: true, interfaceGuard: interfaceGuardKit, }); -// @ts-expect-error TODO statically recognize harden harden(defineVirtualFarClass); /** @@ -91,7 +89,6 @@ export const defineDurableFarClass = ( thisfulMethods: true, interfaceGuard, }); -// @ts-expect-error TODO statically recognize harden harden(defineDurableFarClass); /** @@ -117,7 +114,6 @@ export const defineDurableFarClassKit = ( thisfulMethods: true, interfaceGuard: interfaceGuardKit, }); -// @ts-expect-error TODO statically recognize harden harden(defineDurableFarClassKit); /** @@ -145,7 +141,6 @@ export const vivifyFarClass = ( methods, options, ); -// @ts-expect-error TODO statically recognize harden harden(vivifyFarClass); /** @@ -173,7 +168,6 @@ export const vivifyFarClassKit = ( facets, options, ); -// @ts-expect-error TODO statically recognize harden harden(vivifyFarClassKit); /** @@ -203,7 +197,6 @@ export const vivifyFarInstance = ( return provide(baggage, `the_${kindName}`, () => makeSingleton()); }; -// @ts-expect-error TODO statically recognize harden harden(vivifyFarInstance); /** @@ -221,5 +214,4 @@ export const vivifySingleton = ( methods, options = undefined, ) => vivifyFarInstance(baggage, kindName, undefined, methods, options); -// @ts-expect-error TODO statically recognize harden harden(vivifySingleton);