New method BaseValueVisitor._prot_visitSync(). (#431)

As the title says.

CHANGELOG.md

@@ -19,9 +19,14 @@ Breaking changes:
     results when passed `forLogging === true` (see above).
   * Reworked the `toString()` options of `Moment`.
 * `valvis`:
-  * Renamed some methods in `BaseValueVisitor`, for clarity and consistency.
-  * New method `BaseValueVisitor.visitWrap()`, which has the same "synchronous
-    if possible but async if not" behavior that is used internally.
+  * `BaseValueVisitor`:
+    * Renamed some methods, for clarity and consistency.
+    * New method `visitWrap()`, which has the same "synchronous if possible but
+      async if not" behavior that is used internally.
+    * Reworked (the renamed) `_prot_visitWrap()` to have more consistent
+      behavior.
+    * New method `_prot_visitSync()` to parallel the analogous public method
+      `visitSync()`.
   * Reworked `VisitDef` and `VisitRef` to not assume an associated visitor
     instance. This makes them usable in more situations.
 * `loggy-intf` / `loggy`:

src/loggy-intf/private/HumanVisitor.js

@@ -90,7 +90,7 @@ export class HumanVisitor extends BaseValueVisitor {
         result.push(
           this.#maybeStyle(' = ', style),
           ComboText.INDENT,
-          this._prot_visitWrap(node.value).value);
+          this._prot_visitSync(node.value));
       }
       return new ComboText(...result);
     } else if (node instanceof Sexp) {
@@ -300,7 +300,7 @@ export class HumanVisitor extends BaseValueVisitor {
     const parts = [open, ComboText.INDENT];
 
     for (let at = 0; at < args.length; at++) {
-      const arg    = this._prot_visitWrap(args[at]).value;
+      const arg    = this._prot_visitSync(args[at]);
       const isLast = (at === (args.length - 1));
       if (at !== 0) {
         parts.push(ComboText.SPACE);

src/valvis/export/BaseValueVisitor.js

@@ -612,6 +612,20 @@ export class BaseValueVisitor {
     }
   }
 
+  /**
+   * Like {@link #_prot_visitWrap}, except only ever synchronously visits the
+   * given value, throwing an error if that turned out not to be possible.
+   *
+   * @param {*} node Value to visit.
+   * @returns {*} The visit result.
+   * @throws {Error} Thrown if there was trouble with the visit which could be
+   *   determined synchronously, _or_ if the visit could not be completed
+   *   synchronously.
+   */
+  _prot_visitSync(node) {
+    return this.#visitNode(node).extractSync(false);
+  }
+
   /**
    * Visits the given value as a "sub-visit" of the main visit. This can be used
    * in concrete subclasses, for example, to visit the various pieces of

src/valvis/tests/BaseValueVisitor.test.js

@@ -1132,17 +1132,35 @@ ${'_prot_nameFromValue'}  | ${'expectedName'}
   });
 });
 
-describe('_prot_visitWrap()', () => {
+// Tests for the `_prot_visit*()` methods.
+describe.each`
+methodName           | isAsync  | wraps
+${'_prot_visitSync'} | ${false} | ${false}
+${'_prot_visitWrap'} | ${true}  | ${true}
+`('$methodName()', ({ methodName, isAsync, wraps }) => {
+  function checkResult(got, expected) {
+    if (wraps) {
+      expect(got).toBeInstanceOf(VisitResult);
+      got = got.value;
+    }
+
+    expect(got).toBe(expected);
+  }
+
+  async function checkResultPromise(got, expected) {
+    expect(got).toBeInstanceOf(Promise);
+    checkResult(await got, expected);
+  }
+
   test('works synchronously when possible', () => {
     class VisitCheckVisitor extends BaseValueVisitor {
       _impl_visitNumber(node) {
         return `${node}!`;
       }
 
       _impl_visitString(node_unused) {
-        const got = this._prot_visitWrap(9999);
-        expect(got).toBeInstanceOf(VisitResult);
-        expect(got.value).toBe('9999!');
+        const got = this[methodName](9999);
+        checkResult(got, '9999!');
         return 'yep';
       }
     }
@@ -1151,24 +1169,40 @@ describe('_prot_visitWrap()', () => {
     expect(got).toBe('yep');
   });
 
-  test('works asynchronously when necessary', async () => {
-    class VisitCheckVisitor extends BaseValueVisitor {
-      async _impl_visitNumber(node) {
-        return `${node}!`;
+  if (isAsync) {
+    test('works asynchronously when necessary', async () => {
+      class VisitCheckVisitor extends BaseValueVisitor {
+        async _impl_visitNumber(node) {
+          return `${node}!`;
+        }
+
+        async _impl_visitString(node_unused) {
+          const got = this[methodName](98765);
+          await checkResultPromise(got, '98765!');
+          return 'yep';
+        }
       }
 
-      async _impl_visitString(node_unused) {
-        const got = this._prot_visitWrap(98765);
-        expect(got).toBeInstanceOf(Promise);
-        expect(await got).toBeInstanceOf(VisitResult);
-        expect((await got).value).toBe('98765!');
-        return 'yep';
+      const got = await new VisitCheckVisitor('boop').visitAsyncWrap();
+      expect((await got).value).toBe('yep');
+    });
+  } else {
+    test('throws an error when not able to complete synchronously', async () => {
+      class VisitCheckVisitor extends BaseValueVisitor {
+        async _impl_visitNumber(node) {
+          return `${node}!`;
+        }
+
+        async _impl_visitString(node_unused) {
+          expect(() => this[methodName](112233)).toThrow(/Visit did not finish/);
+          return 'yep';
+        }
       }
-    }
 
-    const got = await new VisitCheckVisitor('boop').visitAsyncWrap();
-    expect((await got).value).toBe('yep');
-  });
+      const got = await new VisitCheckVisitor('boop').visitAsyncWrap();
+      expect((await got).value).toBe('yep');
+    });
+  }
 });
 
 describe('_prot_visitProperties()', () => {