Rule
require-fs-sync-try-catch.
Summary
isInsideTryBlock (require-fs-sync-try-catch.ts:33-55) considers a call "protected" whenever it is lexically inside the block of any enclosing TryStatement:
if (ancestor.type === "TryStatement" && !crossedDeferredBoundary) {
const block = ancestor.block;
if (node.range ... within block.range) {
return true; // <- treated as protected
}
}It never checks whether that TryStatement actually has a catch handler. A try { ... } finally { ... } with no catch does not swallow the error — the throw propagates past finally and still crashes the action. Yet the rule marks the fs call inside such a try as safe and does not flag it.
This defeats the rule's own stated contract ("an unhandled throw crashes the action"): a catch-less try/finally provides cleanup, not error handling, so the crash the rule exists to prevent still happens.
Failure scenario
function persist(path, data) {
acquireLock();
try {
fs.writeFileSync(path, data); // throws on ENOSPC / EACCES
} finally {
releaseLock(); // runs, then the throw resumes propagating
}
}The rule sees the enclosing TryStatement and stays silent, but persist re-throws and the action dies with the same unhandled fs error the rule is meant to catch. try/catch (or try/catch/finally) would genuinely handle it; try/finally alone does not.
Grounding
Ungrounded in the current corpus: a scan for try { ... fs.<sync> ... } finally over non-test actions/setup/js/**/*.cjs returned 0 live sites today (the codebase consistently uses try { fs... } catch). This is a latent soundness gap in the protection predicate rather than a currently-firing miss — filing it because it is a small, well-contained logic fix that closes a real hole before the idiom appears, and because the same catch-less-try assumption may be worth auditing in sibling try/catch rules (require-json-parse-try-catch, require-fs-sync-style guards).
Ask
- In
isInsideTryBlock, only treat a TryStatement as protective when it has a catch handler — i.e. require ancestor.handler != null before returning true. (A try/catch/finally still has handler, so it stays protective; a finally-only try does not.)
- Add tests:
- invalid:
try { fs.writeFileSync(p, d); } finally { cleanup(); } — should be flagged.
- valid:
try { fs.writeFileSync(p, d); } catch (e) {} finally { cleanup(); } — should stay unflagged.
- Confirm the existing valid cases (plain
try { fs... } catch (e) {}) are unaffected.
Notes
Statically grounded review (no live lint available: eslint-factory has no node_modules/dist here, npm firewalled). Verified against require-fs-sync-try-catch.ts + try-catch-rule-utils.ts and the live *.cjs corpus.
Generated by 🤖 ESLint Refiner · 307.7 AIC · ⌖ 12.6 AIC · ⊞ 4.6K · ◷
Rule
require-fs-sync-try-catch.Summary
isInsideTryBlock(require-fs-sync-try-catch.ts:33-55) considers a call "protected" whenever it is lexically inside the block of any enclosingTryStatement:It never checks whether that
TryStatementactually has a catch handler. Atry { ... } finally { ... }with nocatchdoes not swallow the error — the throw propagates pastfinallyand still crashes the action. Yet the rule marks the fs call inside such a try as safe and does not flag it.This defeats the rule's own stated contract ("an unhandled throw crashes the action"): a catch-less try/finally provides cleanup, not error handling, so the crash the rule exists to prevent still happens.
Failure scenario
The rule sees the enclosing
TryStatementand stays silent, butpersistre-throws and the action dies with the same unhandledfserror the rule is meant to catch.try/catch(ortry/catch/finally) would genuinely handle it;try/finallyalone does not.Grounding
Ungrounded in the current corpus: a scan for
try { ... fs.<sync> ... } finallyover non-testactions/setup/js/**/*.cjsreturned 0 live sites today (the codebase consistently usestry { fs... } catch). This is a latent soundness gap in the protection predicate rather than a currently-firing miss — filing it because it is a small, well-contained logic fix that closes a real hole before the idiom appears, and because the same catch-less-try assumption may be worth auditing in sibling try/catch rules (require-json-parse-try-catch,require-fs-sync-style guards).Ask
isInsideTryBlock, only treat aTryStatementas protective when it has a catch handler — i.e. requireancestor.handler != nullbefore returningtrue. (A try/catch/finally still hashandler, so it stays protective; a finally-only try does not.)try { fs.writeFileSync(p, d); } finally { cleanup(); }— should be flagged.try { fs.writeFileSync(p, d); } catch (e) {} finally { cleanup(); }— should stay unflagged.try { fs... } catch (e) {}) are unaffected.Notes
Statically grounded review (no live lint available:
eslint-factoryhas nonode_modules/disthere, npm firewalled). Verified againstrequire-fs-sync-try-catch.ts+try-catch-rule-utils.tsand the live*.cjscorpus.