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 67bbd7ef093..7628defafd8 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 @@ -26,6 +26,7 @@ describe("require-return-after-core-setfailed", () => { `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"); }`, `function f() { if (x) { core.setFailed("bad"); } }`, // setFailed has a return inside the if-block; outer doMore() is not reached via setFailed path `function f() { if (x) { core.setFailed("bad"); return; } doMore(); }`, @@ -188,6 +189,34 @@ doMore();`, }); }); + it("invalid: braceless control bodies with continuation", () => { + ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { + valid: [], + invalid: [ + { + code: `function f() { if (bad) core.setFailed("x"); doMore(); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { if (bad) { core.setFailed("x"); return; } doMore(); }` }] }], + }, + { + code: `function f() { if (bad) return; else core["setFailed"]("x"); doMore(); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { if (bad) return; else { core["setFailed"]("x"); return; } doMore(); }` }] }], + }, + { + code: `const c = core; function f() { if (bad) c.setFailed("x"); doMore(); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `const c = core; function f() { if (bad) { c.setFailed("x"); return; } doMore(); }` }] }], + }, + { + code: `const { setFailed } = core; function f() { for (const file of files) setFailed(file); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `const { setFailed } = core; function f() { for (const file of files) { setFailed(file); return; } }` }] }], + }, + { + code: `function f() { for (const file of files) if (!ok(file)) core.setFailed(file); }`, + errors: [{ messageId: "missingReturnAfterSetFailed", suggestions: [{ messageId: "addReturn", output: `function f() { for (const file of files) if (!ok(file)) { core.setFailed(file); return; } }` }] }], + }, + ], + }); + }); + it("valid: continue after setFailed is accepted — known limitation: does not stop post-loop execution", () => { ruleTester.run("require-return-after-core-setfailed", requireReturnAfterCoreSetFailedRule, { valid: [ 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 0d02a8a1c24..cf861f4242c 100644 --- a/eslint-factory/src/rules/require-return-after-core-setfailed.ts +++ b/eslint-factory/src/rules/require-return-after-core-setfailed.ts @@ -71,6 +71,12 @@ function isCallExpressionStatement(node: TSESTree.Statement): boolean { return node.type === AST_NODE_TYPES.ExpressionStatement && node.expression.type === AST_NODE_TYPES.CallExpression; } +function isLoopStatement(node: TSESTree.Node): node is TSESTree.WhileStatement | TSESTree.DoWhileStatement | TSESTree.ForStatement | TSESTree.ForInStatement | TSESTree.ForOfStatement { + return ( + node.type === AST_NODE_TYPES.WhileStatement || node.type === AST_NODE_TYPES.DoWhileStatement || node.type === AST_NODE_TYPES.ForStatement || node.type === AST_NODE_TYPES.ForInStatement || node.type === AST_NODE_TYPES.ForOfStatement + ); +} + /** * When `core.setFailed()` is the last statement in a nested block, searches * ancestor statement lists for the first subsequent statement that would execute @@ -294,6 +300,17 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ // Used for cross-block fall-through: inserts return; after the setFailed node // inside its block rather than before a statement in an outer block. + function getDirectControlBodyContainer(node: TSESTree.Statement): TSESTree.IfStatement | TSESTree.WhileStatement | TSESTree.DoWhileStatement | TSESTree.ForStatement | TSESTree.ForInStatement | TSESTree.ForOfStatement | null { + const parent = node.parent; + if (parent?.type === AST_NODE_TYPES.IfStatement && (parent.consequent === node || parent.alternate === node)) { + return parent; + } + if (parent !== null && isLoopStatement(parent) && parent.body === node) { + return parent; + } + return null; + } + function reportNested(node: TSESTree.Statement): void { context.report({ node, @@ -303,6 +320,11 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ { messageId: "addReturn", fix(fixer) { + const directControlBodyContainer = getDirectControlBodyContainer(node); + if (directControlBodyContainer !== null) { + return fixer.replaceText(node, `{ ${sourceCode.getText(node)} return; }`); + } + const nextTokenOrComment = sourceCode.getTokenAfter(node, { includeComments: true }); const hasTrailingCommentOnSameLine = nextTokenOrComment !== null && (nextTokenOrComment.type === AST_TOKEN_TYPES.Line || nextTokenOrComment.type === AST_TOKEN_TYPES.Block) && nextTokenOrComment.loc.start.line === node.loc.end.line; @@ -327,7 +349,26 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ }); } + function checkNestedContinuation(stmt: TSESTree.Statement): void { + const ancestors = sourceCode.getAncestors(stmt); + const continuation = findContinuationOutsideBlock(stmt, ancestors); + if (continuation !== null && !isControlTransfer(continuation)) { + reportNested(stmt); + } + } + + function checkDirectControlBody(stmt: TSESTree.Statement | null): void { + if (stmt !== null && stmt.type !== AST_NODE_TYPES.BlockStatement && isCoreSetFailedStatement(stmt)) { + checkNestedContinuation(stmt); + } + } + return { + IfStatement(node: TSESTree.IfStatement) { + for (const branch of [node.consequent, node.alternate]) { + checkDirectControlBody(branch); + } + }, // Check statement blocks: if body, else body, while body, function body, etc. BlockStatement(node: TSESTree.BlockStatement) { checkStatementList(node.body, isCoreSetFailedStatement, report); @@ -337,18 +378,29 @@ export const requireReturnAfterCoreSetFailedRule = createRule({ // Example (invalid): if (x) { core.setFailed(...); } doMore(); const lastStmt = node.body[node.body.length - 1]; if (lastStmt && isCoreSetFailedStatement(lastStmt)) { - const ancestors = sourceCode.getAncestors(lastStmt); - const continuation = findContinuationOutsideBlock(lastStmt, ancestors); - if (continuation !== null && !isControlTransfer(continuation)) { - reportNested(lastStmt); - } + checkNestedContinuation(lastStmt); } }, + DoWhileStatement(node: TSESTree.DoWhileStatement) { + checkDirectControlBody(node.body); + }, + ForInStatement(node: TSESTree.ForInStatement) { + checkDirectControlBody(node.body); + }, + ForOfStatement(node: TSESTree.ForOfStatement) { + checkDirectControlBody(node.body); + }, + ForStatement(node: TSESTree.ForStatement) { + checkDirectControlBody(node.body); + }, // 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, isCoreSetFailedStatement, report); }, + WhileStatement(node: TSESTree.WhileStatement) { + checkDirectControlBody(node.body); + }, Program(node: TSESTree.Program) { checkStatementList(node.body.filter(isExecutableStatement), isCoreSetFailedStatement, report); },