Merge pull request #772 from okonet/fixes

Fix a bunch of v10 issues

.lintstagedrc.json

@@ -1,5 +1,4 @@
 {
   "*.{js,json,md}": "prettier --write",
-  "*.js": "npm run lint:base --fix",
-  ".*{rc, json}": "jsonlint  --in-place"
+  "*.js": "npm run lint:base -- --fix"
 }

README.md

@@ -39,6 +39,19 @@ See [examples](#examples) and [configuration](#configuration) for more informati
 
 See [Releases](https://github.com/okonet/lint-staged/releases)
 
+### Migration
+
+#### v10
+
+- From `v10.0.0` onwards any new modifications to originally staged files will be automatically added to the commit.
+  If your task previously contained a `git add` step, please remove this.
+  The automatic behaviour ensures there are less race-conditions,
+  since trying to run multiple git operations at the same time usually results in an error.
+- From `v10.0.0` onwards _lint-staged_ uses git stashes to improve speed and provide backups while running.
+  Since git stashes require at least an initial commit, you shouldn't run _lint-staged_ in an empty repo.
+- From `v10.0.0` onwards _lint-staged_ requires Node.js version 10.13.0 or later.
+- From `v10.0.0` onwards _lint-staged_ will abort the commit if linter tasks undo all staged changes. To allow creating empty commit, please use the `--allow-empty` option.
+
 ## Command line flags
 
 ```bash
@@ -47,7 +60,7 @@ Usage: lint-staged [options]
 
 Options:
   -V, --version                      output the version number
-  --allow-empty                      allow empty commits when tasks revert all staged changes (default: false)
+  --allow-empty                      allow empty commits when tasks undo all staged changes (default: false)
   -c, --config [path]                path to configuration file
   -d, --debug                        print additional debug information (default: false)
   -p, --concurrent <parallel tasks>  the number of tasks to run concurrently, or false to run tasks serially (default: true)
@@ -57,7 +70,7 @@ Options:
   -h, --help                         output usage information
 ```
 
-- **`--allow-empty`**: By default, when after running tasks there are no staged modifications, lint-staged will exit with an error and abort the commit. Use this flag to allow creating empty git commits.
+- **`--allow-empty`**: By default, when linter tasks undo all staged changes, lint-staged will exit with an error and abort the commit. Use this flag to allow creating empty git commits.
 - **`--config [path]`**: Manually specify a path to a config file or npm package name. Note: when used, lint-staged won't perform the config file search and print an error if the specified file cannot be found.
 - **`--debug`**: Run in debug mode. When set, it does the following:
   - uses [debug](https://github.com/visionmedia/debug) internally to log additional information about staged files, commands being executed, location of binaries, etc. Debug logs, which are automatically enabled by passing the flag, can also be enabled by setting the environment variable `$DEBUG` to `lint-staged*`.
@@ -499,6 +512,7 @@ const { CLIEngine } = require('eslint')
 const cli = new CLIEngine({})
 
 module.exports = {
-  '*.js': files => 'eslint --max-warnings=0 ' + files.filter( file => ! cli.isPathIgnored( file ) ).join( ' ' )
+  '*.js': files =>
+    'eslint --max-warnings=0 ' + files.filter(file => !cli.isPathIgnored(file)).join(' ')
 }
 ```

lib/gitWorkflow.js

@@ -111,7 +111,7 @@ class GitWorkflow {
 
       // Save stash of entire original state, including unstaged and untracked changes.
       // `--keep-index leaves only staged files on disk, for tasks.`
-      await this.execGit(['stash', 'save', '--quiet', '--include-untracked', '--keep-index', STASH])
+      await this.execGit(['stash', 'save', '--include-untracked', '--keep-index', STASH])
 
       // Restore meta information about ongoing git merge
       await this.restoreMergeStatus()
@@ -134,6 +134,9 @@ class GitWorkflow {
 
       debug('Done backing up original state!')
     } catch (error) {
+      if (error.message && error.message.includes('You do not have the initial commit yet')) {
+        ctx.emptyGitRepo = true
+      }
       handleError(error, ctx)
     }
   }

lib/resolveTaskFn.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const chalk = require('chalk')
+const { parseArgsStringToArgv } = require('string-argv')
 const dedent = require('dedent')
 const execa = require('execa')
 const symbols = require('log-symbols')
@@ -66,26 +67,30 @@ function makeErr(linter, result, context = {}) {
  * @returns {function(): Promise<Array<string>>}
  */
 module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) {
-  const cmd = isFn ? command : `${command} ${files.join(' ')}`
+  const [cmd, ...args] = parseArgsStringToArgv(command)
   debug('cmd:', cmd)
+  debug('args:', args)
 
   const execaOptions = { preferLocal: true, reject: false, shell }
   if (relative) {
     execaOptions.cwd = process.cwd()
-  } else if (/^git(\.exe)?/i.test(command) && gitDir !== process.cwd()) {
+  } else if (/^git(\.exe)?/i.test(cmd) && gitDir !== process.cwd()) {
     // Only use gitDir as CWD if we are using the git binary
     // e.g `npm` should run tasks in the actual CWD
     execaOptions.cwd = gitDir
   }
   debug('execaOptions:', execaOptions)
 
   return async ctx => {
-    const result = await execa.command(cmd, execaOptions)
+    const promise = shell
+      ? execa.command(isFn ? command : `${command} ${files.join(' ')}`, execaOptions)
+      : execa(cmd, isFn ? args : args.concat(files), execaOptions)
+    const result = await promise
 
     if (result.failed || result.killed || result.signal != null) {
-      throw makeErr(command, result, ctx)
+      throw makeErr(cmd, result, ctx)
     }
 
-    return successMsg(command)
+    return successMsg(cmd)
   }
 }

lib/runAll.js

@@ -85,7 +85,7 @@ module.exports = async function runAll(
         shell
       })
 
-      if (subTasks.some(subTask => subTask.command.includes('git add'))) {
+      if (subTasks.some(subTask => subTask.command === 'git add')) {
         hasDeprecatedGitAdd = true
       }
 
@@ -187,19 +187,22 @@ module.exports = async function runAll(
   ${symbols.warning} ${chalk.yellow(`lint-staged prevented an empty git commit.
     Use the --allow-empty option to continue, or check your task configuration`)}
 `)
-    }
-
-    // Show help text about manual restore in case of git errors.
-    // No sense to show this if the backup stash itself is missing.
-    else if (error.context.gitError && !error.context.gitGetBackupStashError) {
-      logger.error(`
-  ${symbols.error} ${chalk.red(`lint-staged failed due to a git error.
-    Any lost modifications can be restored from a git stash:
+    } else if (error.context.gitError && !error.context.gitGetBackupStashError) {
+      logger.error(`\n  ${symbols.error} ${chalk.red(`lint-staged failed due to a git error.`)}`)
+
+      if (error.context.emptyGitRepo) {
+        logger.error(
+          `\n    The initial commit is needed for lint-staged to work.
+    Please use the --no-verify flag to skip running lint-staged.`
+        )
+      } else {
+        // No sense to show this if the backup stash itself is missing.
+        logger.error(`  Any lost modifications can be restored from a git stash:
 
     > git stash list
     stash@{0}: On master: automatic lint-staged backup
-    > git stash pop stash@{0}`)}
-`)
+    > git stash pop stash@{0}\n`)
+      }
     }
 
     throw error

