Skip to content

Commit

Permalink
Pass absolute path to directoryExists (microsoft#46086)
Browse files Browse the repository at this point in the history
* Pass absolute path to directoryExists

As pointed out by @gretzkiy, the `directoryExists` call added to
`matchFiles` in microsoft#44710 should have been passing the absolute path (since
the current directory might not match `currentDirectory`).

* Add test, simplify/clarify/fix matchFiles and friends

Co-authored-by: Andrew Branch <[email protected]>
  • Loading branch information
amcasey and andrewbranch committed Oct 21, 2021
1 parent 5725cfe commit 55b4928
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 14 deletions.
28 changes: 14 additions & 14 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6613,7 +6613,7 @@ namespace ts {
includeFilePattern: getRegularExpressionForWildcard(includes, absolutePath, "files"),
includeDirectoryPattern: getRegularExpressionForWildcard(includes, absolutePath, "directories"),
excludePattern: getRegularExpressionForWildcard(excludes, absolutePath, "exclude"),
basePaths: getBasePaths(path, includes, useCaseSensitiveFileNames)
basePaths: getBasePaths(absolutePath, includes, useCaseSensitiveFileNames)
};
}

Expand All @@ -6637,22 +6637,22 @@ namespace ts {
const results: string[][] = includeFileRegexes ? includeFileRegexes.map(() => []) : [[]];
const visited = new Map<string, true>();
const toCanonical = createGetCanonicalFileName(useCaseSensitiveFileNames);
for (const basePath of patterns.basePaths) {
if (directoryExists(basePath)) {
visitDirectory(basePath, combinePaths(currentDirectory, basePath), depth);
for (const absoluteBasePath of patterns.basePaths) {
if (directoryExists(absoluteBasePath)) {
visitDirectory(absoluteBasePath, depth);
}
}

return flatten(results);

function visitDirectory(path: string, absolutePath: string, depth: number | undefined) {
function visitDirectory(absolutePath: string, depth: number | undefined) {
const canonicalPath = toCanonical(realpath(absolutePath));
if (visited.has(canonicalPath)) return;
visited.set(canonicalPath, true);
const { files, directories } = getFileSystemEntries(path);
const { files, directories } = getFileSystemEntries(absolutePath);

for (const current of sort<string>(files, compareStringsCaseSensitive)) {
const name = combinePaths(path, current);
const name = combinePaths(absolutePath, current);
const absoluteName = combinePaths(absolutePath, current);
if (extensions && !fileExtensionIsOneOf(name, extensions)) continue;
if (excludeRegex && excludeRegex.test(absoluteName)) continue;
Expand All @@ -6675,32 +6675,32 @@ namespace ts {
}

for (const current of sort<string>(directories, compareStringsCaseSensitive)) {
const name = combinePaths(path, current);
const absoluteName = combinePaths(absolutePath, current);
if ((!includeDirectoryRegex || includeDirectoryRegex.test(absoluteName)) &&
(!excludeRegex || !excludeRegex.test(absoluteName))) {
visitDirectory(name, absoluteName, depth);
visitDirectory(absoluteName, depth);
}
}
}
}

/**
* Computes the unique non-wildcard base paths amongst the provided include patterns.
* @returns Absolute directory paths
*/
function getBasePaths(path: string, includes: readonly string[] | undefined, useCaseSensitiveFileNames: boolean): string[] {
function getBasePaths(absoluteTsconfigPath: string, includes: readonly string[] | undefined, useCaseSensitiveFileNames: boolean): string[] {
// Storage for our results in the form of literal paths (e.g. the paths as written by the user).
const basePaths: string[] = [path];
const basePaths: string[] = [absoluteTsconfigPath];

if (includes) {
// Storage for literal base paths amongst the include patterns.
const includeBasePaths: string[] = [];
for (const include of includes) {
// We also need to check the relative paths by converting them to absolute and normalizing
// in case they escape the base path (e.g "..\somedirectory")
const absolute: string = isRootedDiskPath(include) ? include : normalizePath(combinePaths(path, include));
const absoluteIncludePath: string = isRootedDiskPath(include) ? include : normalizePath(combinePaths(absoluteTsconfigPath, include));
// Append the literal and canonical candidate base paths.
includeBasePaths.push(getIncludeBasePath(absolute));
includeBasePaths.push(getIncludeBasePath(absoluteIncludePath));
}

// Sort the offsets array using either the literal or canonical path representations.
Expand All @@ -6709,7 +6709,7 @@ namespace ts {
// Iterate over each include base path and include unique base paths that are not a
// subpath of an existing base path
for (const includeBasePath of includeBasePaths) {
if (every(basePaths, basePath => !containsPath(basePath, includeBasePath, path, !useCaseSensitiveFileNames))) {
if (every(basePaths, basePath => !containsPath(basePath, includeBasePath, absoluteTsconfigPath, !useCaseSensitiveFileNames))) {
basePaths.push(includeBasePath);
}
}
Expand Down
31 changes: 31 additions & 0 deletions src/testRunner/unittests/publicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,34 @@ describe("unittests:: Public APIs:: getChild* methods on EndOfFileToken with JSD
assert.equal(endOfFileToken.getChildCount(), 1);
assert.notEqual(endOfFileToken.getChildAt(0), /*expected*/ undefined);
});

describe("unittests:: Public APIs:: sys", () => {
it("readDirectory", () => {
// #45990, testing passing a non-absolute path
// `sys.readDirectory` is just `matchFiles` plugged into the real FS
const read = ts.matchFiles(
/*path*/ "",
/*extensions*/ [".ts", ".tsx"],
/*excludes*/ ["node_modules", "dist"],
/*includes*/ ["**/*"],
/*useCaseSensitiveFileNames*/ true,
/*currentDirectory*/ "/",
/*depth*/ undefined,
/*getFileSystemEntries*/ path => {
switch (path) {
case "/": return { directories: [], files: ["file.ts"] };
default: return { directories: [], files: [] };
}
},
/*realpath*/ ts.identity,
/*directoryExists*/ path => {
switch (path) {
case "/": return true;
default: return false;
}
}
);

assert.deepEqual(read, ["/file.ts"]);
});
});

0 comments on commit 55b4928

Please sign in to comment.