From c2cd0343bf42b212d4a144f570f493286ec280ba Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Tue, 8 Aug 2023 18:42:06 -0700 Subject: [PATCH] fix(patterns): type guards --- packages/exo/src/exo-tools.js | 27 +- packages/patterns/index.js | 10 + .../patterns/src/patterns/internal-types.js | 66 ++++ .../patterns/src/patterns/patternMatchers.js | 285 +++++++++++------- packages/patterns/src/types.js | 109 +++++-- 5 files changed, 348 insertions(+), 149 deletions(-) diff --git a/packages/exo/src/exo-tools.js b/packages/exo/src/exo-tools.js index d6a59ca619..0f3bd637cb 100644 --- a/packages/exo/src/exo-tools.js +++ b/packages/exo/src/exo-tools.js @@ -1,6 +1,14 @@ import { E, Far } from '@endo/far'; import { hasOwnPropertyOf } from '@endo/pass-style'; -import { listDifference, objectMap, mustMatch, M } from '@endo/patterns'; +import { + listDifference, + objectMap, + mustMatch, + M, + isAwaitArgGuard, + assertMethodGuard, + assertInterfaceGuard, +} from '@endo/patterns'; /** @typedef {import('@endo/patterns').Method} Method */ /** @typedef {import('@endo/patterns').MethodGuard} MethodGuard */ @@ -47,9 +55,6 @@ const defendSyncMethod = (method, methodGuard, label) => { return syncMethod; }; -const isAwaitArgGuard = argGuard => - argGuard && typeof argGuard === 'object' && argGuard.klass === 'awaitArg'; - const desync = methodGuard => { const { argGuards, optionalArgGuards = [], restArgGuard } = methodGuard; !isAwaitArgGuard(restArgGuard) || @@ -106,8 +111,8 @@ const defendAsyncMethod = (method, methodGuard, label) => { * @param {string} label */ const defendMethod = (method, methodGuard, label) => { - const { klass, callKind } = methodGuard; - assert(klass === 'methodGuard'); + assertMethodGuard(methodGuard); + const { callKind } = methodGuard; if (callKind === 'sync') { return defendSyncMethod(method, methodGuard, label); } else { @@ -257,15 +262,9 @@ export const defendPrototype = ( } let methodGuards; if (interfaceGuard) { - const { - klass, - interfaceName, - methodGuards: mg, - sloppy = false, - } = interfaceGuard; + assertInterfaceGuard(interfaceGuard); + const { interfaceName, methodGuards: mg, sloppy = false } = interfaceGuard; methodGuards = mg; - assert.equal(klass, 'Interface'); - assert.typeof(interfaceName, 'string'); { const methodNames = ownKeys(behaviorMethods); const methodGuardNames = ownKeys(methodGuards); diff --git a/packages/patterns/index.js b/packages/patterns/index.js index e3a313a049..7aa279363a 100644 --- a/packages/patterns/index.js +++ b/packages/patterns/index.js @@ -2,11 +2,17 @@ export { isKey, assertKey, assertScalarKey, + isCopySet, + assertCopySet, makeCopySet, getCopySetKeys, + isCopyBag, + assertCopyBag, makeCopyBag, makeCopyBagFromElements, getCopyBagEntries, + isCopyMap, + assertCopyMap, makeCopyMap, getCopyMapEntries, } from './src/keys/checkKey.js'; @@ -52,6 +58,10 @@ export { assertPattern, matches, mustMatch, + isAwaitArgGuard, + assertAwaitArgGuard, + assertMethodGuard, + assertInterfaceGuard, } 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 c79f167136..c354f879b1 100644 --- a/packages/patterns/src/patterns/internal-types.js +++ b/packages/patterns/src/patterns/internal-types.js @@ -1 +1,67 @@ /// + +/** @typedef {import('@endo/marshal').Passable} Passable */ +/** @typedef {import('@endo/marshal').PassStyle} PassStyle */ +/** @typedef {import('@endo/marshal').CopyTagged} CopyTagged */ +/** @template T @typedef {import('@endo/marshal').CopyRecord} CopyRecord */ +/** @template T @typedef {import('@endo/marshal').CopyArray} CopyArray */ +/** @typedef {import('@endo/marshal').Checker} Checker */ +/** @typedef {import('@endo/marshal').RankCompare} RankCompare */ +/** @typedef {import('@endo/marshal').RankCover} RankCover */ + +/** @typedef {import('../types.js').AwaitArgGuard} AwaitArgGuard */ +/** @typedef {import('../types.js').ArgGuard} ArgGuard */ +/** @typedef {import('../types.js').MethodGuard} MethodGuard */ +/** @typedef {import('../types.js').InterfaceGuard} InterfaceGuard */ +/** @typedef {import('../types.js').MethodGuardMaker0} MethodGuardMaker0 */ + +/** @typedef {import('../types').MatcherNamespace} MatcherNamespace */ +/** @typedef {import('../types').Key} Key */ +/** @typedef {import('../types').Pattern} Pattern */ +/** @typedef {import('../types').CheckPattern} CheckPattern */ +/** @typedef {import('../types').Limits} Limits */ +/** @typedef {import('../types').AllLimits} AllLimits */ +/** @typedef {import('../types').GetRankCover} GetRankCover */ + +/** + * @typedef {object} MatchHelper + * This factors out only the parts specific to each kind of Matcher. It is + * encapsulated, and its methods can make the stated unchecked assumptions + * enforced by the common calling logic. + * + * @property {(allegedPayload: Passable, + * check: Checker + * ) => boolean} checkIsWellFormed + * Reports whether `allegedPayload` is valid as the payload of a CopyTagged + * whose tag corresponds with this MatchHelper's Matchers. + * + * @property {(specimen: Passable, + * matcherPayload: Passable, + * check: Checker, + * ) => boolean} checkMatches + * Assuming validity of `matcherPayload` as the payload of a Matcher corresponding + * with this MatchHelper, reports whether `specimen` is matched by that Matcher. + * + * @property {import('../types').GetRankCover} getRankCover + * Assumes this is the payload of a CopyTagged with the corresponding + * matchTag. Return a RankCover to bound from below and above, + * in rank order, all possible Passables that would match this Matcher. + * The left element must be before or the same rank as any possible + * matching specimen. The right element must be after or the same + * rank as any possible matching specimen. + */ + +/** + * @typedef {object} PatternKit + * @property {(specimen: Passable, + * patt: Passable, + * check: Checker, + * label?: string|number + * ) => boolean} checkMatches + * @property {(specimen: Passable, patt: Pattern) => boolean} matches + * @property {(specimen: Passable, patt: Pattern, label?: string|number) => void} mustMatch + * @property {(patt: Pattern) => void} assertPattern + * @property {(patt: Passable) => boolean} isPattern + * @property {GetRankCover} getRankCover + * @property {MatcherNamespace} M + */ diff --git a/packages/patterns/src/patterns/patternMatchers.js b/packages/patterns/src/patterns/patternMatchers.js index ee49e4d29d..3edeea4032 100644 --- a/packages/patterns/src/patterns/patternMatchers.js +++ b/packages/patterns/src/patterns/patternMatchers.js @@ -32,6 +32,8 @@ import { checkCopyBag, } from '../keys/checkKey.js'; +import './internal-types.js'; + /// const { quote: q, bare: b, details: X, Fail } = assert; @@ -275,6 +277,16 @@ const makePatternKit = () => { return false; }; + /** + * Checks only recognized kinds, and only if the specimen + * passes the invariants associated with that recognition. + * + * @param {Passable} specimen + * @param {Kind} kind + * @returns {boolean} + */ + const isKind = (specimen, kind) => checkKind(specimen, kind, identChecker); + /** * @param {Passable} specimen * @param {Key} keyAsPattern @@ -507,8 +519,8 @@ const makePatternKit = () => { } const { payload: pattPayload } = patt; const { payload: specimenPayload } = specimen; - const pattKeySet = copyMapKeySet(pattPayload); - const specimenKeySet = copyMapKeySet(specimenPayload); + const pattKeySet = copyMapKeySet(patt); + const specimenKeySet = copyMapKeySet(specimen); // Compare keys as copySets if (!checkMatches(specimenKeySet, pattKeySet, check)) { return false; @@ -516,6 +528,11 @@ const makePatternKit = () => { const pattValues = pattPayload.values; const specimenValues = specimenPayload.values; // compare values as copyArrays + // TODO BUG: this assumes that the keys appear in the + // same order, so we can compare values in that order. + // However, we're only guaranteed that they appear in + // the same rankOrder. Thus we must search one of these + // in the other's rankOrder. return checkMatches(specimenValues, pattValues, check); } default: { @@ -665,8 +682,15 @@ const makePatternKit = () => { return getPassStyleCover(passStyle); }; + /** + * @param {Passable[]} array + * @param {Pattern} patt + * @param {Checker} check + * @param {string} [labelPrefix] + * @returns {boolean} + */ const arrayEveryMatchPattern = (array, patt, check, labelPrefix = '') => { - if (checkKind(patt, 'match:any', identChecker)) { + if (isKind(patt, 'match:any')) { // if the pattern is M.any(), we know its true return true; } @@ -723,7 +747,7 @@ const makePatternKit = () => { if ( patts.length === 2 && !matches(specimen, patts[0]) && - checkKind(patts[0], 'match:kind', identChecker) && + isKind(patts[0], 'match:kind') && patts[0].payload === 'undefined' ) { // Worth special casing the optional pattern for @@ -933,8 +957,7 @@ const makePatternKit = () => { /** @type {MatchHelper} */ const matchRemotableHelper = Far('match:remotable helper', { checkMatches: (specimen, remotableDesc, check) => { - // Unfortunate duplication of checkKind logic, but no better choices. - if (checkKind(specimen, 'remotable', identChecker)) { + if (isKind(specimen, 'remotable')) { return true; } if (check === identChecker) { @@ -1544,53 +1567,6 @@ const makePatternKit = () => { return [base]; }; - /** - * @param {'sync'|'async'} callKind - * @param {ArgGuard[]} argGuards - * @param {ArgGuard[]} [optionalArgGuards] - * @param {ArgGuard} [restArgGuard] - * @returns {MethodGuardMaker} - */ - const makeMethodGuardMaker = ( - callKind, - argGuards, - optionalArgGuards = undefined, - restArgGuard = undefined, - ) => - harden({ - optional: (...optArgGuards) => { - optionalArgGuards === undefined || - Fail`Can only have one set of optional guards`; - restArgGuard === undefined || - Fail`optional arg guards must come before rest arg`; - return makeMethodGuardMaker(callKind, argGuards, optArgGuards); - }, - rest: rArgGuard => { - restArgGuard === undefined || Fail`Can only have one rest arg`; - return makeMethodGuardMaker( - callKind, - argGuards, - optionalArgGuards, - rArgGuard, - ); - }, - returns: (returnGuard = UndefinedShape) => - harden({ - klass: /** @type {const} */ ('methodGuard'), - callKind, - argGuards, - optionalArgGuards, - restArgGuard, - returnGuard, - }), - }); - - const makeAwaitArgGuard = argGuard => - harden({ - klass: /** @type {const} */ ('awaitArg'), - argGuard, - }); - // ////////////////// /** @type {MatcherNamespace} */ @@ -1679,22 +1655,19 @@ const makePatternKit = () => { eref: t => M.or(t, M.promise()), opt: t => M.or(M.undefined(), t), - interface: (interfaceName, methodGuards, { sloppy = false } = {}) => { - for (const [_, methodGuard] of entries(methodGuards)) { - methodGuard.klass === 'methodGuard' || - Fail`unrecognize method guard ${methodGuard}`; - } - return harden({ - klass: /** @type {const} */ ('Interface'), - interfaceName, - methodGuards, - sloppy, - }); - }, - call: (...argGuards) => makeMethodGuardMaker('sync', argGuards), - callWhen: (...argGuards) => makeMethodGuardMaker('async', argGuards), + interface: (interfaceName, methodGuards, options) => + // eslint-disable-next-line no-use-before-define + makeInterfaceGuard(interfaceName, methodGuards, options), + call: (...argPatterns) => + // eslint-disable-next-line no-use-before-define + makeMethodGuardMaker('sync', argPatterns), + callWhen: (...argGuards) => + // eslint-disable-next-line no-use-before-define + makeMethodGuardMaker('async', argGuards), - await: argGuard => makeAwaitArgGuard(argGuard), + await: argPattern => + // eslint-disable-next-line no-use-before-define + makeAwaitArgGuard(argPattern), }); return harden({ @@ -1726,50 +1699,138 @@ export const { MM = M; -/** @typedef {import('@endo/marshal').Passable} Passable */ -/** @typedef {import('@endo/marshal').PassStyle} PassStyle */ -/** @typedef {import('@endo/marshal').CopyTagged} CopyTagged */ -/** @template T @typedef {import('@endo/marshal').CopyRecord} CopyRecord */ -/** @template T @typedef {import('@endo/marshal').CopyArray} CopyArray */ -/** @typedef {import('@endo/marshal').Checker} Checker */ -/** @typedef {import('@endo/marshal').RankCompare} RankCompare */ -/** @typedef {import('@endo/marshal').RankCover} RankCover */ - -/** @typedef {import('../types').ArgGuard} ArgGuard */ -/** @typedef {import('../types').MethodGuardMaker} MethodGuardMaker */ -/** @typedef {import('../types').MatcherNamespace} MatcherNamespace */ -/** @typedef {import('../types').Key} Key */ -/** @typedef {import('../types').Pattern} Pattern */ -/** @typedef {import('../types').PatternKit} PatternKit */ -/** @typedef {import('../types').CheckPattern} CheckPattern */ -/** @typedef {import('../types').Limits} Limits */ -/** @typedef {import('../types').AllLimits} AllLimits */ -/** @typedef {import('../types').GetRankCover} GetRankCover */ +// //////////////////////////// Guards /////////////////////////////////////// + +const AwaitArgGuardShape = harden({ + klass: 'awaitArg', + argGuard: M.pattern(), +}); + +export const isAwaitArgGuard = specimen => + matches(specimen, AwaitArgGuardShape); +harden(isAwaitArgGuard); + +export const assertAwaitArgGuard = specimen => { + mustMatch(specimen, AwaitArgGuardShape, 'awaitArgGuard'); +}; +harden(assertAwaitArgGuard); /** - * @typedef {object} MatchHelper - * This factors out only the parts specific to each kind of Matcher. It is - * encapsulated, and its methods can make the stated unchecker assumptions - * enforced by the common calling logic. - * - * @property {(allegedPayload: Passable, - * check: Checker - * ) => boolean} checkIsWellFormed - * Reports whether `allegedPayload` is valid as the payload of a CopyTagged - * whose tag corresponds with this MatchHelper's Matchers. - * - * @property {(specimen: Passable, - * matcherPayload: Passable, - * check: Checker, - * ) => boolean} checkMatches - * Assuming validity of `matcherPayload` as the payload of a Matcher corresponding - * with this MatchHelper, reports whether `specimen` is matched by that Matcher. - * - * @property {import('../types').GetRankCover} getRankCover - * Assumes this is the payload of a CopyTagged with the corresponding - * matchTag. Return a RankCover to bound from below and above, - * in rank order, all possible Passables that would match this Matcher. - * The left element must be before or the same rank as any possible - * matching specimen. The right element must be after or the same - * rank as any possible matching specimen. + * @param {Pattern} argPattern + * @returns {AwaitArgGuard} */ +const makeAwaitArgGuard = argPattern => { + /** @type {AwaitArgGuard} */ + const result = harden({ + klass: 'awaitArg', + argGuard: argPattern, + }); + assertAwaitArgGuard(result); + return result; +}; + +const PatternListShape = M.arrayOf(M.pattern()); + +const ArgGuardShape = M.or(M.pattern(), AwaitArgGuardShape); +const ArgGuardListShape = M.arrayOf(ArgGuardShape); + +const SyncMethodGuardShape = harden({ + klass: 'methodGuard', + callKind: 'sync', + argGuards: PatternListShape, + optionalArgGuards: M.opt(PatternListShape), + restArgGuard: M.opt(M.pattern()), + returnGuard: M.pattern(), +}); + +const AsyncMethodGuardShape = harden({ + klass: 'methodGuard', + callKind: 'async', + argGuards: ArgGuardListShape, + optionalArgGuards: M.opt(ArgGuardListShape), + restArgGuard: M.opt(M.pattern()), + returnGuard: M.pattern(), +}); + +const MethodGuardShape = M.or(SyncMethodGuardShape, AsyncMethodGuardShape); + +export const assertMethodGuard = specimen => { + mustMatch(specimen, MethodGuardShape, 'methodGuard'); +}; +harden(assertMethodGuard); + +/** + * @param {'sync'|'async'} callKind + * @param {ArgGuard[]} argGuards + * @param {ArgGuard[]} [optionalArgGuards] + * @param {ArgGuard} [restArgGuard] + * @returns {MethodGuardMaker0} + */ +const makeMethodGuardMaker = ( + callKind, + argGuards, + optionalArgGuards = undefined, + restArgGuard = undefined, +) => + harden({ + optional: (...optArgGuards) => { + optionalArgGuards === undefined || + Fail`Can only have one set of optional guards`; + restArgGuard === undefined || + Fail`optional arg guards must come before rest arg`; + return makeMethodGuardMaker(callKind, argGuards, optArgGuards); + }, + rest: rArgGuard => { + restArgGuard === undefined || Fail`Can only have one rest arg`; + return makeMethodGuardMaker( + callKind, + argGuards, + optionalArgGuards, + rArgGuard, + ); + }, + returns: (returnGuard = M.undefined()) => { + /** @type {MethodGuard} */ + const result = harden({ + klass: 'methodGuard', + callKind, + argGuards, + optionalArgGuards, + restArgGuard, + returnGuard, + }); + assertMethodGuard(result); + return result; + }, + }); + +const InterfaceGuardShape = harden({ + klass: 'Interface', + interfaceName: M.string(), + methodGuards: M.recordOf(M.string(), MethodGuardShape), + sloppy: M.boolean(), +}); + +export const assertInterfaceGuard = specimen => { + mustMatch(specimen, InterfaceGuardShape, 'interfaceGuard'); +}; +harden(assertInterfaceGuard); + +/** + * @param {string} interfaceName + * @param {Record} methodGuards + * @param {{sloppy?: boolean}} [options] + * @returns {InterfaceGuard} + */ +const makeInterfaceGuard = (interfaceName, methodGuards, options = {}) => { + const { sloppy = false } = options; + /** @type {InterfaceGuard} */ + const result = harden({ + klass: 'Interface', + interfaceName, + methodGuards, + sloppy, + }); + assertInterfaceGuard(result); + return result; +}; diff --git a/packages/patterns/src/types.js b/packages/patterns/src/types.js index ca7b6c4ad3..0935301790 100644 --- a/packages/patterns/src/types.js +++ b/packages/patterns/src/types.js @@ -489,12 +489,18 @@ export {}; /** * @typedef {object} GuardMakers - * @property {MakeInterfaceGuard} interface Guard an interface to a far object or facet - * @property {(...argGuards: ArgGuard[]) => MethodGuardMaker} call Guard a synchronous call * - * @property {(...argGuards: ArgGuard[]) => MethodGuardMaker} callWhen Guard an async call + * @property {MakeInterfaceGuard} interface + * Guard the interface of an exo object * - * @property {(argGuard: ArgGuard) => ArgGuard} await Guard an await + * @property {(...argPatterns: Pattern[]) => MethodGuardMaker0} call + * Guard a synchronous call + * + * @property {(...argGuards: ArgGuard[]) => MethodGuardMaker0} callWhen + * Guard an async call + * + * @property {(argPattern: Pattern) => AwaitArgGuard} await + * Guard an await */ /** @@ -506,15 +512,53 @@ export {}; /** * @template {Record} [T=Record] * @typedef {{ - * klass: 'Interface', - * interfaceName: string, - * methodGuards: T - * sloppy?: boolean + * klass: 'Interface', + * interfaceName: string, + * methodGuards: T + * sloppy?: boolean * }} InterfaceGuard + * + * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine + * guard that is distinct from a copyRecord + */ + +/** + * @typedef {object} MethodGuardMaker0 + * A method name and parameter/return signature like: + * ```js + * foo(a, b, c = d, ...e) => f + * ``` + * should be guarded by something like: + * ```js + * { + * ...otherMethodGuards, + * foo: M.call(AShape, BShape).optional(CShape).rest(EShape).returns(FShape), + * } + * ``` + * @property {(...optArgGuards: ArgGuard[]) => MethodGuardMaker1} optional + * @property {(rArgGuard: Pattern) => MethodGuardMaker2} rest + * @property {(returnGuard?: Pattern) => MethodGuard} returns + */ + +/** + * @typedef {object} MethodGuardMaker1 + * A method name and parameter/return signature like: + * ```js + * foo(a, b, c = d, ...e) => f + * ``` + * should be guarded by something like: + * ```js + * { + * ...otherMethodGuards, + * foo: M.call(AShape, BShape).optional(CShape).rest(EShape).returns(FShape), + * } + * ``` + * @property {(rArgGuard: Pattern) => MethodGuardMaker2} rest + * @property {(returnGuard?: Pattern) => MethodGuard} returns */ /** - * @typedef {any} MethodGuardMaker + * @typedef {object} MethodGuardMaker2 * A method name and parameter/return signature like: * ```js * foo(a, b, c = d, ...e) => f @@ -526,22 +570,41 @@ export {}; * foo: M.call(AShape, BShape).optional(CShape).rest(EShape).returns(FShape), * } * ``` + * @property {(returnGuard?: Pattern) => MethodGuard} returns */ -/** @typedef {{ klass: 'methodGuard', callKind: 'sync' | 'async', returnGuard: unknown }} MethodGuard */ -/** @typedef {any} ArgGuard */ +/** + * @typedef {{ + * klass: 'methodGuard', + * callKind: 'sync' | 'async', + * argGuards: ArgGuard[] + * optionalArgGuards?: ArgGuard[] + * restArgGuard?: Pattern + * returnGuard: Pattern + * }} MethodGuard + * + * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine + * guard that is distinct from a copyRecord. + * Once we're ready for such a compat break, we might also take the + * opportunity to rename `restArgGuard` and `returnGuard` + * to reflect that their value must be a Pattern rather that a + * non-pattern guard. + */ /** - * @typedef {object} PatternKit - * @property {(specimen: Passable, - * patt: Passable, - * check: Checker, - * label?: string|number - * ) => boolean} checkMatches - * @property {(specimen: Passable, patt: Pattern) => boolean} matches - * @property {(specimen: Passable, patt: Pattern, label?: string|number) => void} mustMatch - * @property {(patt: Pattern) => void} assertPattern - * @property {(patt: Passable) => boolean} isPattern - * @property {GetRankCover} getRankCover - * @property {MatcherNamespace} M + * @typedef {{ + * klass: 'awaitArg', + * argGuard: Pattern + * }} AwaitArgGuard + * + * TODO https://github.com/endojs/endo/pull/1712 to make it into a genuine + * guard that is distinct from a copyRecord. + * Unlike InterfaceGuard or MethodGuard, for AwaitArgGuard it is a correctness + * issue, so that the guard not be mistaken for the copyRecord as key/pattern. + * Once we're ready for such a compat break, we might also take the + * opportunity to rename `argGuard` + * to reflect that its value must be a Pattern rather that a + * non-pattern guard. */ + +/** @typedef {AwaitArgGuard | Pattern} ArgGuard */