Skip to content

refactor(pass-style): refactor-only avoid symbol-named methods #2840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions packages/daemon/src/reader-ref.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ export const makeIteratorRef = iterable => {
}
return harden({ done: true, value: undefined });
},
[Symbol.asyncIterator]() {
return this;
},
});
};

Expand Down
5 changes: 3 additions & 2 deletions packages/daemon/src/ref-reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import { E } from '@endo/far';
* @param {import('@endo/far').ERef<AsyncIterator<TValue, TReturn, TNext>>} iteratorRef
*/
export const makeRefIterator = iteratorRef => {
const iterator = {
const iterator = harden({
/** @param {[] | [TNext]} args */
next: async (...args) => E(iteratorRef).next(...args),
/** @param {[] | [TReturn]} args */
return: async (...args) => E(iteratorRef).return(...args),
/** @param {any} error */
throw: async error => E(iteratorRef).throw(error),
[Symbol.asyncIterator]: () => iterator,
};
});
return iterator;
};
harden(makeRefIterator);

/**
* @param {import('@endo/far').ERef<AsyncIterator<string>>} readerRef
Expand Down
1 change: 1 addition & 0 deletions packages/exo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
},
"devDependencies": {
"@endo/init": "workspace:^",
"@endo/marshal": "workspace:^",
"@endo/ses-ava": "workspace:^",
"ava": "^6.1.3",
"babel-eslint": "^10.1.0",
Expand Down
29 changes: 15 additions & 14 deletions packages/exo/src/exo-tools.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { getMethodNames } from '@endo/eventual-send/utils.js';
import { hasOwnPropertyOf, toThrowable } from '@endo/pass-style';
import { E, Far } from '@endo/far';
import { E } from '@endo/eventual-send';
import {
getRemotableMethodNames,
hasOwnPropertyOf,
toThrowable,
Far,
} from '@endo/pass-style';
import {
mustMatch,
M,
Expand All @@ -17,8 +21,10 @@ import { q, Fail } from '@endo/errors';
import { GET_INTERFACE_GUARD } from './get-interface.js';

/**
* @import {InterfaceGuard, Method, MethodGuard, MethodGuardPayload} from '@endo/patterns'
* @import {RemotableMethodName} from '@endo/pass-style';
* @import {InterfaceGuard, Method, MethodGuard, MethodGuardPayload, DefaultGuardType} from '@endo/patterns'
* @import {ClassContext, ContextProvider, FacetName, KitContext, KitContextProvider, MatchConfig, Methods} from './types.js'
* @import {GetInterfaceGuard} from './get-interface.js';
*/

const { apply, ownKeys } = Reflect;
Expand Down Expand Up @@ -344,7 +350,7 @@ const bindMethod = (
};

/**
* @template {Record<PropertyKey, CallableFunction>} T
* @template {Record<RemotableMethodName, CallableFunction>} T
* @param {string} tag
* @param {ContextProvider} contextProvider
* @param {T} behaviorMethods
Expand All @@ -359,7 +365,7 @@ export const defendPrototype = (
interfaceGuard = undefined,
) => {
const prototype = {};
const methodNames = getMethodNames(behaviorMethods).filter(
const methodNames = getRemotableMethodNames(behaviorMethods).filter(
// By ignoring any method that seems to be a constructor, we can use a
// class.prototype as a behaviorMethods.
key => {
Expand All @@ -373,9 +379,9 @@ export const defendPrototype = (
);
},
);
/** @type {Record<PropertyKey, MethodGuard> | undefined} */
/** @type {Record<RemotableMethodName, MethodGuard> | undefined} */
let methodGuards;
/** @type {import('@endo/patterns').DefaultGuardType} */
/** @type {DefaultGuardType} */
let defaultGuards;
if (interfaceGuard) {
const {
Expand Down Expand Up @@ -461,12 +467,7 @@ export const defendPrototype = (
);
}

return Far(
tag,
/** @type {T & import('./get-interface.js').GetInterfaceGuard<T>} */ (
prototype
),
);
return Far(tag, /** @type {T & GetInterfaceGuard<T>} */ (prototype));
};
harden(defendPrototype);

Expand Down
12 changes: 7 additions & 5 deletions packages/exo/src/get-interface.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
// @ts-check

/**
* @import {RemotableMethodName} from '@endo/pass-style';
*/

/**
* The name of the automatically added default meta-method for
* obtaining an exo's interface, if it has one.
*
* Intended to be similar to `GET_METHOD_NAMES` from `@endo/pass-style`.
*
* TODO Name to be bikeshed. Perhaps even whether it is a
* string or symbol to be bikeshed. See
* https://github.com/endojs/endo/pull/1809#discussion_r1388052454
* See https://github.com/endojs/endo/pull/1809#discussion_r1388052454
*
* TODO Beware that an exo's interface can change across an upgrade,
* Beware that an exo's interface can change across an upgrade,
* so remotes that cache it can become stale.
*/
export const GET_INTERFACE_GUARD = '__getInterfaceGuard__';

