fix: generateTasks handles parent dir globs correctly

lint-staged · Aug 17, 2019 · 82b5182 · 82b5182
1 parent f485e51
commit 82b5182
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 101 deletions.
diff --git 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
@@ -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
@@ -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
   })