test(gap): lock commander's outputError/writeErr indirection — #10711 does not reproduce

changelog.d/10728-property-fn-param-callback-lock.md

@@ -0,0 +1,32 @@
+### Testing
+
+- Lock commander's `_displayError` indirection in the gap suite: an object-property
+  function (`outputError(str, write)`) invoking a second object-property function
+  handed to it as a parameter (`writeErr`). #10711 reports that Perry silently drops
+  that inner call and loses commander's error text; it does not reproduce. The
+  reporter's own isolated repro matches Node 26.5.1 on current `main` (v0.5.1598), on
+  the `main` commit their branch forks from (`8df83f8c`), and on their actual tree
+  (PR #10712 over #10699) — each a full `-p perry -p perry-runtime-static
+  -p perry-stdlib-static` build with `PERRY_RUNTIME_DIR` pinned, so no arm could have
+  linked a stale archive. Real commander 14.0.3 compiled from source through
+  `perry.compilePackages` is byte-identical to Node across the whole surface the issue
+  names — `--help`, `--version`, missing required argument, unknown option, unknown
+  command and `program.error()` — under the default output configuration and under a
+  `configureOutput()` override, as are 32 further spellings of the same indirection
+  (method shorthand, class field, spread, nested receiver, cross-object writer,
+  computed key, getter, `Object.create` chain, `Object.freeze`, destructuring, three
+  levels, async caller, nested closure, loop, and the shape inside a CommonJS module).
+
+  The fixture is therefore a regression lock, not a fix, and it passes on unfixed
+  `main`. The shape still earns a gate: #10689 — an inherited property read folding to
+  the constant `undefined` on a scalar-replaced object — landed one commit before
+  #10711 was filed, is the same family, and was silent in the same way, and nothing in
+  `test-files/` covered this indirection.
+
+  Two cases keep it from passing vacuously. One traces `before` / `typeof write` /
+  `after` around the inner call so that "the outer body ran and the inner call
+  evaporated" cannot read as a pass. The other omits the writer and asserts a
+  `TypeError`, because a missing callee being loud is the property that keeps this bug
+  class from presenting as a plausible wrong answer rather than a crash. Every writer
+  sinks to stdout: the parity harness merges stdout and stderr into one compared
+  stream, so a fixture using both would race on the interleaving.

test-files/test_gap_10711_property_fn_param_callback.ts

@@ -0,0 +1,127 @@
+// #10711: a function read from an object property must not drop its own call
+// to a second function handed to it as a parameter (also read from an object
+// property).
+//
+// This is commander's `_displayError` shape. `lib/command.js` builds a default
+// output configuration holding two function properties —
+//
+//     writeErr:    (str) => process.stderr.write(str),
+//     outputError: (str, write) => write(str),
+//
+// — and every error path calls
+// `this._outputConfiguration.outputError(msg, this._outputConfiguration.writeErr)`:
+// an object-property function invoking a SECOND object-property function that
+// was passed to it as a parameter. If the inner `write(str)` evaporates, the
+// program still runs and still throws the right CommanderError — it just
+// prints nothing. A silently dropped call is the worst failure mode there is,
+// so this fixture asserts the inner call RAN, not merely that something got
+// printed.
+//
+// Every writer sinks to stdout on purpose. The parity harness merges stdout
+// and stderr into one compared stream, so a fixture that used both would race
+// on the interleaving; the stream is incidental to the indirection under test.
+
+function out(s: string): void {
+  process.stdout.write(s);
+}
+
+// ── 1. The reported shape, verbatim: object-literal arrow properties, both
+//       reached through one level of plain-function call. ──────────────────
+const config: any = {
+  writeErr: (str: string) => out(str),
+  outputError: (str: string, write: (s: string) => void) => write(str),
+};
+
+function fireError(cfg: any, message: string) {
+  cfg.outputError(message, cfg.writeErr);
+}
+
+fireError(config, "1 verbatim: error: something went wrong\n");
+
+// ── 2. The inner call is observably entered and left. Printing "before" and
+//       "after" around it is what separates "the call ran" from "the outer
+//       body ran and the inner call vanished" — the two look identical when
+//       only the payload is checked. ──────────────────────────────────────
+const traced: any = {
+  writeErr: (str: string) => out("   inner: " + str),
+  outputError: (str: string, write: (s: string) => void) => {
+    out("2 before, typeof write=" + typeof write + "\n");
+    write(str);
+    out("2 after\n");
+  },
+};
+fireError(traced, "payload\n");
+
+// ── 3. Method-shorthand spelling of the same object. ────────────────────────
+const shorthand: any = {
+  writeErr(str: string) {
+    out(str);
+  },
+  outputError(str: string, write: (s: string) => void) {
+    write(str);
+  },
+};
+fireError(shorthand, "3 shorthand: error: something went wrong\n");
+
+// ── 4. commander's real home for it: a class field holding the config, the
+//       receiver reached as `this.<field>` inside a method. ────────────────
+class Reporter {
+  _outputConfiguration: any = {
+    writeOut: (str: string) => out(str),
+    writeErr: (str: string) => out(str),
+    outputError: (str: string, write: (s: string) => void) => write(str),
+    getOutHelpWidth: () => 80,
+  };
+  configureOutput(cfg: any): Reporter {
+    Object.assign(this._outputConfiguration, cfg);
+    return this;
+  }
+  error(message: string): void {
+    this._outputConfiguration.outputError(
+      `${message}\n`,
+      this._outputConfiguration.writeErr,
+    );
+  }
+}
+
+const reporter = new Reporter();
+reporter.error("4 class field: error: something went wrong");
+
+// ── 5. `configureOutput` replaces the writer after construction — the call
+//       must reach the REPLACEMENT, not a value baked in at literal-creation
+//       time. ────────────────────────────────────────────────────────────
+reporter.configureOutput({ writeErr: (str: string) => out("[override]" + str) });
+reporter.error("5 after configureOutput");
+
+// ── 6. Spread-built config, nested receiver, and a writer taken from a
+//       DIFFERENT object than the one holding `outputError`. ──────────────
+const defaults: any = {
+  writeErr: (str: string) => out(str),
+  outputError: (str: string, write: (s: string) => void) => write(str),
+};
+const nested: any = { io: { ...defaults } };
+nested.io.outputError("6 nested spread: ok\n", nested.io.writeErr);
+
+const sink: any = { writeErr: (str: string) => out("[other]" + str) };
+nested.io.outputError("6 cross-object writer\n", sink.writeErr);
+
+// ── 7. Repeated dispatch: the shape must survive a loop, where the call site
+//       is re-entered and any per-site caching gets a second look. ─────────
+for (let i = 0; i < 3; i++) {
+  fireError(config, "7 loop " + i + "\n");
+}
+
+// ── 8. And when the writer really is missing, the call must be LOUD. A
+//       TypeError here is the property that keeps every future instance of
+//       this bug class from presenting as a plausible wrong answer. (Only the
+//       error's name is printed: Node names the callee — "write is not a
+//       function" — where Perry says "value is not a function".) ───────────
+const noWriter: any = {
+  outputError: (str: string, write: (s: string) => void) => write(str),
+};
+try {
+  fireError(noWriter, "never printed\n");
+  out("8 MISSING WRITER SILENTLY DROPPED THE CALL\n");
+} catch (e: any) {
+  out("8 threw " + e.name + "\n");
+}