types(marshal): fix #1257 typing of deeplyFulfilled (#2312)

Closes: #1257 
Refs: Agoric/agoric-sdk#5993
Agoric/agoric-sdk#6816
#1455

## Description

Low urgency. This is a minor reduction of tech debt.

Agoric/agoric-sdk#5993 introduced
`deeplyFulfilledObject` to work around the deficiencies in the typing of
`deeplyFulfilled` explained at #1257 . This PR fixes those deficiencies.
This does not necessary make `deeplyFulfilledObject` unneeded, but it
should enable many callers of `deeplyFulfilledObject` to call
`deeplyFulfilled` instead without loss of type fidelity.

Note: the code in this first commit probably does not use TS typing
well, resulting in many internal `@ts-expect-error` directives in the
implementation. This is because I still don't really understand what I'm
doing with complex TS types. Reviewers, help appreciated.

But IIUC, the external type should now be as good as
`deeplyFulfilledObject`. Hopefully, the only typing flaws are
encapsulated by this implementation. Yes?

### Security Considerations

More accurate typing helps security. More precise but inaccurate unsound
typing hurts security. Our security practices are already adapted to
this dilemma, and this PR should not have much effect.

### Scaling Considerations

none
### Documentation Considerations

It would be nice if this PR eventually let us drop the need to explain
`deeplyFulfilledObject` and why it co-exists with `deeplyFulfilled`

### Testing Considerations

- [ ] since this is all about changing static types, I need to write
some static type tests.

Even though the code itself is also a bit different, these differences
are with high confidence a pure refactor, so the existing dynamic tests
should adequately test that.

### Compatibility Considerations

Assuming that these changes are a pure refactor with no dynamic behavior
changes, there should be no dynamic compat issues.

The tighter typing of `deeplyFulfilled` does raise the possibility that
some call sites will no longer pass static type checks. But no such
problem appears within the PR's CI, and therefore within the endo repo.

### Upgrade Considerations

none.

- [ ] Include `*BREAKING*:` in the commit message with migration
instructions for any breaking change.
- [ ] Update `NEWS.md` for user-facing changes.

packages/marshal/src/deeplyFulfilled.js

@@ -1,17 +1,54 @@
-/// <reference types="ses"/>
-
+import { X, q } from '@endo/errors';
 import { E } from '@endo/eventual-send';
 import { isPromise } from '@endo/promise-kit';
 import { getTag, isObject, makeTagged, passStyleOf } from '@endo/pass-style';
 
-/** @import {Passable} from '@endo/pass-style' */
-
-import { X, q } from '@endo/errors';
+/**
+ * @import {Passable, Primitive, CopyRecord, CopyArray, CopyTagged, RemotableObject} from '@endo/pass-style'
+ */
 
 const { ownKeys } = Reflect;
 const { fromEntries } = Object;
 
