From 71d79c62d055a288a9766f86069d82549275023e Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 14 Jul 2017 22:35:07 -0700 Subject: [PATCH] Some refactoring to combine files removal from inferred project --- src/server/editorServices.ts | 193 +++++++++++++++-------------------- src/server/lsHost.ts | 20 ++-- src/server/project.ts | 15 ++- 3 files changed, 105 insertions(+), 123 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 0aed0ab99fa..abd8618b060 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -245,20 +245,6 @@ namespace ts.server { } } - /** - * TODO: enforce invariants: - * - script info can be never migrate to state - root file in inferred project, this is only a starting point - * - if script info has more that one containing projects - it is not a root file in inferred project because: - * - references in inferred project supercede the root part - * - root/reference in non-inferred project beats root in inferred project - */ - function isRootFileInInferredProject(info: ScriptInfo): boolean { - if (info.containingProjects.length === 0) { - return false; - } - return info.containingProjects[0].projectKind === ProjectKind.Inferred && info.containingProjects[0].isRoot(info); - } - class DirectoryWatchers { /** * a path to directory watcher map that detects added tsconfig files @@ -386,6 +372,7 @@ namespace ts.server { private pendingProjectUpdates = createMap(); private pendingInferredProjectUpdate: boolean; + readonly currentDirectory: string; readonly toCanonicalFileName: (f: string) => string; public readonly host: ServerHost; @@ -417,6 +404,7 @@ namespace ts.server { Debug.assert(!!this.host.createHash, "'ServerHost.createHash' is required for ProjectService"); + this.currentDirectory = this.host.getCurrentDirectory(); this.toCanonicalFileName = createGetCanonicalFileName(this.host.useCaseSensitiveFileNames); this.directoryWatchers = new DirectoryWatchers(this); this.throttledOperations = new ThrottledOperations(this.host); @@ -431,7 +419,11 @@ namespace ts.server { extraFileExtensions: [] }; - this.documentRegistry = createDocumentRegistry(this.host.useCaseSensitiveFileNames, this.host.getCurrentDirectory()); + this.documentRegistry = createDocumentRegistry(this.host.useCaseSensitiveFileNames, this.currentDirectory); + } + + toPath(fileName: string, basePath = this.currentDirectory) { + return toPath(fileName, basePath, this.toCanonicalFileName); } /* @internal */ @@ -721,7 +713,7 @@ namespace ts.server { this.removeProject(project); // Reload the configured projects for these open files in the project as // they could be held up by another config file somewhere in the parent directory - const openFilesInProject = this.getOrphanFiles(); + const openFilesInProject = filter(this.openFiles, file => file.containingProjects.length === 0); this.reloadConfiguredProjectForFiles(openFilesInProject, project => { project.pendingReload = true; this.delayUpdateProjectGraph(project); }); this.delayInferredProjectsRefresh(); } @@ -762,6 +754,7 @@ namespace ts.server { this.projectToSizeMap.delete((project as ExternalProject).externalProjectName); break; case ProjectKind.Configured: + // Update the map of mapOfKnownTsConfigFiles removeItemFromSet(this.configuredProjects, project); this.projectToSizeMap.delete((project as ConfiguredProject).canonicalConfigFilePath); break; @@ -772,59 +765,38 @@ namespace ts.server { } private assignScriptInfoToInferredProjectIfNecessary(info: ScriptInfo, addToListOfOpenFiles: boolean): void { - const externalProject = this.findContainingExternalProject(info.fileName); - if (externalProject) { - // file is already included in some external project - do nothing - if (addToListOfOpenFiles) { - this.openFiles.push(info); - } - return; - } - - let foundConfiguredProject = false; - for (const p of info.containingProjects) { - // file is the part of configured project - if (p.projectKind === ProjectKind.Configured) { - foundConfiguredProject = true; - if (addToListOfOpenFiles) { - ((p)).addOpenRef(); - } - } - } - if (foundConfiguredProject) { - if (addToListOfOpenFiles) { - this.openFiles.push(info); - } - return; - } - if (info.containingProjects.length === 0) { // create new inferred project p with the newly opened file as root // or add root to existing inferred project if 'useOneInferredProject' is true - const inferredProject = this.createInferredProjectWithRootFileIfNecessary(info); - if (!this.useSingleInferredProject) { - // if useOneInferredProject is not set then try to fixup ownership of open files - // check 'defaultProject !== inferredProject' is necessary to handle cases - // when creation inferred project for some file has added other open files into this project (i.e. as referenced files) - // we definitely don't want to delete the project that was just created - for (const f of this.openFiles) { - if (f.containingProjects.length === 0 || !inferredProject.containsScriptInfo(f)) { - // this is orphaned file that we have not processed yet - skip it - continue; - } + this.createInferredProjectWithRootFileIfNecessary(info); - for (const fContainingProject of f.containingProjects) { - if (fContainingProject.projectKind === ProjectKind.Inferred && - fContainingProject.isRoot(f) && - fContainingProject !== inferredProject) { - - // open file used to be root in inferred project, - // this inferred project is different from the one we've just created for current file - // and new inferred project references this open file. - // We should delete old inferred project and attach open file to the new one - this.removeProject(fContainingProject); - f.attachToProject(inferredProject); - } + // if useOneInferredProject is not set then try to fixup ownership of open files + // check 'defaultProject !== inferredProject' is necessary to handle cases + // when creation inferred project for some file has added other open files into this project + // (i.e.as referenced files) + // we definitely don't want to delete the project that was just created + // Also note that we need to create a copy of the array since the list of project will change + for (const inferredProject of this.inferredProjects.slice(0, this.inferredProjects.length - 1)) { + Debug.assert(!this.useSingleInferredProject); + // Remove this file from the root of inferred project if its part of more than 2 projects + // This logic is same as iterating over all open files and calling + // this.removRootOfInferredProjectIfNowPartOfOtherProject(f); + // Since this is also called from refreshInferredProject and closeOpen file + // to update inferred projects of the open file, this iteration might be faster + // instead of scanning all open files + const root = inferredProject.getRootScriptInfos(); + Debug.assert(root.length === 1); + if (root[0].containingProjects.length > 1) { + this.removeProject(inferredProject); + } + } + } + else { + for (const p of info.containingProjects) { + // file is the part of configured project + if (p.projectKind === ProjectKind.Configured) { + if (addToListOfOpenFiles) { + ((p)).addOpenRef(); } } } @@ -835,17 +807,6 @@ namespace ts.server { } } - private getOrphanFiles() { - let orphanFiles: ScriptInfo[]; - // for all open files - for (const f of this.openFiles) { - if (f.containingProjects.length === 0) { - (orphanFiles || (orphanFiles = [])).push(f); - } - } - return orphanFiles; - } - /** * Remove this file from the set of open, non-configured files. * @param info The file that has been closed or newly configured @@ -871,8 +832,15 @@ namespace ts.server { } } else if (p.projectKind === ProjectKind.Inferred && p.isRoot(info)) { - // open file in inferred project - (projectsToRemove || (projectsToRemove = [])).push(p); + // If this was the open root file of inferred project + if ((p as InferredProject).isProjectWithSingleRoot()) { + // - when useSingleInferredProject is not set, we can guarantee that this will be the only root + // - other wise remove the project if it is the only root + (projectsToRemove || (projectsToRemove = [])).push(p); + } + else { + p.removeFile(info); + } } if (!p.languageServiceEnabled) { @@ -888,11 +856,10 @@ namespace ts.server { } // collect orphanted files and try to re-add them as newly opened - const orphanFiles = this.getOrphanFiles(); - // treat orphaned files as newly opened - if (orphanFiles) { - for (const f of orphanFiles) { + // for all open files + for (const f of this.openFiles) { + if (f.containingProjects.length === 0) { this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false); } } @@ -1003,9 +970,9 @@ namespace ts.server { private findConfiguredProjectByProjectName(configFileName: NormalizedPath) { // make sure that casing of config file name is consistent - configFileName = asNormalizedPath(this.toCanonicalFileName(configFileName)); + const canonicalConfigFilePath = asNormalizedPath(this.toCanonicalFileName(configFileName)); for (const proj of this.configuredProjects) { - if (proj.canonicalConfigFilePath === configFileName) { + if (proj.canonicalConfigFilePath === canonicalConfigFilePath) { return proj; } } @@ -1186,7 +1153,6 @@ namespace ts.server { languageServiceEnabled, projectOptions.compileOnSave === undefined ? false : projectOptions.compileOnSave, cachedServerHost); - this.addFilesToProjectAndUpdateGraph(project, projectOptions.files, fileNamePropertyReader, clientFileName, projectOptions.typeAcquisition, configFileErrors); project.configFileSpecs = configFileSpecs; project.configFileWatcher = this.addFileWatcher(WatchType.ConfigFilePath, project, @@ -1197,6 +1163,8 @@ namespace ts.server { project.watchTypeRoots(); } + this.addFilesToProjectAndUpdateGraph(project, projectOptions.files, fileNamePropertyReader, clientFileName, projectOptions.typeAcquisition, configFileErrors); + this.configuredProjects.push(project); this.sendProjectTelemetry(project.getConfigFilePath(), project, projectOptions); return project; @@ -1242,7 +1210,7 @@ namespace ts.server { let scriptInfo: ScriptInfo | NormalizedPath; let path: Path; if (!project.lsHost.fileExists(newRootFile)) { - path = normalizedPathToPath(normalizedPath, this.host.getCurrentDirectory(), this.toCanonicalFileName); + path = normalizedPathToPath(normalizedPath, this.currentDirectory, this.toCanonicalFileName); const existingValue = projectRootFilesMap.get(path); if (isScriptInfo(existingValue)) { project.removeFile(existingValue); @@ -1257,16 +1225,12 @@ namespace ts.server { path = scriptInfo.path; // If this script info is not already a root add it if (!project.isRoot(scriptInfo)) { - if (scriptInfo.isScriptOpen() && isRootFileInInferredProject(scriptInfo)) { + project.addRoot(scriptInfo); + if (scriptInfo.isScriptOpen()) { // if file is already root in some inferred project // - remove the file from that project and delete the project if necessary - const inferredProject = scriptInfo.containingProjects[0]; - inferredProject.removeFile(scriptInfo); - if (!inferredProject.hasRoots()) { - this.removeProject(inferredProject); - } + this.removRootOfInferredProjectIfNowPartOfOtherProject(scriptInfo); } - project.addRoot(scriptInfo); } } newRootScriptInfoMap.set(path, scriptInfo); @@ -1362,7 +1326,6 @@ namespace ts.server { root.fileName, project, fileName => this.onConfigFileAddedForInferredProject(fileName)); - project.updateGraph(); if (!useExistingProject) { @@ -1403,7 +1366,7 @@ namespace ts.server { } getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) { - const path = toPath(fileName, this.host.getCurrentDirectory(), this.toCanonicalFileName); + const path = normalizedPathToPath(fileName, this.currentDirectory, this.toCanonicalFileName); let info = this.getScriptInfoForPath(path); if (!info) { if (openedByClient || this.host.fileExists(fileName)) { @@ -1438,7 +1401,7 @@ namespace ts.server { } getScriptInfoForNormalizedPath(fileName: NormalizedPath) { - return this.getScriptInfoForPath(normalizedPathToPath(fileName, this.host.getCurrentDirectory(), this.toCanonicalFileName)); + return this.getScriptInfoForPath(normalizedPathToPath(fileName, this.currentDirectory, this.toCanonicalFileName)); } getScriptInfoForPath(fileName: Path) { @@ -1539,6 +1502,26 @@ namespace ts.server { } } + /** + * - script info can be never migrate to state - root file in inferred project, this is only a starting point + * - if script info has more that one containing projects - it is not a root file in inferred project because: + * - references in inferred project supercede the root part + * - root/reference in non-inferred project beats root in inferred project + */ + private removRootOfInferredProjectIfNowPartOfOtherProject(info: ScriptInfo) { + if (info.containingProjects.length > 1 && + info.containingProjects[0].projectKind === ProjectKind.Inferred && + info.containingProjects[0].isRoot(info)) { + const inferredProject = info.containingProjects[0] as InferredProject; + if (inferredProject.isProjectWithSingleRoot()) { + this.removeProject(inferredProject); + } + else { + inferredProject.removeFile(info); + } + } + } + /** * This function is to update the project structure for every projects. * It is called on the premise that all the configured projects are @@ -1548,26 +1531,16 @@ namespace ts.server { this.logger.info("updating project structure from ..."); this.printProjects(); - const orphantedFiles: ScriptInfo[] = []; - // collect all orphanted script infos from open files for (const info of this.openFiles) { + // collect all orphanted script infos from open files if (info.containingProjects.length === 0) { - orphantedFiles.push(info); + this.assignScriptInfoToInferredProjectIfNecessary(info, /*addToListOfOpenFiles*/ false); } + // Or remove the root of inferred project if is referenced in more than one projects else { - if (isRootFileInInferredProject(info) && info.containingProjects.length > 1) { - const inferredProject = info.containingProjects[0]; - Debug.assert(inferredProject.projectKind === ProjectKind.Inferred); - inferredProject.removeFile(info); - if (!inferredProject.hasRoots()) { - this.removeProject(inferredProject); - } - } + this.removRootOfInferredProjectIfNowPartOfOtherProject(info); } } - for (const f of orphantedFiles) { - this.assignScriptInfoToInferredProjectIfNecessary(f, /*addToListOfOpenFiles*/ false); - } for (const p of this.inferredProjects) { p.updateGraph(); diff --git a/src/server/lsHost.ts b/src/server/lsHost.ts index a7e9c5e231c..1be0f203206 100644 --- a/src/server/lsHost.ts +++ b/src/server/lsHost.ts @@ -27,8 +27,12 @@ namespace ts.server { this.currentDirectory = this.host.getCurrentDirectory(); } + private toPath(fileName: string) { + return toPath(fileName, this.currentDirectory, this.getCanonicalFileName); + } + private getFileSystemEntries(rootDir: string) { - const path = toPath(rootDir, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(rootDir); const cachedResult = this.cachedReadDirectoryResult.get(path); if (cachedResult) { return cachedResult; @@ -45,7 +49,7 @@ namespace ts.server { private canWorkWithCacheForDir(rootDir: string) { // Some of the hosts might not be able to handle read directory or getDirectories - const path = toPath(rootDir, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(rootDir); if (this.cachedReadDirectoryResult.get(path)) { return true; } @@ -62,7 +66,7 @@ namespace ts.server { } writeFile(fileName: string, data: string, writeByteOrderMark?: boolean) { - const path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(fileName); const result = this.cachedReadDirectoryResult.get(getDirectoryPath(path)); const baseFileName = getBaseFileName(toNormalizedPath(fileName)); if (result) { @@ -111,14 +115,14 @@ namespace ts.server { } fileExists(fileName: string): boolean { - const path = toPath(fileName, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(fileName); const result = this.cachedReadDirectoryResult.get(getDirectoryPath(path)); const baseName = getBaseFileName(toNormalizedPath(fileName)); return (result && this.hasEntry(result.files, baseName)) || this.host.fileExists(fileName); } directoryExists(dirPath: string) { - const path = toPath(dirPath, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(dirPath); return this.cachedReadDirectoryResult.has(path) || this.host.directoryExists(dirPath); } @@ -147,7 +151,7 @@ namespace ts.server { } addOrDeleteFileOrFolder(fileOrFolder: NormalizedPath) { - const path = toPath(fileOrFolder, this.currentDirectory, this.getCanonicalFileName); + const path = this.toPath(fileOrFolder); const existingResult = this.cachedReadDirectoryResult.get(path); if (existingResult) { if (!this.host.directoryExists(fileOrFolder)) { @@ -262,7 +266,7 @@ namespace ts.server { getResultFileName: (result: R) => string | undefined, logChanges: boolean): R[] { - const path = toPath(containingFile, this.host.getCurrentDirectory(), this.project.projectService.toCanonicalFileName); + const path = this.project.projectService.toPath(containingFile); const currentResolutionsInFile = cache.get(path); const newResolutions: Map = createMap(); @@ -403,7 +407,7 @@ namespace ts.server { fileExists(file: string): boolean { // As an optimization, don't hit the disks for files we already know don't exist // (because we're watching for their creation). - const path = toPath(file, this.host.getCurrentDirectory(), this.project.projectService.toCanonicalFileName); + const path = this.project.projectService.toPath(file); return !this.project.isWatchedMissingFile(path) && this.host.fileExists(file); } diff --git a/src/server/project.ts b/src/server/project.ts index aa492d246dc..1b99e76545e 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -496,8 +496,7 @@ namespace ts.server { // add a root file to project addMissingFileRoot(fileName: NormalizedPath) { - const path = toPath(fileName, this.projectService.host.getCurrentDirectory(), - this.projectService.toCanonicalFileName); + const path = this.projectService.toPath(fileName); this.rootFilesMap.set(path, fileName); this.markAsDirty(); } @@ -842,7 +841,7 @@ namespace ts.server { // Handle triple slash references if (sourceFile.referencedFiles && sourceFile.referencedFiles.length > 0) { for (const referencedFile of sourceFile.referencedFiles) { - const referencedPath = toPath(referencedFile.fileName, currentDirectory, this.projectService.toCanonicalFileName); + const referencedPath = this.projectService.toPath(referencedFile.fileName, currentDirectory); referencedFiles.set(referencedPath, true); } } @@ -855,7 +854,7 @@ namespace ts.server { } const fileName = resolvedTypeReferenceDirective.resolvedFileName; - const typeFilePath = toPath(fileName, currentDirectory, this.projectService.toCanonicalFileName); + const typeFilePath = this.projectService.toPath(fileName, currentDirectory); referencedFiles.set(typeFilePath, true); }); } @@ -943,6 +942,12 @@ namespace ts.server { } } + isProjectWithSingleRoot() { + // - when useSingleInferredProject is not set, we can guarantee that this will be the only root + // - other wise it has single root if it has single root script info + return !this.projectService.useSingleInferredProject || this.getRootScriptInfos().length === 1; + } + getProjectRootPath() { // Single inferred project does not have a project root. if (this.projectService.useSingleInferredProject) { @@ -1143,7 +1148,7 @@ namespace ts.server { const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0; return existingRecursive !== recursive; }, - // Create new watch + // Create new watch and recursive info (directory, flag) => { const recursive = (flag & WatchDirectoryFlags.Recursive) !== 0; return {