fix: prevent Listr from hiding git add warning

lint-staged · Nov 14, 2019 · cce9809 · cce9809
1 parent c7d0592
commit cce9809
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 118 deletions.
diff --git a/lib/generateTasks.js b/lib/generateTasks.js
@@ -15,7 +15,6 @@ const debug = require('debug')('lint-staged:gen-tasks')
  * @param {boolean} [options.gitDir] - Git root directory
  * @param {boolean} [options.files] - Staged filepaths
  * @param {boolean} [options.relative] - Whether filepaths to should be relative to gitDir
- * @returns {Promise}
  */
 module.exports = function generateTasks({
   config,

diff --git a/lib/makeCmdTasks.js b/lib/makeCmdTasks.js
@@ -11,10 +11,9 @@ const debug = require('debug')('lint-staged:make-cmd-tasks')
  * @param {Array<string|Function>|string|Function} options.commands
  * @param {Array<string>} options.files
  * @param {string} options.gitDir
- * @param {Function} [options.logger]
  * @param {Boolean} shell
  */
-module.exports = async function makeCmdTasks({ commands, files, gitDir, logger, shell }) {
+module.exports = function makeCmdTasks({ commands, files, gitDir, shell }) {
   debug('Creating listr tasks for commands %o', commands)
   const commandsArray = Array.isArray(commands) ? commands : [commands]
 
@@ -42,8 +41,11 @@ module.exports = async function makeCmdTasks({ commands, files, gitDir, logger,
         title = mockCommands[i].replace(/\[file\].*\[file\]/, '[file]')
       }
 
-      const task = { title, task: resolveTaskFn({ command, files, gitDir, isFn, logger, shell }) }
-      tasks.push(task)
+      tasks.push({
+        title,
+        command,
+        task: resolveTaskFn({ command, files, gitDir, isFn, shell })
+      })
     })
 
     return tasks

diff --git a/lib/resolveTaskFn.js b/lib/resolveTaskFn.js
@@ -62,30 +62,14 @@ function makeErr(linter, result, context = {}) {
  * @param {String} options.gitDir - Current git repo path
  * @param {Boolean} options.isFn - Whether the linter task is a function
  * @param {Array<string>} options.files — Filepaths to run the linter task against
- * @param {Function} [options.logger]
  * @param {Boolean} [options.relative] — Whether the filepaths should be relative
  * @param {Boolean} [options.shell] — Whether to skip parsing linter task for better shell support
  * @returns {function(): Promise<Array<string>>}
  */
-module.exports = function resolveTaskFn({
-  command,
-  files,
-  gitDir,
-  isFn,
-  logger,
-  relative,
-  shell = false
-}) {
+module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) {
   const cmd = isFn ? command : `${command} ${files.join(' ')}`
   debug('cmd:', cmd)
 
-  if (logger && cmd.includes('git add')) {
-    logger.warn(`
- ${symbols.warning}  ${chalk.yellow(
-      `Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.`
-    )}`)
-  }
-
   const execaOptions = { preferLocal: true, reject: false, shell }
   if (relative) {
     execaOptions.cwd = process.cwd()

diff --git a/lib/runAll.js b/lib/runAll.js
@@ -68,75 +68,89 @@ module.exports = async function runAll(
 `)
   }
 
-  const tasks = generateTasks({ config, cwd, gitDir, files, relative }).map(task => ({
-    title: `Running tasks for ${task.pattern}`,
-    task: async () =>
-      new Listr(
-        await makeCmdTasks({
-          commands: task.commands,
-          files: task.fileList,
-          gitDir,
-          logger,
-          shell
-        }),
-        {
+  const tasks = generateTasks({ config, cwd, gitDir, files, relative })
+
+  // lint-staged 10 will automatically add modifications to index
+  // Warn user when their command includes `git add`
+  let hasDeprecatedGitAdd = false
+
+  const listrTasks = tasks.map(task => {
+    const subTasks = makeCmdTasks({ commands: task.commands, files: task.fileList, gitDir, shell })
+
+    if (subTasks.some(subTask => subTask.command.includes('git add'))) {
+      hasDeprecatedGitAdd = true
+    }
+
+    return {
+      title: `Running tasks for ${task.pattern}`,
+      task: async () =>
+        new Listr(subTasks, {
           // In sub-tasks we don't want to run concurrently
           // and we want to abort on errors
           dateFormat: false,
           concurrent: false,
           exitOnError: true
+        }),
+      skip: () => {
+        if (task.fileList.length === 0) {
+          return `No staged files match ${task.pattern}`
         }
-      ),
-    skip: () => {
-      if (task.fileList.length === 0) {
-        return `No staged files match ${task.pattern}`
+        return false
       }
-      return false
     }
-  }))
+  })
 
-  const listrOptions = {
-    dateFormat: false,
-    renderer: (quiet && 'silent') || (debug && 'verbose') || 'update'
+  if (hasDeprecatedGitAdd) {
+    logger.warn(`${symbols.warning} ${chalk.yellow(
+      `Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.`
+    )}
+`)
   }
 
-  // If all of the configured "linters" should be skipped
+  // If all of the configured tasks should be skipped
   // avoid executing any lint-staged logic
-  if (tasks.every(task => task.skip())) {
+  if (listrTasks.every(task => task.skip())) {
     logger.log('No staged files match any of provided globs.')
     return 'No tasks to run.'
   }
 
+  const listrOptions = {
+    dateFormat: false,
+    renderer: (quiet && 'silent') || (debug && 'verbose') || 'update'
+  }
+
   const git = new GitWorkflow(gitDir)
 
+  const runner = new Listr(
+    [
+      {
+        title: 'Preparing...',
+        task: () => git.stashBackup()
+      },
+      {
+        title: 'Running tasks...',
+        task: () => new Listr(listrTasks, { ...listrOptions, concurrent: true, exitOnError: false })
+      },
+      {
+        title: 'Applying modifications...',
+        skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks',
+        task: () => git.applyModifications()
+      },
+      {
+        title: 'Reverting to original state...',
+        enabled: ctx => ctx.hasErrors,
+        task: () => git.restoreOriginalState()
+      },
+      {
+        title: 'Cleaning up...',
+        task: () => git.dropBackup()
+      }
+    ],
+    listrOptions
+  )
+
   try {
-    await new Listr(
-      [
-        {
-          title: 'Preparing...',
-          task: () => git.stashBackup()
-        },
-        {
-          title: 'Running tasks...',
-          task: () => new Listr(tasks, { ...listrOptions, concurrent: true, exitOnError: false })
-        },
-        {
-          title: 'Applying modifications...',
-          skip: ctx => ctx.hasErrors && 'Skipped because of errors from tasks',
-          task: () => git.applyModifications()
-        },
-        {
-          title: 'Reverting to original state...',
-          enabled: ctx => ctx.hasErrors,
-          task: () => git.restoreOriginalState()
-        },
-        {
-          title: 'Cleaning up...',
-          task: () => git.dropBackup()
-        }
-      ],
-      listrOptions
-    ).run()
+    await runner.run()
   } catch (error) {
     if (error.message.includes('Another git process seems to be running in this repository')) {
       logger.error(`

diff --git a/test/resolveTaskFn.spec.js b/test/resolveTaskFn.spec.js
@@ -1,5 +1,4 @@
 import execa from 'execa'
-import makeConsoleMock from 'consolemock'
 
 import resolveTaskFn from '../lib/resolveTaskFn'
 
@@ -201,19 +200,4 @@ describe('resolveTaskFn', () => {
       expect(context.hasErrors).toEqual(true)
     }
   })
-
-  it('should warn when tasks include git add', async () => {
-    const logger = makeConsoleMock()
-    await resolveTaskFn({
-      ...defaultOpts,
-      command: 'git add',
-      logger,
-      relative: true
-    })
-    expect(logger.printHistory()).toMatchInlineSnapshot(`
-      "
-      WARN 
-       ‼  Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command."
-    `)
-  })
 })
diff --git a/test/runAll.unmocked.spec.js b/test/runAll.unmocked.spec.js
@@ -340,8 +340,8 @@ describe('runAll', () => {
       expect(error.message).toMatch('Another git process seems to be running in this repository')
       expect(console.printHistory()).toMatchInlineSnapshot(`
         "
-        WARN 
-         ‼  Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.
+        WARN ‼ Detected a task using \`git add\`. Lint-staged version 10 will automatically add any task modifications to the git index, and you should remove this command.
+
         ERROR 
           × lint-staged failed due to a git error.
             Any lost modifications can be restored from a git stash:
@@ -360,17 +360,17 @@ describe('runAll', () => {
     // But local modifications are gone
     expect(await execGit(['diff'])).not.toEqual(diff)
     expect(await execGit(['diff'])).toMatchInlineSnapshot(`
-                                                            "diff --git a/test.js b/test.js
-                                                            index f80f875..1c5643c 100644
-                                                            --- a/test.js
-                                                            +++ b/test.js
-                                                            @@ -1,3 +1,3 @@
-                                                             module.exports = {
-                                                            -    'foo': 'bar',
-                                                            -}
-                                                            +  foo: \\"bar\\"
-                                                            +};"
-                                        `)
+                                                                  "diff --git a/test.js b/test.js
+                                                                  index f80f875..1c5643c 100644
+                                                                  --- a/test.js
+                                                                  +++ b/test.js
+                                                                  @@ -1,3 +1,3 @@
+                                                                   module.exports = {
+                                                                  -    'foo': 'bar',
+                                                                  -}
+                                                                  +  foo: \\"bar\\"
+                                                                  +};"
+                                            `)
 
     expect(await readFile('test.js')).not.toEqual(testJsFileUgly + appended)
     expect(await readFile('test.js')).toEqual(testJsFilePretty)
@@ -424,13 +424,13 @@ describe('runAll', () => {
     }
 
     expect(await readFile('test.js')).toMatchInlineSnapshot(`
-                                                            "<<<<<<< HEAD
-                                                            module.exports = \\"foo\\";
-                                                            =======
-                                                            module.exports = \\"bar\\";
-                                                            >>>>>>> branch-b
-                                                            "
-                                        `)
+                                                                  "<<<<<<< HEAD
+                                                                  module.exports = \\"foo\\";
+                                                                  =======
+                                                                  module.exports = \\"bar\\";
+                                                                  >>>>>>> branch-b
+                                                                  "
+                                            `)
 
     // Fix conflict and commit using lint-staged
     await writeFile('test.js', fileInBranchB)
@@ -444,12 +444,12 @@ describe('runAll', () => {
     // Nothing is wrong, so a new commit is created and file is pretty
     expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('4')
     expect(await execGit(['log', '-1', '--pretty=%B'])).toMatchInlineSnapshot(`
-                                                "Merge branch 'branch-b'
+                                                      "Merge branch 'branch-b'
 
-                                                # Conflicts:
-                                                #	test.js
-                                                "
-                                `)
+                                                      # Conflicts:
+                                                      #	test.js
+                                                      "
+                                    `)
     expect(await readFile('test.js')).toEqual(fileInBranchBFixed)
   })
 
@@ -490,13 +490,13 @@ describe('runAll', () => {
     expect(await execGit(['rev-list', '--count', 'HEAD'])).toEqual('1')
     expect(await execGit(['log', '-1', '--pretty=%B'])).toMatch('initial commit')
     expect(await readFile('README.md')).toMatchInlineSnapshot(`
-            "# Test
+                  "# Test
 
-            ## Amended
+                  ## Amended
 
-            ## Edited
-            "
-        `)
+                  ## Edited
+                  "
+            `)
     expect(await readFile('test-untracked.js')).toEqual(testJsFilePretty)
     const status = await execGit(['status'])
     expect(status).toMatch('modified:   README.md')