From 5d49661cf56b785c0b8140f88aa0b2d36cba8909 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:32:42 +0000 Subject: [PATCH 1/2] Initial plan From 89f074c5a5b6d1ce9d79d7ec44cefafc42563652 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 8 Jul 2026 09:44:03 +0000 Subject: [PATCH 2/2] fix: require catch handler in isInsideTryBlock for fs-sync and json-parse rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A try/finally without catch does not handle the error — the throw still propagates. Only treat a TryStatement as protective when it has a catch handler (ancestor.handler != null). try/catch/finally remains protective. Add tests for both rules: - invalid: try/finally without catch is flagged - valid: try/catch/finally is treated as protective Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../rules/require-fs-sync-try-catch.test.ts | 45 +++++++++++++++++++ .../src/rules/require-fs-sync-try-catch.ts | 2 +- .../require-json-parse-try-catch.test.ts | 30 +++++++++++++ .../src/rules/require-json-parse-try-catch.ts | 2 +- 4 files changed, 77 insertions(+), 2 deletions(-) diff --git a/eslint-factory/src/rules/require-fs-sync-try-catch.test.ts b/eslint-factory/src/rules/require-fs-sync-try-catch.test.ts index f8f17afbfc9..8cccf68b663 100644 --- a/eslint-factory/src/rules/require-fs-sync-try-catch.test.ts +++ b/eslint-factory/src/rules/require-fs-sync-try-catch.test.ts @@ -67,6 +67,51 @@ describe("require-fs-sync-try-catch", () => { }); }); + it("valid: try/catch/finally is treated as protective", () => { + cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, { + valid: [`try { fs.writeFileSync(path, data); } catch (e) {} finally { cleanup(); }`, `try { fs.readFileSync(path, "utf8"); } catch (e) { handleErr(e); } finally { releaseLock(); }`], + invalid: [], + }); + }); + + it("invalid: try/finally without catch is not protective", () => { + cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, { + valid: [], + invalid: [ + { + code: `try { fs.writeFileSync(path, data); } finally { cleanup(); }`, + errors: [ + { + messageId: "requireTryCatch", + data: { method: "writeFileSync", arg: "path" }, + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `try { try {\n fs.writeFileSync(path, data);\n} catch (err) {\n // TODO: handle I/O failure for this fs.writeFileSync call.\n throw new Error(\n "fs.writeFileSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} } finally { cleanup(); }`, + }, + ], + }, + ], + }, + { + code: `try { fs.readFileSync(path, "utf8"); } finally { releaseLock(); }`, + errors: [ + { + messageId: "requireTryCatch", + data: { method: "readFileSync", arg: "path" }, + suggestions: [ + { + messageId: "wrapInTryCatch", + output: `try { try {\n fs.readFileSync(path, "utf8");\n} catch (err) {\n // TODO: handle I/O failure for this fs.readFileSync call.\n throw new Error(\n "fs.readFileSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} } finally { releaseLock(); }`, + }, + ], + }, + ], + }, + ], + }); + }); + it("valid: synchronous callbacks inside try block are protected", () => { cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, { valid: [ diff --git a/eslint-factory/src/rules/require-fs-sync-try-catch.ts b/eslint-factory/src/rules/require-fs-sync-try-catch.ts index fc6f170acf6..929d8c39a16 100644 --- a/eslint-factory/src/rules/require-fs-sync-try-catch.ts +++ b/eslint-factory/src/rules/require-fs-sync-try-catch.ts @@ -43,7 +43,7 @@ export const requireFsSyncTryCatchRule = createRule({ crossedDeferredBoundary = true; } - if (ancestor.type === "TryStatement" && !crossedDeferredBoundary) { + if (ancestor.type === "TryStatement" && !crossedDeferredBoundary && ancestor.handler != null) { const block = ancestor.block; if (node.range != null && block.range != null && node.range[0] >= block.range[0] && node.range[1] <= block.range[1]) { return true; diff --git a/eslint-factory/src/rules/require-json-parse-try-catch.test.ts b/eslint-factory/src/rules/require-json-parse-try-catch.test.ts index c58db7c8878..7968dbd2360 100644 --- a/eslint-factory/src/rules/require-json-parse-try-catch.test.ts +++ b/eslint-factory/src/rules/require-json-parse-try-catch.test.ts @@ -152,6 +152,36 @@ describe("require-json-parse-try-catch", () => { }); }); + it("valid: try/catch/finally is treated as protective", () => { + cjsRuleTester.run("require-json-parse-try-catch", requireJsonParseTryCatchRule, { + valid: [`try { const x = JSON.parse(str); } catch (e) {} finally { cleanup(); }`, `try { JSON.parse(str); } catch (e) { handleErr(e); } finally { release(); }`], + invalid: [], + }); + }); + + it("invalid: try/finally without catch is not protective", () => { + cjsRuleTester.run("require-json-parse-try-catch", requireJsonParseTryCatchRule, { + valid: [], + invalid: [ + { + code: `try { const x = JSON.parse(str); } finally { cleanup(); }`, + errors: [ + { + messageId: "requireTryCatch", + data: { arg: "str" }, + suggestions: [ + { + messageId: "useHelper", + output: `try { try {\n const x = JSON.parse(str);\n} catch (err) {\n // TODO: handle parse failure for this code path.\n throw new Error(\n "Failed to parse JSON: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n} } finally { cleanup(); }`, + }, + ], + }, + ], + }, + ], + }); + }); + it("valid: synchronous callbacks inside try block are protected", () => { cjsRuleTester.run("require-json-parse-try-catch", requireJsonParseTryCatchRule, { valid: [ diff --git a/eslint-factory/src/rules/require-json-parse-try-catch.ts b/eslint-factory/src/rules/require-json-parse-try-catch.ts index 59af7a10362..890ba74fa0b 100644 --- a/eslint-factory/src/rules/require-json-parse-try-catch.ts +++ b/eslint-factory/src/rules/require-json-parse-try-catch.ts @@ -38,7 +38,7 @@ export const requireJsonParseTryCatchRule = createRule({ crossedDeferredBoundary = true; } - if (ancestor.type === "TryStatement" && !crossedDeferredBoundary) { + if (ancestor.type === "TryStatement" && !crossedDeferredBoundary && ancestor.handler != null) { const block = ancestor.block; if (node.range[0] >= block.range[0] && node.range[1] <= block.range[1]) { return true;