Skip to content

Add no-nested-try-catch lint rule#29

Open
danielchen0 wants to merge 2 commits intomainfrom
add-no-nested-try-catch-rule-v2
Open

Add no-nested-try-catch lint rule#29
danielchen0 wants to merge 2 commits intomainfrom
add-no-nested-try-catch-rule-v2

Conversation

@danielchen0
Copy link
Collaborator

Summary

  • Adds no-nested-try-catch rule that flags try-catch blocks nested inside other try/catch/finally blocks
  • Suggests extracting inner try-catch to separate functions

Test plan

  • All existing tests pass
  • 7 new tests pass
  • TypeScript compiles
  • Prettier passes

Copy link
Contributor

@arnavsurve arnavsurve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-structured addition. The rule implementation follows established patterns (matches no-type-assertion.ts style), tests are thorough (7 cases covering try/catch/finally nesting, deep nesting, sibling try-catch, and the function boundary edge case), and the README documentation is properly updated.

One minor observation: the ancestor walk doesn't stop at function boundaries, so try { const fn = () => { try { ... } catch {} }; } catch {} will be flagged. The test explicitly documents this as intentional/acceptable, which I agree with -- it nudges users toward extracting the inner function outside the try block entirely, which is the cleaner pattern.

The comment on line 12 says "Check if any ancestor is also a TryStatement's block" but the code checks for TryStatement ancestors generally (not just the block property). The logic is still correct since any try nested anywhere within a TryStatement node (block, handler, or finalizer) should be flagged. The comment could be slightly more precise, but this is a pure nit.

LGTM.

🤖 Generated with Claude Code


traverse(ast, {
TryStatement(path) {
// Check if any ancestor is also a TryStatement's block
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The comment says "Check if any ancestor is also a TryStatement's block" but the code checks for any TryStatement ancestor generally (not specifically the .block child). The logic is correct regardless -- just a minor wording thing. Could be simplified to "Check if any ancestor is a TryStatement" to match what the code actually does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants