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..9b7a6a22a20 --- /dev/null +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.test.ts @@ -0,0 +1,101 @@ +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; } }`, + `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"); } }`, + ], + 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"); 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"); 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 }], + }, + ], + }); + }); + + 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..89a8bd91568 --- /dev/null +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.ts @@ -0,0 +1,141 @@ +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 { + 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(...) + 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.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, 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: { + 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 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) { + 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: 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, + }); + } + + 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); + }, + Program(node: TSESTree.Program) { + checkStatementList(node.body.filter(isExecutableStatement), report); + }, + }; + }, +});