Skip to content

eslint-factory: no-core-error-then-setfailed's strict text-equality check never fires on the codebase's dominant core.error/setF [Content truncated due to length] #49491

Description

@github-actions

Rule

no-core-error-then-setfailed (eslint-factory/src/rules/no-core-error-then-setfailed.ts).

Problem

The rule only reports a redundant core.error(msg); core.setFailed(msg) pair when the two message arguments are exact source-text matches (sourceCode.getText(errorArg) !== sourceCode.getText(setFailedArg) at line 152). In practice, every live adjacent core.error(...); core.setFailed(...) pair in the corpus uses core.setFailed with the same interpolated error message prefixed by an error code, which the strict-equality check treats as "carries extra context" and skips — even though it's the same redundant-logging anti-pattern the rule exists to catch (both calls emit near-identical error annotations, one of them purely as noise).

Grounded evidence (9 live sites, all currently unflagged)

  • actions/setup/js/add_reaction.cjs:184-185:
    core.error(`Failed to add reaction: ${errorMessage}`);
    core.setFailed(`${ERR_API}: Failed to add reaction: ${errorMessage}`);
  • actions/setup/js/unlock-issue.cjs:62-63
  • actions/setup/js/check_permissions.cjs:29-30
  • actions/setup/js/start_mcp_gateway.cjs:407-408, 488-489, 855-856

In each case setFailedArg's source text is ${errorArg's text} with a literal error-code prefix template-spliced in front — not textually identical, so the rule's equivalence check always rejects the pair. The result is that the rule has near-zero practical recall against this codebase's actual style: the anti-pattern it targets is pervasive, but the detection logic never triggers on any real instance.

Ask

Relax the equivalence check to also recognize the "same message, prefixed" shape: when both arguments are template literals (or one is a template literal and the other a simple concatenation) and the setFailedArg's text is provably a strict superset of errorArg's text with only a literal prefix inserted (no reordering, no dropped expressions), treat them as redundant for reporting purposes (still gate the auto-remove suggestion on isSideEffectFree, unchanged). Keep the exact-match case working as today; do not widen to messages that add suffix/interleaved context, since that documented case ("extra context not duplicated by setFailed") should remain unflagged.

Acceptance criteria

  • New invalid-case test mirroring add_reaction.cjs:184-185 shape (core.error(msg) then core.setFailed(prefix + msg) via template literal) — now flagged.
  • Existing valid-case test(s) for genuinely different/suffixed messages remain unflagged.
  • Re-verify add_reaction.cjs:184-185 and at least one of the start_mcp_gateway.cjs sites are flagged after the fix (grounding check).

Generated by 🤖 ESLint Refiner · agent · 669.3 AIC · ⌖ 7.12 AIC · ⊞ 4.9K ·

  • expires on Aug 7, 2026, 10:28 PM UTC-08:00

Metadata

Metadata

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions