Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle when default project for file is solution with file actually referenced by one of the project references #37239

Merged
merged 11 commits into from
Mar 13, 2020
Merged
171 changes: 142 additions & 29 deletions src/server/editorServices.ts

Large diffs are not rendered by default.

24 changes: 23 additions & 1 deletion src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2164,6 +2164,24 @@ namespace ts.server {
this.externalProjectRefCount--;
}

/* @internal */
isSolution() {
return this.getRootFilesMap().size === 0 &&
!this.canConfigFileJsonReportNoInputFiles;
}

/* @internal */
getDefaultChildProjectFromSolution(info: ScriptInfo) {
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
Debug.assert(this.isSolution());
return forEachResolvedProjectReferenceProject(
this,
child => projectContainsInfoDirectly(child, info) ?
child :
undefined,
ProjectReferenceProjectLoadKind.Find
);
}

/** Returns true if the project is needed by any of the open script info/external project */
/* @internal */
hasOpenRef() {
Expand All @@ -2184,12 +2202,16 @@ namespace ts.server {
return !!configFileExistenceInfo.openFilesImpactedByConfigFile.size;
}

const isSolution = this.isSolution();

// If there is no pending update for this project,
// We know exact set of open files that get impacted by this configured project as the files in the project
// The project is referenced only if open files impacted by this project are present in this project
return forEachEntry(
configFileExistenceInfo.openFilesImpactedByConfigFile,
(_value, infoPath) => this.containsScriptInfo(this.projectService.getScriptInfoForPath(infoPath as Path)!)
(_value, infoPath) => isSolution ?
!!this.getDefaultChildProjectFromSolution(this.projectService.getScriptInfoForPath(infoPath as Path)!) :
this.containsScriptInfo(this.projectService.getScriptInfoForPath(infoPath as Path)!)
) || false;
}

