Skip to content

[eslint-miner] eslint: add require-fs-sync-try-catch rule #43994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 8 commits into from
Jul 7, 2026
Merged

[eslint-miner] eslint: add require-fs-sync-try-catch rule #43994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
65b765e
eslint: add require-fs-sync-try-catch rule
github-actions[bot] Jul 7, 2026
9646bff
Merge branch 'main' into eslint-miner/require-fs-sync-try-catch-ba95c…
github-actions[bot] Jul 7, 2026
14ee1cf
Merge branch 'main' into eslint-miner/require-fs-sync-try-catch-ba95c…
github-actions[bot] Jul 7, 2026
89abe64
fix: make fs sync try/catch suggestions safe
Copilot Jul 7, 2026
9f0babc
fix: normalize try/catch suggestion indentation
Copilot Jul 7, 2026
3934685
refactor: tidy try/catch rule utilities
Copilot Jul 7, 2026
48740a8
test: cover nextTick and destructured fs cases
Copilot Jul 7, 2026
8b70c9d
refactor: document try/catch helper behavior
Copilot Jul 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eslint-factory/eslint.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ module.exports = [
"gh-aw-custom/require-async-entrypoint-catch": "warn",
"gh-aw-custom/require-await-core-summary-write": "warn",
"gh-aw-custom/require-error-cause-in-rethrow": "warn",
"gh-aw-custom/require-fs-sync-try-catch": "warn",
"gh-aw-custom/require-json-parse-try-catch": "warn",
"gh-aw-custom/require-parseInt-radix": "warn",
},
Expand Down
2 changes: 2 additions & 0 deletions eslint-factory/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { preferGetErrorMessageRule } from "./rules/prefer-get-error-message";
import { preferNumberIsNanRule } from "./rules/prefer-number-isnan";
import { requireAsyncEntrypointCatchRule } from "./rules/require-async-entrypoint-catch";
import { requireAwaitCoreSummaryWriteRule } from "./rules/require-await-core-summary-write";
import { requireFsSyncTryCatchRule } from "./rules/require-fs-sync-try-catch";
import { requireJsonParseTryCatchRule } from "./rules/require-json-parse-try-catch";
import { requireErrorCauseInRethrowRule } from "./rules/require-error-cause-in-rethrow";
import { requireParseIntRadixRule } from "./rules/require-parseInt-radix";
Expand All @@ -25,6 +26,7 @@ const plugin = {
"require-async-entrypoint-catch": requireAsyncEntrypointCatchRule,
"require-await-core-summary-write": requireAwaitCoreSummaryWriteRule,
"require-error-cause-in-rethrow": requireErrorCauseInRethrowRule,
"require-fs-sync-try-catch": requireFsSyncTryCatchRule,
"require-json-parse-try-catch": requireJsonParseTryCatchRule,
"require-parseInt-radix": requireParseIntRadixRule,
},
Expand Down
318 changes: 318 additions & 0 deletions eslint-factory/src/rules/require-fs-sync-try-catch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,318 @@
import { RuleTester } from "eslint";
import { describe, it } from "vitest";
import { requireFsSyncTryCatchRule } from "./require-fs-sync-try-catch";

const cjsRuleTester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: "commonjs",
},
});

const esmRuleTester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: "module",
},
});

describe("require-fs-sync-try-catch", () => {
it("valid: fs.readFileSync inside try block passes (CommonJS)", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [
`try { const x = fs.readFileSync(path, "utf8"); } catch (e) {}`,
`try { return fs.readFileSync(path, "utf8"); } catch (e) {}`,
`function f() { try { fs.readFileSync(path); } catch (e) {} }`,
`try { const x = fs["readFileSync"](path, "utf8"); } catch (e) {}`,
],
invalid: [],
});
});

it("valid: fs.writeFileSync and fs.appendFileSync inside try block pass", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [`try { fs.writeFileSync(path, data); } catch (e) {}`, `try { fs.appendFileSync(path, data); } catch (e) {}`, `try { fs["writeFileSync"](path, data); } catch (e) {}`],
invalid: [],
});
});

it("valid: other fs methods not in scope are ignored", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [
`fs.existsSync(path);`,
`fs.mkdirSync(dir, { recursive: true });`,
`fs.unlinkSync(path);`,
`fs.statSync(path);`,
`fs.readdirSync(dir);`,
`fs.rmSync(dir, { recursive: true });`,
// Non-fs objects with same method names are ignored
`mockFs.readFileSync(path);`,
`storage.writeFileSync(path, data);`,
],
invalid: [],
});
});

it("valid: destructured fs bindings stay out of scope", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [`const { readFileSync } = require("fs"); readFileSync(path, "utf8");`],
invalid: [],
});
});

it("valid: fs inside try block (ES module) passes", () => {
esmRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [`try { const x = fs.readFileSync(path, "utf8"); } catch (e) {}`],
invalid: [],
});
});

it("valid: synchronous callbacks inside try block are protected", () => {

@github-actions github-actions Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] No test for fs.readFileSync used as a bare expression inside an async function body without any await (i.e., async function f() { fs.readFileSync(p); }). The deferred-callback logic is only tested for .on, setTimeout, and new Promise — a regular async function body is not a deferred sink and should pass, but there is no explicit test confirming this.

💡 Suggested test to add 
it("valid: fs.readFileSync in async function body without try is still flagged (not a deferred sink)", () => {
  cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
    valid: [],
    invalid: [
      {
        code: `async function load() { const x = fs.readFileSync(p, "utf8"); }`,
        errors: [{ messageId: "requireTryCatch" }],
      },
    ],
  });
});

