From 0aa4da43adc0f73012a59942d03f2df04954d6be Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 16 Nov 2018 11:52:29 -0800 Subject: [PATCH] Life time of declaration, sources and map infos Map Info and sources is not ideal and need to revisited since we need to update mapper and projects correctly // TODO: lifetime of source project and declaration map --- src/server/editorServices.ts | 44 ++++++--- src/server/scriptInfo.ts | 2 + .../unittests/tsserverProjectSystem.ts | 95 +++++++++++++++---- .../reference/api/tsserverlibrary.d.ts | 4 - 4 files changed, 112 insertions(+), 33 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 49f94f3fc95..d98aa948113 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -437,7 +437,8 @@ namespace ts.server { /** * Container of all known scripts */ - private readonly filenameToScriptInfo = createMap(); + /*@internal*/ + readonly filenameToScriptInfo = createMap(); private readonly scriptInfoInNodeModulesWatchers = createMap (); /** * Contains all the deleted script info's version information so that @@ -2209,6 +2210,7 @@ namespace ts.server { // Create the mapper declarationInfo.mapInfo = undefined; + // TODO: shkamat Lifetime of declarationInfo and mapInfo let readMapFile: ((fileName: string) => string | undefined) | undefined = fileName => { const mapInfo = this.getOrCreateScriptInfoNotOpenedByClient(fileName, project.currentDirectory, project.directoryStructureHost); if (!mapInfo) return undefined; @@ -2218,7 +2220,7 @@ namespace ts.server { }; const projectName = project.projectName; const mapper = getDocumentPositionMapper( - { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName) }, + { getCanonicalFileName: this.toCanonicalFileName, log: s => this.logger.info(s), getSourceFileLike: f => this.getSourceFileLike(f, projectName, declarationInfo) }, declarationInfo.fileName, declarationInfo.getLineInfo(), readMapFile @@ -2229,8 +2231,8 @@ namespace ts.server { } /*@internal*/ - getSourceFileLike(fileName: string, projectName: string | Project) { - const project = (projectName as Project).projectName ? projectName as Project : this.findProject(projectName as string); + getSourceFileLike(fileName: string, projectNameOrProject: string | Project, declarationInfo?: ScriptInfo) { + const project = (projectNameOrProject as Project).projectName ? projectNameOrProject as Project : this.findProject(projectNameOrProject as string); if (project) { const path = project.toPath(fileName); const sourceFile = project.getSourceFile(path); @@ -2241,6 +2243,11 @@ namespace ts.server { const info = this.getOrCreateScriptInfoNotOpenedByClient(fileName, (project || this).currentDirectory, project ? project.directoryStructureHost : this.host); if (!info) return undefined; + // Attach as source + if (declarationInfo && declarationInfo.mapInfo && info !== declarationInfo) { + (declarationInfo.mapInfo.sourceInfos || (declarationInfo.mapInfo.sourceInfos = createMap())).set(info.path, true); + } + // Key doesnt matter since its only for text and lines if (info.cacheSourceFile) return info.cacheSourceFile.sourceFile; @@ -2556,13 +2563,7 @@ namespace ts.server { // when some file/s were closed which resulted in project removal. // It was then postponed to cleanup these script infos so that they can be reused if // the file from that old project is reopened because of opening file from here. - this.filenameToScriptInfo.forEach(info => { - if (!info.isScriptOpen() && info.isOrphan()) { - // if there are not projects that include this script info - delete it - this.stopWatchingScriptInfo(info); - this.deleteScriptInfo(info); - } - }); + this.removeOrphanScriptInfos(); this.printProjects(); @@ -2605,6 +2606,27 @@ namespace ts.server { } } + private removeOrphanScriptInfos() { + const toRemoveScriptInfos = cloneMap(this.filenameToScriptInfo); + this.filenameToScriptInfo.forEach(info => { + if (info.isScriptOpen() || !info.isOrphan()) { + toRemoveScriptInfos.delete(info.path); + if (info.mapInfo) { + toRemoveScriptInfos.delete(info.mapInfo.path); + if (info.mapInfo.sourceInfos) { + info.mapInfo.sourceInfos.forEach((_value, path) => toRemoveScriptInfos.delete(path)); + } + } + } + }); + + toRemoveScriptInfos.forEach(info => { + // if there are not projects that include this script info - delete it + this.stopWatchingScriptInfo(info); + this.deleteScriptInfo(info); + }); + } + private telemetryOnOpenFile(scriptInfo: ScriptInfo): void { if (this.syntaxOnly || !this.eventHandler || !scriptInfo.isJavaScript() || !addToSeen(this.allJsFilesForOpenFileTelemetry, scriptInfo.path)) { return; diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 71b8fe2f2b5..3d8c737064f 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -302,6 +302,8 @@ namespace ts.server { /*@internal*/ mapInfo?: ScriptInfo; /*@internal*/ + sourceInfos?: Map; + /*@internal*/ mapper: DocumentPositionMapper | false | undefined = false; /*@internal*/ sourceFileLike: SourceFileLike | undefined; diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 6681b4bfa54..217149efd88 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -475,6 +475,10 @@ namespace ts.projectSystem { checkArray("Open files", arrayFrom(projectService.openFiles.keys(), path => projectService.getScriptInfoForPath(path as Path)!.fileName), expectedFiles.map(file => file.path)); } + function checkScriptInfos(projectService: server.ProjectService, expectedFiles: ReadonlyArray) { + checkArray("ScriptInfos files", arrayFrom(projectService.filenameToScriptInfo.values(), info => info.fileName), expectedFiles); + } + function protocolLocationFromSubstring(str: string, substring: string): protocol.Location { const start = str.indexOf(substring); Debug.assert(start !== -1); @@ -10667,7 +10671,7 @@ declare class TestLib { }); }); - it("can go to definition correctly", () => { + describe("with main and depedency project", () => { const projectLocation = "/user/username/projects/myproject"; const dependecyLocation = `${projectLocation}/dependency`; const mainLocation = `${projectLocation}/main`; @@ -10704,24 +10708,79 @@ fn5();` }) }; - const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile]; - const host = createHost(files, [mainConfig.path]); - const session = createSession(host); - const service = session.getProjectService(); - openFilesForSession([mainTs], session); - checkNumberOfProjects(service, { configuredProjects: 1 }); - checkProjectActualFiles(service.configuredProjects.get(mainConfig.path)!, [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`]); - for (let i = 0; i < 5; i++) { - const startSpan = { line: i + 5, offset: 1 }; - const response = session.executeCommandSeq({ - command: protocol.CommandTypes.DefinitionAndBoundSpan, - arguments: { file: mainTs.path, ...startSpan } - }).response as protocol.DefinitionInfoAndBoundSpan; - assert.deepEqual(response, { - definitions: [{ file: dependencyTs.path, start: { line: i + 1, offset: 17 }, end: { line: i + 1, offset: 20 } }], - textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } - }); + const randomFile: File = { + path: `${projectLocation}/random/random.ts`, + content: "let a = 10;" + }; + const randomConfig: File = { + path: `${projectLocation}/random/tsconfig.json`, + content: "{}" + }; + + const files = [dependencyTs, dependencyConfig, mainTs, mainConfig, libFile, randomFile, randomConfig]; + + function verifyInfos(service: server.ProjectService, host: TestServerHost, openInfos: ReadonlyArray, closedInfos: ReadonlyArray, otherWatchedFiles: ReadonlyArray) { + checkScriptInfos(service, openInfos.concat(closedInfos)); + checkWatchedFiles(host, closedInfos.concat(otherWatchedFiles).map(f => f.toLowerCase())); } + + it("can go to definition correctly", () => { + const host = createHost(files, [mainConfig.path]); + const session = createSession(host); + const service = session.getProjectService(); + openFilesForSession([mainTs], session); + checkNumberOfProjects(service, { configuredProjects: 1 }); + checkProjectActualFiles(service.configuredProjects.get(mainConfig.path)!, [mainTs.path, libFile.path, mainConfig.path, `${dependecyLocation}/fns.d.ts`]); + for (let i = 0; i < 5; i++) { + const startSpan = { line: i + 5, offset: 1 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.DefinitionAndBoundSpan, + arguments: { file: mainTs.path, ...startSpan } + }).response as protocol.DefinitionInfoAndBoundSpan; + assert.deepEqual(response, { + definitions: [{ file: dependencyTs.path, start: { line: i + 1, offset: 17 }, end: { line: i + 1, offset: 20 } }], + textSpan: { start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } } + }); + } + checkNumberOfProjects(service, { configuredProjects: 1 }); + const closedInfos = [dependencyTs.path, dependencyConfig.path, libFile.path, `${dependecyLocation}/fns.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + verifyInfos(service, host, [mainTs.path], closedInfos, [mainConfig.path]); + + openFilesForSession([randomFile], session); + verifyInfos(service, host, [mainTs.path, randomFile.path], closedInfos, [mainConfig.path, randomConfig.path]); + }); + + it("rename locations from depedency", () => { + const host = createHost(files, [mainConfig.path]); + const session = createSession(host); + const service = session.getProjectService(); + openFilesForSession([dependencyTs, randomFile], session); + checkNumberOfProjects(service, { configuredProjects: 2 }); + checkProjectActualFiles(service.configuredProjects.get(dependencyConfig.path)!, [dependencyTs.path, libFile.path, dependencyConfig.path]); + debugger; + for (let i = 0; i < 5; i++) { + const startSpan = { line: i + 1, offset: 17 }; + const response = session.executeCommandSeq({ + command: protocol.CommandTypes.Rename, + arguments: { file: dependencyTs.path, ...startSpan } + }).response as protocol.RenameResponseBody; + assert.deepEqual(response.locs, [{ + file: dependencyTs.path, + locs: [{ start: startSpan, end: { line: startSpan.line, offset: startSpan.offset + 3 } }] + }]); + } + checkNumberOfProjects(service, { configuredProjects: 2 }); + const openInfos = [dependencyTs.path, randomFile.path]; + const closedInfos = [libFile.path, `${dependecyLocation}/FnS.d.ts`, `${dependecyLocation}/FnS.d.ts.map`]; + const otherWatchedFiles = [dependencyConfig.path, randomConfig.path]; + verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); + + // Collect the orphan projects and infos + closeFilesForSession([randomFile], session); + openFilesForSession([randomFile], session); + + verifyInfos(service, host, openInfos, closedInfos, otherWatchedFiles); + }); }); }); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 2b78178587e..2bf91fac4f4 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8497,10 +8497,6 @@ declare namespace ts.server { syntaxOnly?: boolean; } class ProjectService { - /** - * Container of all known scripts - */ - private readonly filenameToScriptInfo; private readonly scriptInfoInNodeModulesWatchers; /** * Contains all the deleted script info's version information so that