From 7736ea4e93ba867667cc128bb07926f6d5aed2c8 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 7 Nov 2018 14:55:19 -0800 Subject: [PATCH 1/6] Add tests that create relative path even though the script info isnt dynamic --- .../unittests/tsserverProjectSystem.ts | 31 ++++++++++++++----- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 171070e2cb4..31ca9403215 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -3301,6 +3301,23 @@ namespace ts.projectSystem { }); }); + it("dynamic file with reference paths external project", () => { + const file: File = { + path: "^walkThroughSnippet:/Users/UserName/projects/someProject/out/someFile#1.js", + content: `/// +/// +var x = 10;` + }; + const host = createServerHost([libFile]); + const projectService = createProjectService(host); + projectService.openClientFile(file.path, file.content); + + projectService.checkNumberOfProjects({ inferredProjects: 1 }); + const project = projectService.inferredProjects[0]; + checkProjectRootFiles(project, [file.path]); + checkProjectActualFiles(project, [file.path, libFile.path]); + }); + it("files opened, closed affecting multiple projects", () => { const file: File = { path: "/a/b/projects/config/file.ts", @@ -3893,8 +3910,8 @@ namespace ts.projectSystem { command: server.CommandNames.Open, arguments: { file: untitledFile, - fileContent: "", - scriptKindName: "JS", + fileContent: `/// `, + scriptKindName: "TS", projectRootPath: useProjectRoot ? folderPath : undefined } }); @@ -10772,16 +10789,16 @@ fn5();` const untitledFile = "untitled:^Untitled-1"; executeSessionRequestNoResponse(session, protocol.CommandTypes.Open, { file: untitledFile, - fileContent: "let foo = 1;\nfooo/**/", + fileContent: `/// \nlet foo = 1;\nfooo/**/`, scriptKindName: "TS", projectRootPath: "/proj", }); const response = executeSessionRequest(session, protocol.CommandTypes.GetCodeFixes, { file: untitledFile, - startLine: 2, + startLine: 3, startOffset: 1, - endLine: 2, + endLine: 3, endOffset: 5, errorCodes: [Diagnostics.Cannot_find_name_0_Did_you_mean_1.code], }); @@ -10794,8 +10811,8 @@ fn5();` changes: [{ fileName: untitledFile, textChanges: [{ - start: { line: 2, offset: 1 }, - end: { line: 2, offset: 5 }, + start: { line: 3, offset: 1 }, + end: { line: 3, offset: 5 }, newText: "foo", }], }], From 9eecf3bd681f458cb60de211678feb841c1d063f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 19 Oct 2018 15:03:05 -0700 Subject: [PATCH 2/6] Some reorg --- src/server/editorServices.ts | 4 +- src/server/session.ts | 93 +++++++++++++++++++++++------------- 2 files changed, 62 insertions(+), 35 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 2f3c6204937..eb9b7118c5f 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -835,9 +835,9 @@ namespace ts.server { /* @internal */ private forEachProject(cb: (project: Project) => void) { - this.inferredProjects.forEach(cb); - this.configuredProjects.forEach(cb); this.externalProjects.forEach(cb); + this.configuredProjects.forEach(cb); + this.inferredProjects.forEach(cb); } /* @internal */ diff --git a/src/server/session.ts b/src/server/session.ts index 10399e21680..43b624668de 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -289,7 +289,6 @@ namespace ts.server { function combineProjectOutputWhileOpeningReferencedProjects( projects: Projects, defaultProject: Project, - projectService: ProjectService, action: (project: Project) => ReadonlyArray, getLocation: (t: T) => sourcemaps.SourceMappableLocation, resultsEqual: (a: T, b: T) => boolean, @@ -299,7 +298,6 @@ namespace ts.server { projects, defaultProject, /*initialLocation*/ undefined, - projectService, ({ project }, tryAddToTodo) => { for (const output of action(project)) { if (!contains(outputs, output, resultsEqual) && !tryAddToTodo(project, getLocation(output))) { @@ -312,17 +310,27 @@ namespace ts.server { } function combineProjectOutputForRenameLocations( - projects: Projects, defaultProject: Project, initialLocation: sourcemaps.SourceMappableLocation, projectService: ProjectService, findInStrings: boolean, findInComments: boolean + projects: Projects, + defaultProject: Project, + initialLocation: sourcemaps.SourceMappableLocation, + findInStrings: boolean, + findInComments: boolean ): ReadonlyArray { const outputs: RenameLocation[] = []; - combineProjectOutputWorker(projects, defaultProject, initialLocation, projectService, ({ project, location }, tryAddToTodo) => { - for (const output of project.getLanguageService().findRenameLocations(location.fileName, location.position, findInStrings, findInComments) || emptyArray) { - if (!contains(outputs, output, documentSpansEqual) && !tryAddToTodo(project, documentSpanLocation(output))) { - outputs.push(output); + combineProjectOutputWorker( + projects, + defaultProject, + initialLocation, + ({ project, location }, tryAddToTodo) => { + for (const output of project.getLanguageService().findRenameLocations(location.fileName, location.position, findInStrings, findInComments) || emptyArray) { + if (!contains(outputs, output, documentSpansEqual) && !tryAddToTodo(project, documentSpanLocation(output))) { + outputs.push(output); + } } - } - }, () => getDefinitionLocation(defaultProject, initialLocation)); + }, + () => getDefinitionLocation(defaultProject, initialLocation) + ); return outputs; } @@ -333,31 +341,41 @@ namespace ts.server { return info && { fileName: info.fileName, position: info.textSpan.start }; } - function combineProjectOutputForReferences(projects: Projects, defaultProject: Project, initialLocation: sourcemaps.SourceMappableLocation, projectService: ProjectService): ReadonlyArray { + function combineProjectOutputForReferences( + projects: Projects, + defaultProject: Project, + initialLocation: sourcemaps.SourceMappableLocation + ): ReadonlyArray { const outputs: ReferencedSymbol[] = []; - combineProjectOutputWorker(projects, defaultProject, initialLocation, projectService, ({ project, location }, getMappedLocation) => { - for (const outputReferencedSymbol of project.getLanguageService().findReferences(location.fileName, location.position) || emptyArray) { - const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(outputReferencedSymbol.definition)); - const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ? outputReferencedSymbol.definition : { - ...outputReferencedSymbol.definition, - textSpan: createTextSpan(mappedDefinitionFile.position, outputReferencedSymbol.definition.textSpan.length), - fileName: mappedDefinitionFile.fileName, - }; - let symbolToAddTo = find(outputs, o => documentSpansEqual(o.definition, definition)); - if (!symbolToAddTo) { - symbolToAddTo = { definition, references: [] }; - outputs.push(symbolToAddTo); - } + combineProjectOutputWorker( + projects, + defaultProject, + initialLocation, + ({ project, location }, getMappedLocation) => { + for (const outputReferencedSymbol of project.getLanguageService().findReferences(location.fileName, location.position) || emptyArray) { + const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(outputReferencedSymbol.definition)); + const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ? outputReferencedSymbol.definition : { + ...outputReferencedSymbol.definition, + textSpan: createTextSpan(mappedDefinitionFile.position, outputReferencedSymbol.definition.textSpan.length), + fileName: mappedDefinitionFile.fileName, + }; + let symbolToAddTo = find(outputs, o => documentSpansEqual(o.definition, definition)); + if (!symbolToAddTo) { + symbolToAddTo = { definition, references: [] }; + outputs.push(symbolToAddTo); + } - for (const ref of outputReferencedSymbol.references) { - // If it's in a mapped file, that is added to the todo list by `getMappedLocation`. - if (!contains(symbolToAddTo.references, ref, documentSpansEqual) && !getMappedLocation(project, documentSpanLocation(ref))) { - symbolToAddTo.references.push(ref); + for (const ref of outputReferencedSymbol.references) { + // If it's in a mapped file, that is added to the todo list by `getMappedLocation`. + if (!contains(symbolToAddTo.references, ref, documentSpansEqual) && !getMappedLocation(project, documentSpanLocation(ref))) { + symbolToAddTo.references.push(ref); + } } } - } - }, () => getDefinitionLocation(defaultProject, initialLocation)); + }, + () => getDefinitionLocation(defaultProject, initialLocation) + ); return outputs.filter(o => o.references.length !== 0); } @@ -389,10 +407,10 @@ namespace ts.server { projects: Projects, defaultProject: Project, initialLocation: TLocation, - projectService: ProjectService, cb: CombineProjectOutputCallback, getDefinition: (() => sourcemaps.SourceMappableLocation | undefined) | undefined, ): void { + const projectService = defaultProject.projectService; let toDo: ProjectAndLocation[] | undefined; const seenProjects = createMap(); forEachProjectInProjects(projects, initialLocation && initialLocation.fileName, (project, path) => { @@ -1208,7 +1226,13 @@ namespace ts.server { const position = this.getPositionInFile(args, file); const projects = this.getProjects(args); - const locations = combineProjectOutputForRenameLocations(projects, this.getDefaultProject(args), { fileName: args.file, position }, this.projectService, !!args.findInStrings, !!args.findInComments); + const locations = combineProjectOutputForRenameLocations( + projects, + this.getDefaultProject(args), + { fileName: args.file, position }, + !!args.findInStrings, + !!args.findInComments + ); if (!simplifiedResult) return locations; const defaultProject = this.getDefaultProject(args); @@ -1242,7 +1266,11 @@ namespace ts.server { const file = toNormalizedPath(args.file); const projects = this.getProjects(args); const position = this.getPositionInFile(args, file); - const references = combineProjectOutputForReferences(projects, this.getDefaultProject(args), { fileName: args.file, position }, this.projectService); + const references = combineProjectOutputForReferences( + projects, + this.getDefaultProject(args), + { fileName: args.file, position }, + ); if (simplifiedResult) { const defaultProject = this.getDefaultProject(args); @@ -1749,7 +1777,6 @@ namespace ts.server { return combineProjectOutputWhileOpeningReferencedProjects( this.getProjects(args), this.getDefaultProject(args), - this.projectService, project => project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*fileName*/ undefined, /*excludeDts*/ project.isNonTsProject()), documentSpanLocation, From f97274383d9c31a5dd341d5343dcea425b71729a Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 22 Oct 2018 18:06:19 -0700 Subject: [PATCH 3/6] Log project name when removing project --- src/server/editorServices.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index eb9b7118c5f..83e38625e73 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1037,7 +1037,7 @@ namespace ts.server { } private removeProject(project: Project) { - this.logger.info(`remove project: ${project.getRootFiles().toString()}`); + this.logger.info(`remove project: ${project.projectName}\nFiles::${project.getRootFiles().toString()}`); project.close(); if (Debug.shouldAssert(AssertionLevel.Normal)) { From 41b19309d48584c3131575ea54194bf5656b1915 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 29 Oct 2018 16:32:58 -0700 Subject: [PATCH 4/6] Better project logging --- src/server/editorServices.ts | 27 +++++++++++++-------------- src/server/project.ts | 6 ++++++ 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 83e38625e73..3ea03ff0358 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -1037,7 +1037,8 @@ namespace ts.server { } private removeProject(project: Project) { - this.logger.info(`remove project: ${project.projectName}\nFiles::${project.getRootFiles().toString()}`); + this.logger.info("`remove Project::"); + project.print(); project.close(); if (Debug.shouldAssert(AssertionLevel.Normal)) { @@ -1477,19 +1478,9 @@ namespace ts.server { const writeProjectFileNames = this.logger.hasLevel(LogLevel.verbose); this.logger.startGroup(); - let counter = 0; - const printProjects = (projects: Project[], counter: number): number => { - for (const project of projects) { - this.logger.info(`Project '${project.getProjectName()}' (${ProjectKind[project.projectKind]}) ${counter}`); - this.logger.info(project.filesToString(writeProjectFileNames)); - this.logger.info("-----------------------------------------------"); - counter++; - } - return counter; - }; - counter = printProjects(this.externalProjects, counter); - counter = printProjects(arrayFrom(this.configuredProjects.values()), counter); - printProjects(this.inferredProjects, counter); + let counter = printProjectsWithCounter(this.externalProjects, 0); + counter = printProjectsWithCounter(arrayFrom(this.configuredProjects.values()), counter); + printProjectsWithCounter(this.inferredProjects, counter); this.logger.info("Open files: "); this.openFiles.forEach((projectRootPath, path) => { @@ -2909,4 +2900,12 @@ namespace ts.server { export function isConfigFile(config: ScriptInfoOrConfig): config is TsConfigSourceFile { return (config as TsConfigSourceFile).kind !== undefined; } + + function printProjectsWithCounter(projects: Project[], counter: number) { + for (const project of projects) { + project.print(counter); + counter++; + } + return counter; + } } diff --git a/src/server/project.ts b/src/server/project.ts index 89bac4060bb..7dd8da8a4e0 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -995,6 +995,12 @@ namespace ts.server { return strBuilder; } + print(counter?: number) { + this.writeLog(`Project '${this.projectName}' (${ProjectKind[this.projectKind]}) ${counter === undefined ? "" : counter}`); + this.writeLog(this.filesToString(this.projectService.logger.hasLevel(LogLevel.verbose))); + this.writeLog("-----------------------------------------------"); + } + setCompilerOptions(compilerOptions: CompilerOptions) { if (compilerOptions) { compilerOptions.allowNonTsExtensions = true; From 55fb052d7a6e9070e295219cc69c9d53e45911b4 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 7 Nov 2018 14:55:19 -0800 Subject: [PATCH 5/6] Allow creation of relative path file infos only for open script infos Fixes #20476 --- src/server/editorServices.ts | 19 +++++++-- .../unittests/tsserverProjectSystem.ts | 41 +++++++++++++++---- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 3ea03ff0358..7009619b26d 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -2109,7 +2109,20 @@ namespace ts.server { } private getOrCreateScriptInfoNotOpenedByClientForNormalizedPath(fileName: NormalizedPath, currentDirectory: string, scriptKind: ScriptKind | undefined, hasMixedContent: boolean | undefined, hostToQueryFileExistsOn: DirectoryStructureHost | undefined) { - return this.getOrCreateScriptInfoWorker(fileName, currentDirectory, /*openedByClient*/ false, /*fileContent*/ undefined, scriptKind, hasMixedContent, hostToQueryFileExistsOn); + if (isRootedDiskPath(fileName) || isDynamicFileName(fileName)) { + return this.getOrCreateScriptInfoWorker(fileName, currentDirectory, /*openedByClient*/ false, /*fileContent*/ undefined, scriptKind, hasMixedContent, hostToQueryFileExistsOn); + } + + // This is non rooted path with different current directory than project service current directory + // Only paths recognized are open relative file paths + const info = this.openFilesWithNonRootedDiskPath.get(this.toCanonicalFileName(fileName)); + if (info) { + return info; + } + + // This means triple slash references wont be resolved in dynamic and unsaved files + // which is intentional since we dont know what it means to be relative to non disk files + return undefined; } private getOrCreateScriptInfoOpenedByClientForNormalizedPath(fileName: NormalizedPath, currentDirectory: string, fileContent: string | undefined, scriptKind: ScriptKind | undefined, hasMixedContent: boolean | undefined) { @@ -2126,7 +2139,7 @@ namespace ts.server { let info = this.getScriptInfoForPath(path); if (!info) { const isDynamic = isDynamicFileName(fileName); - Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nScript info with non-dynamic relative file name can only be open script info`); + Debug.assert(isRootedDiskPath(fileName) || isDynamic || openedByClient, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nScript info with non-dynamic relative file name can only be open script info or in context of host currentDirectory`); Debug.assert(!isRootedDiskPath(fileName) || this.currentDirectory === currentDirectory || !this.openFilesWithNonRootedDiskPath.has(this.toCanonicalFileName(fileName)), "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nOpen script files with non rooted disk path opened with current directory context cannot have same canonical names`); Debug.assert(!isDynamic || this.currentDirectory === currentDirectory, "", () => `${JSON.stringify({ fileName, currentDirectory, hostCurrentDirectory: this.currentDirectory, openKeys: arrayFrom(this.openFilesWithNonRootedDiskPath.keys()) })}\nDynamic files must always have current directory context since containing external project name will always match the script info name.`); // If the file is not opened by client and the file doesnot exist on the disk, return @@ -2139,7 +2152,7 @@ namespace ts.server { if (!openedByClient) { this.watchClosedScriptInfo(info); } - else if (!isRootedDiskPath(fileName) && currentDirectory !== this.currentDirectory) { + else if (!isRootedDiskPath(fileName) && !isDynamic) { // File that is opened by user but isn't rooted disk path this.openFilesWithNonRootedDiskPath.set(this.toCanonicalFileName(fileName), info); } diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index 31ca9403215..a113ea14d74 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -3301,7 +3301,7 @@ namespace ts.projectSystem { }); }); - it("dynamic file with reference paths external project", () => { + it("dynamic file with reference paths without external project", () => { const file: File = { path: "^walkThroughSnippet:/Users/UserName/projects/someProject/out/someFile#1.js", content: `/// @@ -3899,18 +3899,30 @@ var x = 10;` describe("when opening new file that doesnt exist on disk yet", () => { function verifyNonExistentFile(useProjectRoot: boolean) { - const host = createServerHost([libFile]); + const folderPath = "/user/someuser/projects/someFolder"; + const fileInRoot: File = { + path: `/src/somefile.d.ts`, + content: "class c { }" + }; + const fileInProjectRoot: File = { + path: `${folderPath}/src/somefile.d.ts`, + content: "class c { }" + }; + const host = createServerHost([libFile, fileInRoot, fileInProjectRoot]); const { hasError, errorLogger } = createErrorLogger(); const session = createSession(host, { canUseEvents: true, logger: errorLogger, useInferredProjectPerProjectRoot: true }); - const folderPath = "/user/someuser/projects/someFolder"; const projectService = session.getProjectService(); const untitledFile = "untitled:Untitled-1"; + const refPathNotFound1 = "../../../../../../typings/@epic/Core.d.ts"; + const refPathNotFound2 = "./src/somefile.d.ts"; + const fileContent = `/// +/// `; session.executeCommandSeq({ command: server.CommandNames.Open, arguments: { file: untitledFile, - fileContent: `/// `, + fileContent, scriptKindName: "TS", projectRootPath: useProjectRoot ? folderPath : undefined } @@ -3918,6 +3930,8 @@ var x = 10;` checkNumberOfProjects(projectService, { inferredProjects: 1 }); const infoForUntitledAtProjectRoot = projectService.getScriptInfoForPath(`${folderPath.toLowerCase()}/${untitledFile.toLowerCase()}` as Path); const infoForUnitiledAtRoot = projectService.getScriptInfoForPath(`/${untitledFile.toLowerCase()}` as Path); + const infoForSomefileAtProjectRoot = projectService.getScriptInfoForPath(`/${folderPath.toLowerCase()}/src/somefile.d.ts` as Path); + const infoForSomefileAtRoot = projectService.getScriptInfoForPath(`${fileInRoot.path.toLowerCase()}` as Path); if (useProjectRoot) { assert.isDefined(infoForUntitledAtProjectRoot); assert.isUndefined(infoForUnitiledAtRoot); @@ -3926,7 +3940,11 @@ var x = 10;` assert.isDefined(infoForUnitiledAtRoot); assert.isUndefined(infoForUntitledAtProjectRoot); } - host.checkTimeoutQueueLength(2); + assert.isUndefined(infoForSomefileAtRoot); + assert.isUndefined(infoForSomefileAtProjectRoot); + + // Since this is not js project so no typings are queued + host.checkTimeoutQueueLength(0); const newTimeoutId = host.getNextTimeoutId(); const expectedSequenceId = session.getNextSeq(); @@ -3937,19 +3955,26 @@ var x = 10;` files: [untitledFile] } }); - host.checkTimeoutQueueLength(3); + host.checkTimeoutQueueLength(1); // Run the last one = get error request host.runQueuedTimeoutCallbacks(newTimeoutId); assert.isFalse(hasError()); - host.checkTimeoutQueueLength(2); + host.checkTimeoutQueueLength(0); checkErrorMessage(session, "syntaxDiag", { file: untitledFile, diagnostics: [] }); session.clearMessages(); host.runQueuedImmediateCallbacks(); assert.isFalse(hasError()); - checkErrorMessage(session, "semanticDiag", { file: untitledFile, diagnostics: [] }); + const errorOffset = fileContent.indexOf(refPathNotFound1) + 1; + checkErrorMessage(session, "semanticDiag", { + file: untitledFile, + diagnostics: [ + createDiagnostic({ line: 1, offset: errorOffset }, { line: 1, offset: errorOffset + refPathNotFound1.length }, Diagnostics.File_0_not_found, [refPathNotFound1], "error"), + createDiagnostic({ line: 2, offset: errorOffset }, { line: 2, offset: errorOffset + refPathNotFound2.length }, Diagnostics.File_0_not_found, [refPathNotFound2.substr(2)], "error") + ] + }); session.clearMessages(); host.runQueuedImmediateCallbacks(1); From 8dd05d5858b100325ab7c2f96afd58922a274436 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 8 Nov 2018 13:01:22 -0800 Subject: [PATCH 6/6] Fix public API --- src/server/project.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/project.ts b/src/server/project.ts index 7dd8da8a4e0..0fddd3835ed 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -995,6 +995,7 @@ namespace ts.server { return strBuilder; } + /*@internal*/ print(counter?: number) { this.writeLog(`Project '${this.projectName}' (${ProjectKind[this.projectKind]}) ${counter === undefined ? "" : counter}`); this.writeLog(this.filesToString(this.projectService.logger.hasLevel(LogLevel.verbose)));