Skip to content

Commit

Permalink
Platform independent wildcard file include ordering (#22393)
Browse files Browse the repository at this point in the history
* Resolve config wildcards result order in a platform independent manner

* Accept affected user test baseline

* Per reccomendation, just change matchFiles

* Add test asserting the same order on differing case sensitive platforms
  • Loading branch information
weswigham authored Mar 8, 2018
1 parent 45eaf42 commit 88ba1ef
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 5 deletions.
5 changes: 2 additions & 3 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2539,7 +2539,6 @@ namespace ts {
path = normalizePath(path);
currentDirectory = normalizePath(currentDirectory);

const comparer = useCaseSensitiveFileNames ? compareStringsCaseSensitive : compareStringsCaseInsensitive;
const patterns = getFileMatcherPatterns(path, excludes, includes, useCaseSensitiveFileNames, currentDirectory);

const regexFlag = useCaseSensitiveFileNames ? "" : "i";
Expand All @@ -2560,7 +2559,7 @@ namespace ts {
function visitDirectory(path: string, absolutePath: string, depth: number | undefined) {
const { files, directories } = getFileSystemEntries(path);

for (const current of sort(files, comparer)) {
for (const current of sort(files, compareStringsCaseSensitive)) {
const name = combinePaths(path, current);
const absoluteName = combinePaths(absolutePath, current);
if (extensions && !fileExtensionIsOneOf(name, extensions)) continue;
Expand All @@ -2583,7 +2582,7 @@ namespace ts {
}
}

for (const current of sort(directories, comparer)) {
for (const current of sort(directories, compareStringsCaseSensitive)) {
const name = combinePaths(path, current);
const absoluteName = combinePaths(absolutePath, current);
if ((!includeDirectoryRegex || includeDirectoryRegex.test(absoluteName)) &&
Expand Down
32 changes: 32 additions & 0 deletions src/harness/unittests/matchFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,18 @@ namespace ts {
"c:/dev/g.min.js/.g/g.ts"
]);

const caseInsensitiveOrderingDiffersWithCaseHost = new Utils.MockParseConfigHost(caseInsensitiveBasePath, /*useCaseSensitiveFileNames*/ false, [
"c:/dev/xylophone.ts",
"c:/dev/Yosemite.ts",
"c:/dev/zebra.ts",
]);

const caseSensitiveOrderingDiffersWithCaseHost = new Utils.MockParseConfigHost(caseSensitiveBasePath, /*useCaseSensitiveFileNames*/ true, [
"/dev/xylophone.ts",
"/dev/Yosemite.ts",
"/dev/zebra.ts",
]);

function assertParsed(actual: ParsedCommandLine, expected: ParsedCommandLine): void {
assert.deepEqual(actual.fileNames, expected.fileNames);
assert.deepEqual(actual.wildcardDirectories, expected.wildcardDirectories);
Expand Down Expand Up @@ -1482,5 +1494,25 @@ namespace ts {
validateMatches(expected, json, caseSensitiveHost, caseSensitiveBasePath);
});
});

it("can include files in the same order on multiple platforms", () => {
function getExpected(basePath: string): ParsedCommandLine {
return {
options: {},
errors: [],
fileNames: [
`${basePath}Yosemite.ts`, // capital always comes before lowercase letters
`${basePath}xylophone.ts`,
`${basePath}zebra.ts`
],
wildcardDirectories: {
[basePath.slice(0, basePath.length - 1)]: WatchDirectoryFlags.Recursive
},
};
}
const json = {};
validateMatches(getExpected(caseSensitiveBasePath), json, caseSensitiveOrderingDiffersWithCaseHost, caseSensitiveBasePath);
validateMatches(getExpected(caseInsensitiveBasePath), json, caseInsensitiveOrderingDiffersWithCaseHost, caseInsensitiveBasePath);
});
});
}
4 changes: 2 additions & 2 deletions tests/baselines/reference/user/chrome-devtools-frontend.log
Original file line number Diff line number Diff line change
Expand Up @@ -13074,9 +13074,9 @@ node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1057,39):
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1108,15): error TS2339: Property 'valuesArray' does not exist on type 'Set<any>'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1116,15): error TS2339: Property 'firstValue' does not exist on type 'Set<any>'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1126,15): error TS2339: Property 'addAll' does not exist on type 'Set<any>'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1127,17): error TS2495: Type 'T[] | Iterable<T>' is not an array type or a string type.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1127,17): error TS2495: Type 'Iterable<T> | T[]' is not an array type or a string type.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1136,15): error TS2339: Property 'containsAll' does not exist on type 'Set<any>'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1137,17): error TS2495: Type 'T[] | Iterable<T>' is not an array type or a string type.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1137,17): error TS2495: Type 'Iterable<T> | T[]' is not an array type or a string type.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1148,15): error TS2339: Property 'remove' does not exist on type 'Map<any, any>'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1155,21): error TS2304: Cannot find name 'VALUE'.
node_modules/chrome-devtools-frontend/front_end/platform/utilities.js(1157,15): error TS2339: Property 'valuesArray' does not exist on type 'Map<any, any>'.
Expand Down

0 comments on commit 88ba1ef

Please sign in to comment.