docs(eslint-factory): document 10 missing custom rules in README - #50208
Merged
Conversation
4 tasks
Contributor
Triage: docs / low risk
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Update README.md to document missing ESLint rules
docs(eslint-factory): document 10 missing custom rules in README
Aug 4, 2026
Contributor
|
Hey 🤖 — thanks for this documentation fix! The PR documents 10 previously undiscovered ESLint rules in
|
Contributor
There was a problem hiding this comment.
Pull request overview
Documents 10 previously undiscoverable custom ESLint rules, though several descriptions do not match implementation behavior.
Changes:
- Adds rule summary entries and detailed documentation.
- Changes capitalization in an unrelated generated workflow lock file.
Show a summary per file
| File | Description |
|---|---|
eslint-factory/README.md |
Documents the 10 missing rules. |
.github/workflows/smoke-goose.lock.yml |
Changes an unrelated generated step name. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suppressed comments (4)
eslint-factory/README.md:52
- This rule scans forward and reports a later assignment until a barrier; it is not limited to immediately adjacent statements. Update the summary so users know the actual detection scope.
| [`no-core-error-then-process-exitcode`](#no-core-error-then-process-exitcode) | Disallow `core.error()` immediately followed by `process.exitCode = nonzero` |
eslint-factory/README.md:626
- The implementation reports non-adjacent assignments too, but only offers a suggestion for adjacent pairs at module scope or directly inside
main(). The current text incorrectly describes both detection and suggestion as unconditional.
Disallow `core.error()` immediately followed by `process.exitCode = nonzero`.
Prefer `core.setFailed(msg)` to signal action failure; it marks the action failed and allows post-action cleanup hooks to run. Unlike `process.exit(1)`, `process.exitCode = 1` does not halt execution immediately.
The rule provides an autofix suggestion that replaces `core.error(msg); process.exitCode = nonzero;` with `core.setFailed(msg);` (at module top level) or `core.setFailed(msg); return;` (inside `main()`).
eslint-factory/README.md:658
- The implementation recognizes only the two ternary forms below; it never visits a standalone logical-OR expression. Remove the
err.stack || String(err)claim, and qualify the suggestion because it is emitted only whengetErrorMessagealready resolves in scope.
Disallow the `err.stack || String(err)` (and equivalent) fallback pattern for formatting caught errors.
Prefer `getErrorMessage(err)` from `error_helpers.cjs`. The `err.stack` ternary/logical-OR pattern surfaces noisy stack frames; `getErrorMessage()` returns a clean, consistent message.
eslint-factory/README.md:685
- The implementation also matches a
setFailedmessage formed by adding only a prefix to thecore.errormessage, and it withholds the removal suggestion when the shared expression may have side effects. Document both conditions so the section matches actual diagnostics.
Disallow a redundant `core.error()` call immediately before `core.setFailed()` with the same message.
`core.error()` immediately before `core.setFailed()` with the same message is redundant: `core.setFailed()` already logs an error annotation and marks the action failed.
The rule provides an autofix suggestion that removes the redundant `core.error()` call.
- Files reviewed: 2/2 changed files
- Comments generated: 8
- Review effort level: Balanced
| find "/tmp/gh-aw/" -type f -print | ||
| echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/agent_output.json" >> "$GITHUB_OUTPUT" | ||
| - name: Download safe outputs items manifest | ||
| - name: Download Safe Outputs Items Manifest |
| | [`require-spawnsync-error-check`](#require-spawnsync-error-check) | Require checking `result.error` after `spawnSync` calls | | ||
| | [`prefer-get-error-message-over-string`](#prefer-get-error-message-over-string) | Prefer `getErrorMessage(err)` over `String(err)` when interpolating a caught error | | ||
| | [`require-rmsync-try-catch`](#require-rmsync-try-catch) | Require try/catch around `fs.rmSync` calls | | ||
| | [`no-core-error-then-process-exit`](#no-core-error-then-process-exit) | Disallow `core.error()` immediately followed by `process.exit(nonzero)` | |
Comment on lines
+614
to
+618
| Disallow `core.error()` immediately followed by `process.exit(nonzero)`. | ||
|
|
||
| Prefer `core.setFailed(msg)` to signal action failure; it marks the action failed and allows post-action cleanup hooks to run. In standalone `node` scripts, `process.exit(nonzero)` does fail the step, but `core.setFailed` is more portable. | ||
|
|
||
| The rule provides an autofix suggestion that replaces `core.error(msg); process.exit(...);` with `core.setFailed(msg); return;`. |
| | [`no-core-error-then-process-exitcode`](#no-core-error-then-process-exitcode) | Disallow `core.error()` immediately followed by `process.exitCode = nonzero` | | ||
| | [`no-exec-interpolated-command`](#no-exec-interpolated-command) | Disallow interpolated command strings passed to `@actions/exec` | | ||
| | [`no-setfailed-then-exit-zero`](#no-setfailed-then-exit-zero) | Disallow resetting the exit code to success after `core.setFailed()` | | ||
| | [`no-err-stack-then-string-fallback`](#no-err-stack-then-string-fallback) | Disallow the `err.stack \|\| String(err)` fallback pattern | |
Comment on lines
+606
to
+610
| Require `fs.rmSync` calls in `actions/setup/js` scripts to be wrapped in `try/catch`. | ||
|
|
||
| `rmSync` throws synchronously on permission errors, invalid paths, or unexpected filesystem state; an unhandled throw crashes the action without surfacing a useful diagnostic. | ||
|
|
||
| **Not flagged:** Calls already inside an enclosing `try { ... } catch { ... }` block. |
| | [`no-setfailed-then-exit-zero`](#no-setfailed-then-exit-zero) | Disallow resetting the exit code to success after `core.setFailed()` | | ||
| | [`no-err-stack-then-string-fallback`](#no-err-stack-then-string-fallback) | Disallow the `err.stack \|\| String(err)` fallback pattern | | ||
| | [`no-caught-error-interpolation`](#no-caught-error-interpolation) | Disallow directly interpolating a caught error in a template literal | | ||
| | [`no-core-error-then-setfailed`](#no-core-error-then-setfailed) | Disallow a redundant `core.error()` call immediately before `core.setFailed()` with the same message | |
| - `core.setFailed(msg); process.exit(0);` | ||
| - `core.setFailed(msg); process.exitCode = 0;` | ||
|
|
||
| The rule provides an autofix suggestion: replace `process.exit(0)` with `return;`, or remove the `process.exitCode = 0;` assignment. |
|
|
||
| Disallow directly interpolating a caught error variable in a template literal (for example `` `Failed: ${err}` ``). | ||
|
|
||
| Directly interpolating a caught error is unsafe — for `Error` objects it produces `"Error: message"` (a redundant prefix); for non-`Error` throws it produces `"[object Object]"`. Use `${getErrorMessage(err)}` if it is available, or `${String(err)}` as an import-free alternative. |
6 tasks
Contributor
|
🎉 This pull request is included in a new release. Release: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
eslint-factory/src/index.tsregisters 38 custom ESLint rules, buteslint-factory/README.mdonly documented 28 of them, leaving 10 rules undiscoverable from the docs (and theirdocs.descriptionURLs pointing to nonexistent README anchors).Changes
## Rulessummary table.### \rule-name`` section for each of the 10 rules, following the existing doc format (purpose, detected forms, not-flagged cases, autofix behavior where applicable):prefer-get-error-message-over-stringrequire-rmsync-try-catchno-core-error-then-process-exitno-core-error-then-process-exitcodeno-exec-interpolated-commandno-setfailed-then-exit-zerono-err-stack-then-string-fallbackno-caught-error-interpolationno-core-error-then-setfailedrequire-escaped-regexp-interpolationsrc/index.tsnow have a matching table entry and section inREADME.md.No code/logic changes — documentation only.