require-error-code-in-thrown-error: nested identifier aliases inside binary/template messages bypass write-once resolution (fals
- Dominant language
- Go
- Stars
- 5.1k
- Forks
- 541
- Avg merge
- 5h 48m
- Merged PRs (30d)
- 772
Description
### Rule: `require-error-code-in-thrown-error`
`messageReferencesErrorCode` (eslint-factory/src/rules/require-error-code-in-thrown-error.ts:13-32) recursively inspects `BinaryExpression` operands and `TemplateLiteral` expression slots by checking each `Identifier`'s **name** directly against `ERROR_CODE_PATTERN`. It never routes those nested identifiers through `resolveWriteOnceInitializerChain` (eslint-factory/src/rules/command-initializer-utils.ts), unlike the top-level `Identifier` branch of `auditMessageExpression` (require-error-code-in-thrown-error.ts:85-90), which does.
The result: a write-once local alias for an `ERR_*`/`SAFE_OUTPUT_E0xx` constant is only recognized when it aliases the **entire** message expression. The moment that alias is used as *part of* a binary `+` concatenation or a template-literal interpolation, resolution silently stops at the identifier's bare name and the rule reports a false `missingErrorCode`.
### Repro (minimal, would need to be added as a test case)
```js
const { ERR_API } = require("./error_codes.cjs");
function f() {
const prefix = ERR_API;
throw new Error(prefix + ": failed"); // flagged as missingErrorCode — false positive
}
```
```js
const { ERR_API } = require("./error_codes.cjs");
function f() {
const prefix = ERR_API;
throw new Error(`${prefix}: failed`); // same bug, template-literal form — false positive
}
```
```js
const { ERR_API } = require("./error_codes.cjs");
function helper() {
const prefix = ERR_API;
return prefix + ": failed";
}
function f() {
throw new Error(helper()); // same bug via resolveSimpleLocalCallReturns — false positive
}
```
Trace for the first snippet: `messageArg` is a top-level `BinaryExpression`, so `isAuditableMessageExpression` is true and `auditMessageExpression` calls `messageReferencesErrorCode(node)` first. That function recurses into `left` (`Identifier "prefix"`) and `right` (string literal), testing `"prefix"` against the pattern directly — it never resolves `prefix`'s initializer. Both operands fail the regex, so `messageReferencesErrorCode` returns `false`. Back in `auditMessageExpression`, the node is neither `CallExpression` nor `Identifier` (it's the `BinaryExpression` itself), so execution falls through to the final `return "missingCode"` — even though `prefix` provably resolves to `ERR_API`.
### Why this matters
- All ~128 files that import `error_codes.cjs` are in this rule's blast radius. "Assign the code to a locally-scoped `const` before concatenating" is an ordinary refactor a developer would reach for (e.g. to shorten a long template literal, or to share a prefix across two `throw` sites) — nothing else in the codebase's conventions discourages it.
- Exhaustive search across `actions/setup/js/*.cjs` for `throw new Error( + ...)` and `= fetch(`-style aliasing of error codes found **zero live occurrences** of this exact shape today, so this is a latent/structural gap rather than a currently-misfiring live diagnostic — but it is fully reproducible from the rule's own source and will misfire the instant the idiom appears.
### Suggested fix
In `messageReferencesErrorCode`, when recursing into a `BinaryExpression` operand or a `TemplateLiteral` expression that is itself an `Identifier`, resolve it via `resolveWriteOnceInitializerChain(expr, sourceCode)` before testing the name (mirroring what the top-level `Identifier` branch of `auditMessageExpression` already does). Note `messageReferencesErrorCode` doesn't currently receive `sourceCode`/`context`, so this requires threading it through (or moving the resolution into a wrapper in `auditMessageExpression` that pre-resolves `BinaryExpression`/`TemplateLiteral` operands before delegating to the pattern check).
### Acceptance criteria
- [ ] The three repro snippets above are added as `valid` cases in `require-error-code-in-thrown-error.test.ts` and pass.
- [ ] A case where the aliased operand does **not** resolve to a code (e.g. `const prefix = "no code"; throw new Error(prefix + ": failed");`) remains correctly flagged as `missingErrorCode`, to confirm the fix doesn't just suppress the whole branch.
> [!WARNING]
>
> Firewall blocked 1 domain
>
> The following domain was blocked by the firewall during workflow execution:
>
> - `api.anthropic.com`
>
> To allow these domains, add them to the `network.allowed` list in your workflow frontmatter:
>
> ```yaml
> network:
> allowed:
> - defaults
> - "api.anthropic.com"
> ```
>
> See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information.
>
>
> Generated by [🤖 ESLint Refiner](https://github.com/github/gh-aw/actions/runs/35059244673) · claude · agent · 318.3 AIC · ⌖ 7.46 AIC · ⊞ 5.8K · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw+is%3Aissue+%22gh-aw-workflow-call-id%3A+github%2Fgh-aw%2Feslint-refiner%22&type=issues)
> - [x] expires on Sep 22, 2026, 9:36 PM UTC-08:00
Contributor guide
Research direction
Start in eslint-factory/src/rules/require-error-code-in-thrown-error.ts, especially messageReferencesErrorCode and auditMessageExpression, then read resolveWriteOnceInitializerChain in eslint-factory/src/rules/command-initializer-utils.ts. Run the cases in require-error-code-in-thrown-error.test.ts; done means the three aliased forms pass as valid while a non-code alias remains reported as missingErrorCode.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- testing-qa, tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100