Rule
require-return-after-core-setfailed (eslint-factory/src/rules/require-return-after-core-setfailed.ts) — first review since it shipped (15th rule; not yet in the README).
Summary
The control-flow analysis in this rule is strong and well-tested (same-block, cross-block continuation through if/else/else if, loop back-edges, switch fall-through). But detection of the core.setFailed(...) call itself is purely syntactic: isCoreSetFailedStatement only matches a MemberExpression whose object is the Identifier named exactly core and whose property is setFailed (non-computed). Any indirection escapes the rule entirely — the setFailed is not recognized, so a missing control-transfer after it is a false negative.
Escaping forms:
const c = core; c.setFailed(msg); doMore(); // aliased object
const { setFailed } = core; setFailed(msg); doMore(); // destructured
core["setFailed"](msg); doMore(); // computed access
const core2 = require("`@actions/core`"); ... // renamed importEach of these silently passes even though execution continues after failure is declared.
Grounding
Ungrounded / latent — a grep of non-test actions/setup/js/**/*.cjs shows every production core.setFailed(...) uses the literal core.setFailed form and is already followed by return or is block/function-terminal (the corpus is disciplined; the rule currently produces zero live violations). The { setFailed } / = core matches in the tree are all test mocks. This is therefore a regression-guard / coverage refinement, not a live bug — but it mirrors the alias-blindspot false negatives previously filed and fixed for sibling rules (e.g. require-await-core-summary-write #43404, require-error-cause-in-rethrow #43948), so it is worth closing before an aliased usage lands.
Proposed refinement
Resolve core aliases and destructured setFailed bindings via scope analysis (as the sibling rules now do), covering at least:
const c = core; c.setFailed(...) — single-hop const alias of core.const { setFailed } = core; setFailed(...) — destructured binding.core["setFailed"](...) — computed member with a string-literal property.
Exclude locally-shadowed bindings (a local function setFailed() / parameter), consistent with the shadow handling in prefer-number-isnan.
Acceptance criteria
- Aliased (
const c = core; c.setFailed(...)) and destructured (const { setFailed } = core; setFailed(...)) calls not followed by a control transfer are reported, with the same cross-block/continuation semantics as the direct form.
- Computed
core["setFailed"](...) is recognized.
- Locally-shadowed
setFailed bindings are not flagged (tests).
- The existing suggestion (
addReturn) behaves identically for the alias/destructure forms.
Filed by the ESLint Refiner daily review (newest-rule coverage; ungrounded/latent).
Generated by 🤖 ESLint Refiner · 354.5 AIC · ⌖ 12.7 AIC · ⊞ 4.6K · ◷
Rule
require-return-after-core-setfailed(eslint-factory/src/rules/require-return-after-core-setfailed.ts) — first review since it shipped (15th rule; not yet in the README).Summary
The control-flow analysis in this rule is strong and well-tested (same-block, cross-block continuation through
if/else/else if, loop back-edges, switch fall-through). But detection of thecore.setFailed(...)call itself is purely syntactic:isCoreSetFailedStatementonly matches aMemberExpressionwhoseobjectis theIdentifiernamed exactlycoreand whosepropertyissetFailed(non-computed). Any indirection escapes the rule entirely — the setFailed is not recognized, so a missing control-transfer after it is a false negative.Escaping forms:
Each of these silently passes even though execution continues after failure is declared.
Grounding
Ungrounded / latent — a grep of non-test
actions/setup/js/**/*.cjsshows every productioncore.setFailed(...)uses the literalcore.setFailedform and is already followed byreturnor is block/function-terminal (the corpus is disciplined; the rule currently produces zero live violations). The{ setFailed }/= corematches in the tree are all test mocks. This is therefore a regression-guard / coverage refinement, not a live bug — but it mirrors the alias-blindspot false negatives previously filed and fixed for sibling rules (e.g.require-await-core-summary-write#43404,require-error-cause-in-rethrow#43948), so it is worth closing before an aliased usage lands.Proposed refinement
Resolve
corealiases and destructuredsetFailedbindings via scope analysis (as the sibling rules now do), covering at least:const c = core; c.setFailed(...)— single-hop const alias ofcore.const { setFailed } = core; setFailed(...)— destructured binding.core["setFailed"](...)— computed member with a string-literal property.Exclude locally-shadowed bindings (a local
function setFailed()/ parameter), consistent with the shadow handling inprefer-number-isnan.Acceptance criteria
const c = core; c.setFailed(...)) and destructured (const { setFailed } = core; setFailed(...)) calls not followed by a control transfer are reported, with the same cross-block/continuation semantics as the direct form.core["setFailed"](...)is recognized.setFailedbindings are not flagged (tests).addReturn) behaves identically for the alias/destructure forms.Filed by the ESLint Refiner daily review (newest-rule coverage; ungrounded/latent).