From cc412959662882f0fd4497636d2a5e2428c4f661 Mon Sep 17 00:00:00 2001
From: Mathieu Hofman <mathieu@agoric.com>
Date: Tue, 23 Aug 2022 22:14:28 +0000
Subject: [PATCH] refactor(marshal): split promise own prop check

---
 packages/marshal/src/helpers/safe-promise.js | 42 ++++++++++----------
 packages/marshal/test/test-passStyleOf.js    |  4 +-
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/packages/marshal/src/helpers/safe-promise.js b/packages/marshal/src/helpers/safe-promise.js
index fe0e03ba73..4c40544835 100644
--- a/packages/marshal/src/helpers/safe-promise.js
+++ b/packages/marshal/src/helpers/safe-promise.js
@@ -11,6 +11,20 @@ const { details: X, quote: q } = assert;
 const { isFrozen, getPrototypeOf } = Object;
 const { ownKeys } = Reflect;
 
+/**
+ * @param {Promise} pr The value to examine
+ * @param {Checker} [check]
+ * @returns {pr is Promise} Whether it is a safe promise
+ */
+const checkPromiseOwnKeys = (pr, check = x => x) => {
+  const keys = ownKeys(pr);
+
+  return check(
+    keys.length === 0,
+    X`${pr} - Must not have any own properties: ${q(keys)}`,
+  );
+};
+
 /**
  * Under Hardened JS a promise is "safe" if its `then` method can be called
  * synchronously without giving the promise an opportunity for a
@@ -27,26 +41,14 @@ const { ownKeys } = Reflect;
  * @param {Checker} [check]
  * @returns {pr is Promise} Whether it is a safe promise
  */
-const checkSafePromise = (pr, check = x => x) => {
-  let keys;
-  return (
-    check(isFrozen(pr), X`${pr} - Must be frozen`) &&
-    check(isPromise(pr), X`${pr} - Must be a promise`) &&
-    check(
-      getPrototypeOf(pr) === Promise.prototype,
-      X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
-    ) &&
-    check(
-      // Suppressing prettier for the following line because it wants to
-      // remove the "extra" parens around `pr`. However, these parens are
-      // required for the TypeScript case syntax. We know this case is safe
-      // because we only get here if `ifPromise(pr)` already passed.
-      // eslint-disable-next-line prettier/prettier
-      (keys = ownKeys(/** @type {Promise} pr */(pr))).length === 0,
-      X`{pr} - Must not have any own properties: ${q(keys)}`,
-    )
-  );
-};
+const checkSafePromise = (pr, check = x => x) =>
+  check(isFrozen(pr), X`${pr} - Must be frozen`) &&
+  check(isPromise(pr), X`${pr} - Must be a promise`) &&
+  check(
+    getPrototypeOf(pr) === Promise.prototype,
+    X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
+  ) &&
+  checkPromiseOwnKeys(/** @type {Promise} */ (pr), check);
 harden(checkSafePromise);
 
 /**
diff --git a/packages/marshal/test/test-passStyleOf.js b/packages/marshal/test/test-passStyleOf.js
index 6efbb8576a..3a580b0ed3 100644
--- a/packages/marshal/test/test-passStyleOf.js
+++ b/packages/marshal/test/test-passStyleOf.js
@@ -45,14 +45,14 @@ test('some passStyleOf rejections', t => {
   prbad2.extra = 'unexpected own property';
   harden(prbad2);
   t.throws(() => passStyleOf(prbad2), {
-    message: /{pr} - Must not have any own properties: \["extra"\]/,
+    message: /\[Promise\]" - Must not have any own properties: \["extra"\]/,
   });
 
   const prbad3 = Promise.resolve();
   Object.defineProperty(prbad3, 'then', { value: () => 'bad then' });
   harden(prbad3);
   t.throws(() => passStyleOf(prbad3), {
-    message: /{pr} - Must not have any own properties: \["then"\]/,
+    message: /\[Promise\]" - Must not have any own properties: \["then"\]/,
   });
 
   const thenable1 = harden({ then: () => 'thenable' });