From 0e642c3bf0b7b9bc7f7c572d330469b4b8a716be Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Sun, 27 Aug 2023 19:45:51 -0700 Subject: [PATCH] fix(patterns,exo): abstract guard getters --- packages/exo/README.md | 13 +-- packages/exo/src/exo-tools.js | 50 ++++++----- packages/exo/test/test-heap-classes.js | 12 +-- packages/patterns/index.js | 4 + .../patterns/src/patterns/internal-types.js | 3 + .../patterns/src/patterns/patternMatchers.js | 82 +++++++++++++++++++ packages/patterns/src/types.js | 22 ++++- 7 files changed, 142 insertions(+), 44 deletions(-) diff --git a/packages/exo/README.md b/packages/exo/README.md index 87d8f9b823..7c7f1274d7 100644 --- a/packages/exo/README.md +++ b/packages/exo/README.md @@ -11,20 +11,13 @@ When an exo is defined with an InterfaceGuard, the exo is augmented by default w ```js // `GET_INTERFACE_GUARD` holds the name of the meta-method import { GET_INTERFACE_GUARD } from '@endo/exo'; -import { getCopyMapEntries } from '@endo/patterns'; +import { getInterfaceMethodNames } from '@endo/patterns'; ... const interfaceGuard = await E(exo)[GET_INTERFACE_GUARD](); // `methodNames` omits names of automatically added meta-methods like // the value of `GET_INTERFACE_GUARD`. - // Others may also be omitted if `interfaceGuard.partial` - const methodNames = [ - ...Reflect.ownKeys(interfaceGuard.methodGuards), - ...(interfaceGuard.symbolMethodGuards - ? [...getCopyMapEntries(interfaceGuard.symbolMethodGuards)].map( - entry => entry[0], - ) - : []), - ]; + // Others may also be omitted if allowed by interfaceGuard options + const methodNames = getInterfaceMethodNames(interfaceGuard); ... ``` diff --git a/packages/exo/src/exo-tools.js b/packages/exo/src/exo-tools.js index d8a185771a..fbac443134 100644 --- a/packages/exo/src/exo-tools.js +++ b/packages/exo/src/exo-tools.js @@ -6,8 +6,9 @@ import { mustMatch, M, isAwaitArgGuard, - assertMethodGuard, - assertInterfaceGuard, + getAwaitArgGuardPayload, + getMethodGuardPayload, + getInterfaceGuardPayload, getCopyMapEntries, } from '@endo/patterns'; @@ -26,8 +27,8 @@ const { defineProperties, fromEntries } = Object; */ const MinMethodGuard = M.call().rest(M.any()).returns(M.any()); -const defendSyncArgs = (args, methodGuard, label) => { - const { argGuards, optionalArgGuards, restArgGuard } = methodGuard; +const defendSyncArgs = (args, methodGuardPayload, label) => { + const { argGuards, optionalArgGuards, restArgGuard } = methodGuardPayload; const paramsPattern = M.splitArray( argGuards, optionalArgGuards, @@ -38,16 +39,16 @@ const defendSyncArgs = (args, methodGuard, label) => { /** * @param {Method} method - * @param {MethodGuard} methodGuard + * @param {MethodGuardPayload} methodGuardPayload * @param {string} label * @returns {Method} */ -const defendSyncMethod = (method, methodGuard, label) => { - const { returnGuard } = methodGuard; +const defendSyncMethod = (method, methodGuardPayload, label) => { + const { returnGuard } = methodGuardPayload; const { syncMethod } = { // Note purposeful use of `this` and concise method syntax syncMethod(...args) { - defendSyncArgs(harden(args), methodGuard, label); + defendSyncArgs(harden(args), methodGuardPayload, label); const result = apply(method, this, args); mustMatch(harden(result), returnGuard, `${label}: result`); return result; @@ -56,8 +57,12 @@ const defendSyncMethod = (method, methodGuard, label) => { return syncMethod; }; -const desync = methodGuard => { - const { argGuards, optionalArgGuards = [], restArgGuard } = methodGuard; +const desync = methodGuardPayload => { + const { + argGuards, + optionalArgGuards = [], + restArgGuard, + } = methodGuardPayload; !isAwaitArgGuard(restArgGuard) || Fail`Rest args may not be awaited: ${restArgGuard}`; const rawArgGuards = [...argGuards, ...optionalArgGuards]; @@ -66,13 +71,13 @@ const desync = methodGuard => { for (let i = 0; i < rawArgGuards.length; i += 1) { const argGuard = rawArgGuards[i]; if (isAwaitArgGuard(argGuard)) { - rawArgGuards[i] = argGuard.argGuard; + rawArgGuards[i] = getAwaitArgGuardPayload(argGuard).argGuard; awaitIndexes.push(i); } } return { awaitIndexes, - rawMethodGuard: { + rawMethodGuardPayload: { argGuards: rawArgGuards.slice(0, argGuards.length), optionalArgGuards: rawArgGuards.slice(argGuards.length), restArgGuard, @@ -80,9 +85,9 @@ const desync = methodGuard => { }; }; -const defendAsyncMethod = (method, methodGuard, label) => { - const { returnGuard } = methodGuard; - const { awaitIndexes, rawMethodGuard } = desync(methodGuard); +const defendAsyncMethod = (method, methodGuardPayload, label) => { + const { returnGuard } = methodGuardPayload; + const { awaitIndexes, rawMethodGuardPayload } = desync(methodGuardPayload); const { asyncMethod } = { // Note purposeful use of `this` and concise method syntax asyncMethod(...args) { @@ -93,7 +98,7 @@ const defendAsyncMethod = (method, methodGuard, label) => { for (let j = 0; j < awaitIndexes.length; j += 1) { rawArgs[awaitIndexes[j]] = awaitedArgs[j]; } - defendSyncArgs(rawArgs, rawMethodGuard, label); + defendSyncArgs(rawArgs, rawMethodGuardPayload, label); return apply(method, this, rawArgs); }); return E.when(resultP, result => { @@ -112,13 +117,13 @@ const defendAsyncMethod = (method, methodGuard, label) => { * @param {string} label */ const defendMethod = (method, methodGuard, label) => { - assertMethodGuard(methodGuard); - const { callKind } = methodGuard; + const methodGuardPayload = getMethodGuardPayload(methodGuard); + const { callKind } = methodGuardPayload; if (callKind === 'sync') { - return defendSyncMethod(method, methodGuard, label); + return defendSyncMethod(method, methodGuardPayload, label); } else { assert(callKind === 'async'); - return defendAsyncMethod(method, methodGuard, label); + return defendAsyncMethod(method, methodGuardPayload, label); } }; @@ -261,15 +266,16 @@ export const defendPrototype = ( // @ts-expect-error TS misses that hasOwn check makes this safe behaviorMethods = harden(methods); } + /** @type {Record | undefined} */ let methodGuards; if (interfaceGuard) { - assertInterfaceGuard(interfaceGuard); const { interfaceName, methodGuards: mg, symbolMethodGuards, sloppy = false, - } = interfaceGuard; + // @ts-expect-error "missing" type parameter + } = getInterfaceGuardPayload(interfaceGuard); methodGuards = harden({ ...mg, ...(symbolMethodGuards && diff --git a/packages/exo/test/test-heap-classes.js b/packages/exo/test/test-heap-classes.js index fc4f1e5f83..204696add1 100644 --- a/packages/exo/test/test-heap-classes.js +++ b/packages/exo/test/test-heap-classes.js @@ -2,7 +2,7 @@ import { test } from './prepare-test-env-ava.js'; // eslint-disable-next-line import/order -import { getCopyMapEntries, M } from '@endo/patterns'; +import { getInterfaceMethodNames, M } from '@endo/patterns'; import { defineExoClass, defineExoClassKit, @@ -10,8 +10,6 @@ import { } from '../src/exo-makers.js'; import { GET_INTERFACE_GUARD } from '../src/exo-tools.js'; -const { ownKeys } = Reflect; - const UpCounterI = M.interface('UpCounter', { incr: M.call() // TODO M.number() should not be needed to get a better error message @@ -52,7 +50,7 @@ test('test defineExoClass', t => { 'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number', }); t.deepEqual(upCounter[GET_INTERFACE_GUARD](), UpCounterI); - t.deepEqual(ownKeys(UpCounterI.methodGuards), ['incr']); + t.deepEqual(getInterfaceMethodNames(UpCounterI), ['incr']); t.is(UpCounterI.symbolMethodGuards, undefined); const symbolic = Symbol.for('symbolic'); @@ -60,11 +58,7 @@ test('test defineExoClass', t => { m: M.call().returns(), [symbolic]: M.call(M.boolean()).returns(), }); - t.deepEqual(ownKeys(FooI.methodGuards), ['m']); - t.deepEqual( - [...getCopyMapEntries(FooI.symbolMethodGuards)].map(entry => entry[0]), - [Symbol.for('symbolic')], - ); + t.deepEqual(getInterfaceMethodNames(FooI), ['m', Symbol.for('symbolic')]); const makeFoo = defineExoClass('Foo', FooI, () => ({}), { m() {}, [symbolic]() {}, diff --git a/packages/patterns/index.js b/packages/patterns/index.js index 7aa279363a..69be581aa5 100644 --- a/packages/patterns/index.js +++ b/packages/patterns/index.js @@ -60,8 +60,12 @@ export { mustMatch, isAwaitArgGuard, assertAwaitArgGuard, + getAwaitArgGuardPayload, assertMethodGuard, + getMethodGuardPayload, + getInterfaceMethodNames, assertInterfaceGuard, + getInterfaceGuardPayload, } from './src/patterns/patternMatchers.js'; // ////////////////// Temporary, until these find their proper home //////////// diff --git a/packages/patterns/src/patterns/internal-types.js b/packages/patterns/src/patterns/internal-types.js index c354f879b1..0d877475e2 100644 --- a/packages/patterns/src/patterns/internal-types.js +++ b/packages/patterns/src/patterns/internal-types.js @@ -9,9 +9,12 @@ /** @typedef {import('@endo/marshal').RankCompare} RankCompare */ /** @typedef {import('@endo/marshal').RankCover} RankCover */ +/** @typedef {import('../types.js').AwaitArgGuardPayload} AwaitArgGuardPayload */ /** @typedef {import('../types.js').AwaitArgGuard} AwaitArgGuard */ /** @typedef {import('../types.js').ArgGuard} ArgGuard */ +/** @typedef {import('../types.js').MethodGuardPayload} MethodGuardPayload */ /** @typedef {import('../types.js').MethodGuard} MethodGuard */ +/** @typedef {import('../types.js').InterfaceGuardPayload} InterfaceGuardPayload */ /** @typedef {import('../types.js').InterfaceGuard} InterfaceGuard */ /** @typedef {import('../types.js').MethodGuardMaker0} MethodGuardMaker0 */ diff --git a/packages/patterns/src/patterns/patternMatchers.js b/packages/patterns/src/patterns/patternMatchers.js index 1ec395a443..f9daa2cbb7 100644 --- a/packages/patterns/src/patterns/patternMatchers.js +++ b/packages/patterns/src/patterns/patternMatchers.js @@ -31,6 +31,7 @@ import { copyMapKeySet, checkCopyBag, makeCopyMap, + getCopyMapKeys, } from '../keys/checkKey.js'; import './internal-types.js'; @@ -1707,21 +1708,45 @@ const AwaitArgGuardShape = harden({ argGuard: M.pattern(), }); +/** + * @param {any} specimen + * @returns {specimen is AwaitArgGuard} + */ export const isAwaitArgGuard = specimen => matches(specimen, AwaitArgGuardShape); harden(isAwaitArgGuard); +/** + * @param {any} specimen + * @returns {asserts specimen is AwaitArgGuard} + */ export const assertAwaitArgGuard = specimen => { mustMatch(specimen, AwaitArgGuardShape, 'awaitArgGuard'); }; harden(assertAwaitArgGuard); +/** + * By using this abstraction rather than accessing the properties directly, + * we smooth the transition to https://github.com/endojs/endo/pull/1712 + * + * @param {AwaitArgGuard} awaitArgGuard + * @returns {AwaitArgGuardPayload} + */ +export const getAwaitArgGuardPayload = awaitArgGuard => { + assertAwaitArgGuard(awaitArgGuard); + const { klass: _, ...payload } = awaitArgGuard; + /** @type {AwaitArgGuardPayload} */ + // @ts-expect-error inference too weak to see this is ok. + return payload; +}; + /** * @param {Pattern} argPattern * @returns {AwaitArgGuard} */ const makeAwaitArgGuard = argPattern => { /** @type {AwaitArgGuard} */ + // @ts-expect-error inference too weak to see it is ok const result = harden({ klass: 'awaitArg', argGuard: argPattern, @@ -1755,11 +1780,30 @@ const AsyncMethodGuardShape = harden({ const MethodGuardShape = M.or(SyncMethodGuardShape, AsyncMethodGuardShape); +/** + * @param {any} specimen + * @returns {asserts specimen is MethodGuard} + */ export const assertMethodGuard = specimen => { mustMatch(specimen, MethodGuardShape, 'methodGuard'); }; harden(assertMethodGuard); +/** + * By using this abstraction rather than accessing the properties directly, + * we smooth the transition to https://github.com/endojs/endo/pull/1712 + * + * @param {MethodGuard} methodGuard + * @returns {MethodGuardPayload} + */ +export const getMethodGuardPayload = methodGuard => { + assertMethodGuard(methodGuard); + const { klass: _, ...payload } = methodGuard; + /** @type {MethodGuardPayload} */ + // @ts-expect-error inference too weak to see this is ok. + return payload; +}; + /** * @param {'sync'|'async'} callKind * @param {ArgGuard[]} argGuards @@ -1792,6 +1836,7 @@ const makeMethodGuardMaker = ( }, returns: (returnGuard = M.undefined()) => { /** @type {MethodGuard} */ + // @ts-expect-error inference too weak to see it is ok const result = harden({ klass: 'methodGuard', callKind, @@ -1817,11 +1862,47 @@ const InterfaceGuardShape = M.splitRecord( }, ); +/** + * @param {any} specimen + * @returns {asserts specimen is InterfaceGuard} + */ export const assertInterfaceGuard = specimen => { mustMatch(specimen, InterfaceGuardShape, 'interfaceGuard'); }; harden(assertInterfaceGuard); +/** + * By using this abstraction rather than accessing the properties directly, + * we smooth the transition to https://github.com/endojs/endo/pull/1712 + * + * @param {InterfaceGuard} interfaceGuard + * @returns {InterfaceGuardPayload} + */ +export const getInterfaceGuardPayload = interfaceGuard => { + assertInterfaceGuard(interfaceGuard); + const { klass: _, ...payload } = interfaceGuard; + /** @type {InterfaceGuardPayload} */ + // @ts-expect-error inference too weak to see this is ok. + return payload; +}; + +const emptyCopyMap = makeCopyMap([]); + +/** + * @param {InterfaceGuard} interfaceGuard + * @returns {(string | symbol)[]} + */ +export const getInterfaceMethodNames = interfaceGuard => { + const { methodGuards, symbolMethodGuards = emptyCopyMap } = + getInterfaceGuardPayload(interfaceGuard); + /** @type {(string | symbol)[]} */ + // @ts-expect-error inference is too weak to see this is ok + return harden([ + ...Reflect.ownKeys(methodGuards), + ...getCopyMapKeys(symbolMethodGuards), + ]); +}; + /** * @param {string} interfaceName * @param {Record} methodGuards @@ -1846,6 +1927,7 @@ const makeInterfaceGuard = (interfaceName, methodGuards, options = {}) => { } } /** @type {InterfaceGuard} */ + // @ts-expect-error inference too weak to see it is ok const result = harden({ klass: 'Interface', interfaceName, diff --git a/packages/patterns/src/types.js b/packages/patterns/src/types.js index 88e0df4916..d4a1644fe4 100644 --- a/packages/patterns/src/types.js +++ b/packages/patterns/src/types.js @@ -519,11 +519,17 @@ export {}; /** * @template {Record} [T=Record] * @typedef {{ - * klass: 'Interface', * interfaceName: string, * methodGuards: { [K in keyof T]: K extends symbol ? never : T[K] }, * symbolMethodGuards?: CopyMap<(keyof T) & symbol, MethodGuard>, * sloppy?: boolean, + * }} InterfaceGuardPayload + */ + +/** + * @template {Record} [T=Record] + * @typedef {{ + * klass: 'Interface' & InterfaceGuardPayload, * }} InterfaceGuard * * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine @@ -583,12 +589,17 @@ export {}; /** * @typedef {{ - * klass: 'methodGuard', * callKind: 'sync' | 'async', * argGuards: ArgGuard[] * optionalArgGuards?: ArgGuard[] * restArgGuard?: Pattern * returnGuard: Pattern + * }} MethodGuardPayload + */ + +/** + * @typedef {{ + * klass: 'methodGuard' & MethodGuardPayload, * }} MethodGuard * * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine @@ -601,8 +612,13 @@ export {}; /** * @typedef {{ - * klass: 'awaitArg', * argGuard: Pattern + * }} AwaitArgGuardPayload + */ + +/** + * @typedef {{ + * klass: 'awaitArg' & AwaitArgGuardPayload, * }} AwaitArgGuard * * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine