From 0fcba1de63c859ac617b9828ae269674271289ce Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 10 Jul 2026 10:02:52 +0000 Subject: [PATCH 1/2] eslint: add require-return-after-core-setfailed rule core.setFailed() marks the action as failed but does NOT stop execution. Calling it inside an if-branch without a following return/throw lets the action continue in a failed state, which can mask secondary errors or produce confusing output. This rule flags any core.setFailed() call that is immediately followed by a non-control-transfer statement in the same block. It provides a 'return;' autofix suggestion. Findings in actions/setup/js (2 sites), confirming the pattern exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- eslint-factory/eslint.config.cjs | 1 + eslint-factory/src/index.ts | 2 + ...equire-return-after-core-setfailed.test.ts | 72 ++++++++++++ .../require-return-after-core-setfailed.ts | 110 ++++++++++++++++++ 4 files changed, 185 insertions(+) create mode 100644 eslint-factory/src/rules/require-return-after-core-setfailed.test.ts create mode 100644 eslint-factory/src/rules/require-return-after-core-setfailed.ts diff --git a/eslint-factory/eslint.config.cjs b/eslint-factory/eslint.config.cjs index dbc9db60ce9..37bb5436a62 100644 --- a/eslint-factory/eslint.config.cjs +++ b/eslint-factory/eslint.config.cjs @@ -25,6 +25,7 @@ module.exports = [ "gh-aw-custom/require-fs-sync-try-catch": "warn", "gh-aw-custom/require-json-parse-try-catch": "warn", "gh-aw-custom/require-parseInt-radix": "warn", + "gh-aw-custom/require-return-after-core-setfailed": "warn", }, }, { diff --git a/eslint-factory/src/index.ts b/eslint-factory/src/index.ts index 3f1c1405733..1159fcd2268 100644 --- a/eslint-factory/src/index.ts +++ b/eslint-factory/src/index.ts @@ -11,6 +11,7 @@ import { requireFsSyncTryCatchRule } from "./rules/require-fs-sync-try-catch"; import { requireJsonParseTryCatchRule } from "./rules/require-json-parse-try-catch"; import { requireErrorCauseInRethrowRule } from "./rules/require-error-cause-in-rethrow"; import { requireParseIntRadixRule } from "./rules/require-parseInt-radix"; +import { requireReturnAfterCoreSetFailedRule } from "./rules/require-return-after-core-setfailed"; const plugin = { meta: { @@ -31,6 +32,7 @@ const plugin = { "require-fs-sync-try-catch": requireFsSyncTryCatchRule, "require-json-parse-try-catch": requireJsonParseTryCatchRule, "require-parseInt-radix": requireParseIntRadixRule, + "require-return-after-core-setfailed": requireReturnAfterCoreSetFailedRule, }, }; diff --git a/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts b/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts new file mode 100644 index 00000000000..c239236121a --- /dev/null +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts @@ -0,0 +1,72 @@ +import { RuleTester } from "eslint"; +import { describe, expect, it } from "vitest"; +import { requireReturnAfterCoreSetFailedRule } from "./require-return-after-core-setfailed"; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: "commonjs", + }, +}); + +describe("require-return-after-core-setfailed", () => { + it("uses the correct docs URL", () => { + expect(requireReturnAfterCoreSetFailedRule.meta.docs.url).toBe( + "https://github.com/github/gh-aw/tree/main/eslint-factory#require-return-after-core-setfailed", + ); + }); + + it("valid: core.setFailed followed by return", () => { + ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { + valid: [ + `function f() { core.setFailed("bad"); return; }`, + `function f() { core.setFailed("bad"); return null; }`, + `function f() { if (x) { core.setFailed("bad"); return; } }`, + `function f() { core.setFailed("bad"); throw new Error("bad"); }`, + `function f() { core.setFailed("bad"); process.exit(1); }`, + `function f() { for (;;) { core.setFailed("bad"); break; } }`, + // setFailed is the last statement in the block — no next statement to check + `function f() { core.setFailed("bad"); }`, + `function f() { if (x) { core.setFailed("bad"); } }`, + ], + invalid: [], + }); + }); + + it("valid: non-core.setFailed calls are ignored", () => { + ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { + valid: [ + `function f() { other.setFailed("bad"); doMore(); }`, + `function f() { core.setOutput("x", 1); doMore(); }`, + `function f() { setFailed("bad"); doMore(); }`, + ], + invalid: [], + }); + }); + + it("invalid: core.setFailed followed by non-control-transfer statement", () => { + ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { + valid: [], + invalid: [ + { + code: `function f() { core.setFailed("bad"); doMore(); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { core.setFailed("bad");\n return; doMore(); }` }] }], + }, + { + code: `function f() { if (x) { core.setFailed("bad"); doMore(); } }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { if (x) { core.setFailed("bad");\n return; doMore(); } }` }] }], + }, + ], + }); + }); + + it("valid: core.setFailed last in if-block is not flagged (outer block continues)", () => { + ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { + valid: [ + // setFailed is the last statement in the if-block; no sibling in the same block follows it + `function f() { if (!ok) { core.setFailed("msg"); } doMore(); }`, + ], + invalid: [], + }); + }); +}); diff --git a/eslint-factory/src/rules/require-return-after-core-setfailed.ts b/eslint-factory/src/rules/require-return-after-core-setfailed.ts new file mode 100644 index 00000000000..a585cd9be19 --- /dev/null +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.ts @@ -0,0 +1,110 @@ +import { AST_NODE_TYPES, ESLintUtils, TSESTree } from "@typescript-eslint/utils"; + +const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh-aw/tree/main/eslint-factory#${name}`); + +/** + * Returns true when the statement is a call to `core.setFailed(...)`. + */ +function isCoreSetFailedStatement(node: TSESTree.Statement): node is TSESTree.ExpressionStatement { + 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 || callee.computed) return false; + const obj = callee.object; + const prop = callee.property; + return obj.type === AST_NODE_TYPES.Identifier && obj.name === "core" && prop.type === AST_NODE_TYPES.Identifier && prop.name === "setFailed"; +} + +/** + * Returns true when the statement unconditionally transfers control out of the + * current block: `return`, `throw`, `break`, `continue`, or `process.exit(...)`. + */ +function isControlTransfer(node: TSESTree.Statement): boolean { + 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(...) + 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; +} + +/** + * Checks a list of sequential statements for `core.setFailed(...)` calls that + * are not immediately followed by a control-transfer statement. + */ +function checkStatementList(stmts: TSESTree.Statement[], report: (node: TSESTree.Node) => void): void { + for (let i = 0; i < stmts.length; i++) { + const stmt = stmts[i]; + if (!isCoreSetFailedStatement(stmt)) continue; + const next = stmts[i + 1]; + if (next && !isControlTransfer(next)) { + report(stmt); + } + } +} + +export const requireReturnAfterCoreSetFailedRule = createRule({ + name: "require-return-after-core-setfailed", + meta: { + type: "problem", + hasSuggestions: true, + docs: { + description: + "Require a return, throw, break, continue, or process.exit() statement immediately after core.setFailed() to prevent execution from continuing after a failure is declared. " + + "core.setFailed() only marks the action as failed at the end; it does not stop execution.", + }, + schema: [], + messages: { + missingReturnAfterSetFailed: + "core.setFailed() does not stop execution — add a 'return' (or 'throw') immediately after to prevent the action from continuing in a failed state.", + addReturn: "Add 'return;' after core.setFailed() to stop execution.", + }, + }, + defaultOptions: [], + create(context) { + function report(node: TSESTree.Node): void { + context.report({ + node, + messageId: "missingReturnAfterSetFailed", + suggest: [ + { + messageId: "addReturn", + fix(fixer) { + return fixer.insertTextAfter(node, "\n return;"); + }, + }, + ], + }); + } + + return { + // Check statement blocks: if body, else body, while body, function body, etc. + BlockStatement(node: TSESTree.BlockStatement) { + checkStatementList(node.body, report); + }, + // Handle single-statement arrow functions (no braces) — rare but safe to skip + // The main case is BlockStatement above. + SwitchCase(node: TSESTree.SwitchCase) { + checkStatementList(node.consequent, report); + }, + }; + }, +}); From 86a8b1ccf46abca22e9d04c057b4c3693e43b27d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Jul 2026 13:51:47 +0000 Subject: [PATCH 2/2] fix(eslint-factory): harden require-return-after-core-setfailed autofix and coverage Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...equire-return-after-core-setfailed.test.ts | 49 +++++++++++--- .../require-return-after-core-setfailed.ts | 67 ++++++++++++++----- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts b/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts index c239236121a..9b7a6a22a20 100644 --- a/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts @@ -11,9 +11,7 @@ const ruleTester = new RuleTester({ describe("require-return-after-core-setfailed", () => { it("uses the correct docs URL", () => { - expect(requireReturnAfterCoreSetFailedRule.meta.docs.url).toBe( - "https://github.com/github/gh-aw/tree/main/eslint-factory#require-return-after-core-setfailed", - ); + expect(requireReturnAfterCoreSetFailedRule.meta.docs.url).toBe("https://github.com/github/gh-aw/tree/main/eslint-factory#require-return-after-core-setfailed"); }); it("valid: core.setFailed followed by return", () => { @@ -25,6 +23,7 @@ describe("require-return-after-core-setfailed", () => { `function f() { core.setFailed("bad"); throw new Error("bad"); }`, `function f() { core.setFailed("bad"); process.exit(1); }`, `function f() { for (;;) { core.setFailed("bad"); break; } }`, + `switch (x) { case "a": core.setFailed("bad"); break; }`, // setFailed is the last statement in the block — no next statement to check `function f() { core.setFailed("bad"); }`, `function f() { if (x) { core.setFailed("bad"); } }`, @@ -35,11 +34,7 @@ describe("require-return-after-core-setfailed", () => { it("valid: non-core.setFailed calls are ignored", () => { ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { - valid: [ - `function f() { other.setFailed("bad"); doMore(); }`, - `function f() { core.setOutput("x", 1); doMore(); }`, - `function f() { setFailed("bad"); doMore(); }`, - ], + valid: [`function f() { other.setFailed("bad"); doMore(); }`, `function f() { core.setOutput("x", 1); doMore(); }`, `function f() { setFailed("bad"); doMore(); }`], invalid: [], }); }); @@ -50,11 +45,45 @@ describe("require-return-after-core-setfailed", () => { invalid: [ { code: `function f() { core.setFailed("bad"); doMore(); }`, - errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { core.setFailed("bad");\n return; doMore(); }` }] }], + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { core.setFailed("bad"); return; doMore(); }` }] }], }, { code: `function f() { if (x) { core.setFailed("bad"); doMore(); } }`, - errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { if (x) { core.setFailed("bad");\n return; doMore(); } }` }] }], + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { if (x) { core.setFailed("bad"); return; doMore(); } }` }] }], + }, + { + code: `function f() { + if (x) { + core.setFailed("bad"); // keep with setFailed + doMore(); + } +}`, + errors: [ + { + messageId: "missingReturnAfterSetFailed", + suggestions: [ + { + messageId: "addReturn", + output: `function f() { + if (x) { + core.setFailed("bad"); // keep with setFailed + return; + doMore(); + } +}`, + }, + ], + }, + ], + }, + { + code: `switch (x) { case "a": core.setFailed("bad"); doMore(); break; }`, + errors: [{ messageId: "missingReturnAfterSetFailed" }], + }, + { + code: `core.setFailed("bad"); +doMore();`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: undefined }], }, ], }); diff --git a/eslint-factory/src/rules/require-return-after-core-setfailed.ts b/eslint-factory/src/rules/require-return-after-core-setfailed.ts index a585cd9be19..89a8bd91568 100644 --- a/eslint-factory/src/rules/require-return-after-core-setfailed.ts +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.ts @@ -21,12 +21,8 @@ function isCoreSetFailedStatement(node: TSESTree.Statement): node is TSESTree.Ex * current block: `return`, `throw`, `break`, `continue`, or `process.exit(...)`. */ function isControlTransfer(node: TSESTree.Statement): boolean { - 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 - ) { + const matchesControlTransferType = node.type === AST_NODE_TYPES.ReturnStatement || node.type === AST_NODE_TYPES.ThrowStatement || node.type === AST_NODE_TYPES.BreakStatement || node.type === AST_NODE_TYPES.ContinueStatement; + if (matchesControlTransferType) { return true; } // process.exit(...) @@ -50,17 +46,27 @@ function isControlTransfer(node: TSESTree.Statement): boolean { * Checks a list of sequential statements for `core.setFailed(...)` calls that * are not immediately followed by a control-transfer statement. */ -function checkStatementList(stmts: TSESTree.Statement[], report: (node: TSESTree.Node) => void): void { +function checkStatementList(stmts: TSESTree.Statement[], report: (node: TSESTree.Statement, next: TSESTree.Statement) => void): void { for (let i = 0; i < stmts.length; i++) { const stmt = stmts[i]; if (!isCoreSetFailedStatement(stmt)) continue; const next = stmts[i + 1]; if (next && !isControlTransfer(next)) { - report(stmt); + report(stmt, next); } } } +function isExecutableStatement(node: TSESTree.ProgramStatement): node is TSESTree.Statement { + return ( + node.type !== AST_NODE_TYPES.ImportDeclaration && + node.type !== AST_NODE_TYPES.ExportAllDeclaration && + node.type !== AST_NODE_TYPES.ExportDefaultDeclaration && + node.type !== AST_NODE_TYPES.ExportNamedDeclaration && + node.type !== AST_NODE_TYPES.TSModuleDeclaration + ); +} + export const requireReturnAfterCoreSetFailedRule = createRule({ name: "require-return-after-core-setfailed", meta: { @@ -74,24 +80,46 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ schema: [], messages: { missingReturnAfterSetFailed: - "core.setFailed() does not stop execution — add a 'return' (or 'throw') immediately after to prevent the action from continuing in a failed state.", + "core.setFailed() does not stop execution — add a control-transfer statement (for example: return, throw, break, continue, or process.exit(...)) immediately after to prevent the action from continuing in a failed state.", addReturn: "Add 'return;' after core.setFailed() to stop execution.", }, }, defaultOptions: [], create(context) { - function report(node: TSESTree.Node): void { + const sourceCode = context.sourceCode; + + function isInsideFunctionLike(node: TSESTree.Node): boolean { + const ancestors = sourceCode.getAncestors(node); + for (let i = ancestors.length - 1; i >= 0; i--) { + const ancestor = ancestors[i]; + const isFunctionLike = ancestor.type === AST_NODE_TYPES.FunctionDeclaration || ancestor.type === AST_NODE_TYPES.FunctionExpression || ancestor.type === AST_NODE_TYPES.ArrowFunctionExpression; + if (isFunctionLike) { + return true; + } + } + return false; + } + + function report(node: TSESTree.Statement, next: TSESTree.Statement): void { context.report({ node, messageId: "missingReturnAfterSetFailed", - suggest: [ - { - messageId: "addReturn", - fix(fixer) { - return fixer.insertTextAfter(node, "\n return;"); - }, - }, - ], + suggest: isInsideFunctionLike(node) + ? [ + { + messageId: "addReturn", + fix(fixer) { + const isOnSameLine = next.loc.start.line === node.loc.end.line; + if (isOnSameLine) { + return fixer.insertTextBefore(next, "return; "); + } + const line = sourceCode.lines[next.loc.start.line - 1] ?? ""; + const indent = /^(\s*)/.exec(line)?.[1] ?? ""; + return fixer.insertTextBefore(next, `return;\n${indent}`); + }, + }, + ] + : undefined, }); } @@ -105,6 +133,9 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ SwitchCase(node: TSESTree.SwitchCase) { checkStatementList(node.consequent, report); }, + Program(node: TSESTree.Program) { + checkStatementList(node.body.filter(isExecutableStatement), report); + }, }; }, });