From e980c9faa6159a499c0104bd5f38041b314bc43a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:03:21 +0000 Subject: [PATCH 1/3] Initial plan From a07fb3d5dd7ca0d40a4a28a0cc05a2e55e13d7a7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 08:30:55 +0000 Subject: [PATCH 2/3] feat(eslint): upgrade no-core-error-then-process-exitcode to forward scan + top-level autofix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add isCoreSetFailedStatement and isControlTransferStatement helpers - Rewrite checkStatements to scan forward from core.error() (like sibling rule) - Stop scan at core.setFailed() or control-transfer statements (return, throw, break, etc.) - Non-adjacent pairs report without autofix suggestion - Add module top-level autofix: enclosingFn === null → core.setFailed(args); no return; - Update Program handler to split body at export/import declarations (preserving export-barrier behavior) - Update rule docs to describe forward-scan semantics - Add new valid tests: setFailed-stops-scan, control-transfer-stops-scan - Add new invalid tests: non-adjacent pair, two-intervening-statements - Update module-top-level and switch-case invalid tests to include autofix suggestions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...o-core-error-then-process-exitcode.test.ts | 29 +++- .../no-core-error-then-process-exitcode.ts | 144 ++++++++++++++---- 2 files changed, 137 insertions(+), 36 deletions(-) diff --git a/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts b/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts index ca3eebd3238..8a10eefcf19 100644 --- a/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts +++ b/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts @@ -27,25 +27,32 @@ describe("no-core-error-then-process-exitcode", () => { `core.warning("msg"); process.exitCode = 1;`, // Variable assignment — runtime value unknown `core.error("msg"); process.exitCode = code;`, - // Exports between statements break adjacency at module scope + // Exports between statements break the scan at module scope `const helper = 1; core.error("msg"); export { helper }; process.exitCode = 1;`, // process.exit (covered by the sibling rule) `core.error("msg"); process.exit(1);`, // Not a simple assignment: += is not flagged `core.error("msg"); process.exitCode += 1;`, + // core.setFailed between error and exitCode stops scanning + `core.error("x"); core.setFailed("y"); process.exitCode = 1;`, + // return between error and exitCode stops scanning (inside a function) + `function run() { core.error("x"); return; process.exitCode = 1; }`, + // throw between error and exitCode stops scanning + `function run() { core.error("x"); throw new Error("x"); process.exitCode = 1; }`, ], invalid: [ { + // module top-level: autofix is safe — no caller continues after replacement code: `core.error("fatal"); process.exitCode = 1;`, - errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [{ messageId: "replaceWithSetFailed", output: 'core.setFailed("fatal");\n ' }] }], }, { code: `core.error("something went wrong"); process.exitCode = 1;`, - errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [{ messageId: "replaceWithSetFailed", output: 'core.setFailed("something went wrong");\n ' }] }], }, { code: "core.error(`ERROR: ${msg}`); process.exitCode = 1;", - errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [{ messageId: "replaceWithSetFailed", output: "core.setFailed(`ERROR: ${msg}`);\n " }] }], }, { // Inside a named function — no autofix suggestion because return; only exits the helper @@ -78,13 +85,23 @@ describe("no-core-error-then-process-exitcode", () => { errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], }, { - // SwitchCase path reports the pattern without an autofix outside main() + // SwitchCase at module top level: autofix is safe (enclosingFn === null) code: `switch (x) { case 1: core.error("fatal"); process.exitCode = 1; break; }`, - errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [{ messageId: "replaceWithSetFailed", output: 'switch (x) { case 1: core.setFailed("fatal");\n break; }' }] }], }, { // exitCode = 2 is also flagged code: `core.error("critical"); process.exitCode = 2;`, + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [{ messageId: "replaceWithSetFailed", output: 'core.setFailed("critical");\n ' }] }], + }, + { + // Non-adjacent pair: intervening statement does not defeat detection; no autofix suggestion + code: `core.error("x"); core.info("y"); process.exitCode = 1;`, + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + }, + { + // Two intervening statements + code: `core.error("fatal"); core.info("a"); core.info("b"); process.exitCode = 1;`, errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], }, ], diff --git a/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts b/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts index b03a7ade4a6..a402477e3b0 100644 --- a/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts +++ b/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts @@ -83,6 +83,58 @@ function isProcessExitCodeNonZero(node: TSESTree.Statement): node is TSESTree.Ex return true; } +/** + * Returns true when `node` is an expression statement containing a call to + * `core.setFailed(...)` (direct, computed, or aliased). + */ +function isCoreSetFailedStatement(node: TSESTree.Statement, sourceCode: SourceCode): boolean { + if (node.type !== AST_NODE_TYPES.ExpressionStatement) return false; + 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; + + const obj = callee.object; + const prop = callee.property; + const isSetFailedNonComputed = !callee.computed && prop.type === AST_NODE_TYPES.Identifier && prop.name === "setFailed"; + const isSetFailedComputed = callee.computed && prop.type === AST_NODE_TYPES.Literal && prop.value === "setFailed"; + if (!isSetFailedNonComputed && !isSetFailedComputed) return false; + if (obj.type !== AST_NODE_TYPES.Identifier) return false; + + return isCoreLikeIdentifier(obj.name) || isCoreAliasIdentifier(obj, sourceCode); +} + +/** + * Returns true when `node` is a control-transfer statement that definitively + * exits the current block: return, throw, break, continue, or process.exit(...). + */ +function isControlTransferStatement(node: TSESTree.Statement): boolean { + // prettier-ignore + if ( + node.type === AST_NODE_TYPES.ReturnStatement || + node.type === AST_NODE_TYPES.ThrowStatement || + node.type === AST_NODE_TYPES.BreakStatement || + node.type === AST_NODE_TYPES.ContinueStatement + ) { + return true; + } + // process.exit(...) — any call, regardless of exit code + if (node.type === AST_NODE_TYPES.ExpressionStatement && node.expression.type === AST_NODE_TYPES.CallExpression) { + const callee = node.expression.callee; + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + !callee.computed && + callee.object.type === AST_NODE_TYPES.Identifier && + callee.object.name === "process" && + callee.property.type === AST_NODE_TYPES.Identifier && + callee.property.name === "exit" + ) { + return true; + } + } + return false; +} + function hasSingleNonSpreadArgument(call: TSESTree.CallExpression): boolean { return call.arguments.length === 1 && call.arguments[0].type !== AST_NODE_TYPES.SpreadElement; } @@ -98,11 +150,13 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ hasSuggestions: true, docs: { description: - "Disallow the pattern `core.error(msg); process.exitCode = nonzero` in GitHub Actions scripts. " + + "Disallow the pattern `core.error(msg); ... ; process.exitCode = nonzero` in GitHub Actions scripts. " + "`core.error()` annotates the log but does not mark the action as failed. " + "Prefer `core.setFailed(msg)` which correctly marks the action as failed and allows post-action " + "cleanup hooks to run. Unlike `process.exit(1)`, `process.exitCode = 1` does not immediately halt " + - "execution, so subsequent code still runs in the failed state.", + "execution, so subsequent code still runs in the failed state. " + + "The rule scans forward from `core.error(...)` for a later `process.exitCode = nonzero`, " + + "stopping at `core.setFailed(...)` or a control-transfer statement.", }, schema: [], messages: { @@ -110,7 +164,7 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ "Avoid `core.error()` followed by `process.exitCode = nonzero`. Prefer `core.setFailed(msg)` to signal " + "action failure; it marks the action failed and allows post-action cleanup hooks to run. " + "Unlike `process.exit(1)`, `process.exitCode = 1` does not halt execution immediately.", - replaceWithSetFailed: "Replace `core.error(msg); process.exitCode = nonzero` with `core.setFailed(msg); return;`.", + replaceWithSetFailed: "Replace `core.error(msg); process.exitCode = nonzero` with `core.setFailed(msg)` (at module top level) or `core.setFailed(msg); return;` (inside main()).", }, }, defaultOptions: [], @@ -120,29 +174,53 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ function checkStatements(stmts: readonly TSESTree.Statement[]): void { for (let i = 0; i < stmts.length - 1; i++) { const current = stmts[i]; - const next = stmts[i + 1]; - if (isCoreErrorStatement(current, sourceCode) && isProcessExitCodeNonZero(next)) { - const enclosingFn = getImmediateEnclosingFunction(current, sourceCode); - const errorCall = current.expression as TSESTree.CallExpression; - const safeToFix = enclosingFn !== null && isFunctionNamedMain(enclosingFn) && hasSingleNonSpreadArgument(errorCall); - - context.report({ - node: current, - messageId: "noCoreErrorThenProcessExitCode", - suggest: safeToFix - ? [ - { - messageId: "replaceWithSetFailed", - fix(fixer: TSESLint.RuleFixer) { - const args = errorCall.arguments.map(a => sourceCode.getText(a)).join(", "); - const callee = errorCall.callee as TSESTree.MemberExpression; - const objectName = sourceCode.getText(callee.object); - return [fixer.replaceText(current, `${objectName}.setFailed(${args}); return;\n`), fixer.remove(next)]; + if (!isCoreErrorStatement(current, sourceCode)) continue; + + // Scan forward for process.exitCode = nonzero, stopping at setFailed or control-transfer. + // Adjacent (j === i+1) keeps autofix; non-adjacent reports without suggestion. + for (let j = i + 1; j < stmts.length; j++) { + const candidate = stmts[j]; + + if (isProcessExitCodeNonZero(candidate)) { + const isAdjacent = j === i + 1; + // The autofix suggestion is only safe when the pair is at module top level or directly + // inside a `main()` entrypoint. Inside helper functions, `return;` only exits the helper + // and lets the caller continue. For non-adjacent pairs we omit the suggestion to avoid + // a fixer that leaves intervening statements between a deleted exitCode and the new setFailed. + const enclosingFn = getImmediateEnclosingFunction(current, sourceCode); + const errorCall = current.expression as TSESTree.CallExpression; + const safeToFix = isAdjacent && (enclosingFn === null || isFunctionNamedMain(enclosingFn)) && hasSingleNonSpreadArgument(errorCall); + + context.report({ + node: current, + messageId: "noCoreErrorThenProcessExitCode", + suggest: safeToFix + ? [ + { + messageId: "replaceWithSetFailed", + fix(fixer: TSESLint.RuleFixer) { + const args = errorCall.arguments.map(a => sourceCode.getText(a)).join(", "); + const callee = errorCall.callee as TSESTree.MemberExpression; + const objectName = sourceCode.getText(callee.object); + + // At module top-level (enclosingFn === null) there is nothing to `return` from, + // so we just replace with setFailed. Inside main() we append `return;` to exit + // the entrypoint cleanly. + const replacement = enclosingFn !== null ? `${objectName}.setFailed(${args}); return;` : `${objectName}.setFailed(${args});`; + + return [fixer.replaceText(current, replacement + "\n"), fixer.remove(candidate)]; + }, }, - }, - ] - : [], - }); + ] + : [], + }); + break; + } + + // Stop scanning if setFailed already handles the failure or a control-transfer exits the block. + if (isCoreSetFailedStatement(candidate, sourceCode) || isControlTransferStatement(candidate)) { + break; + } } } } @@ -155,13 +233,19 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ checkStatements(node.consequent); }, Program(node: TSESTree.Program) { - for (let i = 0; i < node.body.length - 1; i++) { - const current = node.body[i]; - const next = node.body[i + 1]; - if (isProgramStatement(current) && isProgramStatement(next)) { - checkStatements([current, next]); + // At module top level, export declarations act as segment boundaries (they separate + // "regions" of the module). We split the program body at export/import declarations + // so that an export between core.error and process.exitCode breaks the scan. + let segment: TSESTree.Statement[] = []; + for (const stmt of node.body) { + if (isProgramStatement(stmt)) { + segment.push(stmt); + } else { + checkStatements(segment); + segment = []; } } + checkStatements(segment); }, }; }, From 069ebdb9ceace68c7d2ca6ce2570943c057b2f69 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:10:06 +0000 Subject: [PATCH 3/3] fix(eslint): deduplicate exitCode reports, handle computed process[exit], add test coverage for all barriers Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- ...o-core-error-then-process-exitcode.test.ts | 15 ++++ .../no-core-error-then-process-exitcode.ts | 87 ++++++++++--------- 2 files changed, 61 insertions(+), 41 deletions(-) diff --git a/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts b/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts index 8a10eefcf19..9dbd9353183 100644 --- a/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts +++ b/eslint-factory/src/rules/no-core-error-then-process-exitcode.test.ts @@ -39,6 +39,14 @@ describe("no-core-error-then-process-exitcode", () => { `function run() { core.error("x"); return; process.exitCode = 1; }`, // throw between error and exitCode stops scanning `function run() { core.error("x"); throw new Error("x"); process.exitCode = 1; }`, + // break between error and exitCode stops scanning (inside a loop) + `while (true) { core.error("x"); break; process.exitCode = 1; }`, + // continue between error and exitCode stops scanning (inside a loop) + `for (let i = 0; i < 10; i++) { core.error("x"); continue; process.exitCode = 1; }`, + // process.exit() between error and exitCode stops scanning (dot access) + `core.error("x"); process.exit(1); process.exitCode = 1;`, + // process["exit"]() between error and exitCode stops scanning (computed access) + `core.error("x"); process["exit"](1); process.exitCode = 1;`, ], invalid: [ { @@ -104,6 +112,13 @@ describe("no-core-error-then-process-exitcode", () => { code: `core.error("fatal"); core.info("a"); core.info("b"); process.exitCode = 1;`, errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], }, + { + // Two consecutive core.error calls before the same process.exitCode: only the first + // core.error reports (non-adjacent, no autofix) — deduplication prevents a second + // diagnostic and any conflicting autofix from the adjacent core.error("b"). + code: `core.error("a"); core.error("b"); process.exitCode = 1;`, + errors: [{ messageId: "noCoreErrorThenProcessExitCode", suggestions: [] }], + }, ], }); }); diff --git a/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts b/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts index a402477e3b0..a9d1d27773d 100644 --- a/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts +++ b/eslint-factory/src/rules/no-core-error-then-process-exitcode.ts @@ -86,6 +86,8 @@ function isProcessExitCodeNonZero(node: TSESTree.Statement): node is TSESTree.Ex /** * Returns true when `node` is an expression statement containing a call to * `core.setFailed(...)` (direct, computed, or aliased). + * Accepts `sourceCode` for alias resolution via `isCoreAliasIdentifier`; contrast + * with `isControlTransferStatement` which is a pure syntax check and needs no source-code context. */ function isCoreSetFailedStatement(node: TSESTree.Statement, sourceCode: SourceCode): boolean { if (node.type !== AST_NODE_TYPES.ExpressionStatement) return false; @@ -118,18 +120,14 @@ function isControlTransferStatement(node: TSESTree.Statement): boolean { ) { return true; } - // process.exit(...) — any call, regardless of exit code + // process.exit(...) — any call, regardless of exit code; handles both dot and computed access if (node.type === AST_NODE_TYPES.ExpressionStatement && node.expression.type === AST_NODE_TYPES.CallExpression) { const callee = node.expression.callee; - if ( - callee.type === AST_NODE_TYPES.MemberExpression && - !callee.computed && - callee.object.type === AST_NODE_TYPES.Identifier && - callee.object.name === "process" && - callee.property.type === AST_NODE_TYPES.Identifier && - callee.property.name === "exit" - ) { - return true; + if (callee.type === AST_NODE_TYPES.MemberExpression && callee.object.type === AST_NODE_TYPES.Identifier && callee.object.name === "process") { + const prop = callee.property; + const isExitDot = !callee.computed && prop.type === AST_NODE_TYPES.Identifier && prop.name === "exit"; + const isExitComputed = callee.computed && prop.type === AST_NODE_TYPES.Literal && prop.value === "exit"; + if (isExitDot || isExitComputed) return true; } } return false; @@ -172,6 +170,10 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ const sourceCode = context.sourceCode; function checkStatements(stmts: readonly TSESTree.Statement[]): void { + // Track which process.exitCode nodes have already been reported so that two consecutive + // core.error() calls before the same exitCode do not each fire their own diagnostic + // (which could produce conflicting autofixers on the same node). + const reported = new WeakSet(); for (let i = 0; i < stmts.length - 1; i++) { const current = stmts[i]; if (!isCoreErrorStatement(current, sourceCode)) continue; @@ -182,38 +184,41 @@ export const noCoreErrorThenProcessExitCodeRule = createRule({ const candidate = stmts[j]; if (isProcessExitCodeNonZero(candidate)) { - const isAdjacent = j === i + 1; - // The autofix suggestion is only safe when the pair is at module top level or directly - // inside a `main()` entrypoint. Inside helper functions, `return;` only exits the helper - // and lets the caller continue. For non-adjacent pairs we omit the suggestion to avoid - // a fixer that leaves intervening statements between a deleted exitCode and the new setFailed. - const enclosingFn = getImmediateEnclosingFunction(current, sourceCode); - const errorCall = current.expression as TSESTree.CallExpression; - const safeToFix = isAdjacent && (enclosingFn === null || isFunctionNamedMain(enclosingFn)) && hasSingleNonSpreadArgument(errorCall); - - context.report({ - node: current, - messageId: "noCoreErrorThenProcessExitCode", - suggest: safeToFix - ? [ - { - messageId: "replaceWithSetFailed", - fix(fixer: TSESLint.RuleFixer) { - const args = errorCall.arguments.map(a => sourceCode.getText(a)).join(", "); - const callee = errorCall.callee as TSESTree.MemberExpression; - const objectName = sourceCode.getText(callee.object); - - // At module top-level (enclosingFn === null) there is nothing to `return` from, - // so we just replace with setFailed. Inside main() we append `return;` to exit - // the entrypoint cleanly. - const replacement = enclosingFn !== null ? `${objectName}.setFailed(${args}); return;` : `${objectName}.setFailed(${args});`; - - return [fixer.replaceText(current, replacement + "\n"), fixer.remove(candidate)]; + if (!reported.has(candidate)) { + reported.add(candidate); + const isAdjacent = j === i + 1; + // The autofix suggestion is only safe when the pair is at module top level or directly + // inside a `main()` entrypoint. Inside helper functions, `return;` only exits the helper + // and lets the caller continue. For non-adjacent pairs we omit the suggestion to avoid + // a fixer that leaves intervening statements between a deleted exitCode and the new setFailed. + const enclosingFn = getImmediateEnclosingFunction(current, sourceCode); + const errorCall = current.expression as TSESTree.CallExpression; + const safeToFix = isAdjacent && (enclosingFn === null || isFunctionNamedMain(enclosingFn)) && hasSingleNonSpreadArgument(errorCall); + + context.report({ + node: current, + messageId: "noCoreErrorThenProcessExitCode", + suggest: safeToFix + ? [ + { + messageId: "replaceWithSetFailed", + fix(fixer: TSESLint.RuleFixer) { + const args = errorCall.arguments.map(a => sourceCode.getText(a)).join(", "); + const callee = errorCall.callee as TSESTree.MemberExpression; + const objectName = sourceCode.getText(callee.object); + + // At module top-level (enclosingFn === null) there is nothing to `return` from, + // so we just replace with setFailed. Inside main() we append `return;` to exit + // the entrypoint cleanly. + const replacement = enclosingFn !== null ? `${objectName}.setFailed(${args}); return;` : `${objectName}.setFailed(${args});`; + + return [fixer.replaceText(current, replacement + "\n"), fixer.remove(candidate)]; + }, }, - }, - ] - : [], - }); + ] + : [], + }); + } break; }