From 49ad395de197a87ae704537c20eb9feda29202fe Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Mon, 3 Aug 2015 17:42:29 -0700 Subject: [PATCH] resolveModuleName => resolvedModuleNames, added tests --- Jakefile.js | 3 +- src/compiler/program.ts | 51 +++-- src/compiler/types.ts | 2 +- src/server/editorServices.ts | 56 +++-- src/services/services.ts | 9 +- src/services/shims.ts | 14 +- .../cases/unittests/cachingInServerLSHost.ts | 206 ++++++++++++++++++ 7 files changed, 283 insertions(+), 58 deletions(-) create mode 100644 tests/cases/unittests/cachingInServerLSHost.ts diff --git a/Jakefile.js b/Jakefile.js index 059da04b570..396f15e2730 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -142,7 +142,8 @@ var harnessSources = harnessCoreSources.concat([ "versionCache.ts", "convertToBase64.ts", "transpile.ts", - "reuseProgramStructure.ts" + "reuseProgramStructure.ts", + "cachingInServerLSHost.ts" ].map(function (f) { return path.join(unittestsDirectory, f); })).concat([ diff --git a/src/compiler/program.ts b/src/compiler/program.ts index b56423a8c49..4e13de58953 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -224,15 +224,17 @@ namespace ts { host = host || createCompilerHost(options); // initialize resolveModuleNameWorker only if noResolve is false - let resolveModuleNameWorker: (moduleName: string, containingFile: string) => string; + let resolveModuleNamesWorker: (moduleNames: string[], containingFile: string) => string[]; if (!options.noResolve) { - resolveModuleNameWorker = host.resolveModuleName; - if (!resolveModuleNameWorker) { + resolveModuleNamesWorker = host.resolveModuleNames; + if (!resolveModuleNamesWorker) { Debug.assert(host.getModuleResolutionHost !== undefined); let defaultResolver = getDefaultModuleNameResolver(options); - resolveModuleNameWorker = (moduleName, containingFile) => { - let moduleResolution = defaultResolver(moduleName, containingFile, options, host.getModuleResolutionHost()); - return moduleResolution.resolvedFileName; + resolveModuleNamesWorker = (moduleNames, containingFile) => { + return map(moduleNames, moduleName => { + let moduleResolution = defaultResolver(moduleName, containingFile, options, host.getModuleResolutionHost()); + return moduleResolution.resolvedFileName; + }); } } } @@ -347,15 +349,16 @@ namespace ts { return false; } - if (resolveModuleNameWorker) { + if (resolveModuleNamesWorker) { + let moduleNames = map(newSourceFile.imports, name => name.text); + let resolutions = resolveModuleNamesWorker(moduleNames, newSourceFile.fileName); // ensure that module resolution results are still correct - for (let importName of newSourceFile.imports) { - var oldResolution = getResolvedModuleFileName(oldSourceFile, importName.text); - var newResolution = resolveModuleNameWorker(importName.text, newSourceFile.fileName); - if (oldResolution !== newResolution) { + for (let i = 0; i < moduleNames.length; ++i) { + let oldResolution = getResolvedModuleFileName(oldSourceFile, moduleNames[i]); + if (oldResolution !== resolutions[i]) { return false; - } - } + } + } } // pass the cache of module resolutions from the old source file newSourceFile.resolvedModules = oldSourceFile.resolvedModules; @@ -719,10 +722,16 @@ namespace ts { if (file.imports.length) { file.resolvedModules = {}; let oldSourceFile = oldProgram && oldProgram.getSourceFile(file.fileName); - for (let moduleName of file.imports) { - resolveModule(moduleName); - } + let moduleNames = map(file.imports, name => name.text); + let resolutions = resolveModuleNamesWorker(moduleNames, file.fileName); + for (let i = 0; i < file.imports.length; ++i) { + let resolution = resolutions[i]; + setResolvedModuleName(file, moduleNames[i], resolution); + if (resolution) { + findModuleSourceFile(resolution, file.imports[i]); + } + } } else { // no imports - drop cached module resolutions @@ -733,16 +742,6 @@ namespace ts { function findModuleSourceFile(fileName: string, nameLiteral: Expression) { return findSourceFile(fileName, /* isDefaultLib */ false, file, nameLiteral.pos, nameLiteral.end - nameLiteral.pos); } - - function resolveModule(moduleNameExpr: LiteralExpression): void { - Debug.assert(resolveModuleNameWorker !== undefined); - - let resolvedModuleName = resolveModuleNameWorker(moduleNameExpr.text, file.fileName); - setResolvedModuleName(file, moduleNameExpr.text, resolvedModuleName); - if (resolvedModuleName) { - findModuleSourceFile(resolvedModuleName, moduleNameExpr); - } - } } function computeCommonSourceDirectory(sourceFiles: SourceFile[]): string { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index b26c710d397..f7aef302da0 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2266,7 +2266,7 @@ namespace ts { // if getModuleResolutionHost is implemented then compiler will apply one of built-in ways to resolve module names // and ModuleResolutionHost will be used to ask host specific questions // if resolveModuleName is implemented - this will mean that host is completely in charge of module name resolution - resolveModuleName?(moduleName: string, containingFile: string): string; + resolveModuleNames?(moduleNames: string[], containingFile: string): string[]; getModuleResolutionHost?(): ModuleResolutionHost; } diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 3783f16df5e..3d566594ed0 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -98,30 +98,54 @@ namespace ts.server { } } - resolveModuleName(moduleName: string, containingFile: string): string { - let resolutionsInFile = this.resolvedModuleNames.get(containingFile); - if (!resolutionsInFile) { - resolutionsInFile = {}; - this.resolvedModuleNames.set(containingFile, resolutionsInFile); + resolveModuleNames(moduleNames: string[], containingFile: string): string[] { + let currentResolutionsInFile = this.resolvedModuleNames.get(containingFile); + + let newResolutions: Map = {}; + let resolvedFileNames: string[] = []; + + let compilerOptions = this.getCompilationSettings(); + let defaultResolver = ts.getDefaultModuleNameResolver(compilerOptions); + + for (let moduleName of moduleNames) { + // check if this is a duplicate entry in the list + let resolution = lookUp(newResolutions, moduleName); + if (!resolution) { + let existingResolution = currentResolutionsInFile && ts.lookUp(currentResolutionsInFile, moduleName); + if (moduleResolutionIsValid(existingResolution)) { + // ok, it is safe to use existing module resolution results + resolution = existingResolution; + } + else { + resolution = defaultResolver(moduleName, containingFile, compilerOptions, this.moduleResolutionHost); + resolution.lastCheckTime = Date.now(); + newResolutions[moduleName] = resolution; + } + } + + ts.Debug.assert(resolution !== undefined); + + resolvedFileNames.push(resolution.resolvedFileName); } - let resolution = ts.lookUp(resolutionsInFile, moduleName); - if (!moduleResolutionIsValid(resolution)) { - let compilerOptions = this.getCompilationSettings(); - let defaultResolver = ts.getDefaultModuleNameResolver(compilerOptions); - resolution = defaultResolver(moduleName, containingFile, compilerOptions, this.moduleResolutionHost); - resolution.lastCheckTime = Date.now(); - resolutionsInFile[moduleName] = resolution; - } - return resolution.resolvedFileName; + // replace old results with a new one + this.resolvedModuleNames.set(containingFile, newResolutions); + return resolvedFileNames; function moduleResolutionIsValid(resolution: TimestampedResolvedModule): boolean { if (!resolution) { return false; } - // TODO: use lastCheckTime assuming that module resolution results are legal for some period of time - return !resolution.resolvedFileName && resolution.failedLookupLocations.length !== 0; + if (resolution.resolvedFileName) { + // TODO: consider checking failedLookupLocations + // TODO: use lastCheckTime to track expiration for module name resolution + 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; } } diff --git a/src/services/services.ts b/src/services/services.ts index bfc403ebc45..c0a617646b8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -983,7 +983,7 @@ namespace ts { // if resolveModuleName is implemented - this will mean that host is completely in charge of module name resolution // if none of these methods are implemented then language service will try to emulate getModuleResolutionHost atop of 'getScriptSnapshot' getModuleResolutionHost?(): ModuleResolutionHost; - resolveModuleName?(moduleName: string, containingFile: string): string; + resolveModuleNames?(moduleNames: string[], containingFile: string): string[]; } // @@ -2564,7 +2564,8 @@ namespace ts { let oldSettings = program && program.getCompilerOptions(); let newSettings = hostCache.compilationSettings(); - let changesInCompilationSettingsAffectSyntax = oldSettings && oldSettings.target !== newSettings.target; + let changesInCompilationSettingsAffectSyntax = oldSettings && + (oldSettings.target !== newSettings.target || oldSettings.module !== newSettings.module || oldSettings.noResolve !== newSettings.noResolve); // Now create a new compiler let compilerHost: CompilerHost = { @@ -2578,8 +2579,8 @@ namespace ts { getCurrentDirectory: () => host.getCurrentDirectory(), }; - if (host.resolveModuleName) { - compilerHost.resolveModuleName = (moduleName, containingFile) => host.resolveModuleName(moduleName, containingFile) + if (host.resolveModuleNames) { + compilerHost.resolveModuleNames = (moduleNames, containingFile) => host.resolveModuleNames(moduleNames, containingFile) } else if (host.getModuleResolutionHost) { compilerHost.getModuleResolutionHost = () => host.getModuleResolutionHost() diff --git a/src/services/shims.ts b/src/services/shims.ts index 4a2ed7b162f..f290be8ba0b 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -267,22 +267,16 @@ namespace ts { private files: string[]; private loggingEnabled = false; private tracingEnabled = false; - private lastRequestedFile: string; - private lastRequestedModuleResolutions: Map; - public resolveModuleName: (moduleName: string, containingFile: string) => string; + public resolveModuleNames: (moduleName: string[], containingFile: string) => string[]; constructor(private shimHost: LanguageServiceShimHost) { // if shimHost is a COM object then property check will become method call with no arguments. // 'in' does not have this effect. if ("getModuleResolutionsForFile" in this.shimHost) { - this.resolveModuleName = (moduleName: string, containingFile: string) => { - if (this.lastRequestedFile !== containingFile) { - this.lastRequestedModuleResolutions = >JSON.parse(this.shimHost.getModuleResolutionsForFile(containingFile)); - this.lastRequestedFile = containingFile; - } - - return this.lastRequestedModuleResolutions[moduleName]; + this.resolveModuleNames = (moduleNames: string[], containingFile: string) => { + let resolutionsInFile = >JSON.parse(this.shimHost.getModuleResolutionsForFile(containingFile)); + return map(moduleNames, name => lookUp(resolutionsInFile, name)); }; } } diff --git a/tests/cases/unittests/cachingInServerLSHost.ts b/tests/cases/unittests/cachingInServerLSHost.ts new file mode 100644 index 00000000000..ff2da76ba4f --- /dev/null +++ b/tests/cases/unittests/cachingInServerLSHost.ts @@ -0,0 +1,206 @@ +/// + +module ts { + interface File { + name: string; + content: string; + } + + function createDefaultServerHost(fileMap: Map): server.ServerHost { + return { + args: [], + newLine: "\r\n", + useCaseSensitiveFileNames: false, + write: (s: string) => { + }, + readFile: (path: string, encoding?: string): string => { + return hasProperty(fileMap, path) && fileMap[path].content; + }, + writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => { + throw new Error("NYI"); + }, + resolvePath: (path: string): string => { + throw new Error("NYI"); + }, + fileExists: (path: string): boolean => { + return hasProperty(fileMap, path); + }, + directoryExists: (path: string): boolean => { + throw new Error("NYI"); + }, + createDirectory: (path: string) => { + }, + getExecutingFilePath: (): string => { + return ""; + }, + getCurrentDirectory: (): string => { + return ""; + }, + readDirectory: (path: string, extension?: string, exclude?: string[]): string[] => { + throw new Error("NYI"); + }, + exit: (exitCode?: number) => { + }, + watchFile: (path, callback) => { + return { + close: () => { } + } + } + }; + } + + function createProject(rootFile: string, serverHost: server.ServerHost): { project: server.Project, rootScriptInfo: server.ScriptInfo } { + let logger: server.Logger = { + close() { }, + isVerbose: () => false, + loggingEnabled: () => false, + perftrc: (s: string) => { }, + info: (s: string) => { }, + startGroup: () => { }, + endGroup: () => { }, + msg: (s: string, type?: string) => { } + }; + + let projectService = new server.ProjectService(serverHost, logger); + let rootScriptInfo = projectService.openFile(rootFile, /* openedByClient */true); + let project = projectService.createInferredProject(rootScriptInfo); + project.setProjectOptions( {files: [rootScriptInfo.fileName], compilerOptions: {module: ts.ModuleKind.AMD} } ); + return { + project, + rootScriptInfo + }; + } + + describe("Caching in LSHost", () => { + it("works using legacy resolution logic", () => { + let root: File = { + name: "c:/d/f0.ts", + content: `import {x} from "f1"` + }; + + let imported: File = { + name: "c:/f1.ts", + content: `foo()` + }; + + let serverHost = createDefaultServerHost({ [root.name]: root, [imported.name]: imported }); + let { project, rootScriptInfo } = createProject(root.name, serverHost); + + // ensure that imported file was found + let diags = project.compilerService.languageService.getSemanticDiagnostics(imported.name); + assert.equal(diags.length, 1); + + let originalFileExists = serverHost.fileExists; + { + // patch fileExists to make sure that disk is not touched + serverHost.fileExists = (fileName): boolean => { + assert.isTrue(false, "fileExists should not be called"); + return false; + }; + + let newContent = `import {x} from "f1" + var x: string = 1;`; + rootScriptInfo.editContent(0, rootScriptInfo.content.length, newContent); + // trigger synchronization to make sure that import will be fetched from the cache + diags = project.compilerService.languageService.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(fileName); + }; + let newContent = `import {x} from "f2"`; + rootScriptInfo.editContent(0, rootScriptInfo.content.length, newContent); + + try { + // trigger synchronization to make sure that LSHost will try to find 'f2' module on disk + project.compilerService.languageService.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(fileName); + }; + + let newContent = `import {x} from "f1"`; + rootScriptInfo.editContent(0, rootScriptInfo.content.length, newContent); + project.compilerService.languageService.getSemanticDiagnostics(imported.name); + assert.isTrue(fileExistsCalled); + + // setting compiler options discards module resolution cache + fileExistsCalled = false; + + let opts = ts.clone(project.projectOptions); + opts.compilerOptions = ts.clone(opts.compilerOptions); + opts.compilerOptions.target = ts.ScriptTarget.ES5; + project.setProjectOptions(opts); + + project.compilerService.languageService.getSemanticDiagnostics(imported.name); + assert.isTrue(fileExistsCalled); + } + }); + + it("loads missing files from disk", () => { + let root: File = { + name: 'c:/foo.ts', + content: `import {x} from "bar"` + }; + + let imported: File = { + name: 'c:/bar.d.ts', + content: `export var y = 1` + }; + + let fileMap: Map = { [root.name]: root }; + let serverHost = createDefaultServerHost(fileMap); + let 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(fileName); + }; + + let { project, rootScriptInfo } = createProject(root.name, serverHost); + + let diags = project.compilerService.languageService.getSemanticDiagnostics(root.name); + assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called"); + assert.isTrue(diags.length === 1, "one diagnostic expected"); + assert.isTrue(typeof diags[0].messageText === "string" && ((diags[0].messageText).indexOf("Cannot find module") === 0), "should be 'cannot find module' message"); + + // assert that import will success once file appear on disk + fileMap[imported.name] = imported; + fileExistsCalledForBar = false; + rootScriptInfo.editContent(0, rootScriptInfo.content.length, `import {y} from "bar"`) + + diags = project.compilerService.languageService.getSemanticDiagnostics(root.name); + assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called"); + assert.isTrue(diags.length === 0); + }) + }); +} \ No newline at end of file