refactor(lint): remove find dependency and drop mainWithDeps

Author: CMCDragonkai

plans/refactor/STATE.md

@@ -24,6 +24,20 @@
   - removed permissive unknown-option behavior so unknown flags are rejected by the CLI parser
 - Added tests for this slice in [`tests/bin/lint.test.ts`](../../tests/bin/lint.test.ts).
 
+## Implemented in follow-up cleanup slice
+
+- Removed shell-domain dependence on external `find`:
+  - shell discovery now walks filesystem roots in Node and collects `*.sh` files before invoking `shellcheck`
+  - non-existent shell roots are still ignored
+  - explicit shell requests (`--shell ...` or `--only shell`) still fail if `shellcheck` is missing
+  - default shell auto-run still warns/skips when `shellcheck` is missing
+- Removed runtime dependency-injection indirection from CLI entry:
+  - removed `MainDeps`, `defaultMainDeps`, and `mainWithDeps` from `src/bin/lint.ts`
+  - retained `main` as the runtime/default export entrypoint
+- Refactored CLI tests to use Jest spies/mocks instead of `mainWithDeps`:
+  - tests now mock dependency behavior through module spies (for example `utils.commandExists`)
+  - shell assertions updated to verify direct `shellcheck` invocation behavior
+
 ## Current pain points (observed in current implementation)
 
 - Domains run in a fixed sequence and cannot be cleanly targeted.
@@ -43,7 +57,6 @@
 
 - Final naming for the new flags (for example `--domain` vs `--only`, `--skip` vs `--exclude-domain`).
 - Config schema evolution strategy: extend `matrixai-lint-config.json` in place vs introduce a v2 file name.
-- Whether to keep using external `find` for shell file enumeration or replace with Node-based discovery for portability.
 
 ## Next actions
 

plans/refactor/questions.md

@@ -0,0 +1,3 @@
+I noticed that shellcheck run also depend on `find`. I wonder if that's a good idea, since that means `find` also must exist in the environment to run it... what do you think we should do about that?
+
+I also think the lint.ts code @/src/bin/lint.ts is werid with `mainWithDeps`, I don't quite like it. Can you rewrite that script so that only `main` exists, and then explain if in the tests you actually need it? Like can you not make use of jest mocking to do any necessary injection?

src/bin/lint.ts

@@ -83,39 +83,58 @@ function resolveDomainSelection(options: CLIOptions): {
   return { selectedDomains, shellDomainExplicitlyRequested };
 }
 
