From b220831ab7a89fd7e03889911cea738e9daafd31 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 29 Nov 2016 07:01:31 -0800 Subject: [PATCH 1/2] Sort matched files by include order --- src/compiler/commandLineParser.ts | 1 - src/compiler/core.ts | 123 +++++++++++------- src/harness/unittests/matchFiles.ts | 52 ++++++-- .../nodeModulesMaxDepthExceeded.errors.txt | 20 +-- .../amd/nodeModulesMaxDepthExceeded.json | 4 +- .../nodeModulesMaxDepthExceeded.errors.txt | 20 +-- .../node/nodeModulesMaxDepthExceeded.json | 4 +- 7 files changed, 141 insertions(+), 83 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 3c6a86134ea..a81698469e7 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1264,7 +1264,6 @@ namespace ts { const literalFiles = reduceProperties(literalFileMap, addFileToOutput, []); const wildcardFiles = reduceProperties(wildcardFileMap, addFileToOutput, []); - wildcardFiles.sort(host.useCaseSensitiveFileNames ? compareStrings : compareStringsCaseInsensitive); return { fileNames: literalFiles.concat(wildcardFiles), wildcardDirectories diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 89057dd2939..8406404132d 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1668,7 +1668,19 @@ namespace ts { const singleAsteriskRegexFragmentFiles = "([^./]|(\\.(?!min\\.js$))?)*"; const singleAsteriskRegexFragmentOther = "[^/]*"; - export function getRegularExpressionForWildcard(specs: string[], basePath: string, usage: "files" | "directories" | "exclude") { + export function getRegularExpressionForWildcard(specs: string[], basePath: string, usage: "files" | "directories" | "exclude"): string | undefined { + const patterns = getRegularExpressionsForWildcards(specs, basePath, usage); + if (!patterns || !patterns.length) { + return undefined; + } + + const pattern = patterns.map(pattern => `(${pattern})`).join("|"); + // If excluding, match "foo/bar/baz...", but if including, only allow "foo". + const terminator = usage === "exclude" ? "($|/)" : "$"; + return `^(${pattern})${terminator}`; + } + + function getRegularExpressionsForWildcards(specs: string[], basePath: string, usage: "files" | "directories" | "exclude"): string[] | undefined { if (specs === undefined || specs.length === 0) { return undefined; } @@ -1682,33 +1694,8 @@ namespace ts { */ const doubleAsteriskRegexFragment = usage === "exclude" ? "(/.+?)?" : "(/[^/.][^/]*)*?"; - let pattern = ""; - let hasWrittenSubpattern = false; - for (const spec of specs) { - if (!spec) { - continue; - } - - const subPattern = getSubPatternFromSpec(spec, basePath, usage, singleAsteriskRegexFragment, doubleAsteriskRegexFragment, replaceWildcardCharacter); - if (subPattern === undefined) { - continue; - } - - if (hasWrittenSubpattern) { - pattern += "|"; - } - - pattern += "(" + subPattern + ")"; - hasWrittenSubpattern = true; - } - - if (!pattern) { - return undefined; - } - - // If excluding, match "foo/bar/baz...", but if including, only allow "foo". - const terminator = usage === "exclude" ? "($|/)" : "$"; - return `^(${pattern})${terminator}`; + return flatMap(specs, spec => + spec && getSubPatternFromSpec(spec, basePath, usage, singleAsteriskRegexFragment, doubleAsteriskRegexFragment, replaceWildcardCharacter)); } /** @@ -1803,6 +1790,9 @@ namespace ts { } export interface FileMatcherPatterns { + /** One pattern for each "include" spec. */ + includeFilePatterns: string[]; + /** One pattern matching one of any of the "include" specs. */ includeFilePattern: string; includeDirectoryPattern: string; excludePattern: string; @@ -1815,6 +1805,7 @@ namespace ts { const absolutePath = combinePaths(currentDirectory, path); return { + includeFilePatterns: map(getRegularExpressionsForWildcards(includes, absolutePath, "files"), pattern => `^${pattern}$`), includeFilePattern: getRegularExpressionForWildcard(includes, absolutePath, "files"), includeDirectoryPattern: getRegularExpressionForWildcard(includes, absolutePath, "directories"), excludePattern: getRegularExpressionForWildcard(excludes, absolutePath, "exclude"), @@ -1829,38 +1820,76 @@ namespace ts { const patterns = getFileMatcherPatterns(path, excludes, includes, useCaseSensitiveFileNames, currentDirectory); const regexFlag = useCaseSensitiveFileNames ? "" : "i"; - const includeFileRegex = patterns.includeFilePattern && new RegExp(patterns.includeFilePattern, regexFlag); + const includeFileRegexes = patterns.includeFilePatterns && patterns.includeFilePatterns.map(pattern => new RegExp(pattern, regexFlag)); const includeDirectoryRegex = patterns.includeDirectoryPattern && new RegExp(patterns.includeDirectoryPattern, regexFlag); const excludeRegex = patterns.excludePattern && new RegExp(patterns.excludePattern, regexFlag); - const result: string[] = []; + // Associate an array of results with each include regex. This keeps results in order of the "include" order. + // If there are no "includes", then just put everything in results[0]. + const results: string[][] = includeFileRegexes ? includeFileRegexes.map(() => []) : [[]]; + for (const basePath of patterns.basePaths) { - visitDirectory(basePath, combinePaths(currentDirectory, basePath)); + forEachFileInRecursiveDirectories(basePath, combinePaths(currentDirectory, basePath), { useCaseSensitiveFileNames, getFileSystemEntries, includeDirectory, visitFile }); } - return result; + + return flatten(results); + + function includeDirectory(absoluteDirectoryName: string): boolean { + return (!includeDirectoryRegex || includeDirectoryRegex.test(absoluteDirectoryName)) && + (!excludeRegex || !excludeRegex.test(absoluteDirectoryName)); + } + + function visitFile(fileName: string, absoluteFileName: string): void { + if (extensions && !fileExtensionIsAny(fileName, extensions) || + excludeRegex && excludeRegex.test(absoluteFileName)) { + return; + } + + if (!includeFileRegexes) { + results[0].push(fileName); + } + else { + for (let i = 0; i < includeFileRegexes.length; i++) { + if (includeFileRegexes[i].test(absoluteFileName)) { + results[i].push(fileName); + // Only include a file once. + break; + } + } + } + } + } + + interface RecursiveDirectoryVisitor { + useCaseSensitiveFileNames: boolean; + getFileSystemEntries: (path: string) => FileSystemEntries; + includeDirectory: (absoluteDirectoryName: string) => boolean; + visitFile: (fileName: string, absoluteFileName: string) => void; + } + + function forEachFileInRecursiveDirectories(start: string, absoluteStart: string, visitor: RecursiveDirectoryVisitor): void { + visitDirectory(start, absoluteStart); function visitDirectory(path: string, absolutePath: string) { - const { files, directories } = getFileSystemEntries(path); + let { files, directories } = visitor.getFileSystemEntries(path); + files = sorted(files); + directories = sorted(directories); - for (const current of files) { - const name = combinePaths(path, current); - const absoluteName = combinePaths(absolutePath, current); - if ((!extensions || fileExtensionIsAny(name, extensions)) && - (!includeFileRegex || includeFileRegex.test(absoluteName)) && - (!excludeRegex || !excludeRegex.test(absoluteName))) { - result.push(name); - } + for (const file of files) { + visitor.visitFile(combinePaths(path, file), combinePaths(absolutePath, file)); } - for (const current of directories) { - const name = combinePaths(path, current); - const absoluteName = combinePaths(absolutePath, current); - if ((!includeDirectoryRegex || includeDirectoryRegex.test(absoluteName)) && - (!excludeRegex || !excludeRegex.test(absoluteName))) { - visitDirectory(name, absoluteName); + for (const dir of directories) { + const absoluteName = combinePaths(absolutePath, dir); + if (visitor.includeDirectory(absoluteName)) { + visitDirectory(combinePaths(path, dir), absoluteName); } } } + + function sorted(names: string[]): string[] { + return names.slice().sort(visitor.useCaseSensitiveFileNames ? compareStrings : compareStringsCaseInsensitive); + } } /** diff --git a/src/harness/unittests/matchFiles.ts b/src/harness/unittests/matchFiles.ts index c562fcefe91..00efe73c3e2 100644 --- a/src/harness/unittests/matchFiles.ts +++ b/src/harness/unittests/matchFiles.ts @@ -346,9 +346,9 @@ namespace ts { fileNames: [ "c:/dev/a.ts", "c:/dev/b.ts", + "c:/dev/node_modules/a.ts", "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" + "c:/dev/jspm_packages/a.ts" ], wildcardDirectories: {}, }; @@ -373,9 +373,9 @@ namespace ts { options: {}, errors: [], fileNames: [ + "c:/dev/node_modules/a.ts", "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" + "c:/dev/jspm_packages/a.ts" ], wildcardDirectories: {}, }; @@ -398,9 +398,9 @@ namespace ts { fileNames: [ "c:/dev/a.ts", "c:/dev/b.ts", + "c:/dev/node_modules/a.ts", "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" + "c:/dev/jspm_packages/a.ts" ], wildcardDirectories: {}, }; @@ -410,6 +410,36 @@ namespace ts { }); describe("with wildcard include list", () => { + it("is sorted in include order, then in alphabetical order", () => { + const json = { + include: [ + "z/*.ts", + "x/*.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/z/a.ts", + "c:/dev/z/aba.ts", + "c:/dev/z/abz.ts", + "c:/dev/z/b.ts", + "c:/dev/z/bba.ts", + "c:/dev/z/bbz.ts", + "c:/dev/x/a.ts", + "c:/dev/x/aa.ts", + "c:/dev/x/b.ts" + ], + wildcardDirectories: { + "c:/dev/z": ts.WatchDirectoryFlags.None, + "c:/dev/x": ts.WatchDirectoryFlags.None + }, + }; + const actual = ts.parseJsonConfigFileContent(json, caseInsensitiveHost, caseInsensitiveBasePath); + assertParsed(actual, expected); + }); + it("same named declarations are excluded", () => { const json = { include: [ @@ -506,8 +536,8 @@ namespace ts { options: {}, errors: [], fileNames: [ - "c:/dev/x/a.ts", "c:/dev/x/y/a.ts", + "c:/dev/x/a.ts", "c:/dev/z/a.ts" ], wildcardDirectories: { @@ -1230,8 +1260,8 @@ namespace ts { options: {}, errors: [], fileNames: [ - "c:/dev/.z/.b.ts", - "c:/dev/x/.y/a.ts" + "c:/dev/x/.y/a.ts", + "c:/dev/.z/.b.ts" ], wildcardDirectories: {} }; @@ -1271,8 +1301,8 @@ namespace ts { options: {}, errors: [], fileNames: [ - "c:/dev/.z/.b.ts", - "c:/dev/x/.y/a.ts" + "c:/dev/x/.y/a.ts", + "c:/dev/.z/.b.ts" ], wildcardDirectories: { "c:/dev/.z": ts.WatchDirectoryFlags.Recursive, diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt index 99e7b193f0c..5b5199ad7ae 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt @@ -2,19 +2,10 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type '"10"' is not assignable to ty maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. -==== entry.js (0 errors) ==== - var m3 = require("m3"); - - module.exports = { - "a": 42, - "b": "hello, world", - "person": m3.person - }; - ==== relative.js (0 errors) ==== exports.relativeProp = true; -==== maxDepthExceeded/node_modules/m1/index.js (0 errors) ==== +==== index.js (0 errors) ==== var m2 = require('m2'); var rel = require('./relative'); @@ -40,4 +31,13 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i !!! error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any". + +==== entry.js (0 errors) ==== + var m3 = require("m3"); + + module.exports = { + "a": 42, + "b": "hello, world", + "person": m3.person + }; \ No newline at end of file diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json index 86e856dc7b8..9efa0e936ac 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.json @@ -7,10 +7,10 @@ "project": "maxDepthExceeded", "resolvedInputFiles": [ "lib.d.ts", - "maxDepthExceeded/node_modules/m2/entry.js", "maxDepthExceeded/node_modules/m1/relative.js", "maxDepthExceeded/node_modules/m1/index.js", - "maxDepthExceeded/root.ts" + "maxDepthExceeded/root.ts", + "maxDepthExceeded/node_modules/m2/entry.js" ], "emittedFiles": [ "maxDepthExceeded/built/node_modules/m1/relative.js", diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt index 99e7b193f0c..5b5199ad7ae 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt @@ -2,19 +2,10 @@ maxDepthExceeded/root.ts(3,1): error TS2322: Type '"10"' is not assignable to ty maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. -==== entry.js (0 errors) ==== - var m3 = require("m3"); - - module.exports = { - "a": 42, - "b": "hello, world", - "person": m3.person - }; - ==== relative.js (0 errors) ==== exports.relativeProp = true; -==== maxDepthExceeded/node_modules/m1/index.js (0 errors) ==== +==== index.js (0 errors) ==== var m2 = require('m2'); var rel = require('./relative'); @@ -40,4 +31,13 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i !!! error TS2540: Cannot assign to 'rel' because it is a constant or a read-only property. m1.f2.person.age = "10"; // OK if stopped at 2 modules: person will be "any". + +==== entry.js (0 errors) ==== + var m3 = require("m3"); + + module.exports = { + "a": 42, + "b": "hello, world", + "person": m3.person + }; \ No newline at end of file diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json index 86e856dc7b8..9efa0e936ac 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.json @@ -7,10 +7,10 @@ "project": "maxDepthExceeded", "resolvedInputFiles": [ "lib.d.ts", - "maxDepthExceeded/node_modules/m2/entry.js", "maxDepthExceeded/node_modules/m1/relative.js", "maxDepthExceeded/node_modules/m1/index.js", - "maxDepthExceeded/root.ts" + "maxDepthExceeded/root.ts", + "maxDepthExceeded/node_modules/m2/entry.js" ], "emittedFiles": [ "maxDepthExceeded/built/node_modules/m1/relative.js", From 54d64ceb98425d13ae038e91e46849c5c93ab151 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 9 Jan 2017 06:55:42 -0800 Subject: [PATCH 2/2] Respond to PR comment --- src/compiler/core.ts | 83 ++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 49 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 3aab91f53ad..89131e16359 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -164,6 +164,16 @@ namespace ts { return undefined; } + /** Works like Array.prototype.findIndex, returning `-1` if no element satisfying the predicate is found. */ + export function findIndex(array: T[], predicate: (element: T, index: number) => boolean): number { + for (let i = 0; i < array.length; i++) { + if (predicate(array[i], i)) { + return i; + } + } + return -1; + } + /** * Returns the first truthy result of `callback`, or else fails. * This is like `forEach`, but never returns undefined. @@ -1810,68 +1820,43 @@ namespace ts { // If there are no "includes", then just put everything in results[0]. const results: string[][] = includeFileRegexes ? includeFileRegexes.map(() => []) : [[]]; + const comparer = useCaseSensitiveFileNames ? compareStrings : compareStringsCaseInsensitive; for (const basePath of patterns.basePaths) { - forEachFileInRecursiveDirectories(basePath, combinePaths(currentDirectory, basePath), { useCaseSensitiveFileNames, getFileSystemEntries, includeDirectory, visitFile }); + visitDirectory(basePath, combinePaths(currentDirectory, basePath)); } return flatten(results); - function includeDirectory(absoluteDirectoryName: string): boolean { - return (!includeDirectoryRegex || includeDirectoryRegex.test(absoluteDirectoryName)) && - (!excludeRegex || !excludeRegex.test(absoluteDirectoryName)); - } + function visitDirectory(path: string, absolutePath: string) { + let { files, directories } = getFileSystemEntries(path); + files = files.slice().sort(comparer); + directories = directories.slice().sort(comparer); - function visitFile(fileName: string, absoluteFileName: string): void { - if (extensions && !fileExtensionIsAny(fileName, extensions) || - excludeRegex && excludeRegex.test(absoluteFileName)) { - return; - } - - if (!includeFileRegexes) { - results[0].push(fileName); - } - else { - for (let i = 0; i < includeFileRegexes.length; i++) { - if (includeFileRegexes[i].test(absoluteFileName)) { - results[i].push(fileName); - // Only include a file once. - break; + for (const current of files) { + const name = combinePaths(path, current); + const absoluteName = combinePaths(absolutePath, current); + if (extensions && !fileExtensionIsAny(name, extensions)) continue; + if (excludeRegex && excludeRegex.test(absoluteName)) continue; + if (!includeFileRegexes) { + results[0].push(name); + } + else { + const includeIndex = findIndex(includeFileRegexes, re => re.test(absoluteName)); + if (includeIndex !== -1) { + results[includeIndex].push(name); } } } - } - } - interface RecursiveDirectoryVisitor { - useCaseSensitiveFileNames: boolean; - getFileSystemEntries: (path: string) => FileSystemEntries; - includeDirectory: (absoluteDirectoryName: string) => boolean; - visitFile: (fileName: string, absoluteFileName: string) => void; - } - - function forEachFileInRecursiveDirectories(start: string, absoluteStart: string, visitor: RecursiveDirectoryVisitor): void { - visitDirectory(start, absoluteStart); - - function visitDirectory(path: string, absolutePath: string) { - let { files, directories } = visitor.getFileSystemEntries(path); - files = sorted(files); - directories = sorted(directories); - - for (const file of files) { - visitor.visitFile(combinePaths(path, file), combinePaths(absolutePath, file)); - } - - for (const dir of directories) { - const absoluteName = combinePaths(absolutePath, dir); - if (visitor.includeDirectory(absoluteName)) { - visitDirectory(combinePaths(path, dir), absoluteName); + for (const current of directories) { + const name = combinePaths(path, current); + const absoluteName = combinePaths(absolutePath, current); + if ((!includeDirectoryRegex || includeDirectoryRegex.test(absoluteName)) && + (!excludeRegex || !excludeRegex.test(absoluteName))) { + visitDirectory(name, absoluteName); } } } - - function sorted(names: string[]): string[] { - return names.slice().sort(visitor.useCaseSensitiveFileNames ? compareStrings : compareStringsCaseInsensitive); - } } /**