package.json

@@ -45,6 +45,7 @@
     "micromatch": "^4.0.2",
     "normalize-path": "^3.0.0",
     "please-upgrade-node": "^3.2.0",
+    "string-argv": "0.3.1",
     "stringify-object": "^3.3.0"
   },
   "devDependencies": {
@@ -67,7 +68,6 @@
     "husky": "^3.1.0",
     "jest": "^24.9.0",
     "jest-snapshot-serializer-ansi": "^1.0.0",
-    "jsonlint": "^1.6.3",
     "nanoid": "^2.1.7",
     "prettier": "^1.19.1"
   },

test/__snapshots__/index.spec.js.snap

@@ -5,7 +5,7 @@ exports[`lintStaged should exit with code 1 on linter errors 1`] = `
 ERROR 
 
 
-× node -e \\"process.exit(1)\\" found some errors. Please fix them and try committing again."
+× node found some errors. Please fix them and try committing again."
 `;
 
 exports[`lintStaged should load an npm config package when specified 1`] = `

test/__snapshots__/runAll.spec.js.snap

@@ -44,7 +44,7 @@ LOG {
   name: 'ListrError',
   errors: [
     {
-      privateMsg: '\\\\n\\\\n\\\\n× echo \\"sample\\" found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
+      privateMsg: '\\\\n\\\\n\\\\n× echo found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
       context: {taskError: true}
     }
   ],
@@ -75,7 +75,7 @@ LOG {
   name: 'ListrError',
   errors: [
     {
-      privateMsg: '\\\\n\\\\n\\\\n‼ echo \\"sample\\" was terminated with SIGINT',
+      privateMsg: '\\\\n\\\\n\\\\n‼ echo was terminated with SIGINT',
       context: {taskError: true}
     }
   ],

test/makeCmdTasks.spec.js

@@ -42,7 +42,7 @@ describe('makeCmdTasks', () => {
     expect(taskPromise).toBeInstanceOf(Promise)
     await taskPromise
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('test test.js', {
+    expect(execa).lastCalledWith('test', ['test.js'], {
       preferLocal: true,
       reject: false,
       shell: false
@@ -51,7 +51,7 @@ describe('makeCmdTasks', () => {
     expect(taskPromise).toBeInstanceOf(Promise)
     await taskPromise
     expect(execa).toHaveBeenCalledTimes(2)
-    expect(execa).lastCalledWith('test2 test.js', {
+    expect(execa).lastCalledWith('test2', ['test.js'], {
       preferLocal: true,
       reject: false,
       shell: false

test/resolveTaskFn.spec.js

@@ -18,7 +18,7 @@ describe('resolveTaskFn', () => {
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
+    expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
       preferLocal: true,
       reject: false,
       shell: false
@@ -35,7 +35,7 @@ describe('resolveTaskFn', () => {
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
+    expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
       preferLocal: true,
       reject: false,
       shell: false
@@ -87,7 +87,7 @@ describe('resolveTaskFn', () => {
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('git diff test.js', {
+    expect(execa).lastCalledWith('git', ['diff', 'test.js'], {
       cwd: '../',
       preferLocal: true,
       reject: false,
@@ -101,7 +101,7 @@ describe('resolveTaskFn', () => {
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('jest test.js', {
+    expect(execa).lastCalledWith('jest', ['test.js'], {
       preferLocal: true,
       reject: false,
       shell: false
@@ -118,7 +118,7 @@ describe('resolveTaskFn', () => {
 
     await taskFn()
     expect(execa).toHaveBeenCalledTimes(1)
-    expect(execa).lastCalledWith('git diff test.js', {
+    expect(execa).lastCalledWith('git', ['diff', 'test.js'], {
       cwd: process.cwd(),
       preferLocal: true,
       reject: false,

test/resolveTaskFn.unmocked.spec.js

@@ -11,8 +11,6 @@ describe('resolveTaskFn', () => {
       shell: true
     })
 
-    await expect(taskFn()).resolves.toMatchInlineSnapshot(
-      `"√ node -e \\"process.exit(1)\\" || echo $? passed!"`
-    )
+    await expect(taskFn()).resolves.toMatchInlineSnapshot(`"√ node passed!"`)
   })
 })

test/runAll.unmocked.2.spec.js

@@ -120,7 +120,7 @@ describe('runAll', () => {
       LOG → Skipped because of previous git error.
       ERROR 
         × lint-staged failed due to a git error.
-          Any lost modifications can be restored from a git stash:
+      ERROR   Any lost modifications can be restored from a git stash:
 
           > git stash list
           stash@{0}: On master: automatic lint-staged backup

test/runAll.unmocked.spec.js

@@ -121,16 +121,16 @@ describe('runAll', () => {
 
   it('Should commit entire staged file when no errors from linter', async () => {
     // Stage pretty file
-    await appendFile('test.js', testJsFilePretty)
-    await execGit(['add', 'test.js'])
+    await appendFile('test file.js', testJsFilePretty)
+    await execGit(['add', 'test file.js'])
 
     // 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(await readFile('test file.js')).toEqual(testJsFilePretty)
   })
 
   it('Should commit entire staged file when no errors and linter modifies file', async () => {
@@ -381,11 +381,9 @@ describe('runAll', () => {
     ).rejects.toThrowError()
     expect(console.printHistory()).toMatchInlineSnapshot(`
       "
