Skip to content

Commit 49a52d0

Browse files
sheetalkamatsheetalkamat
authored
Merge pull request #33636 from microsoft/rootReferenceRedirect
Fix the issue when file is attached to project because its a root file name but program contains instead its d.ts
2 parents c283809 + 8e4f47c commit 49a52d0

File tree

2 files changed

+154
-8
lines changed

2 files changed

+154
-8
lines changed

‎src/server/session.ts

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ namespace ts.server {
434434
const memGetDefinition = memoize(getDefinition);
435435
projectService.forEachEnabledProject(project => {
436436
if (!addToSeen(seenProjects, project.projectName)) return;
437-
const definition = getDefinitionInProject(memGetDefinition(), defaultProject, project);
437+
const definition = mapDefinitionInProject(memGetDefinition(), defaultProject, project);
438438
if (definition) {
439439
toDo = callbackProjectAndLocation<TLocation>({ project, location: definition as TLocation }, projectService, toDo, seenProjects, cb);
440440
}
@@ -446,22 +446,46 @@ namespace ts.server {
446446
}
447447
}
448448

449-
function getDefinitionInProject(definition: DocumentPosition | undefined, definingProject: Project, project: Project): DocumentPosition | undefined {
450-
if (!definition || project.containsFile(toNormalizedPath(definition.fileName))) return definition;
449+
function mapDefinitionInProject(definition: DocumentPosition | undefined, definingProject: Project, project: Project): DocumentPosition | undefined {
450+
// If the definition is actually from the project, definition is correct as is
451+
if (!definition ||
452+
project.containsFile(toNormalizedPath(definition.fileName)) &&
453+
!isLocationProjectReferenceRedirect(project, definition)) {
454+
return definition;
455+
}
451456
const mappedDefinition = definingProject.isSourceOfProjectReferenceRedirect(definition.fileName) ?
452457
definition :
453458
definingProject.getLanguageService().getSourceMapper().tryGetGeneratedPosition(definition);
454459
return mappedDefinition && project.containsFile(toNormalizedPath(mappedDefinition.fileName)) ? mappedDefinition : undefined;
455460
}
456461

462+
function isLocationProjectReferenceRedirect(project: Project, location: DocumentPosition | undefined) {
463+
if (!location) return false;
464+
const program = project.getLanguageService().getProgram();
465+
if (!program) return false;
466+
const sourceFile = program.getSourceFile(location.fileName);
467+
468+
// It is possible that location is attached to project but
469+
// the program actually includes its redirect instead.
470+
// This happens when rootFile in project is one of the file from referenced project
471+
// Thus root is attached but program doesnt have the actual .ts file but .d.ts
472+
// If this is not the file we were actually looking, return rest of the toDo
473+
return !!sourceFile &&
474+
sourceFile.resolvedPath !== sourceFile.path &&
475+
sourceFile.resolvedPath !== project.toPath(location.fileName);
476+
}
477+
457478
function callbackProjectAndLocation<TLocation extends DocumentPosition | undefined>(
458479
projectAndLocation: ProjectAndLocation<TLocation>,
459480
projectService: ProjectService,
460481
toDo: ProjectAndLocation<TLocation>[] | undefined,
461482
seenProjects: Map<true>,
462483
cb: CombineProjectOutputCallback<TLocation>,
463484
): ProjectAndLocation<TLocation>[] | undefined {
464-
if (projectAndLocation.project.getCancellationToken().isCancellationRequested()) return undefined; // Skip rest of toDo if cancelled
485+
const { project, location } = projectAndLocation;
486+
if (project.getCancellationToken().isCancellationRequested()) return undefined; // Skip rest of toDo if cancelled
487+
// If this is not the file we were actually looking, return rest of the toDo
488+
if (isLocationProjectReferenceRedirect(project, location)) return toDo;
465489
cb(projectAndLocation, (project, location) => {
466490
seenProjects.set(projectAndLocation.project.projectName, true);
467491
const originalLocation = projectService.getOriginalLocationEnsuringConfiguredProject(project, location);
@@ -475,8 +499,8 @@ namespace ts.server {
475499
}
476500
const symlinkedProjectsMap = projectService.getSymlinkedProjects(originalScriptInfo);
477501
if (symlinkedProjectsMap) {
478-
symlinkedProjectsMap.forEach((symlinkedProjects) => {
479-
for (const symlinkedProject of symlinkedProjects) addToTodo({ project: symlinkedProject, location: originalLocation as TLocation }, toDo!, seenProjects);
502+
symlinkedProjectsMap.forEach((symlinkedProjects, symlinkedPath) => {
503+
for (const symlinkedProject of symlinkedProjects) addToTodo({ project: symlinkedProject, location: { fileName: symlinkedPath, pos: originalLocation.pos } as TLocation }, toDo!, seenProjects);
480504
});
481505
}
482506
return originalLocation === location ? undefined : originalLocation;

‎src/testRunner/unittests/tsserver/projectReferences.ts

Lines changed: 124 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace ts.projectSystem {
66
// ts build should succeed
77
const solutionBuilder = tscWatch.createSolutionBuilder(host, rootNames, {});
88
solutionBuilder.build();
9-
assert.equal(host.getOutput().length, 0);
9+
assert.equal(host.getOutput().length, 0, JSON.stringify(host.getOutput(), /*replacer*/ undefined, " "));
1010

1111
return host;
1212
}
@@ -1166,7 +1166,7 @@ ${dependencyTs.content}`);
11661166
dtsFileCreated: "main",
11671167
dtsFileDeleted: ["noDts", noDts => ({
11681168
// Map collection after file open
1169-
closedInfos: noDts.closedInfos.concat(dtsMapLocation) ,
1169+
closedInfos: noDts.closedInfos.concat(dtsMapLocation),
11701170
expectsMap: true
11711171
})],
11721172
dependencyChange: ["change", () => ({
@@ -1179,6 +1179,128 @@ ${dependencyTs.content}`);
11791179
});
11801180
});
11811181

1182+
describe("when root file is file from referenced project", () => {
1183+
function verify(disableSourceOfProjectReferenceRedirect: boolean) {
1184+
const projectLocation = `/user/username/projects/project`;
1185+
const commonConfig: File = {
1186+
path: `${projectLocation}/src/common/tsconfig.json`,
1187+
content: JSON.stringify({
1188+
compilerOptions: {
1189+
composite: true,
1190+
declarationMap: true,
1191+
outDir: "../../out",
1192+
baseUrl: "..",
1193+
disableSourceOfProjectReferenceRedirect
1194+
},
1195+
include: ["./**/*"]
1196+
})
1197+
};
1198+
const keyboardTs: File = {
1199+
path: `${projectLocation}/src/common/input/keyboard.ts`,
1200+
content: `function bar() { return "just a random function so .d.ts location doesnt match"; }
1201+
export function evaluateKeyboardEvent() { }`
1202+
};
1203+
const keyboardTestTs: File = {
1204+
path: `${projectLocation}/src/common/input/keyboard.test.ts`,
1205+
content: `import { evaluateKeyboardEvent } from 'common/input/keyboard';
1206+
function testEvaluateKeyboardEvent() {
1207+
return evaluateKeyboardEvent();
1208+
}
1209+
`
1210+
};
1211+
const srcConfig: File = {
1212+
path: `${projectLocation}/src/tsconfig.json`,
1213+
content: JSON.stringify({
1214+
compilerOptions: {
1215+
composite: true,
1216+
declarationMap: true,
1217+
outDir: "../out",
1218+
baseUrl: ".",
1219+
paths: {
1220+
"common/*": ["./common/*"],
1221+
},
1222+
tsBuildInfoFile: "../out/src.tsconfig.tsbuildinfo",
1223+
disableSourceOfProjectReferenceRedirect
1224+
},
1225+
include: ["./**/*"],
1226+
references: [
1227+
{ path: "./common" }
1228+
]
1229+
})
1230+
};
1231+
const terminalTs: File = {
1232+
path: `${projectLocation}/src/terminal.ts`,
1233+
content: `import { evaluateKeyboardEvent } from 'common/input/keyboard';
1234+
function foo() {
1235+
return evaluateKeyboardEvent();
1236+
}
1237+
`
1238+
};
1239+
const host = createHost(
1240+
[commonConfig, keyboardTs, keyboardTestTs, srcConfig, terminalTs, libFile],
1241+
[srcConfig.path]
1242+
);
1243+
const session = createSession(host);
1244+
openFilesForSession([keyboardTs, terminalTs], session);
1245+
1246+
const searchStr = "evaluateKeyboardEvent";
1247+
const importStr = `import { evaluateKeyboardEvent } from 'common/input/keyboard';`;
1248+
const result = session.executeCommandSeq<protocol.ReferencesRequest>({
1249+
command: protocol.CommandTypes.References,
1250+
arguments: protocolFileLocationFromSubstring(keyboardTs, searchStr)
1251+
}).response as protocol.ReferencesResponseBody;
1252+
assert.deepEqual(result, {
1253+
refs: [
1254+
makeReferenceItem({
1255+
file: keyboardTs,
1256+
text: searchStr,
1257+
contextText: `export function evaluateKeyboardEvent() { }`,
1258+
isDefinition: true,
1259+
lineText: `export function evaluateKeyboardEvent() { }`
1260+
}),
1261+
makeReferenceItem({
1262+
file: keyboardTestTs,
1263+
text: searchStr,
1264+
contextText: importStr,
1265+
isDefinition: true,
1266+
lineText: importStr
1267+
}),
1268+
makeReferenceItem({
1269+
file: keyboardTestTs,
1270+
text: searchStr,
1271+
options: { index: 1 },
1272+
isDefinition: false,
1273+
lineText: ` return evaluateKeyboardEvent();`
1274+
}),
1275+
makeReferenceItem({
1276+
file: terminalTs,
1277+
text: searchStr,
1278+
contextText: importStr,
1279+
isDefinition: true,
1280+
lineText: importStr
1281+
}),
1282+
makeReferenceItem({
1283+
file: terminalTs,
1284+
text: searchStr,
1285+
options: { index: 1 },
1286+
isDefinition: false,
1287+
lineText: ` return evaluateKeyboardEvent();`
1288+
}),
1289+
],
1290+
symbolName: searchStr,
1291+
symbolStartOffset: protocolLocationFromSubstring(keyboardTs.content, searchStr).offset,
1292+
symbolDisplayString: "function evaluateKeyboardEvent(): void"
1293+
});
1294+
}
1295+
1296+
it(`when using declaration file maps to navigate between projects`, () => {
1297+
verify(/*disableSourceOfProjectReferenceRedirect*/ true);
1298+
});
1299+
it(`when using original source files in the project`, () => {
1300+
verify(/*disableSourceOfProjectReferenceRedirect*/ false);
1301+
});
1302+
});
1303+
11821304
it("reusing d.ts files from composite and non composite projects", () => {
11831305
const projectLocation = "/user/username/projects/myproject";
11841306
const configA: File = {

0 commit comments

Comments
 (0)