diff --git a/src/harness/unittests/projectErrors.ts b/src/harness/unittests/projectErrors.ts index 142aa3bf1b1..7c694ec4296 100644 --- a/src/harness/unittests/projectErrors.ts +++ b/src/harness/unittests/projectErrors.ts @@ -20,19 +20,37 @@ namespace ts.projectSystem { } } + function checkDiagnosticsWithLinePos(errors: server.protocol.DiagnosticWithLinePosition[], expectedErrors: string[]) { + assert.equal(errors ? errors.length : 0, expectedErrors.length, `expected ${expectedErrors.length} error in the list`); + if (expectedErrors.length) { + for (let i = 0; i < errors.length; i++) { + const actualMessage = errors[i].message; + const expectedMessage = expectedErrors[i]; + assert.isTrue(actualMessage.indexOf(errors[i].message) === 0, `error message does not match, expected ${actualMessage} to start with ${expectedMessage}`); + } + } + } + it("external project - diagnostics for missing files", () => { const file1 = { path: "/a/b/app.ts", content: "" }; const file2 = { - path: "/a/b/lib.ts", + path: "/a/b/applib.ts", content: "" }; // only file1 exists - expect error - const host = createServerHost([file1]); - const projectService = createProjectService(host); + const host = createServerHost([file1, libFile]); + const session = createSession(host); + const projectService = session.getProjectService(); const projectFileName = "/a/b/test.csproj"; + const compilerOptionsRequest = { + type: "request", + command: server.CommandNames.CompilerOptionsDiagnosticsFull, + seq: 2, + arguments: { projectFileName } + }; { projectService.openExternalProject({ @@ -41,35 +59,27 @@ namespace ts.projectSystem { rootFiles: toExternalFiles([file1.path, file2.path]) }); - projectService.checkNumberOfProjects({ externalProjects: 1 }); - const knownProjects = projectService.synchronizeProjectList([]); - checkProjectErrors(knownProjects[0], ["File '/a/b/lib.ts' not found."]); + checkNumberOfProjects(projectService, { externalProjects: 1 }); + const diags = session.executeCommand(compilerOptionsRequest).response; + checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]); } // only file2 exists - expect error - host.reloadFS([file2]); + host.reloadFS([file2, libFile]); { - projectService.openExternalProject({ - projectFileName, - options: {}, - rootFiles: toExternalFiles([file1.path, file2.path]) - }); - projectService.checkNumberOfProjects({ externalProjects: 1 }); - const knownProjects = projectService.synchronizeProjectList([]); - checkProjectErrors(knownProjects[0], ["File '/a/b/app.ts' not found."]); + host.triggerFileWatcherCallback(file1.path, FileWatcherEventKind.Deleted); + host.triggerFileWatcherCallback(file2.path, FileWatcherEventKind.Created); + checkNumberOfProjects(projectService, { externalProjects: 1 }); + const diags = session.executeCommand(compilerOptionsRequest).response; + checkDiagnosticsWithLinePos(diags, ["File '/a/b/app.ts' not found."]); } // both files exist - expect no errors - host.reloadFS([file1, file2]); + host.reloadFS([file1, file2, libFile]); { - projectService.openExternalProject({ - projectFileName, - options: {}, - rootFiles: toExternalFiles([file1.path, file2.path]) - }); - - projectService.checkNumberOfProjects({ externalProjects: 1 }); - const knownProjects = projectService.synchronizeProjectList([]); - checkProjectErrors(knownProjects[0], []); + host.triggerFileWatcherCallback(file1.path, FileWatcherEventKind.Created); + checkNumberOfProjects(projectService, { externalProjects: 1 }); + const diags = session.executeCommand(compilerOptionsRequest).response; + checkDiagnosticsWithLinePos(diags, []); } }); @@ -79,25 +89,34 @@ namespace ts.projectSystem { content: "" }; const file2 = { - path: "/a/b/lib.ts", + path: "/a/b/applib.ts", content: "" }; const config = { path: "/a/b/tsconfig.json", content: JSON.stringify({ files: [file1, file2].map(f => getBaseFileName(f.path)) }) }; - const host = createServerHost([file1, config]); - const projectService = createProjectService(host); + const host = createServerHost([file1, config, libFile]); + const session = createSession(host); + const projectService = session.getProjectService(); + openFilesForSession([file1], session); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); + const project = projectService.configuredProjects[0]; + const compilerOptionsRequest = { + type: "request", + command: server.CommandNames.CompilerOptionsDiagnosticsFull, + seq: 2, + arguments: { projectFileName: project.getProjectName() } + }; + let diags = session.executeCommand(compilerOptionsRequest).response; + checkDiagnosticsWithLinePos(diags, ["File '/a/b/applib.ts' not found."]); - projectService.openClientFile(file1.path); - projectService.checkNumberOfProjects({ configuredProjects: 1 }); - checkProjectErrors(projectService.synchronizeProjectList([])[0], ["File '/a/b/lib.ts' not found."]); - - host.reloadFS([file1, file2, config]); + host.reloadFS([file1, file2, config, libFile]); host.triggerFileWatcherCallback(file2.path, FileWatcherEventKind.Created); - projectService.checkNumberOfProjects({ configuredProjects: 1 }); - checkProjectErrors(projectService.synchronizeProjectList([])[0], []); + checkNumberOfProjects(projectService, { configuredProjects: 1 }); + diags = session.executeCommand(compilerOptionsRequest).response; + checkDiagnosticsWithLinePos(diags, []); }); it("configured projects - diagnostics for corrupted config 1", () => { diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 83a9b58a004..6ffc636dc7f 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -907,6 +907,41 @@ namespace ts.projectSystem { checkProjectRootFiles(project, [commonFile1.path, commonFile2.path]); }); + it("handles the missing files - that were added to program because they were added with /// { + const file1: FileOrFolder = { + path: "/a/b/commonFile1.ts", + content: `/// + let x = y` + }; + const host = createServerHost([file1, libFile]); + const session = createSession(host); + openFilesForSession([file1], session); + const projectService = session.getProjectService(); + + checkNumberOfInferredProjects(projectService, 1); + const project = projectService.inferredProjects[0]; + checkProjectRootFiles(project, [file1.path]); + checkProjectActualFiles(project, [file1.path, libFile.path]); + const getErrRequest = makeSessionRequest( + server.CommandNames.SemanticDiagnosticsSync, + { file: file1.path } + ); + let diags = session.executeCommand(getErrRequest).response; + + // Two errors: CommonFile2 not found and cannot find name y + assert.equal(diags.length, 2, diags.map(diag => flattenDiagnosticMessageText(diag.text, "\n")).join("\n")); + + host.reloadFS([file1, commonFile2, libFile]); + host.triggerFileWatcherCallback(commonFile2.path, FileWatcherEventKind.Created); + host.runQueuedTimeoutCallbacks(); + checkNumberOfInferredProjects(projectService, 1); + assert.strictEqual(projectService.inferredProjects[0], project, "Inferred project should be same"); + checkProjectRootFiles(project, [file1.path]); + checkProjectActualFiles(project, [file1.path, libFile.path, commonFile2.path]); + diags = session.executeCommand(getErrRequest).response; + assert.equal(diags.length, 0); + }); + it("should create new inferred projects for files excluded from a configured project", () => { const configFile: FileOrFolder = { path: "/a/b/tsconfig.json", @@ -2869,7 +2904,7 @@ namespace ts.projectSystem { moduleFile.path = moduleFileOldPath; host.reloadFS([moduleFile, file1]); - host.triggerFileWatcherCallback(moduleFileNewPath, FileWatcherEventKind.Changed); + host.triggerFileWatcherCallback(moduleFileNewPath, FileWatcherEventKind.Deleted); host.triggerDirectoryWatcherCallback("/a/b", moduleFile.path); host.runQueuedTimeoutCallbacks(); diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 852b4be78d6..7d43a1e67a6 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -245,10 +245,6 @@ namespace ts.server { } } - function createFileNotFoundDiagnostic(fileName: string) { - return createCompilerDiagnostic(Diagnostics.File_0_not_found, fileName); - } - /** * TODO: enforce invariants: * - script info can be never migrate to state - root file in inferred project, this is only a starting point @@ -1166,15 +1162,14 @@ namespace ts.server { const rootFilename = propertyReader.getFileName(f); const scriptKind = propertyReader.getScriptKind(f); const hasMixedContent = propertyReader.hasMixedContent(f, this.hostConfiguration.extraFileExtensions); + const fileName = toNormalizedPath(rootFilename); if (this.host.fileExists(rootFilename)) { - const info = this.getOrCreateScriptInfoForNormalizedPath(toNormalizedPath(rootFilename), /*openedByClient*/ clientFileName === rootFilename, /*fileContent*/ undefined, scriptKind, hasMixedContent); + const info = this.getOrCreateScriptInfoForNormalizedPath(fileName, /*openedByClient*/ clientFileName === rootFilename, /*fileContent*/ undefined, scriptKind, hasMixedContent); project.addRoot(info); } else { - // TODO: (sheetalkamat) because the files are not added as a root, we wont have these available in - // missing files unless someone recreates the project or it was also refrenced in existing sourcefile - // Also these errors wouldnt show correct errors - (configFileErrors || (configFileErrors = [])).push(createFileNotFoundDiagnostic(rootFilename)); + // Create the file root with just the filename so that LS will have correct set of roots + project.addMissingFileRoot(fileName); } } project.setProjectErrors(configFileErrors); @@ -1192,67 +1187,63 @@ namespace ts.server { } private updateNonInferredProject(project: ExternalProject | ConfiguredProject, newUncheckedFiles: T[], propertyReader: FilePropertyReader, newOptions: CompilerOptions, newTypeAcquisition: TypeAcquisition, compileOnSave: boolean, configFileErrors: Diagnostic[]) { - const oldRootScriptInfos = project.getRootScriptInfos(); - const newRootScriptInfos: ScriptInfo[] = []; - const newRootScriptInfoMap: NormalizedPathMap = createNormalizedPathMap(); + const projectRootFilesMap = project.getRootFilesMap(); + const newRootScriptInfoMap: Map = createMap(); - let rootFilesChanged = false; for (const f of newUncheckedFiles) { const newRootFile = propertyReader.getFileName(f); - if (!this.host.fileExists(newRootFile)) { - (configFileErrors || (configFileErrors = [])).push(createFileNotFoundDiagnostic(newRootFile)); - continue; - } const normalizedPath = toNormalizedPath(newRootFile); - let scriptInfo = this.getScriptInfoForNormalizedPath(normalizedPath); - if (!scriptInfo || !project.isRoot(scriptInfo)) { - rootFilesChanged = true; - if (!scriptInfo) { - const scriptKind = propertyReader.getScriptKind(f); - const hasMixedContent = propertyReader.hasMixedContent(f, this.hostConfiguration.extraFileExtensions); - scriptInfo = this.getOrCreateScriptInfoForNormalizedPath(normalizedPath, /*openedByClient*/ false, /*fileContent*/ undefined, scriptKind, hasMixedContent); + let scriptInfo: ScriptInfo | NormalizedPath; + let path: Path; + if (!this.host.fileExists(newRootFile)) { + path = normalizedPathToPath(normalizedPath, this.host.getCurrentDirectory(), this.toCanonicalFileName); + const existingValue = projectRootFilesMap.get(path); + if (isScriptInfo(existingValue)) { + project.removeFile(existingValue); + projectRootFilesMap.set(path, normalizedPath); } + scriptInfo = normalizedPath; } - newRootScriptInfos.push(scriptInfo); - newRootScriptInfoMap.set(scriptInfo.fileName, scriptInfo); - } - - if (rootFilesChanged || newRootScriptInfos.length !== oldRootScriptInfos.length) { - let toAdd: ScriptInfo[]; - let toRemove: ScriptInfo[]; - for (const oldFile of oldRootScriptInfos) { - if (!newRootScriptInfoMap.contains(oldFile.fileName)) { - (toRemove || (toRemove = [])).push(oldFile); - } - } - for (const newFile of newRootScriptInfos) { - if (!project.isRoot(newFile)) { - (toAdd || (toAdd = [])).push(newFile); - } - } - if (toRemove) { - for (const f of toRemove) { - project.removeFile(f); - } - } - if (toAdd) { - for (const f of toAdd) { - if (f.isScriptOpen() && isRootFileInInferredProject(f)) { + else { + const scriptKind = propertyReader.getScriptKind(f); + const hasMixedContent = propertyReader.hasMixedContent(f, this.hostConfiguration.extraFileExtensions); + scriptInfo = this.getOrCreateScriptInfoForNormalizedPath(normalizedPath, /*openedByClient*/ false, /*fileContent*/ undefined, scriptKind, hasMixedContent); + path = scriptInfo.path; + // If this script info is not already a root add it + if (!project.isRoot(scriptInfo)) { + if (scriptInfo.isScriptOpen() && isRootFileInInferredProject(scriptInfo)) { // if file is already root in some inferred project // - remove the file from that project and delete the project if necessary - const inferredProject = f.containingProjects[0]; - inferredProject.removeFile(f); + const inferredProject = scriptInfo.containingProjects[0]; + inferredProject.removeFile(scriptInfo); if (!inferredProject.hasRoots()) { this.removeProject(inferredProject); } } - project.addRoot(f); + project.addRoot(scriptInfo); } } + newRootScriptInfoMap.set(path, scriptInfo); + } + + // project's root file map size is always going to be larger than new roots map + // as we have already all the new files to the project + if (projectRootFilesMap.size > newRootScriptInfoMap.size) { + projectRootFilesMap.forEach((value, path) => { + if (!newRootScriptInfoMap.has(path)) { + if (isScriptInfo(value)) { + project.removeFile(value); + } + else { + projectRootFilesMap.delete(path); + project.markAsDirty(); + } + } + }); } project.setCompilerOptions(newOptions); - (project).setTypeAcquisition(newTypeAcquisition); + project.setTypeAcquisition(newTypeAcquisition); // VS only set the CompileOnSaveEnabled option in the request if the option was changed recently // therefore if it is undefined, it should not be updated. diff --git a/src/server/project.ts b/src/server/project.ts index 6788081176c..7e94de2afac 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -102,9 +102,19 @@ namespace ts.server { (mod: { typescript: typeof ts }): PluginModule; } + /** + * The project root can be script info - if root is present, + * or it could be just normalized path if root wasnt present on the host(only for non inferred project) + */ + export type ProjectRoot = ScriptInfo | NormalizedPath; + /* @internal */ + export function isScriptInfo(value: ProjectRoot): value is ScriptInfo { + return value instanceof ScriptInfo; + } + export abstract class Project { private rootFiles: ScriptInfo[] = []; - private rootFilesMap: Map = createMap(); + private rootFilesMap: Map = createMap(); private program: ts.Program; private externalFiles: SortedReadonlyArray; private missingFilesMap: Map = createMap(); @@ -335,12 +345,13 @@ namespace ts.server { getRootFilesLSHost() { const result: string[] = []; if (this.rootFiles) { - for (const f of this.rootFiles) { - if (this.languageServiceEnabled || f.isScriptOpen()) { + this.rootFilesMap.forEach((value, _path) => { + const f: ScriptInfo = isScriptInfo(value) && value; + if (this.languageServiceEnabled || (f && f.isScriptOpen())) { // if language service is disabled - process only files that are open - result.push(f.fileName); + result.push(f ? f.fileName : value as NormalizedPath); } - } + }); if (this.typingFiles) { for (const f of this.typingFiles) { result.push(f); @@ -350,6 +361,10 @@ namespace ts.server { return result; } + getRootFilesMap() { + return this.rootFilesMap; + } + getRootScriptInfos() { return this.rootFiles; } @@ -458,7 +473,7 @@ namespace ts.server { } isRoot(info: ScriptInfo) { - return this.rootFilesMap && this.rootFilesMap.has(info.path); + return this.rootFilesMap && this.rootFilesMap.get(info.path) === info; } // add a root file to project @@ -472,6 +487,14 @@ namespace ts.server { } } + // add a root file to project + addMissingFileRoot(fileName: NormalizedPath) { + const path = toPath(fileName, this.projectService.host.getCurrentDirectory(), + createGetCanonicalFileName(this.projectService.host.useCaseSensitiveFileNames)); + this.rootFilesMap.set(path, fileName); + this.markAsDirty(); + } + removeFile(info: ScriptInfo, detachFromProject = true) { if (this.isRoot(info)) { this.removeRoot(info); @@ -671,6 +694,12 @@ namespace ts.server { getScriptInfoLSHost(fileName: string) { const scriptInfo = this.projectService.getOrCreateScriptInfo(fileName, /*openedByClient*/ false); if (scriptInfo) { + const existingValue = this.rootFilesMap.get(scriptInfo.path); + if (existingValue !== undefined && existingValue !== scriptInfo) { + // This was missing path earlier but now the file exists. Update the root + this.rootFiles.push(scriptInfo); + this.rootFilesMap.set(scriptInfo.path, scriptInfo); + } scriptInfo.attachToProject(this); } return scriptInfo; @@ -898,12 +927,12 @@ namespace ts.server { } removeRoot(info: ScriptInfo) { + super.removeRoot(info); if (this._isJsInferredProject && info.isJavaScript()) { - if (filter(this.getRootScriptInfos(), info => info.isJavaScript()).length === 0) { + if (!some(this.getRootScriptInfos(), info => info.isJavaScript())) { this.toggleJsInferredProject(/*isJsInferredProject*/ false); } } - super.removeRoot(info); } getProjectRootPath() { diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 534d73a6410..ee5e45c7c47 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -254,8 +254,14 @@ namespace ts.server { detachAllProjects() { for (const p of this.containingProjects) { + const isInfoRoot = p.isRoot(this); // detach is unnecessary since we'll clean the list of containing projects anyways p.removeFile(this, /*detachFromProjects*/ false); + // If the info was for the external or configured project's root, + // add missing file as the root + if (isInfoRoot && p.projectKind !== ProjectKind.Inferred) { + p.addMissingFileRoot(this.fileName); + } } this.containingProjects.length = 0; } @@ -376,4 +382,4 @@ namespace ts.server { return this.scriptKind === ScriptKind.JS || this.scriptKind === ScriptKind.JSX; } } -} \ No newline at end of file +} diff --git a/src/services/services.ts b/src/services/services.ts index 759de900b84..28a29352b80 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -862,15 +862,7 @@ namespace ts { } public getRootFileNames(): string[] { - const fileNames: string[] = []; - - this.fileNameToEntry.forEach(value => { - if (value) { - fileNames.push(value.hostFileName); - } - }); - - return fileNames; + return this.host.getScriptFileNames(); } public getVersion(path: Path): string {