-      WARN ‼ Some of your tasks use \`git add\` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index.
-
       ERROR 
         × lint-staged failed due to a git error.
-          Any lost modifications can be restored from a git stash:
+      ERROR   Any lost modifications can be restored from a git stash:
 
           > git stash list
           stash@{0}: On master: automatic lint-staged backup
@@ -693,12 +691,15 @@ describe('runAll', () => {
 
     // Run lint-staged with prettier --write to automatically fix the file
     // Since prettier reverts all changes, the commit should fail
+    // use the old syntax with manual `git add` to provide a warning message
     await expect(
-      gitCommit({ config: { '*.js': 'prettier --write' } })
+      gitCommit({ config: { '*.js': ['prettier --write', 'git add'] } })
     ).rejects.toThrowErrorMatchingInlineSnapshot(`"Something went wrong"`)
 
     expect(console.printHistory()).toMatchInlineSnapshot(`
       "
+      WARN ‼ Some of your tasks use \`git add\` command. Please remove it from the config since all modifications made by tasks will be automatically added to the git commit index.
+
       WARN 
         ‼ lint-staged prevented an empty git commit.
           Use the --allow-empty option to continue, or check your task configuration
@@ -773,3 +774,28 @@ describe('runAll', () => {
     expect(await readFile('test.js', submoduleDir)).toEqual(testJsFilePretty)
   })
 })
+
+describe('runAll', () => {
+  it('Should throw when run on an empty git repo without an initial commit', async () => {
+    const tmpDir = await createTempDir()
+    const cwd = normalize(tmpDir)
+    const logger = makeConsoleMock()
+
+    await execGit('init', { cwd })
+    await execGit(['config', 'user.name', '"test"'], { cwd })
+    await execGit(['config', 'user.email', '"test@test.com"'], { cwd })
+    await appendFile('test.js', testJsFilePretty, cwd)
+    await execGit(['add', 'test.js'], { cwd })
+    await expect(
+      runAll({ config: { '*.js': 'prettier --list-different' }, cwd, quiet: true }, logger)
+    ).rejects.toThrowErrorMatchingInlineSnapshot(`"Something went wrong"`)
+    expect(logger.printHistory()).toMatchInlineSnapshot(`
+      "
+      ERROR 
+        × lint-staged failed due to a git error.
+      ERROR 
+          The initial commit is needed for lint-staged to work.
+          Please use the --no-verify flag to skip running lint-staged."
+    `)
+  })
+})