Skip to content

require-spawn-error-listener: "has error listener" check is whole-scope, not path-sensitive — conditionally-attached listener si [Content truncated due to length] #51958

Description

@github-actions

Rule

require-spawn-error-listener (eslint-factory/src/rules/require-spawn-error-listener.ts) — second finding from first review since it shipped 2026-08-09 (companion to the DI-fallback issue filed alongside this one).

The gap

Once a const child = spawn(...) binding is resolved, the rule considers the requirement satisfied if any reference to child anywhere in scope matches child.on("error", ...) / child.once("error", ...):

const hasErrorListener = variable.references.some(ref => { ... isErrorListenerCall(grandparent, varName) ... });

There is no check that the matching reference is unconditionally reached relative to the spawn() call — a listener attached only inside an if/else branch, a catch, or after a conditional early return still satisfies hasErrorListener, even though other execution paths never register it:

const child = spawn(cmd, args);
if (verbose) {
  child.on("error", err => { core.info(String(err)); });
}
// on the non-verbose path this spawn() result still has zero 'error' coverage

This is the same whole-block/whole-callback branch-order false-negative shape that has already been found and fixed in sibling rules in this same factory: no-unsafe-promise-catch-error-property (#42915, "guard is whole-callback, causing branch-order false negatives"), no-core-error-then-process-exit (#46993, "only checks adjacent statements — misses intervening statements"), and require-return-after-core-setfailed (#46992). It's a recurring defect class in this factory whenever a rule asks "does X exist anywhere in scope" instead of "does X unconditionally happen on this path."

Grounding

No live occurrence in actions/setup/js today — all 3 production spawn() call sites either attach the listener unconditionally at the top level of the enclosing function or don't attach one at all (a plain miss, already correctly flagged). Filing proactively, not reactively: the fix is small and localized, and every sibling rule in this defect class was found only after real code exercised the gap, at which point the fix landed reactively. Given the pattern has recurred three times already in this factory, it's cheaper to close it now while the rule is one day old and has zero downstream autofix/suggestion consumers to break.

Ask

One of:

  • (a) Tighten hasErrorListener to require the matching .on/.once("error", ...) call to be unconditionally reachable from the declaration — at minimum, reject matches nested inside IfStatement/SwitchCase/loop bodies that don't also contain the spawn() call itself, while continuing to accept the already-tested valid case of a listener registered inside a later same-scope callback (setTimeout(() => { child.on("error", ...) })).
  • (b) If precise reachability analysis is judged out of scope for a rule this size, explicitly document the limitation in the rule's docs.description "Scope:" paragraph (matching the existing style that already documents the assignment-expression and inline-chain limits), so it reads as an intentional trade-off rather than an unreviewed gap.

Acceptance criteria

  • Decision recorded: either (a) branch-conditional listeners no longer fully satisfy the rule, or (b) the docs comment is updated to state this as a known scope limit.
  • If (a): new failing test for const child = spawn(...); if (x) { child.on("error", ...) } with no unconditional listener, expecting missingErrorListener.
  • Existing valid test ("treats nested callback listeners on the same child variable as valid", the setTimeout case) continues to pass unchanged.

Generated by 🤖 ESLint Refiner · agent · 262.1 AIC · ⌖ 28.7 AIC · ⊞ 4.7K ·

  • expires on Aug 17, 2026, 9:48 PM UTC-08:00

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions