Repository: okonet/lint-staged
Pull request #762 (2020-01-08 18:52:55)

feat: throw error to prevent empty commits unless --allow-empty is used
BREAKING CHANGE: Previously, lint-staged would allow empty commits in the situation where a linter task like "prettier --write" reverts all staged changes automatically. Now the default behaviour is to throw an error with a helpful warning message. The --allow empty option can be used to allow empty commits, or `allowEmpty: true` for the Node.js API.

What do you think, @okonet?

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=h1) Report
> Merging [#762](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=desc) into [beta](https://codecov.io/gh/okonet/lint-staged/commit/30b480925a313f5c2b614eb40eb1a340a6cefae5?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/762/graphs/tree.svg?width=650&token=LSoEgsjk4O&height=150&src=pr)](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=tree)

```diff
@@         Coverage Diff         @@
##           beta   #762   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files        13     13           
  Lines       417    429   +12     
  Branches     90     97    +7     
===================================
+ Hits        417    429   +12
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [lib/runAll.js](https://codecov.io/gh/okonet/lint-staged/pull/762/diff?src=pr&el=tree#diff-bGliL3J1bkFsbC5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [lib/gitWorkflow.js](https://codecov.io/gh/okonet/lint-staged/pull/762/diff?src=pr&el=tree#diff-bGliL2dpdFdvcmtmbG93Lmpz) | `100% <100%> (ø)` | :arrow_up: |
| [lib/index.js](https://codecov.io/gh/okonet/lint-staged/pull/762/diff?src=pr&el=tree#diff-bGliL2luZGV4Lmpz) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=footer). Last update [30b4809...fa2cb8a](https://codecov.io/gh/okonet/lint-staged/pull/762?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index 82f1d65..781087e 100644
--- a/README.md
+++ b/README.md
@@ -42,31 +42,34 @@ See [Releases](https://github.com/okonet/lint-staged/releases)
 ## Command line flags
 
 ```bash
-$ npx lint-staged --help
+❯ npx lint-staged --help
 Usage: lint-staged [options]
 
 Options:
-  -V, --version                     output the version number
-  -c, --config [path]               Path to configuration file
-  -r, --relative                    Pass relative filepaths to tasks
-  -x, --shell                       Skip parsing of tasks for better shell support
-  -q, --quiet                       Disable lint-staged’s own console output
-  -d, --debug                       Enable debug mode
-  -p, --concurrent [parallel tasks] The number of tasks to run concurrently, or false to run tasks sequentially
-  -h, --help                        output usage information
+  -V, --version                      output the version number
+  --allow-empty                      allow empty commits when tasks revert 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)
+  -q, --quiet                        disable lint-staged’s own console output (default: false)
+  -r, --relative                     pass relative filepaths to tasks (default: false)
+  -x, --shell                        skip parsing of tasks for better shell support (default: false)
+  -h, --help                         output usage information
 ```
 
+- **`--allow-empty`**: By default lint-stage will exit with an error — aborting the commit — when after running tasks there are no staged modifications. Use this disable this behaviour and create empty git commits.
+  - This can happen when tasks use libraries like _prettier_ or _eslint_ with automatic code formatting
 - **`--config [path]`**: This can be used to manually specify the `lint-staged` config file location. However, if the specified file cannot be found, it will error out instead of performing the usual search. You may pass a npm package name for configuration also.
-- **`--relative`**: By default filepaths will be passed to the linter tasks as _absolute_. This flag makes them relative to `process.cwd()` (where `lint-staged` runs).
-- **`--shell`**: By default linter commands will be parsed for speed and security. This has the side-effect that regular shell scripts might not work as expected. You can skip parsing of commands with this option.
-- **`--quiet`**: By default `lint-staged` will print progress status to console while running linters. Use this flag to supress all output, except for linter scripts.
-- **`--debug`**: Enabling the debug mode does the following:
-  - `lint-staged` uses the [debug](https://github.com/visionmedia/debug) module internally to log 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*`.
-  - Use the [`verbose` renderer](https://github.com/SamVerschueren/listr-verbose-renderer) for `listr`.
 - **`--concurrent [number | (true/false)]`**: Controls the concurrency of tasks being run by lint-staged. **NOTE**: This does NOT affect the concurrency of subtasks (they will always be run sequentially). Possible values are:
   - `false`: Run all tasks serially
   - `true` (default) : _Infinite_ concurrency. Runs as many tasks in parallel as possible.
   - `{number}`: Run the specified number of tasks in parallel, where `1` is equivalent to `false`.
+- **`--debug`**: Enabling the debug mode does the following:
+  - `lint-staged` uses the [debug](https://github.com/visionmedia/debug) module internally to log 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*`.
+  - Use the [`verbose` renderer](https://github.com/SamVerschueren/listr-verbose-renderer) for `listr`.
+- **`--quiet`**: By default `lint-staged` will print progress status to console while running linters. Use this flag to supress all output, except for linter scripts.
+- **`--relative`**: By default filepaths will be passed to the linter tasks as _absolute_. This flag makes them relative to `process.cwd()` (where `lint-staged` runs).
+- **`--shell`**: By default linter commands will be parsed for speed and security. This has the side-effect that regular shell scripts might not work as expected. You can skip parsing of commands with this option.
 
 ## Configuration
 
diff --git a/bin/lint-staged b/bin/lint-staged
index 81f0e6f..ae9a11c 100755
--- a/bin/lint-staged
+++ b/bin/lint-staged
@@ -29,16 +29,17 @@ const debug = debugLib('lint-staged:bin')
 
 cmdline
   .version(pkg.version)
-  .option('-c, --config [path]', 'Path to configuration file')
-  .option('-r, --relative', 'Pass relative filepaths to tasks')
-  .option('-x, --shell', 'Skip parsing of tasks for better shell support')
-  .option('-q, --quiet', 'Disable lint-staged’s own console output')
-  .option('-d, --debug', 'Enable debug mode')
+  .option('--allow-empty', 'allow empty commits when tasks revert all staged changes', false)
+  .option('-c, --config [path]', 'path to configuration file')
+  .option('-d, --debug', 'print additional debug information', false)
   .option(
     '-p, --concurrent <parallel tasks>',
-    'The number of tasks to run concurrently, or false to run tasks serially',
+    'the number of tasks to run concurrently, or false to run tasks serially',
     true
-  )
+    )
+  .option('-q, --quiet', 'disable lint-staged’s own console output', false)
+  .option('-r, --relative', 'pass relative filepaths to tasks', false)
+  .option('-x, --shell', 'skip parsing of tasks for better shell support', false)
   .parse(process.argv)
 
 if (cmdline.debug) {
@@ -66,13 +67,14 @@ const getMaxArgLength = () => {
 }
 
 const options = {
+  allowEmpty: !!cmdline.allowEmpty,
+  concurrent: cmdline.concurrent,
   configPath: cmdline.config,
+  debug: !!cmdline.debug,
   maxArgLength: getMaxArgLength() / 2,
+  quiet: !!cmdline.quiet,
   relative: !!cmdline.relative,
   shell: !!cmdline.shell,
-  quiet: !!cmdline.quiet,
-  debug: !!cmdline.debug,
-  concurrent: cmdline.concurrent
 }
 
 debug('Options parsed from command-line:', options)
diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js
index f1d192a..6fd01ae 100644
--- a/lib/gitWorkflow.js
+++ b/lib/gitWorkflow.js
@@ -37,10 +37,11 @@ const handleError = (error, ctx) => {
 }
 
 class GitWorkflow {
-  constructor({ gitDir, stagedFileChunks }) {
+  constructor({ allowEmpty, gitDir, stagedFileChunks }) {
     this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir })
     this.unstagedDiff = null
     this.gitDir = gitDir
+    this.allowEmpty = allowEmpty
     this.stagedFileChunks = stagedFileChunks
 
     /**
@@ -143,7 +144,7 @@ class GitWorkflow {
    * In case of a merge-conflict retry with 3-way merge.
    */
   async applyModifications(ctx) {
-    let modifiedFiles = await this.execGit(['ls-files', '--modified'])
+    const modifiedFiles = await this.execGit(['ls-files', '--modified'])
     if (modifiedFiles) {
       debug('Detected files modified by tasks:')
       debug(modifiedFiles)
@@ -156,6 +157,14 @@ class GitWorkflow {
       debug('Done adding files to index!')
     }
 
+    const modifiedFilesAfterAdd = await this.execGit(['status', '--porcelain'])
+    if (!modifiedFilesAfterAdd && !this.allowEmpty) {
+      // Tasks reverted all staged changes and the commit would be empty
+      // Throw error to stop commit unless `--allow-empty` was used
+      ctx.gitApplyEmptyCommit = true
+      handleError(new Error('Prevented an empty git commit!'), ctx)
+    }
+
     if (this.unstagedDiff) {
       debug('Restoring unstaged changes...')
       try {
diff --git a/lib/index.js b/lib/index.js
index a75578a..b33b1c7 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -42,8 +42,10 @@ function loadConfig(configPath) {
  * Root lint-staged function that is called from `bin/lint-staged`.
  *
  * @param {object} options
- * @param {string} [options.configPath] - Path to configuration file
+ * @param {Object} [options.allowEmpty] - Allow empty commits when tasks revert all staged changes
+ * @param {boolean | number} [options.concurrent] - The number of tasks to run concurrently, or false to run tasks serially
  * @param {object}  [options.config] - Object with configuration for programmatic API
+ * @param {string} [options.configPath] - Path to configuration file
  * @param {number} [options.maxArgLength] - Maximum argument string length
  * @param {boolean} [options.relative] - Pass relative filepaths to tasks
  * @param {boolean} [options.shell] - Skip parsing of tasks for better shell support
@@ -56,14 +58,15 @@ function loadConfig(configPath) {
  */
 module.exports = async function lintStaged(
   {
-    configPath,
+    allowEmpty = false,
+    concurrent = true,
     config: configObject,
+    configPath,
     maxArgLength,
     relative = false,
     shell = false,
     quiet = false,
-    debug = false,
-    concurrent = true
+    debug = false
   } = {},
   logger = console
 ) {
@@ -90,7 +93,10 @@ module.exports = async function lintStaged(
     }
 
     try {
-      await runAll({ config, maxArgLength, relative, shell, quiet, debug, concurrent }, logger)
+      await runAll(
+        { allowEmpty, concurrent, config, debug, maxArgLength, quiet, relative, shell },
+        logger
+      )
       debugLog('tasks were executed successfully!')
       return true
     } catch (runAllError) {
diff --git a/lib/runAll.js b/lib/runAll.js
index f803441..3203fc1 100644
--- a/lib/runAll.js
+++ b/lib/runAll.js
@@ -19,6 +19,7 @@ const debugLog = require('debug')('lint-staged:run')
  * Executes all tasks and either resolves or rejects the promise
  *
  * @param {object} options
+ * @param {Object} [options.allowEmpty] - Allow empty commits when tasks revert all staged changes
  * @param {Object} [options.config] - Task configuration
  * @param {Object} [options.cwd] - Current working directory
  * @param {number} [options.maxArgLength] - Maximum argument string length
@@ -32,6 +33,7 @@ const debugLog = require('debug')('lint-staged:run')
  */
 module.exports = async function runAll(
   {
+    allowEmpty = false,
     config,
     cwd = process.cwd(),
     debug = false,
@@ -144,13 +146,16 @@ module.exports = async function runAll(
     return 'No tasks to run.'
   }
 
-  const git = new GitWorkflow({ gitDir, stagedFileChunks })
+  const git = new GitWorkflow({ allowEmpty, gitDir, stagedFileChunks })
 
   // Running git reset or dropping the backup stash should be skipped
   // when there are git errors NOT related to applying unstaged modifications.
   // In the latter case, the original state is restored.
   const cleanupNotSafe = ctx =>
-    ctx.gitError && !ctx.gitApplyModificationsError && 'Skipped because of previous git error.'
+    ctx.gitError &&
+    !ctx.gitApplyEmptyCommit &&
+    !ctx.gitApplyModificationsError &&
+    'Skipped because of previous git error.'
 
   const runner = new Listr(
     [
@@ -169,7 +174,7 @@ module.exports = async function runAll(
       },
       {
         title: 'Reverting to original state...',
-        enabled: ctx => ctx.taskError || ctx.gitApplyModificationsError,
+        enabled: ctx => ctx.taskError || ctx.gitApplyEmptyCommit || ctx.gitApplyModificationsError,
         skip: cleanupNotSafe,
         task: ctx => git.restoreOriginalState(ctx)
       },
@@ -185,9 +190,16 @@ module.exports = async function runAll(
   try {
     await runner.run({})
   } catch (error) {
+    if (error.context.gitApplyEmptyCommit) {
+      logger.warn(`
+  ${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.
-    if (error.context.gitError && !error.context.gitGetBackupStashError) {
+    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:


Test patch:
diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js
index bb37f12..eb73049 100644
--- a/test/runAll.unmocked.spec.js
+++ b/test/runAll.unmocked.spec.js
@@ -678,4 +678,61 @@ describe('runAll', () => {
       LOG → lint-staged automatic backup is missing!"
     `)
   })
+
+  it('should fail when task reverts staged changes, to prevent an empty git commit', async () => {
+    // Create and commit a pretty file without running lint-staged
+    // This way the file will be available for the next step
+    await appendFile('test.js', testJsFilePretty)
+    await execGit(['add', 'test.js'])
+    await execGit(['commit', '-m committed pretty file'])
+
+    // Edit file to be ugly
+    await fs.remove(path.resolve(cwd, 'test.js'))
+    await appendFile('test.js', testJsFileUgly)
+    await execGit(['add', 'test.js'])
+
+    // Run lint-staged with prettier --write to automatically fix the file
+    // Since prettier reverts all changes, the commit should fail
+    await expect(
+      gitCommit({ config: { '*.js': 'prettier --write' } })
+    ).rejects.toThrowErrorMatchingInlineSnapshot(`"Something went wrong"`)
+
+    expect(console.printHistory()).toMatchInlineSnapshot(`
+      "
+      WARN 
+        ‼ lint-staged prevented an empty git commit.
+          Use the --allow-empty option to continue, or check your task configuration
+      "
+    `)
+
+    // Something was wrong so the repo is returned to original state
+    expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('2')
+    expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('committed pretty file')
+    expect(await readFile('test.js')).toEqual(testJsFileUgly)
+  })
+
+  it('should create commit when task reverts staged changed and --allow-empty is used', async () => {
+    // Create and commit a pretty file without running lint-staged
+    // This way the file will be available for the next step
+    await appendFile('test.js', testJsFilePretty)
+    await execGit(['add', 'test.js'])
+    await execGit(['commit', '-m committed pretty file'])
+
+    // Edit file to be ugly
+    await writeFile('test.js', testJsFileUgly)
+    await execGit(['add', 'test.js'])
+
+    // Run lint-staged with prettier --write to automatically fix the file
+    // Here we also pass '--allow-empty' to gitCommit because this part is not the full lint-staged
+    await gitCommit({ allowEmpty: true, config: { '*.js': 'prettier --write' } }, [
+      '-m test',
+      '--allow-empty'
+    ])
+
+    // Nothing was wrong so the empty commit is created
+    expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('3')
+    expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('test')
+    expect(await execGit(['diff', '-1'])).toEqual('')
+    expect(await readFile('test.js')).toEqual(testJsFilePretty)
+  })
 })