Fix untracked file handling

.eslintrc.json

@@ -2,8 +2,9 @@
   "extends": ["okonet/node"],
   "rules": {
     "no-console": "off",
-    "node/no-unsupported-features": ["error", "8.12.0"],
-    "node/no-unsupported-features/es-syntax": ["error", { "version": ">=8.12.0" }],
+    "node/no-unsupported-features/node-builtins": "off",
+    "node/no-unsupported-features/es-syntax": ["error", { "version": ">=10.13.0" }],
+    "node/no-unsupported-features/es-builtins": ["error", { "version": ">=10.13.0" }],
     "prettier/prettier": "off",
     "require-atomic-updates": "off"
   }

lib/file.js

@@ -3,11 +3,26 @@
 const debug = require('debug')('lint-staged:file')
 const fs = require('fs')
 
+const fsPromises = fs.promises
+
+/**
+ * Check if a file exists. Returns the filepath if exists.
+ * @param {string} filepath
+ */
+const exists = async filepath => {
+  try {
+    await fsPromises.access(filepath)
+    return filepath
+  } catch {
+    return false
+  }
+}
+
 /**
  * @param {String} filename
  * @returns {Promise<Buffer|Null>}
  */
-module.exports.readBufferFromFile = (filename, rejectENOENT = false) =>
+const readBufferFromFile = (filename, rejectENOENT = false) =>
   new Promise(resolve => {
     debug('Reading buffer from file `%s`', filename)
     fs.readFile(filename, (error, buffer) => {
@@ -20,16 +35,34 @@ module.exports.readBufferFromFile = (filename, rejectENOENT = false) =>
     })
   })
 
+/**
+ * Unlink a file if it exists
+ * @param {*} filepath
+ */
+const unlink = async filepath => {
+  if (filepath) {
+    await fsPromises.access(filepath)
+    await fsPromises.unlink(filepath)
+  }
+}
+
 /**
  * @param {String} filename
  * @param {Buffer} buffer
  * @returns {Promise<Void>}
  */
-module.exports.writeBufferToFile = (filename, buffer) =>
+const writeBufferToFile = (filename, buffer) =>
   new Promise(resolve => {
     debug('Writing buffer to file `%s`', filename)
     fs.writeFile(filename, buffer, () => {
       debug('Done writing buffer to file `%s`!', filename)
       resolve()
     })
   })
+
+module.exports = {
+  exists,
+  readBufferFromFile,
+  unlink,
+  writeBufferToFile
+}

lib/gitWorkflow.js

@@ -4,15 +4,19 @@ const debug = require('debug')('lint-staged:git')
 const path = require('path')
 
 const execGit = require('./execGit')
-const { readBufferFromFile, writeBufferToFile } = require('./file')
+const { exists, readBufferFromFile, unlink, writeBufferToFile } = require('./file')
 
 const MERGE_HEAD = 'MERGE_HEAD'
 const MERGE_MODE = 'MERGE_MODE'
 const MERGE_MSG = 'MERGE_MSG'
 
 const STASH = 'lint-staged automatic backup'
 
-const gitApplyArgs = ['apply', '-v', '--whitespace=nowarn', '--recount', '--unidiff-zero']
+const PATCH_UNSTAGED = 'lint-staged_unstaged.patch'
+const PATCH_UNTRACKED = 'lint-staged_untracked.patch'
+
+const GIT_APPLY_ARGS = ['apply', '-v', '--whitespace=nowarn', '--recount', '--unidiff-zero']
+const GIT_DIFF_ARGS = ['--binary', '--unified=0', '--no-color', '--no-ext-diff', '--patch']
 
 /**
  * Delete untracked files using `git clean`
@@ -39,6 +43,7 @@ const handleError = (error, ctx) => {
 class GitWorkflow {
   constructor({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) {
     this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir })
+    this.gitConfigDir = gitConfigDir
     this.unstagedDiff = null
     this.allowEmpty = allowEmpty
     this.stagedFileChunks = stagedFileChunks
@@ -52,6 +57,27 @@ class GitWorkflow {
     this.mergeMsgFilename = path.resolve(gitConfigDir, MERGE_MSG)
   }
 
+  /**
+   * Get absolute path to file hidden inside .git
+   * @param {string} filename
+   */
+  getHiddenFilepath(filename) {
+    return path.resolve(this.gitConfigDir, `./${filename}`)
+  }
+
+  /**
+   * Check if patch file exists and has content.
+   * @param {string} filename
+   */
+  async hasPatch(filename) {
+    const resolved = this.getHiddenFilepath(filename)
+    const pathIfExists = await exists(resolved)
+    if (!pathIfExists) return false
+    const buffer = await readBufferFromFile(pathIfExists)
+    const patch = buffer.toString().trim()
+    return patch.length ? filename : false
+  }
+
   /**
    * Get name of backup stash
    */
