Skip to content

Commit

Permalink
fix: detect duplicate redundant braces in pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
iiroj committed Aug 12, 2023
1 parent a7f8f29 commit d895aa8
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 23 deletions.
36 changes: 30 additions & 6 deletions lib/validateBraces.js
Expand Up @@ -17,8 +17,8 @@ import { incorrectBraces } from './messages.js'
* braces from user configuration, but this is left to the user (after seeing the warning).
*
* @example <caption>Globs with brace expansions</caption>
* - *.{js,tx} // expanded as *.js, *.ts
* - *.{{j,t}s,css} // expanded as *.js, *.ts, *.css
* - *.{js,tx} // expanded as *.js, *.ts
* - *.{{j,t}s,css} // expanded as *.js, *.ts, *.css
* - file_{1..10}.css // expanded as file_1.css, file_2.css, …, file_10.css
*
* @example <caption>Globs with incorrect brace expansions</caption>
Expand All @@ -28,17 +28,17 @@ import { incorrectBraces } from './messages.js'
* - *.${js} // dollar-sign inhibits expansion, so treated literally
* - *.{js\,ts} // the comma is escaped, so treated literally
*/
export const BRACES_REGEXP = /(?<![\\$])({)(?:(?!(?<!\\),|\.\.|\{|\}).)*?(?<!\\)(})/g
export const INCORRECT_BRACES_REGEXP = /(?<![\\$])({)(?:(?!(?<!\\),|\.\.|\{|\}).)*?(?<!\\)(})/g

/**
* @param {string} pattern
* @returns {string}
*/
const withoutIncorrectBraces = (pattern) => {
const stripIncorrectBraces = (pattern) => {
let output = `${pattern}`
let match = null

while ((match = BRACES_REGEXP.exec(pattern))) {
while ((match = INCORRECT_BRACES_REGEXP.exec(pattern))) {
const fullMatch = match[0]
const withoutBraces = fullMatch.replace(/{/, '').replace(/}/, '')
output = output.replace(fullMatch, withoutBraces)
Expand All @@ -47,6 +47,30 @@ const withoutIncorrectBraces = (pattern) => {
return output
}

/**
* This RegExp matches "duplicate" opening and closing braces, without any other braces
* in between, where the duplication is redundant and should be removed.
*
* @example *.{{js,ts}} // should just be *.{js,ts}
*/
export const DOUBLE_BRACES_REGEXP = /{{[^}{]*}}/

/**
* @param {string} pattern
* @returns {string}
*/
const stripDoubleBraces = (pattern) => {
let output = `${pattern}`
const match = DOUBLE_BRACES_REGEXP.exec(pattern)?.[0]

if (match) {
const withoutBraces = match.replace('{{', '{').replace('}}', '}')
output = output.replace(match, withoutBraces)
}

return output
}

/**
* Validate and remove incorrect brace expansions from glob pattern.
* For example `*.{js}` is incorrect because it doesn't contain a `,` or `..`,
Expand All @@ -57,7 +81,7 @@ const withoutIncorrectBraces = (pattern) => {
* @returns {string}
*/
export const validateBraces = (pattern, logger) => {
const fixedPattern = withoutIncorrectBraces(pattern)
const fixedPattern = stripDoubleBraces(stripIncorrectBraces(pattern))

if (fixedPattern !== pattern) {
logger.warn(incorrectBraces(pattern, fixedPattern))
Expand Down
49 changes: 32 additions & 17 deletions test/unit/validateBraces.spec.js
@@ -1,46 +1,64 @@
import makeConsoleMock from 'consolemock'

import { validateBraces, BRACES_REGEXP } from '../../lib/validateBraces.js'
import {
validateBraces,
INCORRECT_BRACES_REGEXP,
DOUBLE_BRACES_REGEXP,
} from '../../lib/validateBraces.js'

describe('BRACES_REGEXP', () => {
describe('INCORRECT_BRACES_REGEXP', () => {
it(`should match '*.{js}'`, () => {
expect('*.{js}'.match(BRACES_REGEXP)).toBeTruthy()
expect('*.{js}'.match(INCORRECT_BRACES_REGEXP)).toBeTruthy()
})

it(`should match 'file_{10}'`, () => {
expect('file_{test}'.match(BRACES_REGEXP)).toBeTruthy()
expect('file_{test}'.match(INCORRECT_BRACES_REGEXP)).toBeTruthy()
})

it(`should match '*.{spec\\.js}'`, () => {
expect('*.{spec\\.js}'.match(BRACES_REGEXP)).toBeTruthy()
expect('*.{spec\\.js}'.match(INCORRECT_BRACES_REGEXP)).toBeTruthy()
})

it(`should match '*.{js\\,ts}'`, () => {
expect('*.{js\\,ts}'.match(BRACES_REGEXP)).toBeTruthy()
expect('*.{js\\,ts}'.match(INCORRECT_BRACES_REGEXP)).toBeTruthy()
})

it("should not match '*.${js}'", () => {
expect('*.${js}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('*.${js}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match '.{js,ts}'`, () => {
expect('.{js,ts}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('.{js,ts}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match 'file_{1..10}'`, () => {
expect('file_{1..10}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('file_{1..10}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match '*.\\{js\\}'`, () => {
expect('*.\\{js\\}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('*.\\{js\\}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match '*.\\{js}'`, () => {
expect('*.\\{js}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('*.\\{js}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match '*.{js\\}'`, () => {
expect('*.{js\\}'.match(BRACES_REGEXP)).not.toBeTruthy()
expect('*.{js\\}'.match(INCORRECT_BRACES_REGEXP)).toBeFalsy()
})
})

describe('DOUBLE_BRACES_REGEXP', () => {
it(`should match '*.{{js,ts}}'`, () => {
expect('*.{{js,ts}}'.match(DOUBLE_BRACES_REGEXP)).toBeTruthy()
})

it(`should not match '*.{{js,ts},{css}}'`, () => {
expect('*.{{js,ts},{css}}'.match(DOUBLE_BRACES_REGEXP)).toBeFalsy()
})

it(`should not match '*.{{js,ts},{css}}'`, () => {
expect('*.{{js,ts},{css}}'.match(DOUBLE_BRACES_REGEXP)).toBeFalsy()
})
})

Expand Down Expand Up @@ -84,18 +102,15 @@ describe('validateBraces', () => {
`)
})

/**
* @todo This isn't correctly detected even though the outer braces are invalid.
*/
it.skip('should warn about `*.{{js,ts}}` and return fixed pattern', () => {
it('should warn about `*.{{js,ts}}` and return fixed pattern', () => {
const logger = makeConsoleMock()

const fixedBraces = validateBraces('*.{{js,ts}}', logger)

expect(fixedBraces).toEqual('*.{js,ts}')
expect(logger.printHistory()).toMatchInlineSnapshot(`
"
WARN Detected incorrect braces with only single value: \`*.{{js,ts}}\`. Reformatted as: \`*.{js,ts}\`
WARN Detected incorrect braces with only single value: \`*.{{js,ts}}\`. Reformatted as: \`*.{js,ts}\`
"
`)
})
Expand Down

0 comments on commit d895aa8

Please sign in to comment.