From 279863d25ef7dab856ebae418b9863880902bf52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Sun, 23 Jun 2019 20:56:38 +0300 Subject: [PATCH] refactor: Use internal execGit to list staged files (#624) --- .lintstagedrc.json | 2 +- package.json | 3 +- src/getStagedFiles.js | 12 +++++++ src/runAll.js | 20 +++++------ test/__snapshots__/runAll.spec.js.snap | 2 ++ test/getStagedFiles.spec.js | 26 ++++++++++++++ test/gitStash.spec.js | 33 +++++++++--------- test/runAll.spec.js | 47 ++++++++------------------ yarn.lock | 26 +++++++++++--- 9 files changed, 103 insertions(+), 68 deletions(-) create mode 100644 src/getStagedFiles.js create mode 100644 test/getStagedFiles.spec.js diff --git a/.lintstagedrc.json b/.lintstagedrc.json index 71cd2fa0d..533a7f58a 100644 --- a/.lintstagedrc.json +++ b/.lintstagedrc.json @@ -2,6 +2,6 @@ "linters": { "*.{js,json,md}": ["prettier --write", "git add"], "*.js": ["npm run lint:base --fix", "git add"], - ".*{rc, json}": "jsonlint" + ".*{rc, json}": ["jsonlint", "git add"] } } diff --git a/package.json b/package.json index 9af40e7b3..3fb9f917f 100644 --- a/package.json +++ b/package.json @@ -43,9 +43,7 @@ "npm-which": "^3.0.1", "p-map": "^1.1.1", "path-is-inside": "^1.0.2", - "pify": "^3.0.0", "please-upgrade-node": "^3.0.2", - "staged-git-files": "1.1.2", "string-argv": "^0.0.2", "stringify-object": "^3.2.2", "yup": "^0.27.0" @@ -61,6 +59,7 @@ "eslint": "^4.18.1", "eslint-config-okonet": "^5.0.1", "eslint-plugin-node": "^6.0.0", + "fs-extra": "^8.0.1", "husky": "^1.1.4", "jest": "^23.6.0", "jest-snapshot-serializer-ansi": "^1.0.0", diff --git a/src/getStagedFiles.js b/src/getStagedFiles.js new file mode 100644 index 000000000..e9904524d --- /dev/null +++ b/src/getStagedFiles.js @@ -0,0 +1,12 @@ +'use strict' + +const execGit = require('./execGit') + +module.exports = async function getStagedFiles(options) { + try { + const lines = await execGit(['diff', '--staged', '--diff-filter=ACM', '--name-only'], options) + return lines ? lines.split('\n') : [] + } catch (error) { + return null + } +} diff --git a/src/runAll.js b/src/runAll.js index fcf6e99cc..49b4f0300 100644 --- a/src/runAll.js +++ b/src/runAll.js @@ -1,13 +1,12 @@ 'use strict' -const sgf = require('staged-git-files') +const generateTasks = require('./generateTasks') +const git = require('./gitWorkflow') +const getStagedFiles = require('./getStagedFiles') const Listr = require('listr') const has = require('lodash/has') -const pify = require('pify') const makeCmdTasks = require('./makeCmdTasks') -const generateTasks = require('./generateTasks') const resolveGitDir = require('./resolveGitDir') -const git = require('./gitWorkflow') const debug = require('debug')('lint-staged:run') @@ -32,14 +31,15 @@ module.exports = async function runAll(config) { debug('Resolved git directory to be `%s`', gitDir) - sgf.cwd = gitDir + const files = await getStagedFiles({ cwd: gitDir }) + + if (!files) { + throw new Error('Unable to get staged files!') + } - /* files is an Object{ filename: String, status: String } */ - const files = await pify(sgf)('ACM') - const filenames = files.map(file => file.filename) - debug('Loaded list of staged files in git:\n%O', filenames) + debug('Loaded list of staged files in git:\n%O', files) - const tasks = (await generateTasks(config, gitDir, filenames)).map(task => ({ + const tasks = (await generateTasks(config, gitDir, files)).map(task => ({ title: `Running tasks for ${task.pattern}`, task: async () => new Listr( diff --git a/test/__snapshots__/runAll.spec.js.snap b/test/__snapshots__/runAll.spec.js.snap index f87fd370b..38c413186 100644 --- a/test/__snapshots__/runAll.spec.js.snap +++ b/test/__snapshots__/runAll.spec.js.snap @@ -29,6 +29,8 @@ LOG Running tasks for *.js [completed] LOG Running linters... [completed]" `; +exports[`runAll should reject promise when error during getStagedFiles 1`] = `"Unable to get staged files!"`; + exports[`runAll should resolve the promise with no files 1`] = ` " LOG No staged files match any of provided globs." diff --git a/test/getStagedFiles.spec.js b/test/getStagedFiles.spec.js new file mode 100644 index 000000000..3ced4734a --- /dev/null +++ b/test/getStagedFiles.spec.js @@ -0,0 +1,26 @@ +import getStagedFiles from '../src/getStagedFiles' +import execGit from '../src/execGit' + +jest.mock('../src/execGit') + +describe('getStagedFiles', () => { + it('should return array of file names', async () => { + execGit.mockImplementationOnce(async () => 'foo.js\nbar.js') + const staged = await getStagedFiles() + expect(staged).toEqual(['foo.js', 'bar.js']) + }) + + it('should return empty array when no staged files', async () => { + execGit.mockImplementationOnce(async () => '') + const staged = await getStagedFiles() + expect(staged).toEqual([]) + }) + + it('should return null in case of error', async () => { + execGit.mockImplementationOnce(async () => { + throw new Error('fatal: not a git repository (or any of the parent directories): .git') + }) + const staged = await getStagedFiles() + expect(staged).toEqual(null) + }) +}) diff --git a/test/gitStash.spec.js b/test/gitStash.spec.js index cddb9372d..27c95f295 100644 --- a/test/gitStash.spec.js +++ b/test/gitStash.spec.js @@ -2,8 +2,7 @@ const execa = require('execa') const path = require('path') const tmp = require('tmp') const gitflow = require('../src/gitWorkflow') -const pify = require('pify') -const fsp = pify(require('fs')) +const fs = require('fs-extra') tmp.setGracefulCleanup() jest.unmock('execa') @@ -21,7 +20,7 @@ async function gitStatus(opts = gitOpts) { } async function readFile(filepath, dir = wcDirPath) { - const content = await fsp.readFile(path.join(dir, filepath), { encoding: 'utf-8' }) + const content = await fs.readFile(path.join(dir, filepath), { encoding: 'utf-8' }) return content.replace(/\r\n/g, '\n') } @@ -36,7 +35,7 @@ describe('git', () => { // Init repository await gitflow.execGit('init', gitOpts) // Create JS file - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: 'test', @@ -45,7 +44,7 @@ describe('git', () => { } ` ) - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.css'), `.test { border: 1px solid green; @@ -59,9 +58,9 @@ describe('git', () => { // Create inital commit await gitflow.execGit(['commit', '-m', '"Initial commit"'], gitOpts) // Update one of the files - await fsp.writeFile(path.join(wcDirPath, 'test.css'), '.test { border: red; }') + await fs.writeFile(path.join(wcDirPath, 'test.css'), '.test { border: red; }') // Update one of the files - await fsp.writeFile(path.join(wcDirPath, 'test.js'), initialContent) + await fs.writeFile(path.join(wcDirPath, 'test.js'), initialContent) }) afterEach(() => { @@ -97,7 +96,7 @@ describe('git', () => { it('should return true if files are modified and in the index', async () => { await gitflow.execGit(['checkout', 'test.css'], gitOpts) await gitflow.execGit(['add', 'test.js'], gitOpts) - await fsp.writeFile(path.join(wcDirPath, 'test.js'), '') + await fs.writeFile(path.join(wcDirPath, 'test.js'), '') const res = await gitflow.hasPartiallyStagedFiles(gitOpts) expect(res).toEqual(true) }) @@ -226,7 +225,7 @@ M test.js" test: 'test2', test: 'test2', };` - await fsp.writeFile(path.join(wcDirPath, 'test.js'), eslintContent) + await fs.writeFile(path.join(wcDirPath, 'test.js'), eslintContent) // Expect both indexed and modified state on one file expect(await gitStatus()).toMatchInlineSnapshot(`"MM test.js"`) @@ -262,7 +261,7 @@ M test.js" test: 'test2', test: 'test3', }` - await fsp.writeFile(path.join(wcDirPath, 'test.js'), userContent) + await fs.writeFile(path.join(wcDirPath, 'test.js'), userContent) // Expect test.js is in both index and modified expect(await gitStatus()).toMatchInlineSnapshot(` @@ -278,7 +277,7 @@ MM test.js" expect(await gitStatus()).toMatchInlineSnapshot(`"M test.js"`) // Do additional edits (imitate eslint --fix) - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: 'test2', @@ -330,7 +329,7 @@ M test.js" const newContent = `module.exports = { test: "test2", };` - await fsp.writeFile(path.join(wcDirPath, 'test.js'), newContent) + await fs.writeFile(path.join(wcDirPath, 'test.js'), newContent) // and add to index await gitflow.execGit(['add', 'test.js'], gitOpts) await gitflow.updateStash(gitOpts) @@ -372,7 +371,7 @@ M test.js" test: 'test2', test: 'test3' }` - await fsp.writeFile(path.join(wcDirPath, 'test.js'), userContent) + await fs.writeFile(path.join(wcDirPath, 'test.js'), userContent) // Expect test.js is in both index and modified expect(await gitStatus()).toMatchInlineSnapshot(` @@ -389,7 +388,7 @@ MM test.js" expect(await gitflow.execGit(['diff', '--cached'], gitOpts)).toEqual(initialIndex) // Do additional edits (imitate eslint --fix) - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: "test2" @@ -423,7 +422,7 @@ MM test.js" await gitflow.execGit(['checkout', '--', '.'], gitOpts) // Do additional edits and stage them - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: 'test', @@ -441,7 +440,7 @@ MM test.js" await gitflow.execGit(['add', 'test.js'], gitOpts) // Do additional edits without adding to index - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: 'edited', @@ -469,7 +468,7 @@ MM test.js" expect(await gitflow.execGit(['diff', '--cached'], gitOpts)).toEqual(initialIndex) // Imitate running prettier on the version from the index - await fsp.writeFile( + await fs.writeFile( path.join(wcDirPath, 'test.js'), `module.exports = { test: "test", diff --git a/test/runAll.spec.js b/test/runAll.spec.js index f67b6689a..7015f3513 100644 --- a/test/runAll.spec.js +++ b/test/runAll.spec.js @@ -1,16 +1,16 @@ import makeConsoleMock from 'consolemock' -import sgfMock from 'staged-git-files' import execa from 'execa' + import { getConfig } from '../src/getConfig' +import getStagedFiles from '../src/getStagedFiles' import runAll from '../src/runAll' import { hasPartiallyStagedFiles, gitStashSave, gitStashPop, updateStash } from '../src/gitWorkflow' -jest.mock('staged-git-files') +jest.mock('../src/getStagedFiles') jest.mock('../src/gitWorkflow') -sgfMock.mockImplementation((params, callback) => { - callback(null, []) -}) +getStagedFiles.mockImplementation(async () => []) + const globalConsoleTemp = global.console describe('runAll', () => { @@ -60,9 +60,7 @@ describe('runAll', () => { it('should not skip tasks if there are files', async () => { expect.assertions(1) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.js', status: 'sample' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.js']) await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } })) expect(console.printHistory()).toMatchSnapshot() }) @@ -70,9 +68,7 @@ describe('runAll', () => { it('should not skip stashing and restoring if there are partially staged files', async () => { expect.assertions(4) hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true)) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.js', status: 'Modified' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.js']) await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } })) expect(gitStashSave).toHaveBeenCalledTimes(1) expect(updateStash).toHaveBeenCalledTimes(1) @@ -83,9 +79,7 @@ describe('runAll', () => { it('should skip stashing and restoring if there are no partially staged files', async () => { expect.assertions(4) hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(false)) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.js', status: 'Modified' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.js']) await runAll(getConfig({ linters: { '*.js': ['echo "sample"'] } })) expect(gitStashSave).toHaveBeenCalledTimes(0) expect(updateStash).toHaveBeenCalledTimes(0) @@ -96,9 +90,7 @@ describe('runAll', () => { it('should skip updating stash if there are errors during linting', async () => { expect.assertions(4) hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true)) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.js', status: 'Modified' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.js']) execa.mockImplementation(() => Promise.resolve({ stdout: '', @@ -123,9 +115,7 @@ describe('runAll', () => { it('should skip linters and stash update but perform working copy restore if terminated', async () => { expect.assertions(4) hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true)) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.js', status: 'Modified' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.js']) execa.mockImplementation(() => Promise.resolve({ stdout: '', @@ -149,25 +139,16 @@ describe('runAll', () => { expect(gitStashPop).toHaveBeenCalledTimes(1) }) - it('should reject promise when staged-git-files errors', async () => { + it('should reject promise when error during getStagedFiles', async () => { expect.assertions(1) - sgfMock.mockImplementationOnce((params, callback) => { - callback('test', undefined) - }) - - try { - await runAll(getConfig({})) - } catch (err) { - expect(err).toEqual('test') - } + getStagedFiles.mockImplementationOnce(async () => null) + await expect(runAll(getConfig({}))).rejects.toThrowErrorMatchingSnapshot() }) it('should skip stashing changes if no lint-staged files are changed', async () => { expect.assertions(4) hasPartiallyStagedFiles.mockImplementationOnce(() => Promise.resolve(true)) - sgfMock.mockImplementationOnce((params, callback) => { - callback(null, [{ filename: 'sample.java', status: 'Modified' }]) - }) + getStagedFiles.mockImplementationOnce(async () => ['sample.java']) execa.mockImplementationOnce(() => Promise.resolve({ stdout: '', diff --git a/yarn.lock b/yarn.lock index 327439134..f535d0b05 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2190,6 +2190,15 @@ fs-extra@^1.0.0: jsonfile "^2.1.0" klaw "^1.0.0" +fs-extra@^8.0.1: + version "8.0.1" + resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-8.0.1.tgz#90294081f978b1f182f347a440a209154344285b" + integrity sha512-W+XLrggcDzlle47X/XnS7FXrXu9sDo+Ze9zpndeBxdgv88FHLm1HtmkhEwavruS6koanBjp098rUpHs65EmG7A== + dependencies: + graceful-fs "^4.1.2" + jsonfile "^4.0.0" + universalify "^0.1.0" + fs-minipass@^1.2.5: version "1.2.5" resolved "https://registry.yarnpkg.com/fs-minipass/-/fs-minipass-1.2.5.tgz#06c277218454ec288df77ada54a03b8702aacb9d" @@ -3475,6 +3484,13 @@ jsonfile@^2.1.0: optionalDependencies: graceful-fs "^4.1.6" +jsonfile@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/jsonfile/-/jsonfile-4.0.0.tgz#8771aae0799b64076b76640fca058f9c10e33ecb" + integrity sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss= + optionalDependencies: + graceful-fs "^4.1.6" + jsonlint@^1.6.2: version "1.6.3" resolved "https://registry.yarnpkg.com/jsonlint/-/jsonlint-1.6.3.tgz#cb5e31efc0b78291d0d862fbef05900adf212988" @@ -5144,11 +5160,6 @@ stack-utils@^1.0.1: resolved "https://registry.yarnpkg.com/stack-utils/-/stack-utils-1.0.2.tgz#33eba3897788558bebfc2db059dc158ec36cebb8" integrity sha512-MTX+MeG5U994cazkjd/9KNAapsHnibjMLnfXodlkXw76JEea0UiNzrqidzo1emMwk7w5Qhc9jd4Bn9TBb1MFwA== -staged-git-files@1.1.2: - version "1.1.2" - resolved "https://registry.yarnpkg.com/staged-git-files/-/staged-git-files-1.1.2.tgz#4326d33886dc9ecfa29a6193bf511ba90a46454b" - integrity sha512-0Eyrk6uXW6tg9PYkhi/V/J4zHp33aNyi2hOCmhFLqLTIhbgqWn5jlSzI+IU0VqrZq6+DbHcabQl/WP6P3BG0QA== - static-extend@^0.1.1: version "0.1.2" resolved "https://registry.yarnpkg.com/static-extend/-/static-extend-0.1.2.tgz#60809c39cbff55337226fd5e0b520f341f1fb5c6" @@ -5468,6 +5479,11 @@ union-value@^1.0.0: is-extendable "^0.1.1" set-value "^0.4.3" +universalify@^0.1.0: + version "0.1.2" + resolved "https://registry.yarnpkg.com/universalify/-/universalify-0.1.2.tgz#b646f69be3942dabcecc9d6639c80dc105efaa66" + integrity sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg== + unset-value@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/unset-value/-/unset-value-1.0.0.tgz#8376873f7d2335179ffb1e6fc3a8ed0dfc8ab559"