Skip to content

[ESLint Factory] Consolidate require-mkdirsync-try-catch and require-fs-sync-try-catch: shared fs-sync-wrap machinery + unified [Content truncated due to length] #45928

Description

@github-actions

Context

eslint-factory/src/rules/require-mkdirsync-try-catch.ts (new, 18th rule) and eslint-factory/src/rules/require-fs-sync-try-catch.ts are two structurally near-identical rules that enforce the same defect class ("wrap a throwing synchronous fs call in try/catch"). They differ only in the method set:

  • require-fs-sync-try-catch: { readFileSync, writeFileSync, appendFileSync }
  • require-mkdirsync-try-catch: { mkdirSync }

Everything else is duplicated: isInsideTryBlock (incl. the handler != null catch-less-try/finally guard and the deferred-callback boundary), findEnclosingStatement, isRequireFsCall, isIdentifierBoundToFsModule, the require/destructure/alias resolution, and the buildTryCatchSuggestion wiring from try-catch-rule-utils.

The divergence that motivates this (not just cosmetic duplication)

The two copies have already drifted in behavior:

  • require-mkdirsync-try-catch resolves ESM ImportBinding shapes — isFsImportBinding / isMkdirSyncImportBinding recognize import { mkdirSync } from "fs" / "node:fs" (see require-mkdirsync-try-catch.ts:76-85, 92-95, 122-126).
  • require-fs-sync-try-catch resolves only require(...) bindings — it has no ImportBinding handling, so import { readFileSync } from "node:fs" (and aliases thereof) silently escapes the rule.

So the exact same import style is covered for mkdirSync but not for readFileSync/writeFileSync/appendFileSync. Every future improvement to fs-binding resolution has to be made twice and will keep drifting.

Grounded corpus activity

Both rules fire heavily on actions/setup/js/**/*.cjs, so this is live surface, not a hypothetical:

  • mkdirSync (unwrapped, would be flagged by require-mkdirsync-try-catch): e.g. install_frontmatter_skills.cjs:149, pick_experiment.cjs:120 and :387, mount_mcp_as_cli.cjs:404/:405, extract_inline_skills.cjs:227, run_evals.cjs:72/:87, safe_outputs_handlers.cjs:2088 (~60 call sites total).
  • Correctly-wrapped true negatives shared by both: send_otlp_span.cjs:661, apply_samples.cjs fs writes, etc.

(The corpus is CommonJS, so the ESM ImportBinding gap above is currently latent — but it is a real coverage inconsistency between the twins and the right time to fix it is at consolidation.)

Proposal

  1. Introduce a createFsSyncTryCatchRule(methods, { messageIds, todo/errorPrefix }) factory (or add mkdirSync to FS_SYNC_METHODS in require-fs-sync-try-catch if a single rule name is acceptable) so the matcher, try-detection, and suggestion machinery live in one place — ideally in try-catch-rule-utils.ts.
  2. Lift the ESM ImportBinding resolution (currently only in the mkdirsync rule) into the shared helper so all fs-sync methods get uniform require + import + destructure + alias coverage.
  3. Keep the per-method diagnostic text (fs.{{method}}(...)) parameterized.
  4. Preserve the existing index.ts registration and docs URLs (or deprecate one name with a clear migration note).

Acceptance criteria

  • One shared implementation; require-fs-sync-try-catch and require-mkdirsync-try-catch (or their successor) no longer duplicate isInsideTryBlock/binding-resolution logic.
  • import { readFileSync } from "node:fs"; readFileSync(p) (unwrapped) is now flagged, matching the mkdirSync ESM behavior.
  • Existing tests for both rules still pass; add ESM-import test cases for the previously-uncovered fs-sync methods.
  • No change in flagging for the grounded mkdirSync/readFileSync CommonJS sites above.

Generated by 🤖 ESLint Refiner · 517.5 AIC · ⌖ 13.1 AIC · ⊞ 4.6K ·

  • expires on Jul 22, 2026, 10:18 PM UTC-08:00

Metadata

Metadata

Labels

code-qualitycookieIssue Monster Loves Cookies!eslintjavascriptPull requests that update javascript code

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions