Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISSUE-394: expand patterns with brace expansion to avoid some matching issues #395

Merged
merged 1 commit into from
May 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 0 additions & 2 deletions src/benchmark/suites/product/stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ class Glob {

const matches = await func();

console.dir(matches, { colors: true });

const count = matches.length;
const memory = utils.getMemory();
const time = utils.timeEnd(timeStart);
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.

27 changes: 25 additions & 2 deletions src/managers/tasks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,31 @@ 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);
});

it('should do not expand patterns with brace expansion when the `braceExpansion` option is disabled', () => {
const settings = new Settings({ braceExpansion: false });

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

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

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

describe('.convertPatternsToTasks', () => {
Expand All @@ -65,8 +90,6 @@ describe('Managers → Task', () => {

const actual = manager.convertPatternsToTasks(['*', 'a/*', '../*.md'], ['*.md'], /* dynamic */ true);

console.dir(actual, { colors: true });

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

Expand Down
27 changes: 25 additions & 2 deletions src/managers/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,12 @@ export type Task = {
negative: Pattern[];
};

export function generate(patterns: Pattern[], settings: Settings): Task[] {
export function generate(input: Pattern[], settings: Settings): Task[] {
const patterns = processPatterns(input, settings);
const ignore = processPatterns(settings.ignore, settings);

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 All @@ -23,6 +26,26 @@ export function generate(patterns: Pattern[], settings: Settings): Task[] {
return staticTasks.concat(dynamicTasks);
}

function processPatterns(input: Pattern[], settings: Settings): Pattern[] {
let patterns: Pattern[] = input;

/**
* 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.
*/
if (settings.braceExpansion) {
patterns = utils.pattern.expandPatternsWithBraceExpansion(patterns);
}

/**
* This method also removes duplicate slashes that may have been in the pattern or formed as a result of expansion.
*/
return patterns.map((pattern) => utils.pattern.removeDuplicateSlashes(pattern));
}

/**
* Returns tasks grouped by basic pattern directories.
*
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 @@ -499,7 +499,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' } }
]
});
2 changes: 1 addition & 1 deletion src/tests/e2e/patterns/root.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,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 sort expanded patterns by length', () => {
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:/');
});
});
});
27 changes: 23 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,15 @@ 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 });

/**
* 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 +204,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, '/');
}