This pins the boundary: async function bodies are not deferred sinks — the call is still synchronous relative to the function's frame.

@copilot please address this.

cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [
// Array map is synchronous — try block is genuinely protective
`try { const results = paths.map(p => fs.readFileSync(p, "utf8")); } catch (e) {}`,
`try { items.forEach(p => { fs.writeFileSync(p, data); }); } catch (e) {}`,
// Locally-defined nextTick can be synchronous, so the surrounding try is protective
`try { const nextTick = fn => fn(); nextTick(() => { fs.readFileSync(path, "utf8"); }); } catch (e) {}`,
],
invalid: [],
});
});

it("invalid: bare fs.readFileSync is flagged with correct message and suggestion", () => {

@github-actions github-actions Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[/tdd] The valid-case test for mockFs.readFileSync and storage.writeFileSync (non-fs objects) correctly uses the same method names, but there is no test for a destructured import pattern: const { readFileSync } = require('fs'); readFileSync(p);. The current implementation intentionally does not flag this, but the omission isn't documented by a test — a future contributor could unknowingly add destructured-reference support and break the scope guarantee.

💡 Suggested test to add (one-liner) 
// Destructured binding — intentionally out of scope for this rule
`const { readFileSync } = require('fs'); readFileSync(path, 'utf8');`,

Adding this to the valid cases explicitly documents the intentional gap.

@copilot please address this.

cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `const content = fs.readFileSync(filePath, "utf8");`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "filePath" },
suggestions: [],
},
],
},
{
code: `fs.readFileSync(configPath, "utf8");`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "configPath" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try {\n fs.readFileSync(configPath, "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}`,
},
],
},
],
},
],
});
});

it("invalid: bare fs.writeFileSync is flagged", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `fs.writeFileSync(outputPath, JSON.stringify(data));`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "writeFileSync", arg: "outputPath" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try {\n fs.writeFileSync(outputPath, JSON.stringify(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}`,
},
],
},
],
},
],
});
});

it("invalid: bare fs.appendFileSync is flagged", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `fs.appendFileSync(logPath, line + "\\n");`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "appendFileSync", arg: "logPath" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try {\n fs.appendFileSync(logPath, line + "\\n");\n} catch (err) {\n // TODO: handle I/O failure for this fs.appendFileSync call.\n throw new Error(\n "fs.appendFileSync failed: " + (err instanceof Error ? err.message : String(err)),\n { cause: err },\n );\n}`,
},
],
},
],
},
],
});
});

it('invalid: computed fs["readFileSync"] access is flagged when not in try block', () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `const data = fs["readFileSync"](path, "utf8");`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "path" },
suggestions: [],
},
],
},
],
});
});

it("invalid: fs.readFileSync in deferred callback is not protected by surrounding try", () => {
cjsRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
// EventEmitter .on — callback fires asynchronously
{
code: `try { emitter.on("data", () => { fs.readFileSync(path, "utf8"); }); } catch (e) {}`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "path" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try { emitter.on("data", () => { 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} }); } catch (e) {}`,
},
],
},
],
},
// setTimeout — callback fires asynchronously
{
code: `try { setTimeout(() => { fs.writeFileSync(p, d); }, 0); } catch (e) {}`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "writeFileSync", arg: "p" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try { setTimeout(() => { try {\n fs.writeFileSync(p, d);\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} }, 0); } catch (e) {}`,
},
],
},
],
},
// process.nextTick — callback runs after the surrounding try has returned
{
code: `try { process.nextTick(() => { fs.readFileSync(path, "utf8"); }); } catch (e) {}`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "path" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try { process.nextTick(() => { 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} }); } catch (e) {}`,
},
],
},
],
},
// async function bodies are still synchronous relative to their own frame
{
code: `async function load() { fs.readFileSync(path, "utf8"); }`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "path" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `async function load() { 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} }`,
},
],
},
],
},
// new Promise executor — Promise captures throws
{
code: `try { new Promise(resolve => { fs.readFileSync(path); }); } catch (e) {}`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "path" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try { new Promise(resolve => { try {\n fs.readFileSync(path);\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} }); } catch (e) {}`,
},
],
},
],
},
],
});
});

it("invalid: fs.readFileSync inside if-branch without surrounding try is flagged (ES module)", () => {
esmRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `if (cond) {\n const raw = fs.readFileSync(p, "utf8");\n}`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "p" },
suggestions: [],
},
],
},
],
});
});

it("invalid: unsupported positions and multi-line expressions handle suggestions safely", () => {
esmRuleTester.run("require-fs-sync-try-catch", requireFsSyncTryCatchRule, {
valid: [],
invalid: [
{
code: `export default fs.readFileSync(config, "utf8");`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "config" },
suggestions: [],
},
],
},
{
code: `fs.readFileSync(\n config,\n "utf8",\n);`,
errors: [
{
messageId: "requireTryCatch",
data: { method: "readFileSync", arg: "config" },
suggestions: [
{
messageId: "wrapInTryCatch",
output: `try {\n fs.readFileSync(\n config,\n "utf8",\n );\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}`,
},
],
},
],
},
],
});
});
});
Loading
Loading