From 8bdeec067f425150722bd0ee78e310e0992a1444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Wed, 8 Jan 2020 21:53:38 +0200 Subject: [PATCH] feat: throw error to prevent empty commits unless --allow-empty is used (#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. --- README.md | 33 +++++++++++---------- bin/lint-staged | 22 +++++++------- lib/gitWorkflow.js | 13 ++++++-- lib/index.js | 16 ++++++---- lib/runAll.js | 20 ++++++++++--- test/runAll.unmocked.spec.js | 57 ++++++++++++++++++++++++++++++++++++ 6 files changed, 125 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 82f1d653e..781087e03 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 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 81f0e6f2c..ae9a11c05 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 ', - '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 f1d192af3..6fd01ae82 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 a75578a1b..b33b1c7af 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 f803441b1..3203fc1d1 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: diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index bb37f12dd..eb730494e 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) + }) })