From c386e4cf9646dc0953213e9a0ef857cb9664af37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Sat, 14 Mar 2020 15:00:04 +0200 Subject: [PATCH] feat: add `--no-stash` option to disable the backup stash, and not revert in case of errors --- README.md | 14 ++- bin/lint-staged.js | 2 + lib/gitWorkflow.js | 17 +-- lib/index.js | 13 ++- lib/runAll.js | 33 ++++-- test/__mocks__/gitWorkflow.js | 6 +- test/__snapshots__/runAll.spec.js.snap | 24 ++-- test/gitWorkflow.spec.js | 4 +- test/runAll.unmocked.2.spec.js | 8 +- test/runAll.unmocked.spec.js | 156 +++++++++++++++++++++++-- 10 files changed, 217 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 3a0bd7965..b166e5e0c 100644 --- a/README.md +++ b/README.md @@ -60,13 +60,18 @@ Usage: lint-staged [options] Options: -V, --version output the version number - --allow-empty allow empty commits when tasks undo all staged changes (default: false) + --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) + --no-stash disable the backup stash, and do not revert in case of + errors + -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) + -x, --shell skip parsing of tasks for better shell support (default: + false) -h, --help output usage information ``` @@ -79,6 +84,7 @@ Options: - `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`. +- **`--no-stash`**: By default a backup stash will be created before running the tasks, and all task modifications will be reverted in case of an error. This option will disable creating the stash, and instead leave all modifications in the index when aborting the commit. - **`--quiet`**: Supress all CLI output, except from tasks. - **`--relative`**: Pass filepaths relative to `process.cwd()` (where `lint-staged` runs) to tasks. Default is `false`. - **`--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. @@ -168,7 +174,7 @@ Pass arguments to your commands separated by space as you would do in the shell. ## Running multiple commands in a sequence -You can run multiple commands in a sequence on every glob. To do so, pass an array of commands instead of a single one. This is useful for running autoformatting tools like `eslint --fix` or `stylefmt` but can be used for any arbitrary sequences. +You can run multiple commands in a sequence on every glob. To do so, pass an array of commands instead of a single one. This is useful for running autoformatting tools like `eslint --fix` or `stylefmt` but can be used for any arbitrary sequences. For example: diff --git a/bin/lint-staged.js b/bin/lint-staged.js index aadfe16f8..b36f38a3e 100755 --- a/bin/lint-staged.js +++ b/bin/lint-staged.js @@ -31,6 +31,7 @@ cmdline .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('--no-stash', 'disable the backup stash, and do not revert in case of errors', false) .option( '-p, --concurrent ', 'the number of tasks to run concurrently, or false to run tasks serially', @@ -71,6 +72,7 @@ const options = { configPath: cmdline.config, debug: !!cmdline.debug, maxArgLength: getMaxArgLength() / 2, + stash: !!cmdline.stash, // commander inverts `no-` flags to `!x` quiet: !!cmdline.quiet, relative: !!cmdline.relative, shell: !!cmdline.shell diff --git a/lib/gitWorkflow.js b/lib/gitWorkflow.js index f1021bf05..254054868 100644 --- a/lib/gitWorkflow.js +++ b/lib/gitWorkflow.js @@ -54,6 +54,7 @@ const handleError = (error, ctx) => { class GitWorkflow { constructor({ allowEmpty, gitConfigDir, gitDir, stagedFileChunks }) { this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir }) + this.deletedFiles = [] this.gitConfigDir = gitConfigDir this.gitDir = gitDir this.unstagedDiff = null @@ -161,10 +162,9 @@ class GitWorkflow { } /** - * Create backup stashes, one of everything and one of only staged changes - * Staged files are left in the index for running tasks + * Create a diff of partially staged files and backup stash if enabled. */ - async createBackup(ctx) { + async prepare(ctx, stash) { try { debug('Backing up original state...') @@ -179,6 +179,11 @@ class GitWorkflow { await this.execGit(['diff', ...GIT_DIFF_ARGS, '--output', unstagedPatch, '--', ...files]) } + /** + * If backup stash should be skipped, no need to continue + */ + if (!stash) return + // Get a list of unstaged deleted files, because certain bugs might cause them to reappear: // - in git versions =< 2.13.0 the `--keep-index` flag resurrects deleted files // - git stash can't infer RD or MD states correctly, and will lose the deletion @@ -291,9 +296,7 @@ class GitWorkflow { await Promise.all(this.deletedFiles.map(file => unlink(file))) // Clean out patch - if (this.partiallyStagedFiles) { - await unlink(PATCH_UNSTAGED) - } + if (this.partiallyStagedFiles) await unlink(PATCH_UNSTAGED) debug('Done restoring original state!') } catch (error) { @@ -305,7 +308,7 @@ class GitWorkflow { /** * Drop the created stashes after everything has run */ - async dropBackup(ctx) { + async cleanup(ctx) { try { debug('Dropping backup stash...') await this.execGit(['stash', 'drop', '--quiet', await this.getBackupStash(ctx)]) diff --git a/lib/index.js b/lib/index.js index 2fe455955..4ccffa5a8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -46,12 +46,12 @@ function loadConfig(configPath) { * @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 {boolean} [options.debug] - Enable debug mode * @param {number} [options.maxArgLength] - Maximum argument string length + * @param {boolean} [options.quiet] - Disable lint-staged’s own console output * @param {boolean} [options.relative] - Pass relative filepaths to tasks * @param {boolean} [options.shell] - Skip parsing of tasks for better shell support - * @param {boolean} [options.quiet] - Disable lint-staged’s own console output - * @param {boolean} [options.debug] - Enable debug mode - * @param {boolean | number} [options.concurrent] - The number of tasks to run concurrently, or false to run tasks serially + * @param {boolean} [options.stash] - Enable the backup stash, and revert in case of errors * @param {Logger} [logger] * * @returns {Promise} Promise of whether the linting passed or failed @@ -62,11 +62,12 @@ module.exports = async function lintStaged( concurrent = true, config: configObject, configPath, + debug = false, maxArgLength, + quiet = false, relative = false, shell = false, - quiet = false, - debug = false + stash = true } = {}, logger = console ) { @@ -98,7 +99,7 @@ module.exports = async function lintStaged( try { await runAll( - { allowEmpty, concurrent, config, debug, maxArgLength, quiet, relative, shell }, + { allowEmpty, concurrent, config, debug, maxArgLength, stash, quiet, relative, shell }, logger ) debugLog('tasks were executed successfully!') diff --git a/lib/runAll.js b/lib/runAll.js index d167566c4..66951c0c1 100644 --- a/lib/runAll.js +++ b/lib/runAll.js @@ -62,20 +62,22 @@ const shouldSkipCleanup = ctx => { * * @param {object} options * @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] - Task configuration * @param {Object} [options.cwd] - Current working directory + * @param {boolean} [options.debug] - Enable debug mode * @param {number} [options.maxArgLength] - Maximum argument string length + * @param {boolean} [options.quiet] - Disable lint-staged’s own console output * @param {boolean} [options.relative] - Pass relative filepaths to tasks * @param {boolean} [options.shell] - Skip parsing of tasks for better shell support - * @param {boolean} [options.quiet] - Disable lint-staged’s own console output - * @param {boolean} [options.debug] - Enable debug mode - * @param {boolean | number} [options.concurrent] - The number of tasks to run concurrently, or false to run tasks serially + * @param {boolean} [options.stash] - Enable the backup stash, and revert in case of errors * @param {Logger} logger * @returns {Promise} */ const runAll = async ( { allowEmpty = false, + concurrent = true, config, cwd = process.cwd(), debug = false, @@ -83,12 +85,18 @@ const runAll = async ( quiet = false, relative = false, shell = false, - concurrent = true + stash = true }, logger = console ) => { debugLog('Running all linter scripts') + if (!stash) { + logger.warn( + `${symbols.warning} ${chalk.yellow('Skipping backup because `--no-stash` was used.')}` + ) + } + const { gitDir, gitConfigDir } = await resolveGitRepo(cwd) if (!gitDir) throw new Error('Current directory is not a git directory!') @@ -188,8 +196,8 @@ const runAll = async ( const runner = new Listr( [ { - title: 'Creating backup...', - task: ctx => git.createBackup(ctx) + title: 'Preparing...', + task: ctx => git.prepare(ctx, stash) }, { title: 'Hiding unstaged changes to partially staged files...', @@ -200,7 +208,8 @@ const runAll = async ( { title: 'Applying modifications...', task: ctx => git.applyModifications(ctx), - skip: shouldSkipApplyModifications + // Always apply back unstaged modifications when skipping backup + skip: ctx => stash && shouldSkipApplyModifications(ctx) }, { title: 'Restoring unstaged changes to partially staged files...', @@ -212,12 +221,14 @@ const runAll = async ( title: 'Reverting to original state because of errors...', task: ctx => git.restoreOriginalState(ctx), enabled: ctx => - ctx.taskError || ctx.gitApplyEmptyCommitError || ctx.gitRestoreUnstagedChangesError, + stash && + (ctx.taskError || ctx.gitApplyEmptyCommitError || ctx.gitRestoreUnstagedChangesError), skip: shouldSkipRevert }, { - title: 'Cleaning up backup...', - task: ctx => git.dropBackup(ctx), + title: 'Cleaning up...', + task: ctx => git.cleanup(ctx), + enabled: () => stash, skip: shouldSkipCleanup } ], @@ -240,7 +251,7 @@ const runAll = async ( `\n The initial commit is needed for lint-staged to work. Please use the --no-verify flag to skip running lint-staged.` ) - } else { + } else if (stash) { // No sense to show this if the backup stash itself is missing. logger.error(` Any lost modifications can be restored from a git stash: diff --git a/test/__mocks__/gitWorkflow.js b/test/__mocks__/gitWorkflow.js index 212655108..77b146fa8 100644 --- a/test/__mocks__/gitWorkflow.js +++ b/test/__mocks__/gitWorkflow.js @@ -1,8 +1,10 @@ const stub = { - stashBackup: jest.fn().mockImplementation(() => Promise.resolve()), + prepare: jest.fn().mockImplementation(() => Promise.resolve()), + hideUnstagedChanges: jest.fn().mockImplementation(() => Promise.resolve()), applyModifications: jest.fn().mockImplementation(() => Promise.resolve()), + restoreUnstagedChanges: jest.fn().mockImplementation(() => Promise.resolve()), restoreOriginalState: jest.fn().mockImplementation(() => Promise.resolve()), - dropBackup: jest.fn().mockImplementation(() => Promise.resolve()) + cleanup: jest.fn().mockImplementation(() => Promise.resolve()) } module.exports = () => stub diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap index b47659b28..2140eb414 100644 --- a/test/__snapshots__/runAll.spec.js.snap +++ b/test/__snapshots__/runAll.spec.js.snap @@ -2,8 +2,8 @@ exports[`runAll should not skip tasks if there are files 1`] = ` " -LOG Creating backup... [started] -LOG Creating backup... [completed] +LOG Preparing... [started] +LOG Preparing... [completed] LOG Running tasks... [started] LOG Running tasks for *.js [started] LOG echo \\"sample\\" [started] @@ -12,8 +12,8 @@ LOG Running tasks for *.js [completed] LOG Running tasks... [completed] LOG Applying modifications... [started] LOG Applying modifications... [completed] -LOG Cleaning up backup... [started] -LOG Cleaning up backup... [completed]" +LOG Cleaning up... [started] +LOG Cleaning up... [completed]" `; exports[`runAll should resolve the promise with no files 1`] = ` @@ -23,8 +23,8 @@ LOG No staged files found." exports[`runAll should skip applying unstaged modifications if there are errors during linting 1`] = ` " -LOG Creating backup... [started] -LOG Creating backup... [completed] +LOG Preparing... [started] +LOG Preparing... [completed] LOG Running tasks... [started] LOG Running tasks for *.js [started] LOG echo \\"sample\\" [started] @@ -38,8 +38,8 @@ LOG Applying modifications... [skipped] LOG → Skipped because of errors from tasks. LOG Reverting to original state because of errors... [started] LOG Reverting to original state because of errors... [completed] -LOG Cleaning up backup... [started] -LOG Cleaning up backup... [completed] +LOG Cleaning up... [started] +LOG Cleaning up... [completed] LOG { name: 'ListrError', errors: [ @@ -54,8 +54,8 @@ LOG { exports[`runAll should skip tasks and restore state if terminated 1`] = ` " -LOG Creating backup... [started] -LOG Creating backup... [completed] +LOG Preparing... [started] +LOG Preparing... [completed] LOG Running tasks... [started] LOG Running tasks for *.js [started] LOG echo \\"sample\\" [started] @@ -69,8 +69,8 @@ LOG Applying modifications... [skipped] LOG → Skipped because of errors from tasks. LOG Reverting to original state because of errors... [started] LOG Reverting to original state because of errors... [completed] -LOG Cleaning up backup... [started] -LOG Cleaning up backup... [completed] +LOG Cleaning up... [started] +LOG Cleaning up... [completed] LOG { name: 'ListrError', errors: [ diff --git a/test/gitWorkflow.spec.js b/test/gitWorkflow.spec.js index 7ae7b34ed..6837e242d 100644 --- a/test/gitWorkflow.spec.js +++ b/test/gitWorkflow.spec.js @@ -65,14 +65,14 @@ describe('gitWorkflow', () => { } }) - describe('dropBackup', () => { + describe('cleanup', () => { 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( + await expect(gitWorkflow.cleanup(ctx)).rejects.toThrowErrorMatchingInlineSnapshot( `"lint-staged automatic backup is missing!"` ) expect(ctx).toEqual({ diff --git a/test/runAll.unmocked.2.spec.js b/test/runAll.unmocked.2.spec.js index 04c26e1c2..86aea6588 100644 --- a/test/runAll.unmocked.2.spec.js +++ b/test/runAll.unmocked.2.spec.js @@ -106,8 +106,8 @@ describe('runAll', () => { expect(console.printHistory()).toMatchInlineSnapshot(` " - LOG Creating backup... [started] - LOG Creating backup... [failed] + LOG Preparing... [started] + LOG Preparing... [failed] LOG → Merge state could not be restored due to an error! LOG Running tasks... [started] LOG Running tasks... [skipped] @@ -115,8 +115,8 @@ describe('runAll', () => { LOG Applying modifications... [started] LOG Applying modifications... [skipped] LOG → Skipped because of previous git error. - LOG Cleaning up backup... [started] - LOG Cleaning up backup... [skipped] + LOG Cleaning up... [started] + LOG Cleaning up... [skipped] LOG → Skipped because of previous git error. ERROR × lint-staged failed due to a git error. diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js index 373712441..f4ec637de 100644 --- a/test/runAll.unmocked.spec.js +++ b/test/runAll.unmocked.spec.js @@ -237,8 +237,8 @@ describe('runAll', () => { await gitCommit({ config: { '*.js': 'prettier --list-different' }, quiet: false }) expect(console.printHistory()).toMatchInlineSnapshot(` " - LOG Creating backup... [started] - LOG Creating backup... [completed] + LOG Preparing... [started] + LOG Preparing... [completed] LOG Hiding unstaged changes to partially staged files... [started] LOG Hiding unstaged changes to partially staged files... [completed] LOG Running tasks... [started] @@ -251,8 +251,8 @@ describe('runAll', () => { LOG Applying modifications... [completed] LOG Restoring unstaged changes to partially staged files... [started] LOG Restoring unstaged changes to partially staged files... [completed] - LOG Cleaning up backup... [started] - LOG Cleaning up backup... [completed]" + LOG Cleaning up... [started] + LOG Cleaning up... [completed]" `) // Nothing is wrong, so a new commit is created and file is pretty @@ -315,8 +315,8 @@ describe('runAll', () => { ).rejects.toThrowError() expect(console.printHistory()).toMatchInlineSnapshot(` " - LOG Creating backup... [started] - LOG Creating backup... [completed] + LOG Preparing... [started] + LOG Preparing... [completed] LOG Hiding unstaged changes to partially staged files... [started] LOG Hiding unstaged changes to partially staged files... [completed] LOG Running tasks... [started] @@ -335,8 +335,8 @@ describe('runAll', () => { LOG → Skipped because of errors from tasks. LOG Reverting to original state because of errors... [started] LOG Reverting to original state because of errors... [completed] - LOG Cleaning up backup... [started] - LOG Cleaning up backup... [completed]" + LOG Cleaning up... [started] + LOG Cleaning up... [completed]" `) // Something was wrong so the repo is returned to original state @@ -744,8 +744,8 @@ describe('runAll', () => { expect(console.printHistory()).toMatchInlineSnapshot(` " - LOG Creating backup... [started] - LOG Creating backup... [completed] + LOG Preparing... [started] + LOG Preparing... [completed] LOG Running tasks... [started] LOG Running tasks for *.js [started] LOG [Function] git ... [started] @@ -754,8 +754,8 @@ describe('runAll', () => { LOG Running tasks... [completed] LOG Applying modifications... [started] LOG Applying modifications... [completed] - LOG Cleaning up backup... [started] - LOG Cleaning up backup... [failed] + LOG Cleaning up... [started] + LOG Cleaning up... [failed] LOG → lint-staged automatic backup is missing!" `) }) @@ -907,6 +907,138 @@ describe('runAll', () => { expect(await readFile('👋.js')).toEqual(testJsFilePretty) } ) + + it('should skip backup and revert with --no-backup', async () => { + // Stage pretty file + await appendFile('test.js', testJsFileUgly) + await execGit(['add', 'test.js']) + + // Run lint-staged with --no-stash + await gitCommit({ + ...fixJsConfig, + stash: false, + quiet: false + }) + + expect(console.printHistory()).toMatchInlineSnapshot(` + " + WARN ‼ Skipping backup because \`--no-stash\` was used. + LOG Preparing... [started] + LOG Preparing... [completed] + LOG Running tasks... [started] + LOG Running tasks for *.js [started] + LOG prettier --write [started] + LOG prettier --write [completed] + LOG Running tasks for *.js [completed] + LOG Running tasks... [completed] + LOG Applying modifications... [started] + LOG Applying modifications... [completed]" + `) + + // 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) + }) + + it('should abort commit without reverting with --no-stash 1', async () => { + // Stage file + await appendFile('test.js', testJsFileUgly) + await execGit(['add', 'test.js']) + + // Run lint-staged with action that does horrible things to the file, causing a merge conflict + const testFile = path.resolve(cwd, 'test.js') + await expect( + gitCommit({ + config: { + '*.js': () => { + fs.writeFileSync(testFile, Buffer.from(testJsFileUnfixable, 'binary')) + return `prettier --write ${testFile}` + } + }, + quiet: false, + stash: false + }) + ).rejects.toThrowError() + + expect(console.printHistory()).toMatchInlineSnapshot(` + " + WARN ‼ Skipping backup because \`--no-stash\` was used. + LOG Preparing... [started] + LOG Preparing... [completed] + LOG Hiding unstaged changes to partially staged files... [started] + LOG Hiding unstaged changes to partially staged files... [completed] + LOG Running tasks... [started] + LOG Running tasks for *.js [started] + LOG [Function] prettier ... [started] + LOG [Function] prettier ... [completed] + LOG Running tasks for *.js [completed] + LOG Running tasks... [completed] + LOG Applying modifications... [started] + LOG Applying modifications... [completed] + LOG Restoring unstaged changes to partially staged files... [started] + LOG Restoring unstaged changes to partially staged files... [failed] + LOG → Unstaged changes could not be restored due to a merge conflict! + ERROR + × lint-staged failed due to a git error." + `) + + // Something was wrong so the commit was aborted + expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1') + expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('initial commit') + expect(await execGit(['status', '--porcelain'])).toMatchInlineSnapshot(`"UU test.js"`) + // Without revert, the merge conflict is left in-place + expect(await readFile('test.js')).toMatchInlineSnapshot(` + "<<<<<<< ours + module.exports = { + foo: \\"bar\\" + }; + ======= + const obj = { + 'foo': 'bar' + >>>>>>> theirs + " + `) + }) + + it('should abort commit without reverting with --no-stash 2', async () => { + await appendFile('test.js', testJsFileUgly) + await execGit(['add', 'test.js']) + await appendFile('test2.js', testJsFileUnfixable) + await execGit(['add', 'test2.js']) + + // Run lint-staged with --no-stash + await expect( + gitCommit({ + ...fixJsConfig, + quiet: false, + stash: false + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Something went wrong"`) + + expect(console.printHistory()).toMatchInlineSnapshot(` + " + WARN ‼ Skipping backup because \`--no-stash\` was used. + LOG Preparing... [started] + LOG Preparing... [completed] + LOG Running tasks... [started] + LOG Running tasks for *.js [started] + LOG prettier --write [started] + LOG prettier --write [failed] + LOG → + LOG Running tasks for *.js [failed] + LOG → + LOG Running tasks... [failed] + LOG Applying modifications... [started] + LOG Applying modifications... [completed]" + `) + + // Something was wrong, so the commit was aborted + expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1') + expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('initial commit') + expect(await readFile('test.js')).toEqual(testJsFilePretty) // file was still fixed + expect(await readFile('test2.js')).toEqual(testJsFileUnfixable) + }) }) describe('runAll', () => {