feat(eslint): add no-setfailed-then-exit-zero rule - #47232
Conversation
Adds a new ESLint rule that detects `core.setFailed()` followed by `process.exit(0)` (or `process.exit()` with no args), which silently resets the exit code to success and hides the failure from GitHub Actions. - New rule: `no-setfailed-then-exit-zero` - Detects adjacent and non-adjacent `core.setFailed()` + `process.exit(0)` in BlockStatement, SwitchCase, and Program statement lists - Also catches `process.exit()` (no args, defaults to 0) - Provides an autofixable suggestion to replace `process.exit(0)` with `return;` for adjacent pairs - Registered in the plugin index Closes #47228 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
no-setfailed-then-exit-zero rule
There was a problem hiding this comment.
Pull request overview
Adds an ESLint rule to detect failure codes overridden by process.exit(0).
Changes:
- Implements and registers the rule.
- Adds detection and suggestion tests.
Show a summary per file
| File | Description |
|---|---|
no-setfailed-then-exit-zero.ts |
Implements detection and suggestions. |
no-setfailed-then-exit-zero.test.ts |
Tests supported patterns. |
index.ts |
Registers the rule. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
eslint-factory/src/rules/no-setfailed-then-exit-zero.ts:55
process.exit()does not reset a previously assignedprocess.exitCode: when no argument is supplied, Node uses the existingprocess.exitCodeand only defaults to 0 when it is unset. Sincecore.setFailed()synchronously sets that value to 1, this valid failure path is currently reported and offered an unnecessary rewrite. Restrict this predicate to an explicit zero argument (and move the no-argument test tovalid), unless the rule also proves that intervening code resetprocess.exitCode.
// process.exit() with no arguments defaults to exit code 0
if (expr.arguments.length === 0) return true;
eslint-factory/src/rules/no-setfailed-then-exit-zero.ts:157
- Checking each body/consequent in isolation misses sequential control flow across block boundaries and switch fall-through. For example,
if (bad) { core.setFailed("x"); } process.exit(0);is not reported; the existingrequire-return-after-core-setfailedtraversal treats the outerprocess.exitas the next valid control transfer, so this exact footgun passes both rules. Reuse equivalent ancestor-continuation and switch-fall-through traversal fromrequire-return-after-core-setfailed.ts:81-151.
return {
BlockStatement(node: TSESTree.BlockStatement) {
checkStatements(node.body);
},
SwitchCase(node: TSESTree.SwitchCase) {
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Medium
| const expr = node.expression; | ||
| if (expr.type !== AST_NODE_TYPES.CallExpression) return false; | ||
| const callee = expr.callee; | ||
| if (callee.type !== AST_NODE_TYPES.MemberExpression) return false; |
| "no-core-error-then-process-exitcode": noCoreErrorThenProcessExitCodeRule, | ||
| "no-exec-interpolated-command": noExecInterpolatedCommandRule, | ||
| "require-execsync-try-catch": requireExecSyncTryCatchRule, | ||
| "no-setfailed-then-exit-zero": noSetFailedThenExitZeroRule, |
|
@copilot please run the This PR has review coverage already; please work through the open feedback, then rerun checks after the branch update. Run: https://github.com/github/gh-aw/actions/runs/29906399216 Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "pi.dev"See Network Configuration for more information.
|
….config.cjs
- Add isDestructuredCoreMethodIdentifier support to no-setfailed-then-exit-zero rule
so patterns like `const { setFailed } = core; setFailed("bad"); process.exit(0);`
are correctly flagged
- Enable the rule in eslint-factory/eslint.config.cjs so repository lint runs
execute and verify the rule
- Add test cases for destructured setFailed (direct and renamed binding)
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Both review threads addressed in commit
Test cases for both destructured forms (direct and renamed binding) were also added. All local validation passes (fmt, lint, test-unit). |
|
🎉 This pull request is included in a new release. Release: |
core.setFailed(msg)only schedules a non-zero exit code — it does not terminate the process. Callingprocess.exit(0)afterward overrides that exit code to success, causing the Actions step to appear green despite the declared failure.New rule:
no-setfailed-then-exit-zeroDetects
core.setFailed(...)followed byprocess.exit(0)orprocess.exit()(no-arg form also exits 0) in any sequential statement list.core.info()) before theprocess.exit(0)— adjacent pairs get a suggestion fix, non-adjacent pairs are reported without an auto-fixcore["setFailed"]) and aliased (const c = core; c.setFailed(...)) formsreturn,throw,break,process.exit(nonzero), etc.)BlockStatement,SwitchCase, and top-levelProgramnodesno-setfailed-then-exit-zeroRun: https://github.com/github/gh-aw/actions/runs/29906399216
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
pi.devSee Network Configuration for more information.