Skip to content

Commit

Permalink
fix: expand patterns with brace expansion to avoid some matching issues
Browse files Browse the repository at this point in the history
  • Loading branch information
mrmlnc committed May 13, 2023
1 parent 499018b commit 24e36e2
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 97 deletions.
6 changes: 0 additions & 6 deletions __snapshots__/dot.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ exports['Options Dot {"pattern":"fixtures/**/{.,}*","options":{}} (stream) 1'] =
]

exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (sync) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand All @@ -191,7 +190,6 @@ exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (sync) 1'] = [
]

exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (async) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand All @@ -206,7 +204,6 @@ exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (async) 1'] =
]

exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (stream) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand All @@ -221,7 +218,6 @@ exports['Options Dot {"pattern":"fixtures/{.**,**}","options":{}} (stream) 1'] =
]

exports['Options Dot {"pattern":"fixtures/{**/.*,**}","options":{}} (sync) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand All @@ -236,7 +232,6 @@ exports['Options Dot {"pattern":"fixtures/{**/.*,**}","options":{}} (sync) 1'] =
]

exports['Options Dot {"pattern":"fixtures/{**/.*,**}","options":{}} (async) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand All @@ -251,7 +246,6 @@ exports['Options Dot {"pattern":"fixtures/{**/.*,**}","options":{}} (async) 1']
]

exports['Options Dot {"pattern":"fixtures/{**/.*,**}","options":{}} (stream) 1'] = [
"fixtures/.directory/file.md",
"fixtures/.file",
"fixtures/file.md",
"fixtures/first/file.md",
Expand Down
15 changes: 12 additions & 3 deletions __snapshots__/regular.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -5464,11 +5464,20 @@ exports['Patterns Regular (negative group) {"pattern":"**/!(*.md)","options":{"c
"nested/directory/file.json"
]

exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (sync) 1'] = []
exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (sync) 1'] = [
"library/a/book.md",
"library/b/book.md"
]

exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (async) 1'] = []
exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (async) 1'] = [
"library/a/book.md",
"library/b/book.md"
]

exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (stream) 1'] = []
exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,**/library/*/book.md}","options":{"cwd":"fixtures/third"}} (stream) 1'] = [
"library/a/book.md",
"library/b/book.md"
]

exports['Patterns Regular (segmented lists) {"pattern":"{book.xml,library/**/a/book.md}","options":{"cwd":"fixtures/third"}} (sync) 1'] = [
"library/a/book.md"
Expand Down
5 changes: 2 additions & 3 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as taskManager from './managers/tasks';
import * as patternManager from './managers/patterns';
import ProviderAsync from './providers/async';
import Provider from './providers/provider';
import ProviderStream from './providers/stream';
Expand Down Expand Up @@ -59,7 +58,7 @@ namespace FastGlob {
export function generateTasks(source: PatternInternal | PatternInternal[], options?: OptionsInternal): Task[] {
assertPatternsInput(source);

const patterns = patternManager.transform(([] as PatternInternal[]).concat(source));
const patterns = ([] as PatternInternal[]).concat(source);
const settings = new Settings(options);

return taskManager.generate(patterns, settings);
Expand All @@ -81,7 +80,7 @@ namespace FastGlob {
}

function getWorks<T>(source: PatternInternal | PatternInternal[], _Provider: new (settings: Settings) => Provider<T>, options?: OptionsInternal): T[] {
const patterns = patternManager.transform(([] as PatternInternal[]).concat(source));
const patterns = ([] as PatternInternal[]).concat(source);
const settings = new Settings(options);

const tasks = taskManager.generate(patterns, settings);
Expand Down
51 changes: 0 additions & 51 deletions src/managers/patterns.spec.ts

This file was deleted.

18 changes: 0 additions & 18 deletions src/managers/patterns.ts

This file was deleted.

13 changes: 13 additions & 0 deletions src/managers/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,19 @@ describe('Managers → Task', () => {

assert.deepStrictEqual(actual, expected);
});

it('should expand patterns with brace expansion', () => {
const settings = new Settings();

const expected = [
tests.task.builder().base('a').positive('a/*').build(),
tests.task.builder().base('a/b').positive('a/b/*').build()
];

const actual = manager.generate(['a/{b,}/*'], settings);

assert.deepStrictEqual(actual, expected);
});
});

describe('.convertPatternsToTasks', () => {
Expand Down
14 changes: 12 additions & 2 deletions src/managers/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,19 @@ export type Task = {
negative: Pattern[];
};

export function generate(patterns: Pattern[], settings: Settings): Task[] {
export function generate(input: Pattern[], settings: Settings): Task[] {
/**
* The original pattern like `{,*,**,a/*}` can lead to problems checking the depth when matching entry
* and some problems with the micromatch package (see fast-glob issues: #365, #394).
*
* To solve this problem, we expand all patterns containing brace expansion. This can lead to a slight slowdown
* in matching in the case of a large set of patterns after expansion.
*/
const patterns = utils.pattern.expandPatternsWithBraceExpansion(input);
const ignore = utils.pattern.expandPatternsWithBraceExpansion(settings.ignore);

const positivePatterns = getPositivePatterns(patterns);
const negativePatterns = getNegativePatternsAsPositive(patterns, settings.ignore);
const negativePatterns = getNegativePatternsAsPositive(patterns, ignore);

const staticPatterns = positivePatterns.filter((pattern) => utils.pattern.isStaticPattern(pattern, settings));
const dynamicPatterns = positivePatterns.filter((pattern) => utils.pattern.isDynamicPattern(pattern, settings));
Expand Down
8 changes: 1 addition & 7 deletions src/providers/matchers/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,7 @@ export default abstract class Matcher {
}

private _fillStorage(): void {
/**
* The original pattern may include `{,*,**,a/*}`, which will lead to problems with matching (unresolved level).
* So, before expand patterns with brace expansion into separated patterns.
*/
const patterns = utils.pattern.expandPatternsWithBraceExpansion(this._patterns);

for (const pattern of patterns) {
for (const pattern of this._patterns) {
const segments = this._getPatternSegments(pattern);
const sections = this._splitSegmentsIntoSections(segments);

Expand Down
2 changes: 1 addition & 1 deletion src/tests/e2e/patterns/regular.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ runner.suite('Patterns Regular (negative group)', {

runner.suite('Patterns Regular (segmented lists)', {
tests: [
{ pattern: '{book.xml,**/library/*/book.md}', options: { cwd: 'fixtures/third' }, issue: 365 },
{ pattern: '{book.xml,**/library/*/book.md}', options: { cwd: 'fixtures/third' } },
{ pattern: '{book.xml,library/**/a/book.md}', options: { cwd: 'fixtures/third' } }
]
});
4 changes: 3 additions & 1 deletion src/tests/e2e/patterns/root.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ function getRootEntries(root: string, withBase: boolean = false): string[] {
result = getRootEntriesWithStatsCall(root);
}

console.dir(result, { colors: true, compact: false, depth: null });

if (withBase) {
const separator = root.endsWith('/') ? '' : '/';

Expand Down Expand Up @@ -48,7 +50,7 @@ runner.suite('Patterns Root', {
{
pattern: '/tmp/*',
condition: () => !utils.platform.isWindows(),
expected: () => getRootEntries('/tmp')
expected: () => getRootEntries('/tmp', /** withBase */ true)
},
{
pattern: '/*',
Expand Down
46 changes: 45 additions & 1 deletion src/utils/pattern.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -408,12 +408,20 @@ describe('Utils → Pattern', () => {

describe('.expandBraceExpansion', () => {
it('should return an array of expanded patterns with brace expansion without dupes', () => {
const expected = ['a/b', 'a/c/d', 'a/c'];
const expected = ['a/b', 'a/c', 'a/c/d'];

const actual = util.expandBraceExpansion('a/{b,c/d,{b,c}}');

assert.deepStrictEqual(actual, expected);
});

it('should remove duplicate slashes', () => {
const expected = ['a/*', 'a/b/*', 'a/c/*', 'a/b/c/*'];

const actual = util.expandBraceExpansion('a/{b,}/{c,}/*');

assert.deepStrictEqual(actual, expected);
});
});

describe('.getPatternParts', () => {
Expand Down Expand Up @@ -479,4 +487,40 @@ describe('Utils → Pattern', () => {
assert.ok(!actual);
});
});

describe('.removeDuplicateSlashes', () => {
it('should do not change patterns', () => {
const action = util.removeDuplicateSlashes;

assert.strictEqual(action('directory/file.md'), 'directory/file.md');
assert.strictEqual(action('files{.txt,/file.md}'), 'files{.txt,/file.md}');
});

it('should do not change the device path in patterns with UNC parts', () => {
const action = util.removeDuplicateSlashes;

assert.strictEqual(action('//?//D://'), '//?/D:/');
assert.strictEqual(action('//.//D:///'), '//./D:/');
assert.strictEqual(action('//LOCALHOST//d$//'), '//LOCALHOST/d$/');
assert.strictEqual(action('//127.0.0.1///d$//'), '//127.0.0.1/d$/');
assert.strictEqual(action('//./UNC////LOCALHOST///d$//'), '//./UNC/LOCALHOST/d$/');
});

it('should remove duplicate slashes in the middle and the of the pattern', () => {
const action = util.removeDuplicateSlashes;

assert.strictEqual(action('a//b'), 'a/b');
assert.strictEqual(action('b///c'), 'b/c');
assert.strictEqual(action('c/d///'), 'c/d/');
assert.strictEqual(action('//?//D://'), '//?/D:/');
});

it('should form double slashes at the beginning of the pattern', () => {
const action = util.removeDuplicateSlashes;

assert.strictEqual(action('///*'), '//*');
assert.strictEqual(action('////?'), '//?');
assert.strictEqual(action('///?/D:/'), '//?/D:/');
});
});
});
29 changes: 25 additions & 4 deletions src/utils/pattern.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ const REGEX_GROUP_SYMBOLS_RE = /(?:^|[^!*+?@])\([^(]*\|[^|]*\)/;
const GLOB_EXTENSION_SYMBOLS_RE = /[!*+?@]\([^(]*\)/;
const BRACE_EXPANSION_SEPARATORS_RE = /,|\.\./;

/**
* Matches a sequence of two or more consecutive slashes, excluding the first two slashes at the beginning of the string.
* The latter is due to the presence of the device path at the beginning of the UNC path.
*/
const DOUBLE_SLASH_RE = /(?!^)\/{2,}/g;

type PatternTypeOptions = {
braceExpansion?: boolean;
caseSensitiveMatch?: boolean;
Expand Down Expand Up @@ -150,10 +156,17 @@ export function expandPatternsWithBraceExpansion(patterns: Pattern[]): Pattern[]
}

export function expandBraceExpansion(pattern: Pattern): Pattern[] {
return micromatch.braces(pattern, {
expand: true,
nodupes: true
});
const patterns = micromatch
.braces(pattern, { expand: true, nodupes: true })
.map((pattern) => removeDuplicateSlashes(pattern));

/**
* Sort the patterns by length so that the same depth patterns are processed side by side.
* `a/{b,}/{c,}/*` – `['a/*', 'a/b/*', 'a/c/*', 'a/b/c/*']`
*/
patterns.sort((a, b) => a.length - b.length);

return patterns;
}

export function getPatternParts(pattern: Pattern, options: MicromatchOptions): Pattern[] {
Expand Down Expand Up @@ -193,3 +206,11 @@ export function convertPatternsToRe(patterns: Pattern[], options: MicromatchOpti
export function matchAny(entry: string, patternsRe: PatternRe[]): boolean {
return patternsRe.some((patternRe) => patternRe.test(entry));
}

/**
* This package only works with forward slashes as a path separator.
* Because of this, we cannot use the standard `path.normalize` method, because on Windows platform it will use of backslashes.
*/
export function removeDuplicateSlashes(pattern: string): string {
return pattern.replace(DOUBLE_SLASH_RE, '/');
}

0 comments on commit 24e36e2

Please sign in to comment.