Skip to content

Commit

Permalink
fix: correctly handle incorrect patterns withs braces of only single …
Browse files Browse the repository at this point in the history
…value

Technically brace patterns like `*.{js}` are invalid because braces should
always contain at least one comma or a sequence,
for example `*.{js,ts}` or `file{1..10}.js`.

The `micromatch` library used to match patterns to files will silently ignore
such invalid braces and thus lead to the pattern always matching zero files.
This is a unintuitive, so lint-staged should normalize such cases
by simply removing the unnecessary braces before matching files.
  • Loading branch information
iiroj committed Jul 13, 2021
1 parent 447e8b6 commit 80bd709
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 22 deletions.
71 changes: 51 additions & 20 deletions lib/generateTasks.js
Expand Up @@ -6,6 +6,19 @@ const path = require('path')

const debug = require('debug')('lint-staged:gen-tasks')

const { incorrectBraces } = require('./messages')

// Braces with a single value like `*.{js}` are invalid
// and thus ignored by micromatch. This regex matches all occurrences of
// two curly braces without a `,` or `..` between them, to make sure
// users can still accidentally use them without
// some linters never matching anything.
//
// For example `.{js,ts}` or `file_{1..10}` are valid but `*.{js}` is not.
//
// See: https://www.gnu.org/software/bash/manual/html_node/Brace-Expansion.html
const BRACES_REGEXP = /({)(?:(?!,|\.\.).)*?(})/g

/**
* Generates all task commands, and filelist
*
Expand All @@ -16,33 +29,51 @@ const debug = require('debug')('lint-staged:gen-tasks')
* @param {boolean} [options.files] - Staged filepaths
* @param {boolean} [options.relative] - Whether filepaths to should be relative to gitDir
*/
const generateTasks = ({ config, cwd = process.cwd(), gitDir, files, relative = false }) => {
const generateTasks = (
{ config, cwd = process.cwd(), gitDir, files, relative = false },
logger
) => {
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]) => {
return Object.entries(config).map(([rawPattern, commands]) => {
let pattern = rawPattern

const isParentDirPattern = pattern.startsWith('../')

const fileList = micromatch(
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
return !file.startsWith('..') && !path.isAbsolute(file)
}),
pattern,
{
cwd,
dot: true,
// 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('/'),
}
).map((file) => normalize(relative ? file : path.resolve(cwd, file)))
// Only worry about children of the CWD unless the pattern explicitly
// specifies that it concerns a parent directory.
const filteredFiles = relativeFiles.filter((file) => {
if (isParentDirPattern) return true
return !file.startsWith('..') && !path.isAbsolute(file)
})

// Remove "extra" brackets when they contain only a single value
let hadIncorrectBraces = false
while (BRACES_REGEXP.exec(rawPattern)) {
hadIncorrectBraces = true
pattern = pattern.replace(/{/, '')
pattern = pattern.replace(/}/, '')
}

// Warn the user about incorrect brackets usage
if (hadIncorrectBraces) {
logger.warn(incorrectBraces(rawPattern, pattern))
}

const matches = micromatch(filteredFiles, pattern, {
cwd,
dot: true,
// If the 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('/'),
strictBrackets: true,
})

const fileList = matches.map((file) => normalize(relative ? file : path.resolve(cwd, file)))

const task = { pattern, commands, fileList }
debug('Generated task: \n%O', task)
Expand Down
6 changes: 6 additions & 0 deletions lib/messages.js
Expand Up @@ -21,6 +21,11 @@ const DEPRECATED_GIT_ADD = `${warning} ${chalk.yellow(
)}
`

const incorrectBraces = (before, after) => `${warning} ${chalk.yellow(
`Detected incorrect braces with only single value: \`${before}\`. Reformatted as: \`${after}\``
)}
`

const TASK_ERROR = 'Skipped because of errors from tasks.'

const SKIPPED_GIT_ERROR = 'Skipped because of previous git error.'
Expand All @@ -44,6 +49,7 @@ const CONFIG_STDIN_ERROR = 'Error: Could not read config from stdin.'
module.exports = {
NOT_GIT_REPO,
FAILED_GET_STAGED_FILES,
incorrectBraces,
NO_STAGED_FILES,
NO_TASKS,
skippingBackup,
Expand Down
2 changes: 1 addition & 1 deletion lib/runAll.js
Expand Up @@ -129,7 +129,7 @@ const runAll = async (
const matchedFiles = new Set()

for (const [index, files] of stagedFileChunks.entries()) {
const chunkTasks = generateTasks({ config, cwd, gitDir, files, relative })
const chunkTasks = generateTasks({ config, cwd, gitDir, files, relative }, logger)
const chunkListrTasks = []

for (const task of chunkTasks) {
Expand Down
71 changes: 70 additions & 1 deletion test/generateTasks.spec.js
@@ -1,5 +1,6 @@
import os from 'os'
import makeConsoleMock from 'consolemock'
import normalize from 'normalize-path'
import os from 'os'
import path from 'path'

import generateTasks from '../lib/generateTasks'
Expand Down Expand Up @@ -153,6 +154,74 @@ describe('generateTasks', () => {
})
})

it('should match pattern "*.{js}" and show warning', async () => {
const logger = makeConsoleMock()
const result = await generateTasks(
{
config: {
'*.{js}': 'lint',
},
cwd,
gitDir,
files,
},
logger
)
const linter = result.find((item) => item.pattern === '*.js')

expect(linter).toEqual({
pattern: '*.js',
commands: 'lint',
fileList: [
`${gitDir}/test.js`,
`${gitDir}/deeper/test.js`,
`${gitDir}/deeper/test2.js`,
`${gitDir}/even/deeper/test.js`,
`${gitDir}/.hidden/test.js`,
].map(normalizePath),
})

expect(logger.printHistory()).toMatchInlineSnapshot(`
"
WARN ‼ Detected incorrect braces with only single value: \`*.{js}\`. Reformatted as: \`*.js\`
"
`)
})

it('should match pattern "*.{c}{s}{s}" and show warning', async () => {
const logger = makeConsoleMock()
const result = await generateTasks(
{
config: {
'*.{c}{s}{s}': 'lint',
},
cwd,
gitDir,
files,
},
logger
)
const linter = result.find((item) => item.pattern === '*.css')

expect(linter).toEqual({
pattern: '*.css',
commands: 'lint',
fileList: [
`${gitDir}/test.css`,
`${gitDir}/deeper/test.css`,
`${gitDir}/deeper/test2.css`,
`${gitDir}/even/deeper/test.css`,
`${gitDir}/.hidden/test.css`,
].map(normalizePath),
})

expect(logger.printHistory()).toMatchInlineSnapshot(`
"
WARN ‼ Detected incorrect braces with only single value: \`*.{c}{s}{s}\`. Reformatted as: \`*.css\`
"
`)
})

it('should not match files in parent directory by default', async () => {
const result = await generateTasks({
config,
Expand Down

0 comments on commit 80bd709

Please sign in to comment.