feat(lint): repo-local no-inline-lazy-node-getter rule via fleet config factory

jdalton · jdalton · commit 9b164b45406d · 2026-06-05T08:28:26.000-04:00
socket-lib's .config/repo/oxlint.config.mts imports the fleet oxlint factory
(config()) and augments it with a repo-local plugin + the
socket-repo/no-inline-lazy-node-getter rule. Flags inline-chained lazy
node-module getters (getFs().existsSync(x)) and autofixes to a bound const,
hoisted before any leading oxlint-disable comment, deduped per statement. The
factory call (not oxlint extends) makes both the fleet + repo plugins load with
fleet rules/overrides/ignores intact.
diff --git a/.config/repo/oxlint-plugin/index.mts b/.config/repo/oxlint-plugin/index.mts
@@ -0,0 +1,20 @@
+/**
+ * @file Socket-lib repo-local oxlint plugin. Rules that encode conventions
+ *   specific to socket-lib's own module shape (not fleet-canonical, so not in
+ *   `.config/fleet/oxlint-plugin/`). Wiring: `.config/repo/oxlintrc.json`
+ *   extends the fleet config, adds this plugin to `jsPlugins`, and enables its
+ *   rules under the `socket-repo/` namespace.
+ */
+
+import noInlineLazyNodeGetter from './rules/no-inline-lazy-node-getter.mts'
+
+const plugin = {
+  meta: {
+    name: 'socket-repo',
+  },
+  rules: {
+    'no-inline-lazy-node-getter': noInlineLazyNodeGetter,
+  },
+}
+
+export default plugin
diff --git a/.config/repo/oxlint-plugin/rules/no-inline-lazy-node-getter.mts b/.config/repo/oxlint-plugin/rules/no-inline-lazy-node-getter.mts
@@ -0,0 +1,187 @@
+/**
+ * @file Repo-local rule: forbid inline-chained member access on a lazy
+ *   node-module getter — `getFs().existsSync(x)`, `getPath().join(a, b)`,
+ *   `getNodeChildProcess().spawn(...)`. socket-lib loads Node builtins through
+ *   lazy `getNode*()` getters (`src/node/*`) and their short aliases (`getFs`,
+ *   `getPath`, …) so a builtin is resolved once and memoized. Calling the
+ *   getter inline at every use defeats the memoization read-site ergonomics and
+ *   reads noisily — `getFs().existsSync(a) … getFs().statSync(b)` re-invokes
+ *   the getter on every line. The fleet convention is to bind it once: const fs
+ *   = getFs() if (fs.existsSync(a)) { … } const s = fs.statSync(b) This rule
+ *   flags `getX().member` where `getX` is one of the lazy node-module getters
+ *   and rewrites it: it hoists `const <name> = getX()` immediately before the
+ *   enclosing statement and replaces the inline call with `<name>`. `<name>` is
+ *   derived from the getter (`getFs`/`getNodeFs` → `fs`, `getNodePath` →
+ *   `path`, `getNodeChildProcess` → `childProcess`). Repo-local (not
+ *   fleet-canonical): the `getNode*` lazy-getter convention is socket-lib's own
+ *   module shape, so the rule lives under `.config/repo/oxlint-plugin/` and is
+ *   wired via `.config/repo/oxlintrc.json`, not cascaded.
+ */
+
+import type {
+  AstNode,
+  RuleContext,
+  RuleFixer,
+} from '../../../fleet/oxlint-plugin/lib/rule-types.mts'
+
+/**
+ * Lazy node-module getter names → the conventional const name to bind them to.
+ * Both the canonical `getNode*` form and the short `get*` alias map to the same
+ * variable name so the rewrite reads like hand-written code.
+ */
+const GETTER_TO_BINDING: Record<string, string> = {
+  __proto__: null,
+  getCrypto: 'crypto',
+  getFs: 'fs',
+  getFsPromises: 'fsPromises',
+  getHttp: 'http',
+  getHttps: 'https',
+  getNodeAsyncHooks: 'asyncHooks',
+  getNodeChildProcess: 'childProcess',
+  getNodeCrypto: 'crypto',
+  getNodeEvents: 'events',
+  getNodeFs: 'fs',
+  getNodeFsPromises: 'fsPromises',
+  getNodeHttp: 'http',
+  getNodeHttps: 'https',
+  getNodeModule: 'nodeModule',
+  getNodeOs: 'os',
+  getNodePath: 'path',
+  getNodeTimersPromises: 'timersPromises',
+  getNodeUrl: 'url',
+  getNodeUtil: 'util',
+  getPath: 'path',
+  getSemver: 'semver',
+  getUtil: 'util',
+} as Record<string, string>
+
+/**
+ * Walk up from a node to the nearest enclosing statement — the node whose
+ * parent is a block / program / switch-case body. The hoisted `const` is
+ * inserted before it.
+ */
+function findEnclosingStatement(node: AstNode): AstNode | undefined {
+  let current = node
+  let parent = current.parent
+  while (parent) {
+    const parentType = parent.type
+    if (
+      parentType === 'BlockStatement' ||
+      parentType === 'Program' ||
+      parentType === 'StaticBlock' ||
+      parentType === 'SwitchCase'
+    ) {
+      return current
+    }
+    current = parent
+    parent = current.parent
+  }
+  return undefined
+}
+
+const rule = {
+  create(context: RuleContext) {
+    const sourceCode = context.getSourceCode
+      ? context.getSourceCode()
+      : context.sourceCode
+
+    // Dedup hoists within a single lint pass. When one statement holds two
+    // inline calls of the SAME getter — `path.basename(x, getNodePath()
+    // .extname(x))` next to another `getNodePath()` — emitting the
+    // `const path = getNodePath()` hoist for each would write the
+    // declaration twice. Key by enclosing-statement range + binding: the
+    // first occurrence hoists + rewrites, the rest only rewrite to the
+    // already-declared binding.
+    const hoisted = new Set<string>()
+
+    return {
+      // Match `<getter>().<member>` — a MemberExpression whose object is a
+      // CallExpression of a bare getter identifier.
+      MemberExpression(node: AstNode) {
+        const object = node.object
+        if (
+          !object ||
+          object.type !== 'CallExpression' ||
+          object.callee?.type !== 'Identifier' ||
+          // The getter takes no args; a call with args isn't one of ours.
+          (object.arguments && object.arguments.length > 0)
+        ) {
+          return
+        }
+        const getter = object.callee.name
+        const binding = GETTER_TO_BINDING[getter]
+        if (!binding) {
+          return
+        }
+        const member =
+          node.property?.type === 'Identifier' ? node.property.name : '…'
+
+        const enclosing = findEnclosingStatement(node)
+        const hoistKey = enclosing
+          ? `${enclosing.range?.[0] ?? enclosing.start ?? enclosing.loc?.start?.line}:${binding}`
+          : ''
+
+        context.report({
+          node: object,
+          messageId: 'inlineGetter',
+          data: { getter, member, binding },
+          fix(fixer: RuleFixer) {
+            // Can't safely hoist without an enclosing statement to anchor to.
+            if (!enclosing) {
+              return undefined
+            }
+            // Second+ inline call of the same getter in one statement: the
+            // hoist already exists, just point this call at the binding.
+            if (hoisted.has(hoistKey)) {
+              return fixer.replaceText(object, binding)
+            }
+            hoisted.add(hoistKey)
+            const indentMatch = /^[ \t]*/.exec(
+              sourceCode.lines?.[enclosing.loc.start.line - 1] ?? '',
+            )
+            const indent = indentMatch ? indentMatch[0] : ''
+            // Anchor the hoist BEFORE any line comment that directly precedes
+            // the statement. A leading `// oxlint-disable-next-line …` (or any
+            // explanatory comment) targets the statement on the next line;
+            // inserting the `const` between the comment and the statement would
+            // orphan the directive onto the injected line and re-expose the
+            // statement to the disabled rule.
+            const commentsBefore =
+              sourceCode.getCommentsBefore?.(enclosing) ?? []
+            const lastComment = commentsBefore[commentsBefore.length - 1]
+            const anchor =
+              lastComment &&
+              lastComment.loc?.end?.line === enclosing.loc.start.line - 1
+                ? lastComment
+                : enclosing
+            return [
+              fixer.insertTextBefore(
+                anchor,
+                `const ${binding} = ${getter}()\n${indent}`,
+              ),
+              fixer.replaceText(object, binding),
+            ]
+          },
+        })
+      },
+    }
+  },
+
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description:
+        'Bind a lazy node-module getter to a const once (`const fs = getFs()`) instead of calling it inline at each use (`getFs().existsSync(x)`).',
+      category: 'Best Practices',
+      recommended: true,
+    },
+    fixable: 'code',
+    messages: {
+      inlineGetter:
+        '`{{getter}}().{{member}}` calls the lazy node-module getter inline. Bind it once — `const {{binding}} = {{getter}}()` — then use `{{binding}}.{{member}}(…)`. Re-invoking the getter at every call site is noisy and defeats the single-read convention.',
+    },
+    schema: [],
+  },
+}
+
+export default rule
diff --git a/.config/repo/oxlint-plugin/test/no-inline-lazy-node-getter.test.mts b/.config/repo/oxlint-plugin/test/no-inline-lazy-node-getter.test.mts
@@ -0,0 +1,185 @@
+/**
+ * @file Integration test for socket-repo/no-inline-lazy-node-getter. The fleet
+ *   RuleTester is hardcoded to the fleet plugin + `socket/` namespace, so a
+ *   repo rule under `socket-repo/` can't use it. Instead this test exercises
+ *   the real repo overlay (`.config/repo/oxlintrc.json` → extends fleet + loads
+ *   the repo plugin) end-to-end: write a fixture, run oxlint with the repo
+ *   config, assert the finding fires (and `--fix` rewrites to the bound-const
+ *   form).
+ */
+
+import assert from 'node:assert/strict'
+import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'
+import os from 'node:os'
+import path from 'node:path'
+import { fileURLToPath } from 'node:url'
+import { describe, test } from 'node:test'
+
+import { whichSync } from '@socketsecurity/lib-stable/bin/which'
+import { safeDeleteSync } from '@socketsecurity/lib-stable/fs/safe'
+import { spawnSync } from '@socketsecurity/lib-stable/process/spawn/child'
+
+const HERE = path.dirname(fileURLToPath(import.meta.url))
+// Plugin entry, absolute so the spawned oxlint resolves it from a tmpdir.
+const PLUGIN_INDEX = path.resolve(HERE, '..', 'index.mts')
+const NODE_MODULES_BIN = path.resolve(
+  HERE,
+  '..',
+  '..',
+  '..',
+  '..',
+  'node_modules',
+  '.bin',
+)
+
+const RULE_NAME = 'no-inline-lazy-node-getter'
+
+/**
+ * Oxlint renders the rule id as either `socket-repo/<name>` (diagnostic ruleId)
+ * or `socket-repo(<name>)` (compact code form) depending on reporter. Match
+ * either.
+ */
+function hasRuleFinding(stdout: string): boolean {
+  return (
+    stdout.includes(`socket-repo/${RULE_NAME}`) ||
+    stdout.includes(`socket-repo(${RULE_NAME})`)
+  )
+}
+
+/**
+ * Resolve the locally-installed oxlint, never a global PATH copy (per
+ * socket/no-which-for-local-bin). Returns undefined when it isn't installed so
+ * the cases skip gracefully instead of failing.
+ */
+function resolveOxlintBinary(): string | undefined {
+  const found = whichSync('oxlint', { path: NODE_MODULES_BIN, nothrow: true })
+  return typeof found === 'string' ? found : undefined
+}
+
+function runOxlint(
+  code: string,
+  fix: boolean,
+): { stdout: string; code: string } {
+  const tmpdir = mkdtempSync(path.join(os.tmpdir(), 'oxlint-repo-rule-'))
+  const fixture = path.join(tmpdir, 'fixture.ts')
+  // Minimal isolated config: just this plugin + rule, no `extends`/
+  // ignorePatterns (the full overlay's fleet inheritance is verified by
+  // `pnpm run lint`, not here). Keeps the unit test independent of the
+  // fleet config and resolvable from a tmpdir.
+  const config = path.join(tmpdir, '.oxlintrc.json')
+  writeFileSync(
+    config,
+    JSON.stringify({
+      plugins: [],
+      jsPlugins: [PLUGIN_INDEX],
+      rules: { 'socket-repo/no-inline-lazy-node-getter': 'error' },
+    }),
+  )
+  writeFileSync(fixture, code)
+  try {
+    const args = ['-c', config]
+    if (fix) {
+      args.push('--fix')
+    }
+    args.push(fixture)
+    const result = spawnSync(resolveOxlintBinary() ?? 'oxlint', args, {
+      encoding: 'utf8',
+    })
+    return {
+      stdout: String(result.stdout ?? ''),
+      code: fix ? readFileSync(fixture, 'utf8') : '',
+    }
+  } finally {
+    safeDeleteSync(tmpdir)
+  }
+}
+
+describe('socket-repo/no-inline-lazy-node-getter', () => {
+  test('flags getFs().member inline access', () => {
+    if (!resolveOxlintBinary()) {
+      return
+    }
+    const { stdout } = runOxlint(
+      'function f() {\n  return getFs().existsSync("/x")\n}\n',
+      false,
+    )
+    assert.ok(
+      hasRuleFinding(stdout),
+      `expected ${RULE_NAME} finding, got:\n${stdout}`,
+    )
+  })
+
+  test('does not flag a bound-const getter', () => {
+    if (!resolveOxlintBinary()) {
+      return
+    }
+    const { stdout } = runOxlint(
+      'function f() {\n  const fs = getFs()\n  return fs.existsSync("/x")\n}\n',
+      false,
+    )
+    assert.ok(
+      !hasRuleFinding(stdout),
+      `expected no ${RULE_NAME} finding, got:\n${stdout}`,
+    )
+  })
+
+  test('autofix hoists the const and rewrites the call', () => {
+    if (!resolveOxlintBinary()) {
+      return
+    }
+    const { code } = runOxlint(
+      'function f() {\n  return getNodePath().join("a", "b")\n}\n',
+      true,
+    )
+    assert.ok(
+      code.includes('const path = getNodePath()'),
+      `expected hoisted const, got:\n${code}`,
+    )
+    assert.ok(
+      code.includes('path.join("a", "b")'),
+      `expected rewritten call, got:\n${code}`,
+    )
+    assert.ok(
+      !/getNodePath\(\)\.join/.test(code),
+      `expected no remaining inline call, got:\n${code}`,
+    )
+  })
+
+  test('autofix dedupes two inline calls of the same getter in one statement', () => {
+    if (!resolveOxlintBinary()) {
+      return
+    }
+    const { code } = runOxlint(
+      'function f(x) {\n  return getNodePath().basename(x, getNodePath().extname(x))\n}\n',
+      true,
+    )
+    const hoists = (code.match(/const path = getNodePath\(\)/g) ?? []).length
+    assert.equal(
+      hoists,
+      1,
+      `expected exactly one hoisted const, got ${hoists}:\n${code}`,
+    )
+    assert.ok(
+      code.includes('path.basename(x, path.extname(x))'),
+      `expected both calls rewritten, got:\n${code}`,
+    )
+  })
+
+  test('autofix keeps a leading oxlint-disable-next-line attached to its statement', () => {
+    if (!resolveOxlintBinary()) {
+      return
+    }
+    const { code } = runOxlint(
+      'function f(p) {\n  // oxlint-disable-next-line some/rule -- intentional\n  return getFs().statSync(p)\n}\n',
+      true,
+    )
+    // The hoisted const must land BEFORE the disable comment, so the directive
+    // still sits on the line directly above the statSync statement.
+    assert.ok(
+      /const fs = getFs\(\)\n\s*\/\/ oxlint-disable-next-line[^\n]*\n\s*return fs\.statSync/.test(
+        code,
+      ),
+      `expected hoist before the disable comment, got:\n${code}`,
+    )
+  })
+})
diff --git a/.config/repo/oxlint.config.mts b/.config/repo/oxlint.config.mts
