fix: run all commands returned by function linters

Function linters have to be evaluated at makeCmdTasks, and resolveTaskFn has to run for each of the returned commands.

Author: iiroj

src/makeCmdTasks.js

@@ -4,19 +4,6 @@ const resolveTaskFn = require('./resolveTaskFn')
 
 const debug = require('debug')('lint-staged:make-cmd-tasks')
 
-/**
- * Get title for linter task. For a function, evaluate by passing single [file].
- * For strings, return as-is
- * @param {string|Function} linter
- */
-const getTitle = linter => {
-  if (typeof linter === 'function') {
-    const resolved = linter(['[file]'])
-    return Array.isArray(resolved) ? resolved[0] : resolved
-  }
-  return linter
-}
-
 /**
  * Creates and returns an array of listr tasks which map to the given commands.
  *
@@ -27,16 +14,23 @@ const getTitle = linter => {
  */
 module.exports = async function makeCmdTasks(commands, shell, gitDir, pathsToLint) {
   debug('Creating listr tasks for commands %o', commands)
+  const commandsArray = Array.isArray(commands) ? commands : [commands]
+
+  return commandsArray.reduce((tasks, command) => {
+    // linter function may return array of commands that already include `pathsToLit`
+    const isFn = typeof command === 'function'
+    const resolved = isFn ? command(pathsToLint) : command
+    const linters = Array.isArray(resolved) ? resolved : [resolved] // Wrap non-array linter as array
 
-  const lintersArray = Array.isArray(commands) ? commands : [commands]
+    linters.forEach(linter => {
+      const task = {
+        title: linter,
+        task: resolveTaskFn({ gitDir, isFn, linter, pathsToLint, shell })
+      }
 
-  return lintersArray.map(linter => ({
-    title: getTitle(linter),
-    task: resolveTaskFn({
-      linter,
-      shell,
-      gitDir,
-      pathsToLint
+      tasks.push(task)
     })
-  }))
+
+    return tasks
+  }, [])
 }

src/resolveTaskFn.js

@@ -13,9 +13,11 @@ const debug = require('debug')('lint-staged:task')
  * return the promise.
  *
  * @param {string} cmd
+ * @param {Array<string>} args
+ * @param {Object} execaOptions
  * @return {Promise} child_process
  */
-const execLinter = (cmd, args, execaOptions = {}) => {
+const execLinter = (cmd, args, execaOptions) => {
   debug('cmd:', cmd)
   if (args) debug('args:', args)
   debug('execaOptions:', execaOptions)
@@ -75,52 +77,42 @@ function makeErr(linter, result, context = {}) {
  * if the OS is Windows.
  *
  * @param {Object} options
+ * @param {string} options.gitDir
+ * @param {Boolean} options.isFn
  * @param {string} options.linter
  * @param {Boolean} options.shellMode
- * @param {string} options.gitDir
  * @param {Array<string>} options.pathsToLint
  * @returns {function(): Promise<Array<string>>}
  */
-module.exports = function resolveTaskFn({ gitDir, linter, pathsToLint, shell = false }) {
-  // If `linter` is a function, it should return a string when evaluated with `pathsToLint`.
-  // Else, it's a already a string
-  const fnLinter = typeof linter === 'function'
-  const linterString = fnLinter ? linter(pathsToLint) : linter
-  // Support arrays of strings/functions by treating everything as arrays
-  const linters = Array.isArray(linterString) ? linterString : [linterString]
-
+module.exports = function resolveTaskFn({ gitDir, isFn, linter, pathsToLint, shell = false }) {
   const execaOptions = { preferLocal: true, reject: false, shell }
 
-  const tasks = linters.map(command => {
-    let cmd
-    let args
+  let cmd
+  let args
 
-    if (shell) {
-      execaOptions.shell = true
-      // If `shell`, passed command shouldn't be parsed
-      // If `linter` is a function, command already includes `pathsToLint`.
-      cmd = fnLinter ? command : `${command} ${pathsToLint.join(' ')}`
-    } else {
-      const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(command)
-      cmd = parsedCmd
-      args = fnLinter ? parsedArgs : parsedArgs.concat(pathsToLint)
-    }
-
-    // Only use gitDir as CWD if we are using the git binary
-    // e.g `npm` should run tasks in the actual CWD
-    if (/^git(\.exe)?/i.test(command) && gitDir !== process.cwd()) {
-      execaOptions.cwd = gitDir
-    }
+  if (shell) {
+    execaOptions.shell = true
+    // If `shell`, passed command shouldn't be parsed
+    // If `linter` is a function, command already includes `pathsToLint`.
+    cmd = isFn ? linter : `${linter} ${pathsToLint.join(' ')}`
+  } else {
+    const [parsedCmd, ...parsedArgs] = stringArgv.parseArgsStringToArgv(linter)
+    cmd = parsedCmd
+    args = isFn ? parsedArgs : parsedArgs.concat(pathsToLint)
+  }
 
-    return ctx =>
-      execLinter(cmd, args, execaOptions).then(result => {
-        if (result.failed || result.killed || result.signal != null) {
-          throw makeErr(linter, result, ctx)
-        }
+  // Only use gitDir as CWD if we are using the git binary
+  // e.g `npm` should run tasks in the actual CWD
+  if (/^git(\.exe)?/i.test(linter) && gitDir !== process.cwd()) {
+    execaOptions.cwd = gitDir
+  }
 
-        return successMsg(linter)
-      })
-  })
+  return ctx =>
+    execLinter(cmd, args, execaOptions).then(result => {
+      if (result.failed || result.killed || result.signal != null) {
+        throw makeErr(linter, result, ctx)
+      }
 
-  return ctx => Promise.all(tasks.map(task => task(ctx)))
+      return successMsg(linter)
+    })
 }

test/makeCmdTasks.spec.js

@@ -61,25 +61,35 @@ describe('makeCmdTasks', () => {
 
   it('should work with function linter returning array of string', async () => {
     const res = await makeCmdTasks(() => ['test', 'test2'], false, gitDir, ['test.js'])
-    expect(res.length).toBe(1)
+    expect(res.length).toBe(2)
     expect(res[0].title).toEqual('test')
+    expect(res[1].title).toEqual('test2')
   })
 
   it('should work with function linter accepting arguments', async () => {
     const res = await makeCmdTasks(
       filenames => filenames.map(file => `test ${file}`),
       false,
       gitDir,
-      ['test.js']
+      ['test.js', 'test2.js']
     )
-    expect(res.length).toBe(1)
-    expect(res[0].title).toEqual('test [file]')
+    expect(res.length).toBe(2)
+    expect(res[0].title).toEqual('test test.js')
+    expect(res[1].title).toEqual('test test2.js')
   })
 
   it('should work with array of mixed string and function linters', async () => {
-    const res = await makeCmdTasks([() => 'test', 'test2'], false, gitDir, ['test.js'])
-    expect(res.length).toBe(2)
+    const res = await makeCmdTasks(
+      [() => 'test', 'test2', files => files.map(file => `test ${file}`)],
+      false,
+      gitDir,
+      ['test.js', 'test2.js', 'test3.js']
+    )
+    expect(res.length).toBe(5)
     expect(res[0].title).toEqual('test')
     expect(res[1].title).toEqual('test2')
+    expect(res[2].title).toEqual('test test.js')
+    expect(res[3].title).toEqual('test test2.js')
+    expect(res[4].title).toEqual('test test3.js')
   })
 })

test/resolveTaskFn.spec.js

@@ -24,11 +24,12 @@ describe('resolveTaskFn', () => {
     })
   })
 
-  it('should support function linters that return string', async () => {
+  it('should not append pathsToLint when isFn', async () => {
     expect.assertions(2)
     const taskFn = resolveTaskFn({
       ...defaultOpts,
-      linter: filenames => `node --arg=true ./myscript.js ${filenames}`
+      isFn: true,
+      linter: 'node --arg=true ./myscript.js test.js'
     })
 
     await taskFn()
@@ -40,25 +41,38 @@ describe('resolveTaskFn', () => {
     })
   })
 
-  it('should support function linters that return array of strings', async () => {
-    expect.assertions(3)
+  it('should not append pathsToLint when isFn and shell', async () => {
+    expect.assertions(2)
     const taskFn = resolveTaskFn({
       ...defaultOpts,
-      pathsToLint: ['foo.js', 'bar.js'],
-      linter: filenames => filenames.map(filename => `node --arg=true ./myscript.js ${filename}`)
+      isFn: true,
+      shell: true,
+      linter: 'node --arg=true ./myscript.js test.js'
     })
 
     await taskFn()
-    expect(execa).toHaveBeenCalledTimes(2)
-    expect(execa).nthCalledWith(1, 'node', ['--arg=true', './myscript.js', 'foo.js'], {
+    expect(execa).toHaveBeenCalledTimes(1)
+    expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
       preferLocal: true,
       reject: false,
-      shell: false
+      shell: true
+    })
+  })
+
+  it('should work with shell', async () => {
+    expect.assertions(2)
+    const taskFn = resolveTaskFn({
+      ...defaultOpts,
+      shell: true,
+      linter: 'node --arg=true ./myscript.js'
     })
-    expect(execa).nthCalledWith(2, 'node', ['--arg=true', './myscript.js', 'bar.js'], {
+
+    await taskFn()
+    expect(execa).toHaveBeenCalledTimes(1)
+    expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
       preferLocal: true,
       reject: false,
-      shell: false
+      shell: true
     })
   })
 

test/resolveTaskFn.unmocked.spec.js

@@ -6,16 +6,13 @@ describe('resolveTaskFn', () => {
   it('should call execa with shell when configured so', async () => {
     const taskFn = resolveTaskFn({
       pathsToLint: ['package.json'],
-      linter: () => 'node -e "process.exit(1)" || echo $?',
+      isFn: true,
+      linter: 'node -e "process.exit(1)" || echo $?',
       shell: true
     })
 
-    await expect(taskFn()).resolves.toMatchInlineSnapshot(`
-Array [
-  "√ function linter() {
-        return 'node -e \\"process.exit(1)\\" || echo $?';
-      } passed!",
-]
-`)
+    await expect(taskFn()).resolves.toMatchInlineSnapshot(
+      `"√ node -e \\"process.exit(1)\\" || echo $? passed!"`
+    )
   })
 })