fix(exo): allow non-enumerable methods

packages/exo/src/exo-tools.js

@@ -22,7 +22,8 @@ import {
 
 const { quote: q, Fail } = assert;
 const { apply, ownKeys } = Reflect;
-const { defineProperties, fromEntries } = Object;
+const { defineProperties, fromEntries, getOwnPropertyDescriptors, create } =
+  Object;
 
 /**
  * A method guard, for inclusion in an interface guard, that enforces only that
@@ -255,17 +256,22 @@ export const GET_INTERFACE_GUARD = Symbol.for('getInterfaceGuard');
 
 /**
  *
- * @template {Record<PropertyKey, CallableFunction>} T
- * @param {T} behaviorMethods
+ * @template {Record<PropertyKey, TypedPropertyDescriptor<CallableFunction>>} T
+ * @param {T} methodDescs
  * @param {InterfaceGuard<{ [M in keyof T]: MethodGuard }>} interfaceGuard
  * @returns {T}
  */
-const withGetInterfaceGuardMethod = (behaviorMethods, interfaceGuard) =>
+const withGetInterfaceGuardMethodDesc = (methodDescs, interfaceGuard) =>
   harden({
-    [GET_INTERFACE_GUARD]() {
-      return interfaceGuard;
+    [GET_INTERFACE_GUARD]: {
+      value() {
+        return interfaceGuard;
+      },
+      writable: false,
+      enumerable: false,
+      configurable: false,
     },
-    ...behaviorMethods,
+    ...methodDescs,
   });
 
 /**
@@ -285,12 +291,22 @@ export const defendPrototype = (
   interfaceGuard = undefined,
 ) => {
   const prototype = {};
-  if (hasOwnPropertyOf(behaviorMethods, 'constructor')) {
-    // By ignoring any method named "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);
+  let methodDescs = getOwnPropertyDescriptors(behaviorMethods);
+  if (
+    hasOwnPropertyOf(behaviorMethods, 'constructor') &&
+    typeof behaviorMethods.constructor === 'function' &&
+    hasOwnPropertyOf(behaviorMethods.constructor, 'prototype') &&
+    behaviorMethods.constructor.prototype === behaviorMethods
+  ) {
+    // By ignoring any method named "constructor" that might be a
+    // class, we can use a class.prototype as a behaviorMethods.
+    // TODO: Because this looks only at own properties, it is
+    // currently useful only on base classes. Better would be
+    // for it to deal with subclasses as well.
+    const { constructor: _, ...otherDescs } =
+      getOwnPropertyDescriptors(behaviorMethods);
+    // @ts-expect-error typing ignoring `constructor`
+    methodDescs = otherDescs;
   }
   /** @type {Record<PropertyKey, MethodGuard> | undefined} */
   let methodGuards;
@@ -307,7 +323,7 @@ export const defendPrototype = (
         fromEntries(getCopyMapEntries(symbolMethodGuards))),
     });
     {
-      const methodNames = ownKeys(behaviorMethods);
+      const methodNames = ownKeys(methodDescs);
       const methodGuardNames = ownKeys(methodGuards);
       const unimplemented = listDifference(methodGuardNames, methodNames);
       unimplemented.length === 0 ||
@@ -318,17 +334,16 @@ export const defendPrototype = (
           Fail`methods ${q(unguarded)} not guarded by ${q(interfaceName)}`;
       }
     }
-    behaviorMethods = withGetInterfaceGuardMethod(
-      behaviorMethods,
-      interfaceGuard,
-    );
+    methodDescs = withGetInterfaceGuardMethodDesc(methodDescs, interfaceGuard);
   }
 
-  for (const prop of ownKeys(behaviorMethods)) {
+  const methods = create(Object.prototype, methodDescs);
+
+  for (const prop of ownKeys(methods)) {
     prototype[prop] = bindMethod(
       `In ${q(prop)} method of (${tag})`,
       contextProvider,
-      behaviorMethods[prop],
+      methods[prop],
       thisfulMethods,
       // TODO some tool does not yet understand the `?.[` syntax
       methodGuards && methodGuards[prop],

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

@@ -0,0 +1,76 @@
+// eslint-disable-next-line import/order
+import { test } from './prepare-test-env-ava.js';
+
+// eslint-disable-next-line import/order
+import { getInterfaceMethodKeys, M, objectMap } from '@endo/patterns';
+import { defineExoClass } from '../src/exo-makers.js';
+import { GET_INTERFACE_GUARD } from '../src/exo-tools.js';
+
+const { getOwnPropertyDescriptors, create, 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 =>
+  create(
+    getPrototypeOf(obj),
+    objectMap(getOwnPropertyDescriptors(obj), desc => ({
+      ...desc,
+      enumerable: false,
+    })),
+  );
+
+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',
+  });
+});