/**
* @template {Record<PropertyKey, CallableFunction>} M
* @template {Record<RemotableMethodName, CallableFunction>} M
* @typedef {{
* [GET_INTERFACE_GUARD]?: () =>
* import('@endo/patterns').InterfaceGuard<{
Expand Down
4 changes: 2 additions & 2 deletions packages/exo/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { RemotableBrand } from '@endo/eventual-send';
import type { RemotableObject } from '@endo/pass-style';
import type { RemotableObject, RemotableMethodName } from '@endo/pass-style';
import type { InterfaceGuard, MethodGuard, Pattern } from '@endo/patterns';
import type { GetInterfaceGuard } from './get-interface.js';

Expand All @@ -11,7 +11,7 @@ export type MatchConfig = {
redactedIndices: number[];
};
export type FacetName = string;
export type Methods = Record<PropertyKey, CallableFunction>;
export type Methods = Record<RemotableMethodName, CallableFunction>;
export type ClassContext<S = any, M extends Methods = any> = {
state: S;
self: M;
Expand Down
15 changes: 5 additions & 10 deletions packages/exo/test/heap-classes.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// @ts-check
import test from '@endo/ses-ava/prepare-endo.js';

import { passableSymbolForName } from '@endo/pass-style';
import { getInterfaceMethodKeys, M } from '@endo/patterns';
import {
GET_INTERFACE_GUARD,
Expand Down Expand Up @@ -81,25 +80,21 @@ test('test defineExoClass', t => {
t.deepEqual(upCounter[GET_INTERFACE_GUARD]?.(), UpCounterI);
t.deepEqual(getInterfaceMethodKeys(UpCounterI), ['incr']);

const symbolic = passableSymbolForName('symbolic');
const FooI = M.interface('Foo', {
m: M.call().returns(),
[symbolic]: M.call(M.boolean()).returns(),
m2: M.call(M.boolean()).returns(),
});
t.deepEqual(getInterfaceMethodKeys(FooI), [
'm',
passableSymbolForName('symbolic'),
]);
t.deepEqual(getInterfaceMethodKeys(FooI), ['m', 'm2']);
const makeFoo = defineExoClass('Foo', FooI, () => ({}), {
m() {},
[symbolic]() {},
m2() {},
});
const foo = makeFoo();
t.deepEqual(foo[GET_INTERFACE_GUARD]?.(), FooI);
// @ts-expect-error intentional for test
t.throws(() => foo[symbolic]('invalid arg'), {
t.throws(() => foo.m2('invalid arg'), {
message:
'In "[Symbol(symbolic)]" method of (Foo): arg 0: string "invalid arg" - Must be a boolean',
'In "m2" method of (Foo): arg 0: string "invalid arg" - Must be a boolean',
});
});

Expand Down
15 changes: 5 additions & 10 deletions packages/exo/test/non-enumerable-methods.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import test from '@endo/ses-ava/prepare-endo.js';

import { objectMetaMap } from '@endo/common/object-meta-map.js';
import { passableSymbolForName } from '@endo/pass-style';
import { getInterfaceMethodKeys, M } from '@endo/patterns';
import { defineExoClass } from '../src/exo-makers.js';
import { GET_INTERFACE_GUARD } from '../src/get-interface.js';
Expand Down Expand Up @@ -50,28 +49,24 @@ test('test defineExoClass', t => {
t.deepEqual(upCounter[GET_INTERFACE_GUARD](), UpCounterI);
t.deepEqual(getInterfaceMethodKeys(UpCounterI), ['incr']);

const symbolic = passableSymbolForName('symbolic');
const FooI = M.interface('Foo', {
m: M.call().returns(),
[symbolic]: M.call(M.boolean()).returns(),
m2: M.call(M.boolean()).returns(),
});
t.deepEqual(getInterfaceMethodKeys(FooI), [
'm',
passableSymbolForName('symbolic'),
]);
t.deepEqual(getInterfaceMethodKeys(FooI), ['m', 'm2']);
const makeFoo = defineExoClass(
'Foo',
FooI,
() => ({}),
denumerate({
m() {},
[symbolic]() {},
m2() {},
}),
);
const foo = makeFoo();
t.deepEqual(foo[GET_INTERFACE_GUARD](), FooI);
t.throws(() => foo[symbolic]('invalid arg'), {
t.throws(() => foo.m2('invalid arg'), {
message:
'In "[Symbol(symbolic)]" method of (Foo): arg 0: string "invalid arg" - Must be a boolean',
'In "m2" method of (Foo): arg 0: string "invalid arg" - Must be a boolean',
});
});
10 changes: 7 additions & 3 deletions packages/marshal/src/dot-membrane.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@ import {
getInterfaceOf,
Far,
passStyleOf,
getRemotableMethodNames,
} from '@endo/pass-style';
import { Fail } from '@endo/errors';
import { makeMarshal } from './marshal.js';

/**
* @import {RemotableMethodName} from '@endo/pass-style';
*/

