Skip to content

eslint-factory: prefer-actions-exec-over-child-process retainsCallResult is too broad, masking real exec()/execFile() migration #55945

Description

@github-actions

Summary

prefer-actions-exec-over-child-process exempts exec()/execFile() calls (the HANDLE_RETURNING_METHODS) whenever the call result is "retained", per retainsCallResult:

function retainsCallResult(node: TSESTree.CallExpression): boolean {
  return node.parent != null && node.parent.type !== AST_NODE_TYPES.ExpressionStatement;
}

The intent (per the rule's own doc comment) is to exempt calls whose returned ChildProcess handle is kept for stdin/stdout streaming or lifecycle management — a capability @actions/exec doesn't have. But the check only distinguishes "bare expression statement" from "everything else"; it does not check how the value is used. Any parent other than ExpressionStatement is treated as retention, including parents that discard or immediately consume the value without ever touching a ChildProcess handle:

  • await exec("git", args, cb) — parent is AwaitExpression. child_process.exec/execFile return a ChildProcess, not a thenable, so await here is a no-op left over from copy-pasting an @actions/exec call pattern (await exec(cmd, args)) onto a still-unmigrated child_process import — exactly the kind of confusion this rule exists to catch, but it is silently exempted.
  • void exec("git", args, cb); — parent is UnaryExpression, exempted even though void explicitly discards the result.
  • exec("git", args, cb) || fallback(); / if (exec(...)) {} / x, exec(...)LogicalExpression/ConditionalExpression/SequenceExpression parents are exempted even though none of them provide stdin/stdout streaming or lifecycle control; a ChildProcess object is always truthy so these are almost certainly bugs, not deliberate handle retention.

Genuine handle retention only happens via VariableDeclarator/AssignmentExpression (assigned to a binding for later use), ReturnStatement/implicit arrow return (handed to the caller), or being immediately member-accessed/chained (exec(...).stdout.pipe(...)) or passed as a call argument. retainsCallResult should check for one of those specific shapes rather than "anything but ExpressionStatement".

Grounding

No live occurrence of await exec(/void exec( etc. was found in the current actions/setup/js corpus (checked via grep for await\s+exec(Sync|File|FileSync)?\( across all .cjs files — zero matches), so this is a latent soundness gap rather than an active false negative today. It is cheap to fix and worth closing before it masks a real migration candidate.

Ask

Narrow retainsCallResult to only recognize genuine retention shapes (VariableDeclarator, AssignmentExpression RHS, ReturnStatement/ArrowFunctionExpression implicit return, MemberExpression object, CallExpression argument position), and add regression tests:

  • invalid: await exec("git", args, cb); as a standalone statement → still flagged
  • invalid: void exec("git", args, cb); → still flagged
  • invalid: exec("git", args, cb) || onError(); → still flagged
  • valid (unchanged): const child = exec(...); child.stdin.end(); and exec(...).stdout.pipe(...) → still exempt

Scope

eslint-factory/src/rules/prefer-actions-exec-over-child-process.ts (+ its .test.ts).

Generated by 🤖 ESLint Refiner · claude · agent · 188.2 AIC · ⌖ 7.66 AIC · ⊞ 5.8K ·

  • expires on Sep 1, 2026, 9:31 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions