Conversation
arnavsurve
left a comment
There was a problem hiding this comment.
Overall this is a clean, well-structured rule that follows existing patterns nicely. Good test coverage of the key scenarios. Two issues to address:
-
README rule count is stale. The heading on line 94 says "Available Rules (33 total)" but with this rule it should be "34 total." The test in
config-modes.test.tswas correctly updated to 34, but the README heading was missed. -
Lint message conflates two unrelated concerns. The rule only detects bare
JSON.parse()calls, but the warning message also recommendsfast-safe-stringifyforJSON.stringifycircular reference issues. These are completely different failure modes. The message should focus solely on theJSON.parsetry-catch issue. If stringify guidance is desired, it should be a separate rule or at most a separate README note (which it already is in the code example).
Minor observation (non-blocking): JSON.parse() inside a catch block is silenced because the ancestor walk hits the parent TryStatement. The test documents this as intended, but it means an unguarded JSON.parse in a catch handler won't be flagged. This is a reasonable tradeoff for simplicity.
🤖 Generated with Claude Code
| rule: RULE_NAME, | ||
| message: | ||
| 'Wrap JSON.parse() in a try-catch block to handle malformed input. For JSON.stringify, consider using fast-safe-stringify to handle circular references', | ||
| line: loc?.start.line ?? 0, |
There was a problem hiding this comment.
The second sentence about JSON.stringify and fast-safe-stringify is unrelated to what this rule actually detects. This rule only flags JSON.parse() calls, so recommending a stringify library in the lint output is confusing to the developer seeing the warning. Consider simplifying to just:
message: 'Wrap JSON.parse() in a try-catch block to handle malformed input',The fast-safe-stringify tip is already in the README code example, which is the appropriate place for supplementary guidance.
| | ---------------------- | -------- | --------------------------------------------------------- | | ||
| | `prefer-guard-clauses` | warning | Use early returns instead of nesting if statements | | ||
| | `no-type-assertion` | warning | Avoid `as` type casts; use type narrowing or proper types | | ||
| | `safe-json-parse` | warning | Wrap JSON.parse in try-catch to handle malformed input | |
There was a problem hiding this comment.
The heading on line 94 (## Available Rules (33 total)) needs to be updated to 34 total now that safe-json-parse is being added. The test in config-modes.test.ts was correctly bumped to 34, but this heading was missed.
Address review feedback: update the "Available Rules" heading and config-modes test assertion to reflect the correct rule count after merging main (34 rules) plus safe-json-parse (35). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Test plan