Detect thrown custom Error subclasses in error-code lint rule - #51343
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Hey ✅ Rule enhancement — extends ✅ Test coverage — includes test cases for direct subclasses, nested chains, and subclasses declared after throw sites. ✅ Violations resolved — updates ✅ Clear description — PR body documents the rule behavior, coverage, and reasoning. The diff is focused, well-scoped to the issue (#51311), and ready for review. 🚀
|
PR Triage
Extends error-code lint rule to custom Error subclasses. Isolated eslint-factory change, no CI results yet.
|
There was a problem hiding this comment.
Pull request overview
Extends error-code linting to detect local Error subclasses and updates existing violations.
Changes:
- Detects direct and transitive custom
Errorsubclasses. - Adds subclass rule coverage.
- Prefixes signed-commit errors with standardized codes.
Show a summary per file
| File | Description |
|---|---|
eslint-factory/src/rules/require-error-code-in-thrown-error.ts |
Adds custom subclass detection. |
eslint-factory/src/rules/require-error-code-in-thrown-error.test.ts |
Tests direct, transitive, and later declarations. |
actions/setup/js/push_signed_commits.cjs |
Adds standardized error-code prefixes. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (1)
actions/setup/js/push_signed_commits.cjs:844
- All
PushSignedCommitsPolicyViolationinstances are now constructed with anERR_VALIDATIONprefix, and this wrapper adds the same prefix again. The surfaced message therefore contains duplicate error codes; normalize the nested message here or assign the code at only one layer.
throw new Error(`${ERR_VALIDATION}: pushSignedCommits: refusing unsigned push for branch '${branch}': ${getErrorMessage(err)}`, { cause: err });
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
| if (node.type === AST_NODE_TYPES.ClassDeclaration) { | ||
| const superclassName = getLocalSuperclassName(node); | ||
| if (superclassName && node.id) { | ||
| localClassExtends.set(node.id.name, superclassName); |
| if (err instanceof PushSignedCommitsUnsupportedShape) { | ||
| throw new Error( | ||
| `pushSignedCommits: refusing unsigned push for branch '${branch}': ${getErrorMessage(err)}. ` + | ||
| `${ERR_VALIDATION}: pushSignedCommits: refusing unsigned push for branch '${branch}': ${getErrorMessage(err)}. ` + |
|
@copilot Quick triage nudge for this PR. Please refresh the branch if GitHub can update it cleanly, review any remaining maintainer-facing follow-up, and run the
Run: https://github.com/github/gh-aw/actions/runs/31263551516
|
- Replace program-wide class name map with ESLint scope-based resolution in require-error-code-in-thrown-error rule; this correctly handles shadowing (e.g. a parameter named CustomError shadows the outer class) - Add shadowing test cases to rule tests - Add stripLeadingErrorCode helper in push_signed_commits.cjs and use it when composing wrapper messages so ERR_VALIDATION is not emitted twice Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…level strip Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed the two reviewer issues:
Commits: |
|
🎉 This pull request is included in a new release. Release: |
require-error-code-in-thrown-erroronly checkedthrow new Error(...), so local customErrorsubclasses could throw uncoded messages unnoticed in files already importingerror_codes.cjs.class X extends Errordeclarations.class B extends A.throw new CustomError(...).Coverage
Errorsubclasses.Existing violations
push_signed_commits.cjscustomPushSignedCommits*throw sites to include standardizedERR_*prefixes rather than allowlisting them.