diff --git a/Jakefile.js b/Jakefile.js index 46a744445d3..9e334a9bded 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -121,7 +121,6 @@ var harnessSources = harnessCoreSources.concat([ "transpile.ts", "reuseProgramStructure.ts", "textStorage.ts", - "cachingInServerLSHost.ts", "moduleResolution.ts", "tsconfigParsing.ts", "commandLineParsing.ts", diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 4d4fc6c1d8d..52c16f7a646 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -1127,21 +1127,4 @@ namespace ts { function toSearchResult(value: T | undefined): SearchResult { return value !== undefined ? { value } : undefined; } - - /** Calls `callback` on `directory` and every ancestor directory it has, returning the first defined result. */ - function forEachAncestorDirectory(directory: string, callback: (directory: string) => SearchResult): SearchResult { - while (true) { - const result = callback(directory); - if (result !== undefined) { - return result; - } - - const parentPath = getDirectoryPath(directory); - if (parentPath === directory) { - return undefined; - } - - directory = parentPath; - } - } } diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index 3d5d3e0a845..56dd48aaebf 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -184,9 +184,9 @@ namespace ts { const seenNamesInFile = createMap(); for (const name of names) { - // check if this is a duplicate entry in the list let resolution = resolutionsInFile.get(name); - if (!moduleResolutionIsValid(resolution, name)) { + // Resolution is valid if it is present and not invalidated + if (!resolution || resolution.isInvalidated) { const existingResolution = resolution; const resolutionInDirectory = perDirectoryResolution.get(name); if (resolutionInDirectory) { @@ -221,25 +221,6 @@ namespace ts { return resolvedModules; - function moduleResolutionIsValid(resolution: T, name: string): boolean { - // This is already calculated resolution in this round of synchronization - if (seenNamesInFile.has(name)) { - return true; - } - - if (!resolution || resolution.isInvalidated) { - return false; - } - - const result = getResult(resolution); - if (result) { - return true; - } - // consider situation if we have no candidate locations as valid resolution. - // after all there is no point to invalidate it if we have no idea where to look for the module. - return resolution.failedLookupLocations.length === 0; - } - function resolutionIsEqualTo(oldResolution: T, newResolution: T): boolean { if (oldResolution === newResolution) { return true; diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 19748bfe0de..e17e0e96997 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3609,6 +3609,23 @@ namespace ts { export function closeFileWatcherOf(objWithWatcher: T) { objWithWatcher.watcher.close(); } + + /** Calls `callback` on `directory` and every ancestor directory it has, returning the first defined result. */ + export function forEachAncestorDirectory(directory: string, callback: (directory: string) => T): T { + while (true) { + const result = callback(directory); + if (result !== undefined) { + return result; + } + + const parentPath = getDirectoryPath(directory); + if (parentPath === directory) { + return undefined; + } + + directory = parentPath; + } + } } namespace ts { diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json index 7c3ad0bc5a3..006359c8022 100644 --- a/src/harness/tsconfig.json +++ b/src/harness/tsconfig.json @@ -109,7 +109,6 @@ "./unittests/convertToBase64.ts", "./unittests/transpile.ts", "./unittests/reuseProgramStructure.ts", - "./unittests/cachingInServerLSHost.ts", "./unittests/moduleResolution.ts", "./unittests/tsconfigParsing.ts", "./unittests/commandLineParsing.ts", diff --git a/src/harness/unittests/cachingInServerLSHost.ts b/src/harness/unittests/cachingInServerLSHost.ts deleted file mode 100644 index a1095e7dc48..00000000000 --- a/src/harness/unittests/cachingInServerLSHost.ts +++ /dev/null @@ -1,203 +0,0 @@ -/// - -namespace ts { - interface File { - name: string; - content: string; - } - - function createDefaultServerHost(fileMap: Map): server.ServerHost { - const existingDirectories = createMap(); - forEachKey(fileMap, name => { - let dir = getDirectoryPath(name); - let previous: string; - do { - existingDirectories.set(dir, true); - previous = dir; - dir = getDirectoryPath(dir); - } while (dir !== previous); - }); - return { - args: [], - newLine: "\r\n", - useCaseSensitiveFileNames: false, - write: noop, - readFile: path => { - const file = fileMap.get(path); - return file && file.content; - }, - writeFile: notImplemented, - resolvePath: notImplemented, - fileExists: path => fileMap.has(path), - directoryExists: path => existingDirectories.get(path) || false, - createDirectory: noop, - getExecutingFilePath: () => "", - getCurrentDirectory: () => "", - getDirectories: () => [], - getEnvironmentVariable: () => "", - readDirectory: notImplemented, - exit: noop, - watchFile: () => ({ - close: noop - }), - watchDirectory: () => ({ - close: noop - }), - setTimeout, - clearTimeout, - setImmediate: typeof setImmediate !== "undefined" ? setImmediate : action => setTimeout(action, 0), - clearImmediate: typeof clearImmediate !== "undefined" ? clearImmediate : clearTimeout, - createHash: Harness.mockHash, - }; - } - - function createProject(rootFile: string, serverHost: server.ServerHost): { project: server.Project, rootScriptInfo: server.ScriptInfo } { - const svcOpts: server.ProjectServiceOptions = { - host: serverHost, - logger: projectSystem.nullLogger, - cancellationToken: { isCancellationRequested: () => false }, - useSingleInferredProject: false, - useInferredProjectPerProjectRoot: false, - typingsInstaller: undefined - }; - const projectService = new server.ProjectService(svcOpts); - const rootScriptInfo = projectService.getOrCreateScriptInfoForNormalizedPath(server.toNormalizedPath(rootFile), /*openedByClient*/ true); - - const project = projectService.assignOrphanScriptInfoToInferredProject(rootScriptInfo); - project.setCompilerOptions({ module: ts.ModuleKind.AMD, noLib: true } ); - return { - project, - rootScriptInfo - }; - } - - describe("Caching in LSHost", () => { - it("works using legacy resolution logic", () => { - const root: File = { - name: "c:/d/f0.ts", - content: `import {x} from "f1"` - }; - - const imported: File = { - name: "c:/f1.ts", - content: `foo()` - }; - - const serverHost = createDefaultServerHost(createMapFromTemplate({ [root.name]: root, [imported.name]: imported })); - const { project, rootScriptInfo } = createProject(root.name, serverHost); - - // ensure that imported file was found - let diags = project.getLanguageService().getSemanticDiagnostics(imported.name); - assert.equal(diags.length, 1); - - - const originalFileExists = serverHost.fileExists; - { - // patch fileExists to make sure that disk is not touched - serverHost.fileExists = notImplemented; - - const newContent = `import {x} from "f1" - var x: string = 1;`; - rootScriptInfo.editContent(0, root.content.length, newContent); - // trigger synchronization to make sure that import will be fetched from the cache - diags = project.getLanguageService().getSemanticDiagnostics(imported.name); - // ensure file has correct number of errors after edit - assert.equal(diags.length, 1); - } - { - let fileExistsIsCalled = false; - serverHost.fileExists = (fileName): boolean => { - if (fileName === "lib.d.ts") { - return false; - } - fileExistsIsCalled = true; - assert.isTrue(fileName.indexOf("/f2.") !== -1); - return originalFileExists.call(serverHost, fileName); - }; - const newContent = `import {x} from "f2"`; - rootScriptInfo.editContent(0, root.content.length, newContent); - - try { - // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk - project.getLanguageService().getSemanticDiagnostics(imported.name); - assert.isTrue(false, `should not find file '${imported.name}'`); - } - catch (e) { - assert.isTrue(e.message.indexOf(`Could not find file: '${imported.name}'.`) === 0); - } - - assert.isTrue(fileExistsIsCalled); - } - { - let fileExistsCalled = false; - serverHost.fileExists = (fileName): boolean => { - if (fileName === "lib.d.ts") { - return false; - } - fileExistsCalled = true; - assert.isTrue(fileName.indexOf("/f1.") !== -1); - return originalFileExists.call(serverHost, fileName); - }; - - const newContent = `import {x} from "f1"`; - rootScriptInfo.editContent(0, root.content.length, newContent); - project.getLanguageService().getSemanticDiagnostics(imported.name); - assert.isTrue(fileExistsCalled); - - // setting compiler options discards module resolution cache - fileExistsCalled = false; - - const compilerOptions = ts.cloneCompilerOptions(project.getCompilationSettings()); - compilerOptions.target = ts.ScriptTarget.ES5; - project.setCompilerOptions(compilerOptions); - - project.getLanguageService().getSemanticDiagnostics(imported.name); - assert.isTrue(fileExistsCalled); - } - }); - - it("loads missing files from disk", () => { - const root: File = { - name: `c:/foo.ts`, - content: `import {x} from "bar"` - }; - - const imported: File = { - name: `c:/bar.d.ts`, - content: `export var y = 1` - }; - - const fileMap = createMapFromTemplate({ [root.name]: root }); - const serverHost = createDefaultServerHost(fileMap); - const originalFileExists = serverHost.fileExists; - - let fileExistsCalledForBar = false; - serverHost.fileExists = fileName => { - if (fileName === "lib.d.ts") { - return false; - } - if (!fileExistsCalledForBar) { - fileExistsCalledForBar = fileName.indexOf("/bar.") !== -1; - } - - return originalFileExists.call(serverHost, fileName); - }; - - const { project, rootScriptInfo } = createProject(root.name, serverHost); - - let diags = project.getLanguageService().getSemanticDiagnostics(root.name); - assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called"); - assert.isTrue(diags.length === 1, "one diagnostic expected"); - const messageText = diags[0].messageText; - assert.isTrue(isString(messageText) && messageText.indexOf("Cannot find module") === 0, "should be 'cannot find module' message"); - - fileMap.set(imported.name, imported); - fileExistsCalledForBar = false; - rootScriptInfo.editContent(0, root.content.length, `import {y} from "bar"`); - - diags = project.getLanguageService().getSemanticDiagnostics(root.name); - assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called."); - assert.isTrue(diags.length === 0, "The import should succeed once the imported file appears on disk."); - }); - }); -} diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index d7de270645b..7c04f08938d 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -249,14 +249,9 @@ namespace ts.projectSystem { function getNodeModuleDirectories(dir: string) { const result: string[] = []; - while (true) { - result.push(combinePaths(dir, "node_modules")); - const parentDir = getDirectoryPath(dir); - if (parentDir === dir) { - break; - } - dir = parentDir; - } + forEachAncestorDirectory(dir, ancestor => { + result.push(combinePaths(ancestor, "node_modules")); + }); return result; } @@ -343,14 +338,9 @@ namespace ts.projectSystem { export function getTypeRootsFromLocation(currentDirectory: string) { currentDirectory = normalizePath(currentDirectory); const result: string[] = []; - while (true) { - result.push(combinePaths(currentDirectory, "node_modules/@types")); - const parentDirectory = getDirectoryPath(currentDirectory); - if (parentDirectory === currentDirectory) { - break; - } - currentDirectory = parentDirectory; - } + forEachAncestorDirectory(currentDirectory, ancestor => { + result.push(combinePaths(ancestor, "node_modules/@types")); + }); return result; } @@ -4147,6 +4137,7 @@ namespace ts.projectSystem { verifyCalledOnEachEntry, verifyNoHostCalls, verifyNoHostCallsExceptFileExistsOnce, + verifyCalledOn, clear }; @@ -4179,6 +4170,12 @@ namespace ts.projectSystem { } } + function verifyCalledOn(callback: keyof CalledMaps, name: string) { + const calledMap = calledMaps[callback]; + const result = calledMap.get(name); + assert.isTrue(result && !!result.length, `${callback} should be called with name: ${name}: ${arrayFrom(calledMap.keys())}`); + } + function verifyNoCall(callback: keyof CalledMaps) { const calledMap = calledMaps[callback]; assert.equal(calledMap.size, 0, `${callback} shouldnt be called: ${arrayFrom(calledMap.keys())}`); @@ -4218,6 +4215,161 @@ namespace ts.projectSystem { } } + it("works using legacy resolution logic", () => { + let rootContent = `import {x} from "f1"`; + const root: FileOrFolder = { + path: "/c/d/f0.ts", + content: rootContent + }; + + const imported: FileOrFolder = { + path: "/c/f1.ts", + content: `foo()` + }; + + const host = createServerHost([root, imported]); + const projectService = createProjectService(host); + projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true }); + projectService.openClientFile(root.path); + checkNumberOfProjects(projectService, { inferredProjects: 1 }); + const project = projectService.inferredProjects[0]; + const rootScriptInfo = project.getRootScriptInfos()[0]; + assert.equal(rootScriptInfo.fileName, root.path); + + // ensure that imported file was found + verifyImportedDiagnostics(); + + const callsTrackingHost = createCallsTrackingHost(host); + + // trigger synchronization to make sure that import will be fetched from the cache + // ensure file has correct number of errors after edit + editContent(`import {x} from "f1"; + var x: string = 1;`); + verifyImportedDiagnostics(); + callsTrackingHost.verifyNoHostCalls(); + + // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk + editContent(`import {x} from "f2"`); + try { + // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk + verifyImportedDiagnostics(); + assert.isTrue(false, `should not find file '${imported.path}'`); + } + catch (e) { + assert.isTrue(e.message.indexOf(`Could not find file: '${imported.path}'.`) === 0); + } + const f2Lookups = getLocationsForModuleLookup("f2"); + callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", f2Lookups, 1); + const f2DirLookups = getLocationsForDirectoryLookup(); + callsTrackingHost.verifyCalledOnEachEntry("directoryExists", f2DirLookups); + callsTrackingHost.verifyNoCall("getDirectories"); + callsTrackingHost.verifyNoCall("readFile"); + callsTrackingHost.verifyNoCall("readDirectory"); + + editContent(`import {x} from "f1"`); + verifyImportedDiagnostics(); + const f1Lookups = f2Lookups.map(s => s.replace("f2", "f1")); + f1Lookups.length = f1Lookups.indexOf(imported.path) + 1; + const f1DirLookups = ["/c/d", "/c", typeRootFromTsserverLocation]; + vertifyF1Lookups(); + + // setting compiler options discards module resolution cache + callsTrackingHost.clear(); + projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true, target: ts.ScriptTarget.ES5 }); + verifyImportedDiagnostics(); + vertifyF1Lookups(); + + function vertifyF1Lookups() { + callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", f1Lookups, 1); + callsTrackingHost.verifyCalledOnEachEntryNTimes("directoryExists", f1DirLookups, 1); + callsTrackingHost.verifyNoCall("getDirectories"); + callsTrackingHost.verifyNoCall("readFile"); + callsTrackingHost.verifyNoCall("readDirectory"); + } + + function editContent(newContent: string) { + callsTrackingHost.clear(); + rootScriptInfo.editContent(0, rootContent.length, newContent); + rootContent = newContent; + } + + function verifyImportedDiagnostics() { + const diags = project.getLanguageService().getSemanticDiagnostics(imported.path); + assert.equal(diags.length, 1); + const diag = diags[0]; + assert.equal(diag.code, Diagnostics.Cannot_find_name_0.code); + assert.equal(flattenDiagnosticMessageText(diag.messageText, "\n"), "Cannot find name 'foo'."); + } + + function getLocationsForModuleLookup(module: string) { + const locations: string[] = []; + forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => { + locations.push( + combinePaths(ancestor, `${module}.ts`), + combinePaths(ancestor, `${module}.tsx`), + combinePaths(ancestor, `${module}.d.ts`) + ); + }); + forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => { + locations.push( + combinePaths(ancestor, `${module}.js`), + combinePaths(ancestor, `${module}.jsx`) + ); + }); + return locations; + } + + function getLocationsForDirectoryLookup() { + const result = createMap(); + // Type root + result.set(typeRootFromTsserverLocation, 1); + forEachAncestorDirectory(getDirectoryPath(root.path), ancestor => { + // To resolve modules + result.set(ancestor, 2); + // for type roots + result.set(combinePaths(ancestor, `node_modules`), 1); + }); + return result; + } + }); + + it("loads missing files from disk", () => { + const root: FileOrFolder = { + path: "/c/foo.ts", + content: `import {y} from "bar"` + }; + + const imported: FileOrFolder = { + path: "/c/bar.d.ts", + content: `export var y = 1` + }; + + const host = createServerHost([root]); + const projectService = createProjectService(host); + projectService.setCompilerOptionsForInferredProjects({ module: ts.ModuleKind.AMD, noLib: true }); + const callsTrackingHost = createCallsTrackingHost(host); + projectService.openClientFile(root.path); + checkNumberOfProjects(projectService, { inferredProjects: 1 }); + const project = projectService.inferredProjects[0]; + const rootScriptInfo = project.getRootScriptInfos()[0]; + assert.equal(rootScriptInfo.fileName, root.path); + + let diags = project.getLanguageService().getSemanticDiagnostics(root.path); + assert.equal(diags.length, 1); + const diag = diags[0]; + assert.equal(diag.code, Diagnostics.Cannot_find_module_0.code); + assert.equal(flattenDiagnosticMessageText(diag.messageText, "\n"), "Cannot find module 'bar'."); + callsTrackingHost.verifyCalledOn("fileExists", imported.path); + + + callsTrackingHost.clear(); + host.reloadFS([root, imported]); + host.runQueuedTimeoutCallbacks(); + diags = project.getLanguageService().getSemanticDiagnostics(root.path); + assert.equal(diags.length, 0); + callsTrackingHost.verifyCalledOn("fileExists", imported.path); + }); + it("when calling goto definition of module", () => { const clientFile: FileOrFolder = { path: "/a/b/controllers/vessels/client.ts", @@ -4371,18 +4523,7 @@ namespace ts.projectSystem { const canonicalFile3Path = useCaseSensitiveFileNames ? file3.path : file3.path.toLocaleLowerCase(); const numberOfTimesWatchInvoked = getNumberOfWatchesInvokedForRecursiveWatches(watchingRecursiveDirectories, canonicalFile3Path); callsTrackingHost.verifyCalledOnEachEntryNTimes("fileExists", [canonicalFile3Path], numberOfTimesWatchInvoked); - - // Called for type root resolution - const directoryExistsCalled = createMap(); - for (let dir = frontendDir; dir !== "/"; dir = getDirectoryPath(dir)) { - directoryExistsCalled.set(`${dir}/node_modules`, 2); - } - directoryExistsCalled.set(`/node_modules`, 2); - directoryExistsCalled.set(`${frontendDir}/types`, 2); - directoryExistsCalled.set(`${frontendDir}/node_modules/@types`, 2); - directoryExistsCalled.set(canonicalFile3Path, numberOfTimesWatchInvoked); - callsTrackingHost.verifyCalledOnEachEntry("directoryExists", directoryExistsCalled); - + callsTrackingHost.verifyCalledOnEachEntryNTimes("directoryExists", [canonicalFile3Path], numberOfTimesWatchInvoked); callsTrackingHost.verifyNoCall("getDirectories"); callsTrackingHost.verifyCalledOnEachEntryNTimes("readFile", [file3.path], 1); callsTrackingHost.verifyNoCall("readDirectory");