Repository: okonet/lint-staged
Pull request #1106 (2022-02-15 07:38:41)

fix: search all configs regardless of staged files
This PR searches for all configurations inside the git repo, instead of only those matching the staged files. It's currently only used for the `hasMultipleConfigs` check, but in the future it will also be used to fix "parent directory" globs, which are currently not working.

Without this fix, `hasMultipleConfigs` would be `false` if you only staged files matching a single config, and thus the CWD would be wrong.

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov) Report
> Merging [#1106](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov) (0d5f1c0) into [master](https://codecov.io/gh/okonet/lint-staged/commit/339515010ccd95a2f952dbe65f8366463f94d26a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov) (3395150) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/1106/graphs/tree.svg?width=650&height=150&src=pr&token=LSoEgsjk4O&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov)](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov)

```diff
@@            Coverage Diff            @@
##            master     #1106   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        24    +1     
  Lines          653       675   +22     
  Branches       173       176    +3     
=========================================
+ Hits           653       675   +22     
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov) | Coverage Δ | |
|---|---|---|
| [lib/loadConfig.js](https://codecov.io/gh/okonet/lint-staged/pull/1106/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov#diff-bGliL2xvYWRDb25maWcuanM=) | `100.00% <100.00%> (ø)` | |
| [lib/runAll.js](https://codecov.io/gh/okonet/lint-staged/pull/1106/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov#diff-bGliL3J1bkFsbC5qcw==) | `100.00% <100.00%> (ø)` | |
| [lib/searchConfigs.js](https://codecov.io/gh/okonet/lint-staged/pull/1106/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov#diff-bGliL3NlYXJjaENvbmZpZ3MuanM=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov). Last update [3395150...0d5f1c0](https://codecov.io/gh/okonet/lint-staged/pull/1106?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andrey+Okonetchnikov).

iiroj: Need to add some more tests:
```
--------------------|---------|----------|---------|---------|-------------------
File                | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------|---------|----------|---------|---------|-------------------
All files           |     100 |    99.68 |     100 |     100 |
 searchConfigs.js   |     100 |       80 |     100 |     100 | 26                
```
iiroj: What do you think about this, @okonet ?
iiroj: Ping @okonet, do you think you'd have time for checking this out? I haven't really had time to take it further either, but merging this would fix the most clear issue we have currently in monorepo setups.
iiroj: I hope you are well @okonet.

In the meantime, if someone from the community wants to open this exact same pull request, I am able to approve and release it. Unfortunately with the current repo settings, I'm not able to self-approve my own changes.

Patch:
diff --git a/lib/getConfigGroups.js b/lib/getConfigGroups.js
index db1ce14..eff2460 100644
--- a/lib/getConfigGroups.js
+++ b/lib/getConfigGroups.js
@@ -99,13 +99,6 @@ export const getConfigGroups = async (
   // Discover configs from the base directory of each file
   await Promise.all(Object.entries(filesByDir).map(([dir, files]) => searchConfig(dir, files)))
 
-  // Throw if no configurations were found
-  if (Object.keys(configGroups).length === 0) {
-    debugLog('Found no config groups!')
-    logger.error(`${ConfigNotFoundError.message}.`)
-    throw ConfigNotFoundError
-  }
-
   debugLog('Grouped staged files into %d groups!', Object.keys(configGroups).length)
 
   return configGroups
diff --git a/lib/getStagedFiles.js b/lib/getStagedFiles.js
index a6a5573..363e07a 100644
--- a/lib/getStagedFiles.js
+++ b/lib/getStagedFiles.js
@@ -3,6 +3,7 @@ import path from 'path'
 import normalize from 'normalize-path'
 
 import { execGit } from './execGit.js'
+import { parseGitZOutput } from './parseGitZOutput.js'
 
 export const getStagedFiles = async ({ cwd = process.cwd() } = {}) => {
   try {
@@ -14,15 +15,7 @@ export const getStagedFiles = async ({ cwd = process.cwd() } = {}) => {
 
     if (!lines) return []
 
-    // With `-z`, git prints `fileA\u0000fileB\u0000fileC\u0000` so we need to
-    // remove the last occurrence of `\u0000` before splitting
-    return (
-      lines
-        // eslint-disable-next-line no-control-regex
-        .replace(/\u0000$/, '')
-        .split('\u0000')
-        .map((file) => normalize(path.resolve(cwd, file)))
-    )
+    return parseGitZOutput(lines).map((file) => normalize(path.resolve(cwd, file)))
   } catch {
     return null
   }
diff --git a/lib/loadConfig.js b/lib/loadConfig.js
index 78fa3c7..43e281e 100644
--- a/lib/loadConfig.js
+++ b/lib/loadConfig.js
@@ -13,7 +13,7 @@ const debugLog = debug('lint-staged:loadConfig')
  * The list of files `lint-staged` will read configuration
  * from, in the declared order.
  */
-const searchPlaces = [
+export const searchPlaces = [
   'package.json',
   '.lintstagedrc',
   '.lintstagedrc.json',
diff --git a/lib/parseGitZOutput.js b/lib/parseGitZOutput.js
new file mode 100644
index 0000000..931e163
--- /dev/null
+++ b/lib/parseGitZOutput.js
@@ -0,0 +1,9 @@
+/**
+ * Return array of strings split from the output of `git <something> -z`.
+ * With `-z`, git prints `fileA\u0000fileB\u0000fileC\u0000` so we need to
+ * remove the last occurrence of `\u0000` before splitting
+ */
+export const parseGitZOutput = (input) =>
+  input
+    .replace(/\u0000$/, '') // eslint-disable-line no-control-regex
+    .split('\u0000')
diff --git a/lib/runAll.js b/lib/runAll.js
index 1e28263..db2d1c8 100644
--- a/lib/runAll.js
+++ b/lib/runAll.js
@@ -35,7 +35,8 @@ import {
   restoreOriginalStateSkipped,
   restoreUnstagedChangesSkipped,
 } from './state.js'
-import { GitRepoError, GetStagedFilesError, GitError } from './symbols.js'
+import { GitRepoError, GetStagedFilesError, GitError, ConfigNotFoundError } from './symbols.js'
+import { searchConfigs } from './searchConfigs.js'
 
 const debugLog = debug('lint-staged:runAll')
 
@@ -121,7 +122,19 @@ export const runAll = async (
 
   const configGroups = await getConfigGroups({ configObject, configPath, cwd, files }, logger)
 
-  const hasMultipleConfigs = Object.keys(configGroups).length > 1
+  const hasExplicitConfig = configObject || configPath
+  const foundConfigs = hasExplicitConfig ? null : await searchConfigs(gitDir, logger)
+  const numberOfConfigs = hasExplicitConfig ? 1 : Object.keys(foundConfigs).length
+
+  // Throw if no configurations were found
+  if (numberOfConfigs === 0) {
+    ctx.errors.add(ConfigNotFoundError)
+    throw createError(ctx, ConfigNotFoundError)
+  }
+
+  debugLog('Found %d configs:\n%O', numberOfConfigs, foundConfigs)
+
+  const hasMultipleConfigs = numberOfConfigs > 1
 
   // lint-staged 10 will automatically add modifications to index
   // Warn user when their command includes `git add`
diff --git a/lib/searchConfigs.js b/lib/searchConfigs.js
new file mode 100644
index 0000000..bfca1c4
--- /dev/null
+++ b/lib/searchConfigs.js
@@ -0,0 +1,69 @@
+/** @typedef {import('./index').Logger} Logger */
+
+import { basename, join } from 'path'
+
+import normalize from 'normalize-path'
+
+import { execGit } from './execGit.js'
+import { loadConfig, searchPlaces } from './loadConfig.js'
+import { parseGitZOutput } from './parseGitZOutput.js'
+import { validateConfig } from './validateConfig.js'
+
+const EXEC_GIT = ['ls-files', '-z', '--full-name']
+
+const filterPossibleConfigFiles = (file) => searchPlaces.includes(basename(file))
+
+const numberOfLevels = (file) => file.split('/').length
+
+const sortDeepestParth = (a, b) => (numberOfLevels(a) > numberOfLevels(b) ? -1 : 1)
+
+/**
+ * Search all config files from the git repository
+ *
+ * @param {string} gitDir
+ * @param {Logger} logger
+ * @returns {Promise<{ [key: string]: * }>} found configs with filepath as key, and config as value
+ */
+export const searchConfigs = async (gitDir = process.cwd(), logger) => {
+  /** Get all possible config files known to git */
+  const cachedFiles = parseGitZOutput(await execGit(EXEC_GIT, { cwd: gitDir })).filter(
+    filterPossibleConfigFiles
+  )
+
+  /** Get all possible config files from uncommitted files */
+  const otherFiles = parseGitZOutput(
+    await execGit([...EXEC_GIT, '--others', '--exclude-standard'], { cwd: gitDir })
+  ).filter(filterPossibleConfigFiles)
+
+  /** Sort possible config files so that deepest is first */
+  const possibleConfigFiles = [...cachedFiles, ...otherFiles]
+    .map((file) => join(gitDir, file))
+    .map((file) => normalize(file))
+    .sort(sortDeepestParth)
+
+  /** Create object with key as config file, and value as null */
+  const configs = possibleConfigFiles.reduce(
+    (acc, configPath) => Object.assign(acc, { [configPath]: null }),
+    {}
+  )
+
+  /** Load and validate all configs to the above object */
+  await Promise.all(
+    possibleConfigFiles
+      .map((configPath) => loadConfig({ configPath }, logger))
+      .map((promise) =>
+        promise.then(({ config, filepath }) => {
+          if (config) {
+            configs[filepath] = validateConfig(config, filepath, logger)
+          }
+        })
+      )
+  )
+
+  /** Get validated configs from the above object, without any `null` values (not found) */
+  const foundConfigs = Object.entries(configs)
+    .filter(([, value]) => !!value)
+    .reduce((acc, [key, value]) => ({ ...acc, [key]: value }), {})
+
+  return foundConfigs
+}


Test patch:
diff --git a/test/getConfigGroups.spec.js b/test/getConfigGroups.spec.js
index 6f1cedd..70ada8e 100644
--- a/test/getConfigGroups.spec.js
+++ b/test/getConfigGroups.spec.js
@@ -30,12 +30,6 @@ describe('getConfigGroups', () => {
     )
   })
 
-  it('should throw when config not found', async () => {
-    await expect(
-      getConfigGroups({ files: ['/foo.js'] })
-    ).rejects.toThrowErrorMatchingInlineSnapshot(`"Configuration could not be found"`)
-  })
-
   it('should find config files for all staged files', async () => {
     // Base cwd
     loadConfig.mockResolvedValueOnce({ config, filepath: '/.lintstagedrc.json' })
diff --git a/test/parseGitZOutput.spec.js b/test/parseGitZOutput.spec.js
new file mode 100644
index 0000000..772921a
--- /dev/null
+++ b/test/parseGitZOutput.spec.js
@@ -0,0 +1,13 @@
+import { parseGitZOutput } from '../lib/parseGitZOutput'
+
+describe('parseGitZOutput', () => {
+  it('should split string from `git -z` control character', () => {
+    const input = 'a\u0000b\u0000c'
+    expect(parseGitZOutput(input)).toEqual(['a', 'b', 'c'])
+  })
+
+  it('should remove trailing `git -z` control character', () => {
+    const input = 'a\u0000'
+    expect(parseGitZOutput(input)).toEqual(['a'])
+  })
+})
diff --git a/test/runAll.spec.js b/test/runAll.spec.js
index 01168ec..84fa9c4 100644
--- a/test/runAll.spec.js
+++ b/test/runAll.spec.js
@@ -8,7 +8,8 @@ import { getStagedFiles } from '../lib/getStagedFiles'
 import { GitWorkflow } from '../lib/gitWorkflow'
 import { resolveGitRepo } from '../lib/resolveGitRepo'
 import { runAll } from '../lib/runAll'
-import { GitError } from '../lib/symbols'
+import { ConfigNotFoundError, GitError } from '../lib/symbols'
+import * as searchConfigsNS from '../lib/searchConfigs'
 import * as getConfigGroupsNS from '../lib/getConfigGroups'
 
 jest.mock('../lib/file')
@@ -27,6 +28,7 @@ jest.mock('../lib/resolveConfig', () => ({
   },
 }))
 
+const searchConfigs = jest.spyOn(searchConfigsNS, 'searchConfigs')
 const getConfigGroups = jest.spyOn(getConfigGroupsNS, 'getConfigGroups')
 
 getStagedFiles.mockImplementation(async () => [])
@@ -277,17 +279,18 @@ describe('runAll', () => {
     const cwd = process.cwd()
     // For the test, set cwd in test/
     const innerCwd = path.join(cwd, 'test/')
-    try {
-      // Run lint-staged in `innerCwd` with relative option
-      // This means the sample task will receive `foo.js`
-      await runAll({
+
+    // Run lint-staged in `innerCwd` with relative option
+    // This means the sample task will receive `foo.js`
+    await expect(
+      runAll({
         configObject: { '*.js': mockTask },
         configPath,
         stash: false,
         relative: true,
         cwd: innerCwd,
       })
-    } catch {} // eslint-disable-line no-empty
+    ).rejects.toThrowError()
 
     // task received relative `foo.js`
     expect(mockTask).toHaveBeenCalledTimes(1)
@@ -313,17 +316,22 @@ describe('runAll', () => {
       },
     })
 
+    searchConfigs.mockResolvedValueOnce({
+      '.lintstagedrc.json': { '*.js': mockTask },
+      'test/.lintstagedrc.json': { '*.js': mockTask },
+    })
+
     // We are only interested in the `matchedFileChunks` generation
     let expected
     const mockConstructor = jest.fn(({ matchedFileChunks }) => (expected = matchedFileChunks))
     GitWorkflow.mockImplementationOnce(mockConstructor)
 
-    try {
-      await runAll({
+    await expect(
+      runAll({
         stash: false,
         relative: true,
       })
-    } catch {} // eslint-disable-line no-empty
+    ).rejects.toThrowError()
 
     // task received relative `foo.js` from both directories
     expect(mockTask).toHaveBeenCalledTimes(2)
@@ -355,17 +363,42 @@ describe('runAll', () => {
       },
     })
 
-    try {
-      await runAll({
+    searchConfigs.mockResolvedValueOnce({
+      '.lintstagedrc.json': { '*.js': mockTask },
+      'test/.lintstagedrc.json': { '*.js': mockTask },
+    })
+
+    await expect(
+      runAll({
         cwd: '.',
         stash: false,
         relative: true,
       })
-    } catch {} // eslint-disable-line no-empty
+    ).rejects.toThrowError()
 
     expect(mockTask).toHaveBeenCalledTimes(2)
     expect(mockTask).toHaveBeenNthCalledWith(1, ['foo.js'])
     // This is now relative to "." instead of "test/"
     expect(mockTask).toHaveBeenNthCalledWith(2, ['test/foo.js'])
   })
+
+  it('should error when no configurations found', async () => {
+    getStagedFiles.mockImplementationOnce(async () => ['foo.js', 'test/foo.js'])
+
+    getConfigGroups.mockResolvedValueOnce({})
+
+    searchConfigs.mockResolvedValueOnce({})
+
+    expect.assertions(1)
+
+    try {
+      await runAll({
+        cwd: '.',
+        stash: false,
+        relative: true,
+      })
+    } catch ({ ctx }) {
+      expect(ctx.errors.has(ConfigNotFoundError)).toBe(true)
+    }
+  })
 })