fix(exo): allow richer behaviorMethods

packages/exo/package.json

@@ -33,6 +33,7 @@
   },
   "dependencies": {
     "@endo/env-options": "^0.1.4",
+    "@endo/eventual-send": "^0.17.6",
     "@endo/far": "^0.2.22",
     "@endo/pass-style": "^0.1.7",
     "@endo/patterns": "^0.2.6"

packages/exo/src/exo-tools.js

@@ -1,5 +1,5 @@
+import { getMethodNames } from '@endo/eventual-send/src/local.js';
 import { E, Far } from '@endo/far';
-import { hasOwnPropertyOf } from '@endo/pass-style';
 import {
   listDifference,
   objectMap,
@@ -253,21 +253,6 @@ const bindMethod = (
  */
 export const GET_INTERFACE_GUARD = Symbol.for('getInterfaceGuard');
 
-/**
- *
- * @template {Record<PropertyKey, CallableFunction>} T
- * @param {T} behaviorMethods
- * @param {InterfaceGuard<{ [M in keyof T]: MethodGuard }>} interfaceGuard
- * @returns {T}
- */
-const withGetInterfaceGuardMethod = (behaviorMethods, interfaceGuard) =>
-  harden({
-    [GET_INTERFACE_GUARD]() {
-      return interfaceGuard;
-    },
-    ...behaviorMethods,
-  });
-
 /**
  * @template {Record<PropertyKey, CallableFunction>} T
  * @param {string} tag
@@ -285,13 +270,20 @@ export const defendPrototype = (
   interfaceGuard = undefined,
 ) => {
   const prototype = {};
-  if (hasOwnPropertyOf(behaviorMethods, 'constructor')) {
-    // By ignoring any method named "constructor", we can use a
+  const methodNames = getMethodNames(behaviorMethods).filter(
+    // By ignoring any method that seems to be a constructor, we can use a
     // class.prototype as a behaviorMethods.
-    const { constructor: _, ...methods } = behaviorMethods;
-    // @ts-expect-error TS misses that hasOwn check makes this safe
-    behaviorMethods = harden(methods);
-  }
+    key => {
+      if (key !== 'constructor') {
+        return true;
+      }
+      const constructor = behaviorMethods.constructor;
+      return !(
+        constructor.prototype &&
+        constructor.prototype.constructor === constructor
+      );
+    },
+  );
   /** @type {Record<PropertyKey, MethodGuard> | undefined} */
   let methodGuards;
   if (interfaceGuard) {
@@ -307,7 +299,6 @@ export const defendPrototype = (
         fromEntries(getCopyMapEntries(symbolMethodGuards))),
     });
     {
-      const methodNames = ownKeys(behaviorMethods);
       const methodGuardNames = ownKeys(methodGuards);
       const unimplemented = listDifference(methodGuardNames, methodNames);
       unimplemented.length === 0 ||
@@ -318,13 +309,9 @@ export const defendPrototype = (
           Fail`methods ${q(unguarded)} not guarded by ${q(interfaceName)}`;
       }
     }
-    behaviorMethods = withGetInterfaceGuardMethod(
-      behaviorMethods,
-      interfaceGuard,
-    );
   }
 
-  for (const prop of ownKeys(behaviorMethods)) {
+  for (const prop of methodNames) {
     prototype[prop] = bindMethod(
       `In ${q(prop)} method of (${tag})`,
       contextProvider,
@@ -335,6 +322,21 @@ export const defendPrototype = (
     );
   }
 
+  if (interfaceGuard) {
+    const getInterfaceGuardMethod = {
+      [GET_INTERFACE_GUARD]() {
+        return interfaceGuard;
+      },
+    }[GET_INTERFACE_GUARD];
+    prototype[GET_INTERFACE_GUARD] = bindMethod(
+      `In ${q(GET_INTERFACE_GUARD)} method of (${tag})`,
+      contextProvider,
+      getInterfaceGuardMethod,
+      thisfulMethods,
+      undefined,
+    );
+  }
+
   return Far(tag, /** @type {T} */ (prototype));
 };
 harden(defendPrototype);

packages/exo/test/test-exo-class-js-class.js

@@ -0,0 +1,95 @@
+/* eslint-disable max-classes-per-file */
+/* eslint-disable class-methods-use-this */
+// eslint-disable-next-line import/order
+import { test } from './prepare-test-env-ava.js';
+
+import { passStyleOf, Far } from '@endo/pass-style';
+// import { M, getInterfaceGuardPayload } from '@endo/patterns';
+// import { defineExoClass, makeExo } from '../src/exo-makers.js';
+import { M, getInterfaceGuardPayload } from '@endo/patterns';
+import { makeExo, defineExoClass } from '../src/exo-makers.js';
+
+/**
+ * Classes whose instances should be Far objects may find it convenient to
+ * inherit from this base class. Note that the constructor of this base class
+ * freezes the instance in an empty state, so all is interesting attributes
+ * can only depend on its identity and what it inherits from.
+ * This includes private fields, as those are keyed only on
+ * this object's identity. However, we discourage (but cannot prevent) such
+ * use of private fields, as they cannot easily be refactored into Exo state.
+ */
+const FarBaseClass = class FarBaseClass {
+  constructor() {
+    harden(this);
+  }
+};
+
+Far('FarBaseClass', FarBaseClass.prototype);
+harden(FarBaseClass);
+
+// Based on FarSubclass1 in test-far-class-instances.js
+class DoublerBehaviorClass extends FarBaseClass {
+  double(x) {
+    return x + x;
+  }
+}
+
+const DoublerI = M.interface('Doubler', {
+  double: M.call(M.lte(10)).returns(M.number()),
+});
+
+const doubler = makeExo('doubler', DoublerI, DoublerBehaviorClass.prototype);
+
+test('exo doubler using js classes', t => {
+  t.is(passStyleOf(doubler), 'remotable');
+  t.is(doubler.double(3), 6);
+  t.throws(() => doubler.double('x'), {
+    message:
+      'In "double" method of (doubler): arg 0: "x" - Must be <= 10',
+  });
+  t.throws(() => doubler.double(), {
+    message:
+      'In "double" method of (doubler): Expected at least 1 arguments: []',
+  });
+  t.throws(() => doubler.double(12), {
+    message: 'In "double" method of (doubler): arg 0: 12 - Must be <= 10',
+  });
+});
+
+// Based on FarSubclass2 in test-far-class-instances.js
+class DoubleAdderBehaviorClass extends DoublerBehaviorClass {
+  doubleAddSelfCall(x) {
+    const {
+      state: { y },
+      self,
+    } = this;
+    return self.double(x) + y;
+  }
+
+  doubleAddSuperCall(x) {
+    const { state: { y } } = this;
+    return super.double(x) + y;
+  }
+}
+
+const DoubleAdderI = M.interface('DoubleAdder', {
+  ...getInterfaceGuardPayload(DoublerI).methodGuards,
+  doubleAddSelfCall: M.call(M.number()).returns(M.number()),
+  doubleAddSuperCall: M.call(M.number()).returns(M.number()),
+});
+
+const makeDoubleAdder = defineExoClass(
+  'doubleAdderClass',
+  DoubleAdderI,
+  y => ({ y }),
+  DoubleAdderBehaviorClass.prototype,
+);
+
+test('exo inheritance self vs super call', t => {
+  const da = makeDoubleAdder(5);
+  t.is(da.doubleAddSelfCall(3), 11);
+  t.throws(() => da.doubleAddSelfCall(12), {
+    message: 'In "double" method of (doubleAdderClass): arg 0: 12 - Must be <= 10',
+  });
+  t.is(da.doubleAddSuperCall(12), 29);
+});

packages/exo/test/test-non-enumerable-methods.js

@@ -0,0 +1,74 @@
+// eslint-disable-next-line import/order
+import { test } from './prepare-test-env-ava.js';
+
+// eslint-disable-next-line import/order
+import { getInterfaceMethodKeys, M, objectMetaMap } from '@endo/patterns';
+import { defineExoClass } from '../src/exo-makers.js';
+import { GET_INTERFACE_GUARD } from '../src/exo-tools.js';
+
+const { getPrototypeOf } = Object;
+
+const UpCounterI = M.interface('UpCounter', {
+  incr: M.call()
+    // TODO M.number() should not be needed to get a better error message
+    .optional(M.and(M.number(), M.gte(0)))
+    .returns(M.number()),
+});
+
+const denumerate = obj =>
+  objectMetaMap(
+    obj,
+    desc => ({ ...desc, enumerable: false }),
+    getPrototypeOf(obj),
+  );
+
+test('test defineExoClass', t => {
+  const makeUpCounter = defineExoClass(
+    'UpCounter',
+    UpCounterI,
+    /** @param {number} x */
+    (x = 0) => ({ x }),
+    denumerate({
+      incr(y = 1) {
+        const { state } = this;
+        state.x += y;
+        return state.x;
+      },
+    }),
+  );
+  const upCounter = makeUpCounter(3);
+  t.is(upCounter.incr(5), 8);
+  t.is(upCounter.incr(1), 9);
+  t.throws(() => upCounter.incr(-3), {
+    message: 'In "incr" method of (UpCounter): arg 0?: -3 - Must be >= 0',
+  });
+  // @ts-expect-error bad arg
+  t.throws(() => upCounter.incr('foo'), {
+    message:
+      'In "incr" method of (UpCounter): arg 0?: string "foo" - Must be a number',
+  });
+  t.deepEqual(upCounter[GET_INTERFACE_GUARD](), UpCounterI);
+  t.deepEqual(getInterfaceMethodKeys(UpCounterI), ['incr']);
+
+  const symbolic = Symbol.for('symbolic');
+  const FooI = M.interface('Foo', {
+    m: M.call().returns(),
+    [symbolic]: M.call(M.boolean()).returns(),
+  });
+  t.deepEqual(getInterfaceMethodKeys(FooI), ['m', Symbol.for('symbolic')]);
+  const makeFoo = defineExoClass(
+    'Foo',
+    FooI,
+    () => ({}),
+    denumerate({
+      m() {},
+      [symbolic]() {},
+    }),
+  );
+  const foo = makeFoo();
+  t.deepEqual(foo[GET_INTERFACE_GUARD](), FooI);
+  t.throws(() => foo[symbolic]('invalid arg'), {
+    message:
+      'In "[Symbol(symbolic)]" method of (Foo): arg 0: string "invalid arg" - Must be a boolean',
+  });
+});

packages/patterns/index.js

@@ -70,7 +70,12 @@ export {
 
 // ////////////////// Temporary, until these find their proper home ////////////
 
-export { listDifference, objectMap } from './src/utils.js';
+export {
+  listDifference,
+  objectMap,
+  objectMetaMap,
+  objectMetaAssign,
+} from './src/utils.js';
 
 // eslint-disable-next-line import/export
 export * from './src/types.js';

packages/patterns/src/utils.js

@@ -4,7 +4,13 @@ import { isPromise } from '@endo/promise-kit';
 
 /** @typedef {import('@endo/marshal').Checker} Checker */
 
-const { fromEntries, entries } = Object;
+const {
+  fromEntries,
+  entries,
+  getOwnPropertyDescriptors,
+  create,
+  defineProperties,
+} = Object;
 const { ownKeys } = Reflect;
 
 const { details: X, quote: q, Fail } = assert;
@@ -92,8 +98,8 @@ harden(fromUniqueEntries);
  * a CopyRecord.
  *
  * @template {Record<string, any>} O
- * @param {O} original
  * @template R map result
+ * @param {O} original
  * @param {(value: O[keyof O], key: keyof O) => R} mapFn
  * @returns {{ [P in keyof O]: R}}
  */
@@ -104,6 +110,79 @@ export const objectMap = (original, mapFn) => {
 };
 harden(objectMap);
 
+/**
+ * Like `objectMap`, but at the reflective level of property descriptors
+ * rather than property values.
+ *
+ * Except for hardening, the edge case behavior is mostly the opposite of
+ * the `objectMap` edge cases.
+ *    * No matter how mutable the original object, the returned object is
+ *      hardened.
+ *    * All own properties of the original are mapped, even if symbol-named
+ *      or non-enumerable.
+ *    * If any of the original properties were accessors, the descriptor
+ *      containing the getter and setter are given to `metaMapFn`.
+ *    * The own properties of the returned are according to the descriptors
+ *      returned by `metaMapFn`.
+ *    * The returned object will always be a plain object whose state is
+ *      only these mapped own properties. It will inherit from the third
+ *      argument if provided, defaulting to `Object.prototype` if omitted.
+ *
+ * Because a property descriptor is distinct from `undefined`, we bundle
+ * mapping and filtering together. When the `metaMapFn` returns `undefined`,
+ * that property is omitted from the result.
+ *
+ * @template {Record<PropertyKey, any>} O
+ * @param {O} original
+ * @param {(
+ *   desc: TypedPropertyDescriptor<O[keyof O]>,
+ *   key: keyof O
+ * ) => (PropertyDescriptor | undefined)} metaMapFn
+ * @param {any} [proto]
+ * @returns {any}
+ */
+export const objectMetaMap = (
+  original,
+  metaMapFn,
+  proto = Object.prototype,
+) => {
+  const descs = getOwnPropertyDescriptors(original);
+  const keys = ownKeys(original);
+
+  const descEntries = /** @type {[PropertyKey,PropertyDescriptor][]} */ (
+    keys
+      .map(key => [key, metaMapFn(descs[key], key)])
+      .filter(([_key, optDesc]) => optDesc !== undefined)
+  );
+  return harden(create(proto, fromUniqueEntries(descEntries)));
+};
+harden(objectMetaMap);
+
+/**
+ * Like `Object.assign` but at the reflective level of property descriptors
+ * rather than property values.
+ *
+ * Unlike `Object.assign`, this includes all own properties, whether enumerable
+ * or not. An original accessor property is copied by sharing its getter and
+ * setter, rather than calling the getter to obtain a value. If an original
+ * property is non-configurable, a property of the same name on a later original
+ * that would conflict instead causes the call to `objectMetaAssign` to throw an
+ * error.
+ *
+ * Returns the enhanced `target` after hardening.
+ *
+ * @param {any} target
+ * @param {any[]} originals
+ * @returns {any}
+ */
+export const objectMetaAssign = (target, ...originals) => {
+  for (const original of originals) {
+    defineProperties(target, getOwnPropertyDescriptors(original));
+  }
+  return harden(target);
+};
+harden(objectMetaAssign);
+
 /**
  *
  * @param {Array<string | symbol>} leftNames