const { fromEntries } = Object;
const { ownKeys } = Reflect;

// TODO(erights): Add Converter type
/** @param {any} [mirrorConverter] */
Expand Down Expand Up @@ -63,7 +67,7 @@ const makeConverter = (mirrorConverter = undefined) => {
break;
}
case 'remotable': {
/** @param {PropertyKey} [optVerb] */
/** @param {RemotableMethodName} [optVerb] */
const myMethodToYours =
(optVerb = undefined) =>
(...yourArgs) => {
Expand Down Expand Up @@ -96,7 +100,7 @@ const makeConverter = (mirrorConverter = undefined) => {
// minds.
yours = Far(iface, myMethodToYours());
} else {
const myMethodNames = ownKeys(mine);
const myMethodNames = getRemotableMethodNames(mine);
const yourMethods = myMethodNames.map(name => [
name,
myMethodToYours(name),
Expand Down
2 changes: 1 addition & 1 deletion packages/marshal/src/marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ harden(stringify);

/**
* @param {string} str
* @returns {unknown}
* @returns {Passable}
*/
const parse = str =>
unserialize(
Expand Down
2 changes: 1 addition & 1 deletion packages/pass-style/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export {

export { getErrorConstructor, isErrorLike } from './src/error.js';

export { getInterfaceOf } from './src/remotable.js';
export { getInterfaceOf, getRemotableMethodNames } from './src/remotable.js';

export {
assertPassableSymbol,
Expand Down
2 changes: 1 addition & 1 deletion packages/pass-style/src/copyRecord.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

import {
assertChecker,
canBeMethod,
getOwnDataDescriptor,
CX,
} from './passStyle-helpers.js';
import { canBeMethod } from './remotable.js';

const { ownKeys } = Reflect;
const { getPrototypeOf, prototype: objectPrototype } = Object;
Expand Down
17 changes: 0 additions & 17 deletions packages/pass-style/src/passStyle-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,6 @@ harden(isTypedArray);

export const PASS_STYLE = Symbol.for('passStyle');

/**
* For a function to be a valid method, it must not be passable.
* Otherwise, we risk confusing pass-by-copy data carrying
* far functions with attempts at far objects with methods.
*
* TODO HAZARD Because we check this on the way to hardening a remotable,
* we cannot yet check that `func` is hardened. However, without
* doing so, it's inheritance might change after the `PASS_STYLE`
* check below.
*
* @param {any} func
* @returns {boolean}
*/
export const canBeMethod = func =>
typeof func === 'function' && !(PASS_STYLE in func);
harden(canBeMethod);

/**
* Below we have a series of predicate functions and their (curried) assertion
* functions. The semantics of the assertion function is just to assert that
Expand Down
48 changes: 46 additions & 2 deletions packages/pass-style/src/remotable.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/// <reference types="ses"/>

import { Fail, q } from '@endo/errors';
import { getMethodNames } from '@endo/eventual-send/utils.js';
import {
assertChecker,
canBeMethod,
hasOwnPropertyOf,
PASS_STYLE,
checkTagRecord,
Expand All @@ -14,12 +14,56 @@ import {
} from './passStyle-helpers.js';

/**
* @import {Checker} from './types.js'
* @import {Checker, RemotableMethodName} from './types.js'
* @import {InterfaceSpec, PassStyled} from './types.js'
* @import {PassStyleHelper} from './internal-types.js'
* @import {RemotableObject as Remotable} from './types.js'
*/

/**
* For a function to be a valid method, it must not be passable.
* Otherwise, we risk confusing pass-by-copy data carrying
* far functions with attempts at far objects with methods.
*
* TODO HAZARD Because we check this on the way to hardening a remotable,
* we cannot yet check that `func` is hardened. However, without
* doing so, it's inheritance might change after the `PASS_STYLE`
* check below.
*
* @param {any} func
* @returns {func is CallableFunction}
*/
export const canBeMethod = func =>
typeof func === 'function' && !(PASS_STYLE in func);
harden(canBeMethod);

/**
* Abstract out this test so
* (TODO) a later PR can restrict possible method names
*
* @param {any} key
* @returns {key is RemotableMethodName}
*/
const canBeMethodName = key =>
// typeof key === 'string' || typeof key === 'symbol';
typeof key === 'string' || typeof key === 'symbol' || typeof key === 'number';
harden(canBeMethodName);

/**
* Uses the `getMethodNames` from the eventual-send level of abstraction that
* does not know anything about remotables.
*
* Currently, just alias `getMethodNames` but this abstraction exists so
* a future PR can enforce restrictions on method names of remotables.
*
* @template {Record<string, CallableFunction>} T
* @param {T} behaviorMethods
* @returns {RemotableMethodName[]}
*/
export const getRemotableMethodNames = behaviorMethods =>
getMethodNames(behaviorMethods);
harden(getRemotableMethodNames);

const { ownKeys } = Reflect;
const { isArray } = Array;
const {
Expand Down
Loading