-// TODO return a type contingent on the parameter as deeplyFullfilledObject from agoric-sdk does
+/**
+ * Currently copied from @agoric/internal utils.js.
+ * TODO Should migrate here and then, if needed, reexported there.
+ *
+ * @template T
+ * @typedef {{ [KeyType in keyof T]: T[KeyType] } & {}} Simplify flatten the
+ *   type output to improve type hints shown in editors
+ *   https://github.com/sindresorhus/type-fest/blob/main/source/simplify.d.ts
+ */
+
+/**
+ * Currently copied from @agoric/internal utils.js.
+ * TODO Should migrate here and then, if needed, reexported there.
+ *
+ * @typedef {(...args: any[]) => any} Callable
+ */
+
+/**
+ * Currently copied from @agoric/internal utils.js.
+ * TODO Should migrate here and then, if needed, reexported there.
+ *
+ * @template {{}} T
+ * @typedef {{
+ *   [K in keyof T]: T[K] extends Callable ? T[K] : DeeplyAwaited<T[K]>;
+ * }} DeeplyAwaitedObject
+ */
+
+/**
+ * Currently copied from @agoric/internal utils.js.
+ * TODO Should migrate here and then, if needed, reexported there.
+ *
+ * @template T
+ * @typedef {T extends PromiseLike<any>
+ *     ? Awaited<T>
+ *     : T extends {}
+ *       ? Simplify<DeeplyAwaitedObject<T>>
+ *       : Awaited<T>} DeeplyAwaited
+ */
+
 /**
  * Given a Passable `val` whose pass-by-copy structure may contain leaf
  * promises, return a promise for a replacement Passable,
@@ -29,53 +66,64 @@ const { fromEntries } = Object;
  * Passable, or if any of the transitive promises fulfill to something
  * that is not Passable, then the returned promise rejects.
  *
- * If `val` or its parts are non-key Passables only *because* they contains
+ * If `val` or its parts are non-key Passables only *because* they contain
  * promises, the deeply fulfilled forms of val or its parts may be keys. This
- * is for the higher "store" level of abstraction to determine, because it
- * defines the "key" notion in question.
- *
- * // TODO: That higher level is in the process of being migrated from
- * // `@agoric/store` to `@endo/patterns`. Once that is far enough along,
- * // revise the above comment to match.
- * // See https://github.com/endojs/endo/pull/1451
+ * is for the higher "@endo/patterns" level of abstraction to determine,
+ * because it defines the `Key` notion in question.
  *
- * @param {any} val
- * @returns {Promise<Passable>}
+ * @template {Passable} [T=Passable]
+ * @param {T} val
+ * @returns {Promise<DeeplyAwaited<T>>}
  */
 export const deeplyFulfilled = async val => {
+  // TODO Figure out why we need these at-expect-error directives below
+  // and fix if possible.
+  // https://github.com/endojs/endo/issues/1257 may be relevant.
+
   if (!isObject(val)) {
-    return val;
+    return /** @type {DeeplyAwaited<T>} */ (val);
   }
   if (isPromise(val)) {
     return E.when(val, nonp => deeplyFulfilled(nonp));
   }
   const passStyle = passStyleOf(val);
   switch (passStyle) {
     case 'copyRecord': {
-      const names = ownKeys(val);
-      const valPs = names.map(name => deeplyFulfilled(val[name]));
+      const rec = /** @type {CopyRecord} */ (val);
+      const names = /** @type {string[]} */ (ownKeys(rec));
+      const valPs = names.map(name => deeplyFulfilled(rec[name]));
+      // @ts-expect-error not assignable to type 'DeeplyAwaited<T>'
       return E.when(Promise.all(valPs), vals =>
         harden(fromEntries(vals.map((c, i) => [names[i], c]))),
       );
     }
     case 'copyArray': {
-      const valPs = val.map(p => deeplyFulfilled(p));
+      const arr = /** @type {CopyArray} */ (val);
+      const valPs = arr.map(p => deeplyFulfilled(p));
+      // @ts-expect-error not assignable to type 'DeeplyAwaited<T>'
       return E.when(Promise.all(valPs), vals => harden(vals));
     }
     case 'tagged': {
-      const tag = getTag(val);
-      return E.when(deeplyFulfilled(val.payload), payload =>
+      const tgd = /** @type {CopyTagged} */ (val);
+      const tag = getTag(tgd);
+      // @ts-expect-error not assignable to type 'DeeplyAwaited<T>'
+      return E.when(deeplyFulfilled(tgd.payload), payload =>
         makeTagged(tag, payload),
       );
     }
     case 'remotable': {
-      return val;
+      const rem = /** @type {RemotableObject} */ (val);
+      // @ts-expect-error not assignable to type 'DeeplyAwaited<T>'
+      return rem;
     }
     case 'error': {
-      return val;
+      const err = /** @type {Error} */ (val);
+      // @ts-expect-error not assignable to type 'DeeplyAwaited<T>'
+      return err;
     }
     case 'promise': {
-      return E.when(val, nonp => deeplyFulfilled(nonp));
+      const prom = /** @type {Promise} */ (/** @type {unknown} */ (val));
+      return E.when(prom, nonp => deeplyFulfilled(nonp));
     }
     default: {
       throw assert.fail(X`Unexpected passStyle ${q(passStyle)}`, TypeError);

packages/marshal/test/deeplyFulfilled.test.js

@@ -0,0 +1,24 @@
+import test from '@endo/ses-ava/prepare-endo.js';
+
+import { Far } from '@endo/pass-style';
+import { deeplyFulfilled } from '../src/deeplyFulfilled.js';
+
+// Currently, just copied from deeplyFulfilledObject test.
+// TODO extend to test cases unique to deeplyFulfilled, i.e. primitives
+test('deeplyFulfilled', async t => {
+  const someFar = Far('somefar', { getAsync: () => Promise.resolve('async') });
+  const unfulfilled = harden({
+    obj1: {
+      obj2a: {
+        stringP: Promise.resolve('foo'),
+      },
+      obj2b: someFar,
+    },
+  });
+  const fulfilled = await deeplyFulfilled(unfulfilled);
+  // JS check that it's a now string
+  fulfilled.obj1.obj2a.stringP.length;
+  t.deepEqual(fulfilled, {
+    obj1: { obj2a: { stringP: 'foo' }, obj2b: someFar },
+  });
+});