Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat: throw error to prevent empty commits unless --allow-empty is us…
…ed (#762)

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.
  • Loading branch information
iiroj authored and okonet committed Jan 8, 2020
1 parent 30b4809 commit 8bdeec0
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 36 deletions.
33 changes: 18 additions & 15 deletions README.md
Expand Up @@ -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

Expand Down
22 changes: 12 additions & 10 deletions bin/lint-staged
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 11 additions & 2 deletions lib/gitWorkflow.js
Expand Up @@ -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

/**
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
16 changes: 11 additions & 5 deletions lib/index.js
Expand Up @@ -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
Expand All @@ -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
) {
Expand All @@ -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) {
Expand Down
20 changes: 16 additions & 4 deletions lib/runAll.js
Expand Up @@ -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
Expand All @@ -32,6 +33,7 @@ const debugLog = require('debug')('lint-staged:run')
*/
module.exports = async function runAll(
{
allowEmpty = false,
config,
cwd = process.cwd(),
debug = false,
Expand Down Expand Up @@ -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(
[
Expand All @@ -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)
},
Expand All @@ -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:
Expand Down
57 changes: 57 additions & 0 deletions test/runAll.unmocked.spec.js
Expand Up @@ -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)
})
})

0 comments on commit 8bdeec0

Please sign in to comment.