Skip to content

Commit

Permalink
feat: warn when task contains "git add"
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Nov 14, 2019
1 parent 74ed28d commit 5208399
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 56 deletions.
6 changes: 3 additions & 3 deletions .lintstagedrc.json
@@ -1,5 +1,5 @@
{
"*.{js,json,md}": ["prettier --write"],
"*.js": ["npm run lint:base --fix"],
".*{rc, json}": ["jsonlint --in-place"]
"*.{js,json,md}": "prettier --write",
"*.js": "npm run lint:base --fix",
".*{rc, json}": "jsonlint --in-place"
}
5 changes: 3 additions & 2 deletions lib/makeCmdTasks.js
Expand Up @@ -11,9 +11,10 @@ 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, shell }) {
module.exports = async function makeCmdTasks({ commands, files, gitDir, logger, shell }) {
debug('Creating listr tasks for commands %o', commands)
const commandsArray = Array.isArray(commands) ? commands : [commands]

Expand Down Expand Up @@ -41,7 +42,7 @@ module.exports = async function makeCmdTasks({ commands, files, gitDir, shell })
title = mockCommands[i].replace(/\[file\].*\[file\]/, '[file]')
}

const task = { title, task: resolveTaskFn({ gitDir, isFn, command, files, shell }) }
const task = { title, task: resolveTaskFn({ command, files, gitDir, isFn, logger, shell }) }
tasks.push(task)
})

Expand Down
18 changes: 17 additions & 1 deletion lib/resolveTaskFn.js
Expand Up @@ -62,14 +62,30 @@ 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, relative, shell = false }) {
module.exports = function resolveTaskFn({
command,
files,
gitDir,
isFn,
logger,
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()
Expand Down
8 changes: 7 additions & 1 deletion lib/runAll.js
Expand Up @@ -72,7 +72,13 @@ module.exports = async function runAll(
title: `Running tasks for ${task.pattern}`,
task: async () =>
new Listr(
await makeCmdTasks({ commands: task.commands, files: task.fileList, gitDir, shell }),
await makeCmdTasks({
commands: task.commands,
files: task.fileList,
gitDir,
logger,
shell
}),
{
// In sub-tasks we don't want to run concurrently
// and we want to abort on errors
Expand Down
1 change: 1 addition & 0 deletions test/makeCmdTasks.spec.js
@@ -1,4 +1,5 @@
import execa from 'execa'

import makeCmdTasks from '../lib/makeCmdTasks'

describe('makeCmdTasks', () => {
Expand Down
39 changes: 28 additions & 11 deletions test/resolveTaskFn.spec.js
@@ -1,4 +1,6 @@
import execa from 'execa'
import makeConsoleMock from 'consolemock'

import resolveTaskFn from '../lib/resolveTaskFn'

const defaultOpts = { files: ['test.js'] }
Expand Down Expand Up @@ -80,13 +82,13 @@ describe('resolveTaskFn', () => {
expect.assertions(2)
const taskFn = resolveTaskFn({
...defaultOpts,
command: 'git add',
command: 'git diff',
gitDir: '../'
})

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git add test.js', {
expect(execa).lastCalledWith('git diff test.js', {
cwd: '../',
preferLocal: true,
reject: false,
Expand All @@ -111,13 +113,13 @@ describe('resolveTaskFn', () => {
expect.assertions(2)
const taskFn = resolveTaskFn({
...defaultOpts,
command: 'git add',
command: 'git diff',
relative: true
})

await taskFn()
expect(execa).toHaveBeenCalledTimes(1)
expect(execa).lastCalledWith('git add test.js', {
expect(execa).lastCalledWith('git diff test.js', {
cwd: process.cwd(),
preferLocal: true,
reject: false,
Expand All @@ -140,12 +142,12 @@ describe('resolveTaskFn', () => {
await taskFn()
} catch (err) {
expect(err.privateMsg).toMatchInlineSnapshot(`
"
"
× mock-fail-linter found some errors. Please fix them and try committing again.
Mock error"
`)
× mock-fail-linter found some errors. Please fix them and try committing again.
Mock error"
`)
}
})

Expand All @@ -166,11 +168,11 @@ Mock error"
await taskFn()
} catch (err) {
expect(err.privateMsg).toMatchInlineSnapshot(`
"
"
‼ mock-killed-linter was terminated with SIGINT"
`)
‼ mock-killed-linter was terminated with SIGINT"
`)
}
})

Expand Down Expand Up @@ -199,4 +201,19 @@ Mock error"
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."
`)
})
})
78 changes: 40 additions & 38 deletions test/runAll.unmocked.spec.js
Expand Up @@ -339,16 +339,18 @@ describe('runAll', () => {
} catch (error) {
expect(error.message).toMatch('Another git process seems to be running in this repository')
expect(console.printHistory()).toMatchInlineSnapshot(`
"
ERROR
× lint-staged failed due to a git error.
Any lost modifications can be restored from a git stash:
> git stash list
stash@{0}: On master: automatic lint-staged backup
> git stash pop stash@{0}
"
`)
"
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:
> git stash list
stash@{0}: On master: automatic lint-staged backup
> git stash pop stash@{0}
"
`)
}

// Something was wrong so new commit wasn't created
Expand All @@ -358,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)
Expand Down Expand Up @@ -422,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)
Expand All @@ -442,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)
})

Expand Down Expand Up @@ -488,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')
Expand Down

0 comments on commit 5208399

Please sign in to comment.