Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: parse command string with string-argv unless --shell is used
execa.command splits arguments from spaces even when it's part of a filename
  • Loading branch information
iiroj committed Jan 21, 2020
1 parent 47985b4 commit 4cb4dde
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 21 deletions.
15 changes: 10 additions & 5 deletions lib/resolveTaskFn.js
@@ -1,6 +1,7 @@
'use strict'

const chalk = require('chalk')
const { parseArgsStringToArgv } = require('string-argv')
const dedent = require('dedent')
const execa = require('execa')
const symbols = require('log-symbols')
Expand Down Expand Up @@ -66,26 +67,30 @@ function makeErr(linter, result, context = {}) {
* @returns {function(): Promise<Array<string>>}
*/
module.exports = function resolveTaskFn({ command, files, gitDir, isFn, relative, shell = false }) {
const cmd = isFn ? command : `${command} ${files.join(' ')}`
const [cmd, ...args] = parseArgsStringToArgv(command)
debug('cmd:', cmd)
debug('args:', args)

const execaOptions = { preferLocal: true, reject: false, shell }
if (relative) {
execaOptions.cwd = process.cwd()
} else if (/^git(\.exe)?/i.test(command) && gitDir !== process.cwd()) {
} else if (/^git(\.exe)?/i.test(cmd) && gitDir !== process.cwd()) {
// Only use gitDir as CWD if we are using the git binary
// e.g `npm` should run tasks in the actual CWD
execaOptions.cwd = gitDir
}
debug('execaOptions:', execaOptions)

return async ctx => {
const result = await execa.command(cmd, execaOptions)
const promise = shell
? execa.command(isFn ? command : `${command} ${files.join(' ')}`, execaOptions)
: execa(cmd, isFn ? args : args.concat(files), execaOptions)
const result = await promise

if (result.failed || result.killed || result.signal != null) {
throw makeErr(command, result, ctx)
throw makeErr(cmd, result, ctx)
}

return successMsg(command)
return successMsg(cmd)
}
}
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -45,6 +45,7 @@
"micromatch": "^4.0.2",
"normalize-path": "^3.0.0",
"please-upgrade-node": "^3.2.0",
"string-argv": "0.3.1",
"stringify-object": "^3.3.0"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion test/__snapshots__/index.spec.js.snap
Expand Up @@ -5,7 +5,7 @@ exports[`lintStaged should exit with code 1 on linter errors 1`] = `
ERROR
× node -e \\"process.exit(1)\\" found some errors. Please fix them and try committing again."
× node found some errors. Please fix them and try committing again."
`;

exports[`lintStaged should load an npm config package when specified 1`] = `
Expand Down
4 changes: 2 additions & 2 deletions test/__snapshots__/runAll.spec.js.snap
Expand Up @@ -44,7 +44,7 @@ LOG {
name: 'ListrError',
errors: [
{
privateMsg: '\\\\n\\\\n\\\\n× echo \\"sample\\" found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
privateMsg: '\\\\n\\\\n\\\\n× echo found some errors. Please fix them and try committing again.\\\\n\\\\nLinter finished with error',
context: {taskError: true}
}
],
Expand Down Expand Up @@ -75,7 +75,7 @@ LOG {
name: 'ListrError',
errors: [
{
privateMsg: '\\\\n\\\\n\\\\n‼ echo \\"sample\\" was terminated with SIGINT',
privateMsg: '\\\\n\\\\n\\\\n‼ echo was terminated with SIGINT',
context: {taskError: true}
}
],
Expand Down
4 changes: 2 additions & 2 deletions test/makeCmdTasks.spec.js
Expand Up @@ -42,7 +42,7 @@ describe('makeCmdTasks', () => {
expect(taskPromise).toBeInstanceOf(Promise)
await taskPromise
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('test test.js', {
expect(execa).lastCalledWith('test', ['test.js'], {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -51,7 +51,7 @@ describe('makeCmdTasks', () => {
expect(taskPromise).toBeInstanceOf(Promise)
await taskPromise
expect(execa).toHaveBeenCalledTimes(2)
expect(execa).lastCalledWith('test2 test.js', {
expect(execa).lastCalledWith('test2', ['test.js'], {
preferLocal: true,
reject: false,
shell: false
Expand Down
10 changes: 5 additions & 5 deletions test/resolveTaskFn.spec.js
Expand Up @@ -18,7 +18,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -35,7 +35,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('node --arg=true ./myscript.js test.js', {
expect(execa).lastCalledWith('node', ['--arg=true', './myscript.js', 'test.js'], {
preferLocal: true,
reject: false,
shell: false
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git diff test.js', {
expect(execa).lastCalledWith('git', ['diff', 'test.js'], {
cwd: '../',
preferLocal: true,
reject: false,
Expand All @@ -101,7 +101,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('jest test.js', {
expect(execa).lastCalledWith('jest', ['test.js'], {
preferLocal: true,
reject: false,
shell: false
Expand All @@ -118,7 +118,7 @@ describe('resolveTaskFn', () => {

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git diff test.js', {
expect(execa).lastCalledWith('git', ['diff', 'test.js'], {
cwd: process.cwd(),
preferLocal: true,
reject: false,
Expand Down
4 changes: 1 addition & 3 deletions test/resolveTaskFn.unmocked.spec.js
Expand Up @@ -11,8 +11,6 @@ describe('resolveTaskFn', () => {
shell: true
})

await expect(taskFn()).resolves.toMatchInlineSnapshot(
`"√ node -e \\"process.exit(1)\\" || echo $? passed!"`
)
await expect(taskFn()).resolves.toMatchInlineSnapshot(`"√ node passed!"`)
})
})
6 changes: 3 additions & 3 deletions test/runAll.unmocked.spec.js
Expand Up @@ -121,16 +121,16 @@ describe('runAll', () => {

it('Should commit entire staged file when no errors from linter', async () => {
// Stage pretty file
await appendFile('test.js', testJsFilePretty)
await execGit(['add', 'test.js'])
await appendFile('test file.js', testJsFilePretty)
await execGit(['add', 'test file.js'])

// Run lint-staged with `prettier --list-different` and commit pretty file
await gitCommit({ config: { '*.js': 'prettier --list-different' } })

// 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)
expect(await readFile('test file.js')).toEqual(testJsFilePretty)
})

it('Should commit entire staged file when no errors and linter modifies file', async () => {
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Expand Up @@ -5305,6 +5305,11 @@ stealthy-require@^1.1.1:
resolved "https://registry.yarnpkg.com/stealthy-require/-/stealthy-require-1.1.1.tgz#35b09875b4ff49f26a777e509b3090a3226bf24b"
integrity sha1-NbCYdbT/SfJqd35QmzCQoyJr8ks=

string-argv@0.3.1:
version "0.3.1"
resolved "https://registry.yarnpkg.com/string-argv/-/string-argv-0.3.1.tgz#95e2fbec0427ae19184935f816d74aaa4c5c19da"
integrity sha512-a1uQGz7IyVy9YwhqjZIZu1c8JO8dNIe20xBmSS6qu9kv++k3JGzCVmprbNN5Kn+BgzD5E7YYwg1CcjuJMRNsvg==

string-length@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/string-length/-/string-length-2.0.0.tgz#d40dbb686a3ace960c1cffca562bf2c45f8363ed"
Expand Down

0 comments on commit 4cb4dde

Please sign in to comment.