Conversation
Flags `String(error)` in the else branch of `error instanceof Error` ternaries, where String() produces '[object Object]' for non-Error objects. Suggests using JSON.stringify() instead to preserve object structure.
arnavsurve
left a comment
There was a problem hiding this comment.
Looks good. Clean implementation that follows the established patterns in the codebase.
What the rule does: Flags String(x) in the else branch of x instanceof Error ternaries, suggesting JSON.stringify(x) instead. This catches the common footgun where String() on a non-Error object (e.g. a plain object thrown as an error) produces the unhelpful '[object Object]'.
What I checked:
- The recursive
containsStringCallcorrectly walks child nodes usingVISITOR_KEYS, catching nested usages likenew Error(String(err)) - Variable name matching between the
instanceofcheck and theString()call prevents false positives - The rule correctly scopes to only
instanceof Error(not custom subclasses), which matches the intended use case of catch-all error handling - Test coverage is solid: 6 tests covering the positive case, variable name variants, nested calls, and three false-positive avoidance cases
- All CI checks pass across Node 18/20/22
- README, rule index, and test count are all updated consistently
One minor pre-existing nit (not from this PR): the README intro still says "all 31 rules" on line 17, while the actual count is now 34. Could be fixed in a follow-up.
🤖 Generated with Claude Code
src/rules/no-string-coerce-error.ts
Outdated
| if (stringCall) { | ||
| results.push({ | ||
| rule: RULE_NAME, | ||
| message: `String(${errorName}) produces '[object Object]' for non-Error objects. Use JSON.stringify(${errorName}) instead.`, |
There was a problem hiding this comment.
Nit: The message says String(x) always produces '[object Object]', but that's only true for plain objects -- for strings/numbers it works fine. Since the else branch of instanceof Error could receive anything (string, number, object), the message is slightly overstated. Consider something like: "String(${errorName}) may produce '[object Object]' for non-Error objects. Consider JSON.stringify(${errorName}) to preserve structure." -- but this is very minor, fine to leave as-is.
Summary
no-string-coerce-errorlint rule that flagsString(error)in the else branch oferror instanceof ErrorternariesString()on a non-Error object produces'[object Object]'— the rule suggestsJSON.stringify()instead to preserve object structureTest plan