-type MainDeps = {
-  runESLint: typeof utils.runESLint;
-  findUserESLintConfig: typeof utils.findUserESLintConfig;
-  collectMarkdown: typeof utils.collectMarkdown;
-  commandExists: typeof utils.commandExists;
-  execFileSync: typeof childProcess.execFileSync;
-  fileExists: typeof fs.existsSync;
-  parseAsync: (argv: string[]) => Promise<void>;
-  opts: () => CLIOptions;
-};
-
-const defaultMainDeps: MainDeps = {
-  runESLint: utils.runESLint,
-  findUserESLintConfig: utils.findUserESLintConfig,
-  collectMarkdown: utils.collectMarkdown,
-  commandExists: utils.commandExists,
-  execFileSync: ((...args: unknown[]) =>
-    childProcess.execFileSync(
-      args[0] as string,
-      args[1] as ReadonlyArray<string> | undefined,
-      args[2] as childProcess.ExecFileSyncOptions,
-    )) as typeof childProcess.execFileSync,
-  fileExists: fs.existsSync,
-  parseAsync: async (argv) => {
-    await program.parseAsync(argv);
-  },
-  opts: () => program.opts<CLIOptions>(),
-};
+const SHELL_FILE_PATTERN = /\.sh$/i;
+
+function collectShellFiles(searchRoots: string[]): string[] {
+  const shellFiles = new Set<string>();
+
+  const visitPath = (entryPath: string): void => {
+    let entryStats: fs.Stats;
+    try {
+      entryStats = fs.statSync(entryPath);
+    } catch {
+      return;
+    }
+
+    if (entryStats.isFile()) {
+      if (SHELL_FILE_PATTERN.test(entryPath)) {
+        shellFiles.add(entryPath);
+      }
+      return;
+    }
+
+    if (!entryStats.isDirectory()) {
+      return;
+    }
+
+    let dirEntries: fs.Dirent[];
+    try {
+      dirEntries = fs.readdirSync(entryPath, { withFileTypes: true });
+    } catch {
+      return;
+    }
+
+    for (const dirEntry of dirEntries) {
+      const childPath = path.join(entryPath, dirEntry.name);
+      if (dirEntry.isDirectory()) {
+        visitPath(childPath);
+      } else if (dirEntry.isFile() && SHELL_FILE_PATTERN.test(dirEntry.name)) {
+        shellFiles.add(childPath);
+      }
+    }
+  };
+
+  for (const searchRoot of searchRoots) {
+    visitPath(searchRoot);
+  }
+
+  return [...shellFiles].sort();
+}
 
 /* eslint-disable no-console */
 async function main(argv = process.argv) {
-  await defaultMainDeps.parseAsync(argv);
-  const options = defaultMainDeps.opts();
+  await program.parseAsync(argv);
+  const options = program.opts<CLIOptions>();
 
   const fix = Boolean(options.fix);
   const useUserConfig = Boolean(options.userConfig);
@@ -135,7 +154,7 @@ async function main(argv = process.argv) {
   if (explicitConfigPath !== undefined) {
     const absolutePath = path.resolve(explicitConfigPath);
 
-    if (!defaultMainDeps.fileExists(absolutePath)) {
+    if (!fs.existsSync(absolutePath)) {
       console.error(
         `--config points to "${explicitConfigPath}", but that file does not exist.`,
       );
@@ -145,7 +164,7 @@ async function main(argv = process.argv) {
       chosenConfig = absolutePath;
     }
   } else if (useUserConfig) {
-    chosenConfig = defaultMainDeps.findUserESLintConfig();
+    chosenConfig = utils.findUserESLintConfig();
     if (chosenConfig === undefined) {
       console.error(
         '--user-config given but no local ESLint config was found. Falling back to built-in config.',
@@ -159,7 +178,7 @@ async function main(argv = process.argv) {
       hadFailure = true;
     } else {
       try {
-        const hadLintingErrors = await defaultMainDeps.runESLint({
+        const hadLintingErrors = await utils.runESLint({
           fix,
           configPath: chosenConfig,
           explicitGlobs: eslintPatterns,
@@ -176,60 +195,51 @@ async function main(argv = process.argv) {
   }
 
   if (selectedDomains.has('shell')) {
-    const hasFind = defaultMainDeps.commandExists('find');
-    const hasShellcheck = defaultMainDeps.commandExists('shellcheck');
+    const hasShellcheck = utils.commandExists('shellcheck');
 
-    if (!(hasFind && hasShellcheck)) {
+    if (!hasShellcheck) {
       if (shellDomainExplicitlyRequested) {
         console.error(
-          'Shell domain requested explicitly, but find or shellcheck was not found in environment.',
+          'Shell domain requested explicitly, but shellcheck was not found in environment.',
         );
         hadFailure = true;
       } else {
         console.warn(
-          'Skipping shellcheck: find or shellcheck not found in environment.',
+          'Skipping shellcheck: shellcheck not found in environment.',
         );
       }
     } else {
       const searchRoots = (
         shellPatterns?.length ? shellPatterns : DEFAULT_SHELLCHECK_SEARCH_ROOTS
       )
         .map((p) => path.resolve(process.cwd(), p))
-        .filter((p) => defaultMainDeps.fileExists(p));
-
-      // Linting shell scripts (this does not have auto-fixing)
-      const shellCheckArgs = [
-        ...searchRoots,
-        '-type',
-        'f',
-        '-regextype',
-        'posix-extended',
-        '-regex',
-        '.*\\.(sh)',
-        '-exec',
-        'shellcheck',
-        '{}',
-        '+',
-      ];
+        .filter((p) => fs.existsSync(p));
 
       console.error('Running shellcheck:');
       if (searchRoots.length === 0) {
         console.warn(
           'No search roots found for shellcheck. Skipping shellcheck.',
         );
       } else {
-        console.error(' ' + ['find', ...shellCheckArgs].join(' '));
-        try {
-          defaultMainDeps.execFileSync('find', shellCheckArgs, {
-            stdio: ['inherit', 'inherit', 'inherit'],
-            windowsHide: true,
-            encoding: 'utf-8',
-            shell: platform === 'win32' ? true : false,
-            cwd: process.cwd(),
-          });
-        } catch (err) {
-          console.error('Shellcheck failed. ' + err);
-          hadFailure = true;
+        const shellFiles = collectShellFiles(searchRoots);
+        if (shellFiles.length === 0) {
+          console.warn(
+            'No shell script files found for shellcheck. Skipping shellcheck.',
+          );
+        } else {
+          console.error(' ' + ['shellcheck', ...shellFiles].join(' '));
+          try {
+            childProcess.execFileSync('shellcheck', shellFiles, {
+              stdio: ['inherit', 'inherit', 'inherit'],
+              windowsHide: true,
+              encoding: 'utf-8',
+              shell: platform === 'win32',
+              cwd: process.cwd(),
+            });
+          } catch (err) {
+            console.error('Shellcheck failed. ' + err);
+            hadFailure = true;
+          }
         }
       }
     }
@@ -239,12 +249,12 @@ async function main(argv = process.argv) {
     // Linting markdown files
     // Always include README if it exists
     const markdownFiles: string[] = [];
-    if (defaultMainDeps.fileExists('README.md')) {
+    if (fs.existsSync('README.md')) {
       markdownFiles.push('README.md');
     }
     for (const dir of ['pages', 'blog', 'docs']) {
-      if (defaultMainDeps.fileExists(dir)) {
-        markdownFiles.push(...defaultMainDeps.collectMarkdown(dir));
+      if (fs.existsSync(dir)) {
+        markdownFiles.push(...utils.collectMarkdown(dir));
       }
     }
     if (markdownFiles.length === 0) {
@@ -274,7 +284,7 @@ async function main(argv = process.argv) {
       try {
         if (prettierBin) {
           console.error(` ${prettierBin} \n ${prettierArgs.join('\n' + ' ')}`);
-          defaultMainDeps.execFileSync(
+          childProcess.execFileSync(
             process.execPath,
             [prettierBin, ...prettierArgs],
             {
@@ -286,7 +296,7 @@ async function main(argv = process.argv) {
           );
         } else {
           console.error('prettier' + prettierArgs.join('\n' + ' '));
-          defaultMainDeps.execFileSync('prettier', prettierArgs, {
+          childProcess.execFileSync('prettier', prettierArgs, {
             stdio: 'inherit',
             windowsHide: true,
             encoding: 'utf-8',
@@ -314,21 +324,6 @@ async function main(argv = process.argv) {
 }
 /* eslint-enable no-console */
 
-async function mainWithDeps(
-  argv: string[],
-  deps: Partial<MainDeps>,
-): Promise<void> {
-  const previousDeps = {
-    ...defaultMainDeps,
-  };
-  Object.assign(defaultMainDeps, deps);
-  try {
-    await main(argv);
-  } finally {
-    Object.assign(defaultMainDeps, previousDeps);
-  }
-}
-
 if (import.meta.url.startsWith('file:')) {
   const modulePath = url.fileURLToPath(import.meta.url);
   if (process.argv[1] === modulePath) {
@@ -337,4 +332,3 @@ if (import.meta.url.startsWith('file:')) {
 }
 
 export default main;
-export { mainWithDeps };

tests/bin/lint.test.ts

@@ -2,7 +2,7 @@ import path from 'node:path';
 import fs from 'node:fs';
 import childProcess from 'node:child_process';
 import { jest } from '@jest/globals';
-import main, { mainWithDeps } from '#bin/lint.js';
+import main from '#bin/lint.js';
 
 describe('matrixai-lint CLI domain semantics', () => {
   let capturedExecCalls: Array<{
@@ -85,7 +85,7 @@ describe('matrixai-lint CLI domain semantics', () => {
       ]),
     ).rejects.toBeDefined();
 
-    const shellCalls = capturedExecCalls.filter((c) => c.file === 'find');
+    const shellCalls = capturedExecCalls.filter((c) => c.file === 'shellcheck');
     expect(shellCalls).toHaveLength(0);
 
     const prettierCalls = capturedExecCalls.filter(
@@ -97,16 +97,34 @@ describe('matrixai-lint CLI domain semantics', () => {
   });
 
   test('explicit shell request + missing shellcheck fails', async () => {
+    jest
+      .spyOn(childProcess, 'spawnSync')
+      .mockImplementation((file: string, args?: readonly string[]) => {
+        const commandName = args?.[0];
+        const status =
+          (file === 'which' || file === 'where') && commandName === 'shellcheck'
+            ? 1
+            : 0;
+
+        return {
+          pid: 0,
+          output: [null, null, null],
+          stdout: null,
+          stderr: null,
+          status,
+          signal: null,
+          error: undefined,
+        } as unknown as ReturnType<typeof childProcess.spawnSync>;
+      });
+
     await expect(
-      mainWithDeps(['node', 'matrixai-lint', '--shell', 'scripts'], {
-        commandExists: (cmd: string) => cmd === 'find',
-      }),
+      main(['node', 'matrixai-lint', '--shell', 'scripts']),
     ).rejects.toBeDefined();
 
-    const shellcheckFindCall = capturedExecCalls.find(
-      (c) => c.file === 'find' && c.args.includes('shellcheck'),
+    const shellcheckCall = capturedExecCalls.find(
+      (c) => c.file === 'shellcheck',
     );
-    expect(shellcheckFindCall).toBeUndefined();
+    expect(shellcheckCall).toBeUndefined();
   });
 
   test('default shell missing shellcheck warns/skips (non-fatal unless other failures)', async () => {
@@ -122,14 +140,32 @@ describe('matrixai-lint CLI domain semantics', () => {
       'utf8',
     );
 
-    await mainWithDeps(['node', 'matrixai-lint'], {
-      commandExists: (cmd: string) => cmd === 'find',
-    });
-
-    const shellcheckFindCall = capturedExecCalls.find(
-      (c) => c.file === 'find' && c.args.includes('shellcheck'),
+    jest
+      .spyOn(childProcess, 'spawnSync')
+      .mockImplementation((file: string, args?: readonly string[]) => {
+        const commandName = args?.[0];
+        const status =
+          (file === 'which' || file === 'where') && commandName === 'shellcheck'
+            ? 1
+            : 0;
+
+        return {
+          pid: 0,
+          output: [null, null, null],
+          stdout: null,
+          stderr: null,
+          status,
+          signal: null,
+          error: undefined,
+        } as unknown as ReturnType<typeof childProcess.spawnSync>;
+      });
+
+    await main(['node', 'matrixai-lint']);
+
+    const shellcheckCall = capturedExecCalls.find(
+      (c) => c.file === 'shellcheck',
     );
-    expect(shellcheckFindCall).toBeUndefined();
+    expect(shellcheckCall).toBeUndefined();
 
     const prettierCalls = capturedExecCalls.filter(
       (c) =>