From 1c24e27ce49bd4a905bda91d91f16efe97970bd1 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 26 Sep 2019 15:25:30 -0700 Subject: [PATCH 1/2] Add the test that fails with find all references --- .../unittests/tsserver/projectReferences.ts | 126 +++++++++++++++++- 1 file changed, 124 insertions(+), 2 deletions(-) diff --git a/src/testRunner/unittests/tsserver/projectReferences.ts b/src/testRunner/unittests/tsserver/projectReferences.ts index 2e886e2169e..e4716ac5f2e 100644 --- a/src/testRunner/unittests/tsserver/projectReferences.ts +++ b/src/testRunner/unittests/tsserver/projectReferences.ts @@ -6,7 +6,7 @@ namespace ts.projectSystem { // ts build should succeed const solutionBuilder = tscWatch.createSolutionBuilder(host, rootNames, {}); solutionBuilder.build(); - assert.equal(host.getOutput().length, 0); + assert.equal(host.getOutput().length, 0, JSON.stringify(host.getOutput(), /*replacer*/ undefined, " ")); return host; } @@ -1166,7 +1166,7 @@ ${dependencyTs.content}`); dtsFileCreated: "main", dtsFileDeleted: ["noDts", noDts => ({ // Map collection after file open - closedInfos: noDts.closedInfos.concat(dtsMapLocation) , + closedInfos: noDts.closedInfos.concat(dtsMapLocation), expectsMap: true })], dependencyChange: ["change", () => ({ @@ -1179,6 +1179,128 @@ ${dependencyTs.content}`); }); }); + describe("when root file is file from referenced project", () => { + function verify(disableSourceOfProjectReferenceRedirect: boolean) { + const projectLocation = `/user/username/projects/project`; + const commonConfig: File = { + path: `${projectLocation}/src/common/tsconfig.json`, + content: JSON.stringify({ + compilerOptions: { + composite: true, + declarationMap: true, + outDir: "../../out", + baseUrl: "..", + disableSourceOfProjectReferenceRedirect + }, + include: ["./**/*"] + }) + }; + const keyboardTs: File = { + path: `${projectLocation}/src/common/input/keyboard.ts`, + content: `function bar() { return "just a random function so .d.ts location doesnt match"; } +export function evaluateKeyboardEvent() { }` + }; + const keyboardTestTs: File = { + path: `${projectLocation}/src/common/input/keyboard.test.ts`, + content: `import { evaluateKeyboardEvent } from 'common/input/keyboard'; +function testEvaluateKeyboardEvent() { + return evaluateKeyboardEvent(); +} +` + }; + const srcConfig: File = { + path: `${projectLocation}/src/tsconfig.json`, + content: JSON.stringify({ + compilerOptions: { + composite: true, + declarationMap: true, + outDir: "../out", + baseUrl: ".", + paths: { + "common/*": ["./common/*"], + }, + tsBuildInfoFile: "../out/src.tsconfig.tsbuildinfo", + disableSourceOfProjectReferenceRedirect + }, + include: ["./**/*"], + references: [ + { path: "./common" } + ] + }) + }; + const terminalTs: File = { + path: `${projectLocation}/src/terminal.ts`, + content: `import { evaluateKeyboardEvent } from 'common/input/keyboard'; +function foo() { + return evaluateKeyboardEvent(); +} +` + }; + const host = createHost( + [commonConfig, keyboardTs, keyboardTestTs, srcConfig, terminalTs, libFile], + [srcConfig.path] + ); + const session = createSession(host); + openFilesForSession([keyboardTs, terminalTs], session); + + const searchStr = "evaluateKeyboardEvent"; + const importStr = `import { evaluateKeyboardEvent } from 'common/input/keyboard';`; + const result = session.executeCommandSeq({ + command: protocol.CommandTypes.References, + arguments: protocolFileLocationFromSubstring(keyboardTs, searchStr) + }).response as protocol.ReferencesResponseBody; + assert.deepEqual(result, { + refs: [ + makeReferenceItem({ + file: keyboardTs, + text: searchStr, + contextText: `export function evaluateKeyboardEvent() { }`, + isDefinition: true, + lineText: `export function evaluateKeyboardEvent() { }` + }), + makeReferenceItem({ + file: keyboardTestTs, + text: searchStr, + contextText: importStr, + isDefinition: true, + lineText: importStr + }), + makeReferenceItem({ + file: keyboardTestTs, + text: searchStr, + options: { index: 1 }, + isDefinition: false, + lineText: ` return evaluateKeyboardEvent();` + }), + makeReferenceItem({ + file: terminalTs, + text: searchStr, + contextText: importStr, + isDefinition: true, + lineText: importStr + }), + makeReferenceItem({ + file: terminalTs, + text: searchStr, + options: { index: 1 }, + isDefinition: false, + lineText: ` return evaluateKeyboardEvent();` + }), + ], + symbolName: searchStr, + symbolStartOffset: protocolLocationFromSubstring(keyboardTs.content, searchStr).offset, + symbolDisplayString: "function evaluateKeyboardEvent(): void" + }); + } + + it(`when using declaration file maps to navigate between projects`, () => { + verify(/*disableSourceOfProjectReferenceRedirect*/ true); + }); + it(`when using original source files in the project`, () => { + verify(/*disableSourceOfProjectReferenceRedirect*/ false); + }); + }); + it("reusing d.ts files from composite and non composite projects", () => { const projectLocation = "/user/username/projects/myproject"; const configA: File = { From 8e4f47c8a9b8ac64e667841ba58f502e4593d73e Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 27 Sep 2019 08:58:42 -0700 Subject: [PATCH 2/2] Fix the issue when file is attached to project because its a root file name but program contains instead its d.ts Fixes #33323 --- src/server/session.ts | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 6fe392c722b..e2ca230e942 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -434,7 +434,7 @@ namespace ts.server { const memGetDefinition = memoize(getDefinition); projectService.forEachEnabledProject(project => { if (!addToSeen(seenProjects, project.projectName)) return; - const definition = getDefinitionInProject(memGetDefinition(), defaultProject, project); + const definition = mapDefinitionInProject(memGetDefinition(), defaultProject, project); if (definition) { toDo = callbackProjectAndLocation({ project, location: definition as TLocation }, projectService, toDo, seenProjects, cb); } @@ -446,14 +446,35 @@ namespace ts.server { } } - function getDefinitionInProject(definition: DocumentPosition | undefined, definingProject: Project, project: Project): DocumentPosition | undefined { - if (!definition || project.containsFile(toNormalizedPath(definition.fileName))) return definition; + function mapDefinitionInProject(definition: DocumentPosition | undefined, definingProject: Project, project: Project): DocumentPosition | undefined { + // If the definition is actually from the project, definition is correct as is + if (!definition || + project.containsFile(toNormalizedPath(definition.fileName)) && + !isLocationProjectReferenceRedirect(project, definition)) { + return definition; + } const mappedDefinition = definingProject.isSourceOfProjectReferenceRedirect(definition.fileName) ? definition : definingProject.getLanguageService().getSourceMapper().tryGetGeneratedPosition(definition); return mappedDefinition && project.containsFile(toNormalizedPath(mappedDefinition.fileName)) ? mappedDefinition : undefined; } + function isLocationProjectReferenceRedirect(project: Project, location: DocumentPosition | undefined) { + if (!location) return false; + const program = project.getLanguageService().getProgram(); + if (!program) return false; + const sourceFile = program.getSourceFile(location.fileName); + + // It is possible that location is attached to project but + // the program actually includes its redirect instead. + // This happens when rootFile in project is one of the file from referenced project + // Thus root is attached but program doesnt have the actual .ts file but .d.ts + // If this is not the file we were actually looking, return rest of the toDo + return !!sourceFile && + sourceFile.resolvedPath !== sourceFile.path && + sourceFile.resolvedPath !== project.toPath(location.fileName); + } + function callbackProjectAndLocation( projectAndLocation: ProjectAndLocation, projectService: ProjectService, @@ -461,7 +482,10 @@ namespace ts.server { seenProjects: Map, cb: CombineProjectOutputCallback, ): ProjectAndLocation[] | undefined { - if (projectAndLocation.project.getCancellationToken().isCancellationRequested()) return undefined; // Skip rest of toDo if cancelled + const { project, location } = projectAndLocation; + if (project.getCancellationToken().isCancellationRequested()) return undefined; // Skip rest of toDo if cancelled + // If this is not the file we were actually looking, return rest of the toDo + if (isLocationProjectReferenceRedirect(project, location)) return toDo; cb(projectAndLocation, (project, location) => { seenProjects.set(projectAndLocation.project.projectName, true); const originalLocation = projectService.getOriginalLocationEnsuringConfiguredProject(project, location); @@ -475,8 +499,8 @@ namespace ts.server { } const symlinkedProjectsMap = projectService.getSymlinkedProjects(originalScriptInfo); if (symlinkedProjectsMap) { - symlinkedProjectsMap.forEach((symlinkedProjects) => { - for (const symlinkedProject of symlinkedProjects) addToTodo({ project: symlinkedProject, location: originalLocation as TLocation }, toDo!, seenProjects); + symlinkedProjectsMap.forEach((symlinkedProjects, symlinkedPath) => { + for (const symlinkedProject of symlinkedProjects) addToTodo({ project: symlinkedProject, location: { fileName: symlinkedPath, pos: originalLocation.pos } as TLocation }, toDo!, seenProjects); }); } return originalLocation === location ? undefined : originalLocation;