@@ -121,13 +147,10 @@ class GitWorkflow {
       await cleanUntrackedFiles(this.execGit)
 
       // Get a diff of unstaged changes by diffing the saved stash against what's left on disk.
-      this.unstagedDiff = await this.execGit([
+      await this.execGit([
         'diff',
-        '--binary',
-        '--unified=0',
-        '--no-color',
-        '--no-ext-diff',
-        '--patch',
+        ...GIT_DIFF_ARGS,
+        `--output=${this.getHiddenFilepath(PATCH_UNSTAGED)}`,
         await this.getBackupStash(ctx),
         '-R' // Show diff in reverse
       ])
@@ -167,18 +190,22 @@ class GitWorkflow {
       handleError(new Error('Prevented an empty git commit!'), ctx)
     }
 
-    if (this.unstagedDiff) {
+    // Restore unstaged changes by applying the diff back. If it at first fails,
+    // this is probably because of conflicts between task modifications.
+    // 3-way merge usually fixes this, and in case it doesn't we should just give up and throw.
+    if (await this.hasPatch(PATCH_UNSTAGED)) {
       debug('Restoring unstaged changes...')
+      const unstagedPatch = this.getHiddenFilepath(PATCH_UNSTAGED)
       try {
-        await this.execGit(gitApplyArgs, { input: `${this.unstagedDiff}\n` })
+        await this.execGit([...GIT_APPLY_ARGS, unstagedPatch])
       } catch (error) {
         debug('Error while restoring changes:')
         debug(error)
         debug('Retrying with 3-way merge')
 
         try {
           // Retry with `--3way` if normal apply fails
-          await this.execGit([...gitApplyArgs, '--3way'], { input: `${this.unstagedDiff}\n` })
+          await this.execGit([...GIT_APPLY_ARGS, '--3way', unstagedPatch])
         } catch (error2) {
           debug('Error while restoring unstaged changes using 3-way merge:')
           debug(error2)
@@ -193,15 +220,24 @@ class GitWorkflow {
     }
 
     // Restore untracked files by reading from the third commit associated with the backup stash
-    // Git will return with error code if the commit doesn't exist
     // See https://stackoverflow.com/a/52357762
     try {
       const backupStash = await this.getBackupStash(ctx)
-      const output = await this.execGit(['show', '--format=%b', `${backupStash}^3`])
-      const untrackedDiff = typeof output === 'string' && output.trim() // remove empty lines from start of output
-      if (!untrackedDiff) return
-      await this.execGit([...gitApplyArgs], { input: `${untrackedDiff}\n` })
-    } catch (err) {} // eslint-disable-line no-empty
+      const untrackedPatch = this.getHiddenFilepath(PATCH_UNTRACKED)
+      await this.execGit([
+        'show',
+        ...GIT_DIFF_ARGS,
+        '--format=%b',
+        `--output=${untrackedPatch}`,
+        `${backupStash}^3`
+      ])
+      if (await this.hasPatch(PATCH_UNTRACKED)) {
+        await this.execGit([...GIT_APPLY_ARGS, untrackedPatch])
+      }
+    } catch (error) {
+      ctx.gitRestoreUntrackedError = true
+      handleError(error, ctx)
+    }
   }
 
   /**
@@ -228,6 +264,10 @@ class GitWorkflow {
   async dropBackup(ctx) {
     try {
       debug('Dropping backup stash...')
+      await Promise.all([
+        exists(this.getHiddenFilepath(PATCH_UNSTAGED)).then(unlink),
+        exists(this.getHiddenFilepath(PATCH_UNTRACKED)).then(unlink)
+      ])
       const backupStash = await this.getBackupStash(ctx)
       await this.execGit(['stash', 'drop', '--quiet', backupStash])
       debug('Done dropping backup stash!')

lib/runAll.js

@@ -57,6 +57,12 @@ module.exports = async function runAll(
   if (!files) throw new Error('Unable to get staged files!')
   debugLog('Loaded list of staged files in git:\n%O', files)
 
+  // If there are no files avoid executing any lint-staged logic
+  if (files.length === 0) {
+    logger.log('No staged files found.')
+    return 'No tasks to run.'
+  }
+
   const stagedFileChunks = chunkFiles({ files, gitDir, maxArgLength, relative })
   const chunkCount = stagedFileChunks.length
   if (chunkCount > 1) debugLog(`Chunked staged files into ${chunkCount} part`, chunkCount)

test/__snapshots__/runAll.spec.js.snap

@@ -18,7 +18,7 @@ LOG Cleaning up... [completed]"
 
 exports[`runAll should resolve the promise with no files 1`] = `
 "
-LOG No staged files match any of provided globs."
+LOG No staged files found."
 `;
 
 exports[`runAll should skip applying unstaged modifications if there are errors during linting 1`] = `
@@ -85,5 +85,5 @@ LOG {
 
 exports[`runAll should use an injected logger 1`] = `
 "
-LOG No staged files match any of provided globs."
+LOG No staged files found."
 `;

test/gitWorkflow.spec.js

@@ -5,7 +5,7 @@ import path from 'path'
 import nanoid from 'nanoid'
 
 import execGitBase from '../lib/execGit'
-import { cleanUntrackedFiles } from '../lib/gitWorkflow'
+import GitWorkflow, { cleanUntrackedFiles } from '../lib/gitWorkflow'
 
 jest.unmock('execa')
 
@@ -74,4 +74,31 @@ describe('gitWorkflow', () => {
       expect(await fs.exists(testFile)).toEqual(false)
     })
   })
+
+  describe('hasPatch', () => {
+    it('should return false when patch file not found', async () => {
+      const gitWorkflow = new GitWorkflow({
+        gitDir: cwd,
+        gitConfigDir: path.resolve(cwd, './.git')
+      })
+      expect(await gitWorkflow.hasPatch('foo')).toEqual(false)
+    })
+  })
+
+  describe('dropBackup', () => {
+    it('should handle errors', async () => {
+      const gitWorkflow = new GitWorkflow({
+        gitDir: cwd,
+        gitConfigDir: path.resolve(cwd, './.git')
+      })
+      const ctx = {}
+      await expect(gitWorkflow.dropBackup(ctx)).rejects.toThrowErrorMatchingInlineSnapshot(
+        `"lint-staged automatic backup is missing!"`
+      )
+      expect(ctx).toEqual({
+        gitError: true,
+        gitGetBackupStashError: true
+      })
+    })
+  })
 })

test/runAll.unmocked.spec.js

@@ -82,11 +82,6 @@ describe('runAll', () => {
     )
     await removeTempDir(nonGitDir)
   })
-
-  it('should short-circuit with no staged files', async () => {
-    const status = await runAll({ config: { '*.js': 'echo success' }, cwd })
-    expect(status).toEqual('No tasks to run.')
-  })
 })
 
 const globalConsoleTemp = console
@@ -119,6 +114,11 @@ describe('runAll', () => {
     console = globalConsoleTemp
   })
 
+  it('should exit early with no staged files', async () => {
+    const status = await runAll({ config: { '*.js': 'echo success' }, cwd })
+    expect(status).toEqual('No tasks to run.')
+  })
+
   it('Should commit entire staged file when no errors from linter', async () => {
     // Stage pretty file
     await appendFile('test file.js', testJsFilePretty)
@@ -555,8 +555,10 @@ describe('runAll', () => {
     await appendFile('test.js', testJsFilePretty)
     await execGit(['add', 'test.js'])
 
-    // Add another file, but keep it untracked
+    // Add untracked files
     await appendFile('test-untracked.js', testJsFilePretty)
+    await appendFile('.gitattributes', 'binary\n')
+    await writeFile('binary', Buffer.from('Hello, World!', 'binary'))
 
     // Run lint-staged with `prettier --list-different` and commit pretty file
     await gitCommit({ config: { '*.js': 'prettier --list-different' } })
@@ -566,6 +568,7 @@ describe('runAll', () => {
     expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test')
     expect(await readFile('test.js')).toEqual(testJsFilePretty)
     expect(await readFile('test-untracked.js')).toEqual(testJsFilePretty)
+    expect(Buffer.from(await readFile('binary'), 'binary').toString()).toEqual('Hello, World!')
   })
 
   it('should work when amending previous commit with unstaged changes', async () => {
@@ -611,20 +614,20 @@ describe('runAll', () => {
   })
 
   it('should handle binary files', async () => {
-    // mark test.js as binary file
-    await appendFile('.gitattributes', 'test.js binary\n')
+    // mark file as binary
+    await appendFile('.gitattributes', 'binary\n')
 
     // Stage pretty file
-    await appendFile('test.js', testJsFilePretty)
-    await execGit(['add', 'test.js'])
+    await writeFile('binary', Buffer.from('Hello, World!', 'binary'))
+    await execGit(['add', 'binary'])
 
     // Run lint-staged with `prettier --list-different` and commit pretty file
     await gitCommit({ config: { '*.js': 'prettier --list-different' } })
 
     // Nothing is wrong, so a new commit is created
     expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('2')
     expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test')
-    expect(await readFile('test.js')).toEqual(testJsFilePretty)
+    expect(Buffer.from(await readFile('binary'), 'binary').toString()).toEqual('Hello, World!')
   })
 
   it('should run chunked tasks when necessary', async () => {
@@ -670,10 +673,11 @@ describe('runAll', () => {
       LOG Running tasks for *.js [completed]
       LOG Running tasks... [completed]
       LOG Applying modifications... [started]
-      LOG Applying modifications... [completed]
+      LOG Applying modifications... [failed]
+      LOG → lint-staged automatic backup is missing!
       LOG Cleaning up... [started]
-      LOG Cleaning up... [failed]
-      LOG → lint-staged automatic backup is missing!"
+      LOG Cleaning up... [skipped]
+      LOG → Skipped because of previous git error."
     `)
   })