Expand Down
26 changes: 18 additions & 8 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,16 @@ namespace ts.server {
if (initialLocation) {
const defaultDefinition = getDefinitionLocation(defaultProject, initialLocation!);
if (defaultDefinition) {
const getGeneratedDefinition = memoize(() => defaultProject.isSourceOfProjectReferenceRedirect(defaultDefinition.fileName) ?
defaultDefinition :
defaultProject.getLanguageService().getSourceMapper().tryGetGeneratedPosition(defaultDefinition));
const getSourceDefinition = memoize(() => defaultProject.isSourceOfProjectReferenceRedirect(defaultDefinition.fileName) ?
defaultDefinition :
defaultProject.getLanguageService().getSourceMapper().tryGetSourcePosition(defaultDefinition));
projectService.loadAncestorProjectTree(seenProjects);
projectService.forEachEnabledProject(project => {
if (!addToSeen(seenProjects, project)) return;
const definition = mapDefinitionInProject(defaultDefinition, defaultProject, project);
const definition = mapDefinitionInProject(defaultDefinition, project, getGeneratedDefinition, getSourceDefinition);
if (definition) {
toDo = callbackProjectAndLocation<TLocation>({ project, location: definition as TLocation }, projectService, toDo, seenProjects, cb);
}
Expand All @@ -447,17 +453,21 @@ namespace ts.server {
}
}

function mapDefinitionInProject(definition: DocumentPosition | undefined, definingProject: Project, project: Project): DocumentPosition | undefined {
function mapDefinitionInProject(
definition: DocumentPosition,
project: Project,
getGeneratedDefinition: () => DocumentPosition | undefined,
getSourceDefinition: () => DocumentPosition | undefined
): DocumentPosition | undefined {
// If the definition is actually from the project, definition is correct as is
if (!definition ||
project.containsFile(toNormalizedPath(definition.fileName)) &&
if (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;
const generatedDefinition = getGeneratedDefinition();
if (generatedDefinition && project.containsFile(toNormalizedPath(generatedDefinition.fileName))) return generatedDefinition;
const sourceDefinition = getSourceDefinition();
return sourceDefinition && project.containsFile(toNormalizedPath(sourceDefinition.fileName)) ? sourceDefinition : undefined;
}

function isLocationProjectReferenceRedirect(project: Project, location: DocumentPosition | undefined) {
Expand Down
8 changes: 4 additions & 4 deletions src/testRunner/unittests/publicApi.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
describe("Public APIs", () => {
describe("unittests:: Public APIs", () => {
function verifyApi(fileName: string) {
const builtFile = `built/local/${fileName}`;
const api = `api/${fileName}`;
Expand Down Expand Up @@ -32,7 +32,7 @@ describe("Public APIs", () => {
});
});

describe("Public APIs:: token to string", () => {
describe("unittests:: Public APIs:: token to string", () => {
function assertDefinedTokenToString(initial: ts.SyntaxKind, last: ts.SyntaxKind) {
for (let t = initial; t <= last; t++) {
assert.isDefined(ts.tokenToString(t), `Expected tokenToString defined for ${ts.Debug.formatSyntaxKind(t)}`);
Expand All @@ -47,13 +47,13 @@ describe("Public APIs:: token to string", () => {
});
});

describe("Public APIs:: createPrivateIdentifier", () => {
describe("unittests:: Public APIs:: createPrivateIdentifier", () => {
it("throws when name doesn't start with #", () => {
assert.throw(() => ts.createPrivateIdentifier("not"), "Debug Failure. First character of private identifier must be #: not");
});
});

describe("Public APIs:: isPropertyName", () => {
describe("unittests:: Public APIs:: isPropertyName", () => {
it("checks if a PrivateIdentifier is a valid property name", () => {
const prop = ts.createPrivateIdentifier("#foo");
assert.isTrue(ts.isPropertyName(prop), "PrivateIdentifier must be a valid property name.");
Expand Down
63 changes: 63 additions & 0 deletions src/testRunner/unittests/tsserver/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,69 @@ namespace ts.projectSystem {
assert.isFalse(event.event.endsWith("Diag"), JSON.stringify(event));
}
}
export function projectLoadingStartEvent(projectName: string, reason: string, seq?: number): protocol.ProjectLoadingStartEvent {
return {
seq: seq || 0,
type: "event",
event: server.ProjectLoadingStartEvent,
body: { projectName, reason }
};
}

export function projectLoadingFinishEvent(projectName: string, seq?: number): protocol.ProjectLoadingFinishEvent {
return {
seq: seq || 0,
type: "event",
event: server.ProjectLoadingFinishEvent,
body: { projectName }
};
}

export function projectInfoTelemetryEvent(seq?: number): protocol.TelemetryEvent {
return telemetryEvent(server.ProjectInfoTelemetryEvent, "", seq);
}

function telemetryEvent(telemetryEventName: string, payload: any, seq?: number): protocol.TelemetryEvent {
return {
seq: seq || 0,
type: "event",
event: "telemetry",
body: {
telemetryEventName,
payload
}
};
}

export function configFileDiagEvent(triggerFile: string, configFile: string, diagnostics: protocol.DiagnosticWithFileName[], seq?: number): protocol.ConfigFileDiagnosticEvent {
return {
seq: seq || 0,
type: "event",
event: server.ConfigFileDiagEvent,
body: {
triggerFile,
configFile,
diagnostics
}
};
}

export function checkEvents(session: TestSession, expectedEvents: protocol.Event[]) {
const events = session.events;
assert.equal(events.length, expectedEvents.length, `Actual:: ${JSON.stringify(session.events, /*replacer*/ undefined, " ")}`);
expectedEvents.forEach((expectedEvent, index) => {
if (expectedEvent.event === "telemetry") {
// Ignore payload
const { body, ...actual } = events[index] as protocol.TelemetryEvent;
const { body: expectedBody, ...expected } = expectedEvent as protocol.TelemetryEvent;
assert.deepEqual(actual, expected, `Expected ${JSON.stringify(expectedEvent)} at ${index} in ${JSON.stringify(events)}`);
assert.equal(body.telemetryEventName, expectedBody.telemetryEventName, `Expected ${JSON.stringify(expectedEvent)} at ${index} in ${JSON.stringify(events)}`);
}
else {
checkNthEvent(session, expectedEvent, index, index === expectedEvents.length);
}
});
}

export function checkNthEvent(session: TestSession, expectedEvent: protocol.Event, index: number, isMostRecent: boolean) {
const events = session.events;
Expand Down
Loading