From 82b5182833b288ee826b5eb6769bdde8cddc8ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iiro=20J=C3=A4ppinen?= Date: Sun, 4 Aug 2019 11:10:36 +0300 Subject: [PATCH] fix: generateTasks handles parent dir globs correctly --- src/generateTasks.js | 32 +++------- test/generateTasks.spec.js | 128 +++++++++++++++---------------------- test/resolveGitDir.spec.js | 5 +- 3 files changed, 64 insertions(+), 101 deletions(-) diff --git a/src/generateTasks.js b/src/generateTasks.js index 1f3b8424e..a554e91f6 100644 --- a/src/generateTasks.js +++ b/src/generateTasks.js @@ -1,23 +1,11 @@ 'use strict' const micromatch = require('micromatch') +const normalize = require('normalize-path') const path = require('path') const debug = require('debug')('lint-staged:gen-tasks') -/** - * Test if `child` path is inside `parent` path - * https://stackoverflow.com/a/45242825 - * - * @param {String} parent - * @param {String} child - * @returns {Boolean} - */ -const isPathInside = (parent, child) => { - const relative = path.relative(parent, child) - return relative && !relative.startsWith('..') && !path.isAbsolute(relative) -} - /** * Generates all task commands, and filelist * @@ -37,16 +25,20 @@ module.exports = function generateTasks({ relative = false }) { debug('Generating linter tasks') + + const absoluteFiles = files.map(file => normalize(path.resolve(gitDir, file))) + const relativeFiles = absoluteFiles.map(file => normalize(path.relative(cwd, file))) + return Object.entries(config).map(([pattern, commands]) => { const isParentDirPattern = pattern.startsWith('../') + const fileList = micromatch( - files + relativeFiles // Only worry about children of the CWD unless the pattern explicitly // specifies that it concerns a parent directory. .filter(file => { if (isParentDirPattern) return true - const absolutePath = path.resolve(gitDir, file) - return isPathInside(cwd, absolutePath) + return !file.startsWith('..') && !path.isAbsolute(file) }), pattern, { @@ -55,13 +47,9 @@ module.exports = function generateTasks({ // If pattern doesn't look like a path, enable `matchBase` to // match against filenames in every directory. This makes `*.js` // match both `test.js` and `subdirectory/test.js`. - matchBase: !pattern.includes('/'), - posixSlashes: true + matchBase: !pattern.includes('/') } - ).map(file => - // Return absolute path after the filter is run - relative ? file : cwd + '/' + file - ) + ).map(file => normalize(relative ? file : path.resolve(cwd, file))) const task = { pattern, commands, fileList } debug('Generated task: \n%O', task) diff --git a/test/generateTasks.spec.js b/test/generateTasks.spec.js index 1e312c863..8a73f44f9 100644 --- a/test/generateTasks.spec.js +++ b/test/generateTasks.spec.js @@ -1,8 +1,12 @@ -import path from 'path' import os from 'os' +import normalize from 'normalize-path' +import path from 'path' + import generateTasks from '../src/generateTasks' import resolveGitDir from '../src/resolveGitDir' +const normalizePath = path => normalize(path) + const files = [ 'test.js', 'deeper/test.js', @@ -23,24 +27,11 @@ const files = [ '.hidden/test.txt' ] -const filesSpecialCases = files.concat([ - 'sub-dir/test.js', - 'sub-dir/deeper/test.js', - 'sub-dir/deeper/test2.js', - 'sub-dir/even/deeper/test.js', - 'sub-dir/.hidden/test.js', - - 'test.py', - 'deeper/test.py', - 'deeper/test2.py', - 'even/deeper/test.py', - '.hidden/test.py' -]) - // Mocks get hoisted jest.mock('../src/resolveGitDir.js') const gitDir = path.join(os.tmpdir(), 'tmp-lint-staged') resolveGitDir.mockResolvedValue(gitDir) +const cwd = gitDir const config = { '*.js': 'root-js', @@ -49,18 +40,10 @@ const config = { '.hidden/*.js': 'hidden-js', 'unknown/*.js': 'unknown-js', '*.{css,js}': 'root-css-or-js', - '../**/*.py': 'below-root-py' + '../*.{css,js}': 'parent-dir-css-or-js' } describe('generateTasks', () => { - beforeAll(() => { - jest.spyOn(process, 'cwd').mockReturnValue(gitDir) - }) - - afterAll(() => { - process.cwd.mockRestore() - }) - it('should return absolute paths', async () => { const [task] = await generateTasks({ config: { @@ -76,7 +59,7 @@ describe('generateTasks', () => { it('should not match non-children files', async () => { const relPath = path.join(process.cwd(), '..') - const result = await generateTasks({ config, gitDir: relPath, files }) + const result = await generateTasks({ config, cwd, gitDir: relPath, files }) const linter = result.find(item => item.pattern === '*.js') expect(linter).toEqual({ pattern: '*.js', @@ -86,10 +69,10 @@ describe('generateTasks', () => { }) it('should return an empty file list for linters with no matches.', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) result.forEach(task => { - if (task.commands === 'unknown-js' || task.commands === 'below-root-py') { + if (task.commands === 'unknown-js' || task.commands === 'parent-dir-css-or-js') { expect(task.fileList.length).toEqual(0) } else { expect(task.fileList.length).not.toEqual(0) @@ -98,7 +81,7 @@ describe('generateTasks', () => { }) it('should match pattern "*.js"', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) const linter = result.find(item => item.pattern === '*.js') expect(linter).toEqual({ pattern: '*.js', @@ -109,12 +92,12 @@ describe('generateTasks', () => { `${gitDir}/deeper/test2.js`, `${gitDir}/even/deeper/test.js`, `${gitDir}/.hidden/test.js` - ].map(path.normalize) + ].map(normalizePath) }) }) it('should match pattern "**/*.js"', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) const linter = result.find(item => item.pattern === '**/*.js') expect(linter).toEqual({ pattern: '**/*.js', @@ -125,32 +108,32 @@ describe('generateTasks', () => { `${gitDir}/deeper/test2.js`, `${gitDir}/even/deeper/test.js`, `${gitDir}/.hidden/test.js` - ].map(path.normalize) + ].map(normalizePath) }) }) it('should match pattern "deeper/*.js"', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) const linter = result.find(item => item.pattern === 'deeper/*.js') expect(linter).toEqual({ pattern: 'deeper/*.js', commands: 'deeper-js', - fileList: [`${gitDir}/deeper/test.js`, `${gitDir}/deeper/test2.js`].map(path.normalize) + fileList: [`${gitDir}/deeper/test.js`, `${gitDir}/deeper/test2.js`].map(normalizePath) }) }) it('should match pattern ".hidden/*.js"', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) const linter = result.find(item => item.pattern === '.hidden/*.js') expect(linter).toEqual({ pattern: '.hidden/*.js', commands: 'hidden-js', - fileList: [path.normalize(`${gitDir}/.hidden/test.js`)] + fileList: [`${gitDir}/.hidden/test.js`].map(normalizePath) }) }) it('should match pattern "*.{css,js}"', async () => { - const result = await generateTasks({ config, gitDir, files }) + const result = await generateTasks({ config, cwd, gitDir, files }) const linter = result.find(item => item.pattern === '*.{css,js}') expect(linter).toEqual({ pattern: '*.{css,js}', @@ -166,12 +149,42 @@ describe('generateTasks', () => { `${gitDir}/deeper/test2.css`, `${gitDir}/even/deeper/test.css`, `${gitDir}/.hidden/test.css` - ].map(path.normalize) + ].map(normalizePath) + }) + }) + + it('should not match files in parent directory by default', async () => { + const result = await generateTasks({ + config, + cwd: path.join(gitDir, 'deeper'), + gitDir, + files + }) + const linter = result.find(item => item.pattern === '*.js') + expect(linter).toEqual({ + pattern: '*.js', + commands: 'root-js', + fileList: [`${gitDir}/deeper/test.js`, `${gitDir}/deeper/test2.js`].map(normalizePath) + }) + }) + + it('should match files in parent directory when pattern starts with "../"', async () => { + const result = await generateTasks({ + config, + cwd: path.join(gitDir, 'deeper'), + gitDir, + files + }) + const linter = result.find(item => item.pattern === '../*.{css,js}') + expect(linter).toEqual({ + commands: 'parent-dir-css-or-js', + fileList: [`${gitDir}/test.js`, `${gitDir}/test.css`].map(normalizePath), + pattern: '../*.{css,js}' }) }) it('should be able to return relative paths for "*.{css,js}"', async () => { - const result = await generateTasks({ config, gitDir, files, relative: true }) + const result = await generateTasks({ config, cwd, gitDir, files, relative: true }) const linter = result.find(item => item.pattern === '*.{css,js}') expect(linter).toEqual({ pattern: '*.{css,js}', @@ -187,44 +200,7 @@ describe('generateTasks', () => { 'deeper/test2.css', 'even/deeper/test.css', '.hidden/test.css' - ].map(path.normalize) - }) - }) -}) - -describe('generateTasks Special Cases', () => { - beforeAll(() => { - jest.spyOn(process, 'cwd').mockReturnValue(path.join(gitDir, 'sub-dir')) - }) - - afterAll(() => { - process.cwd.mockRestore() - }) - - it('should not match non-children files', async () => { - const result = await generateTasks({ config, gitDir, files: filesSpecialCases }) - const linter = result.find(item => item.pattern === '*.js') - expect(linter).toEqual({ - pattern: '*.js', - commands: 'root-js', - fileList: filesSpecialCases - .filter(x => /sub-dir/.test(x)) - .map(x => path.join(gitDir, x)) - .map(path.normalize) - }) - }) - - it('should match non-children files when configured', async () => { - // const relPath = path.join(process.cwd(), '..') - const result = await generateTasks({ config, gitDir, files: filesSpecialCases }) - const linter = result.find(item => item.pattern === '../**/*.py') - expect(linter).toEqual({ - pattern: '../**/*.py', - commands: 'below-root-py', - fileList: filesSpecialCases - .filter(x => !/sub-dir/.test(x) && /.py$/.test(x)) - .map(x => path.join(gitDir, x)) - .map(path.normalize) + ].map(normalizePath) }) }) }) diff --git a/test/resolveGitDir.spec.js b/test/resolveGitDir.spec.js index 0c4ed6424..ff2591e0a 100644 --- a/test/resolveGitDir.spec.js +++ b/test/resolveGitDir.spec.js @@ -18,8 +18,7 @@ describe('resolveGitDir', () => { const expected = normalize(path.dirname(__dirname)) const processCwdBkp = process.cwd process.cwd = () => __dirname - // path.resolve to strip trailing slash - expect(path.resolve(await resolveGitDir())).toEqual(expected) + expect(await resolveGitDir()).toEqual(expected) process.cwd = processCwdBkp }) @@ -28,7 +27,7 @@ describe('resolveGitDir', () => { const processCwdBkp = process.cwd process.cwd = () => __dirname process.env.GIT_DIR = 'wrong/path/.git' // refer to https://github.com/DonJayamanne/gitHistoryVSCode/issues/233#issuecomment-375769718 - expect(path.resolve(await resolveGitDir())).toEqual(expected) + expect(await resolveGitDir()).toEqual(expected) process.cwd = processCwdBkp })