Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Mar 13, 2020
Merged

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0716898
Add test where file from referenced project of solution belongs to in…
sheetalkamat Mar 4, 2020
47946ff
Try to find project from project references if the default config pro…
sheetalkamat Mar 4, 2020
8b9c1f9
Add test to verify the correct collection of projects
sheetalkamat Mar 4, 2020
1e41cd8
Handle when default config project is indirectly referenced in the so…
sheetalkamat Mar 4, 2020
2e7ae2b
Include public API tests in unittests
sheetalkamat Mar 4, 2020
57bad74
Make sure default project for script info is calculated correctly
sheetalkamat Mar 4, 2020
01bfca9
Handle reload projects
sheetalkamat Mar 5, 2020
f64ffe9
Ensure to load solution project tree when project is referenced by so…
sheetalkamat Mar 5, 2020
914f358
Find all refs when the file is referenced via d.ts
sheetalkamat Mar 5, 2020
728ca69
Some comments per feedback
sheetalkamat Mar 10, 2020
62ef1ef
Fix typo
sheetalkamat Mar 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 125 additions & 27 deletions src/server/editorServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ namespace ts.server {
}

interface AssignProjectResult extends OpenConfiguredProjectResult {
defaultConfigProject: ConfiguredProject | undefined;
retainProjects: readonly ConfiguredProject[] | ConfiguredProject | undefined;
}

interface FilePropertyReader<T> {
Expand Down Expand Up @@ -424,7 +424,48 @@ namespace ts.server {
return !!(infoOrFileNameOrConfig as AncestorConfigFileInfo).configFileInfo;
}

function forEachResolvedProjectReference<T>(
/*@internal*/
export enum ProjectReferenceProjectLoadKind {
Find,
FindCreate,
FindCreateLoad
}

/*@internal*/
export function forEachResolvedProjectReferenceProject<T>(
project: ConfiguredProject,
cb: (child: ConfiguredProject, configFileName: NormalizedPath) => T | undefined,
projectReferenceProjectLoadKind: ProjectReferenceProjectLoadKind.Find | ProjectReferenceProjectLoadKind.FindCreate
): T | undefined;
/*@internal*/
export function forEachResolvedProjectReferenceProject<T>(
project: ConfiguredProject,
cb: (child: ConfiguredProject, configFileName: NormalizedPath) => T | undefined,
projectReferenceProjectLoadKind: ProjectReferenceProjectLoadKind.FindCreateLoad,
reason: string
): T | undefined;
export function forEachResolvedProjectReferenceProject<T>(
project: ConfiguredProject,
cb: (child: ConfiguredProject, configFileName: NormalizedPath) => T | undefined,
projectReferenceProjectLoadKind: ProjectReferenceProjectLoadKind,
reason?: string
): T | undefined {
return forEachResolvedProjectReference(project, ref => {
if (!ref) return undefined;
const configFileName = toNormalizedPath(ref.sourceFile.fileName);
const child = project.projectService.findConfiguredProjectByProjectName(configFileName) || (
projectReferenceProjectLoadKind === ProjectReferenceProjectLoadKind.FindCreate ?
project.projectService.createConfiguredProject(configFileName) :
projectReferenceProjectLoadKind === ProjectReferenceProjectLoadKind.FindCreateLoad ?
project.projectService.createAndLoadConfiguredProject(configFileName, reason!) :
undefined
);
return child && cb(child, configFileName);
});
}

/*@internal*/
export function forEachResolvedProjectReference<T>(
project: ConfiguredProject,
cb: (resolvedProjectReference: ResolvedProjectReference | undefined, resolvedProjectReferencePath: Path) => T | undefined
): T | undefined {
Expand Down Expand Up @@ -486,6 +527,12 @@ namespace ts.server {
return !info.isScriptOpen() && info.mTime !== undefined;
}

/*@internal*/
export function projectContainsInfoDirectly(project: Project, info: ScriptInfo) {
return project.containsScriptInfo(info) &&
!project.isSourceOfProjectReferenceRedirect(info.path);
}

/*@internal*/
export function updateProjectIfDirty(project: Project) {
return project.dirty && project.updateGraph();
Expand Down Expand Up @@ -1686,8 +1733,12 @@ namespace ts.server {
findDefaultConfiguredProject(info: ScriptInfo) {
if (!info.isScriptOpen()) return undefined;
const configFileName = this.getConfigFileNameForFile(info);
return configFileName &&
const project = configFileName &&
this.findConfiguredProjectByProjectName(configFileName);

return project?.isSolution() ?
project.getDefaultChildProjectFromSolution(info) :
project;
}

/**
Expand Down Expand Up @@ -1737,7 +1788,8 @@ namespace ts.server {
this.logger.endGroup();
}

private findConfiguredProjectByProjectName(configFileName: NormalizedPath): ConfiguredProject | undefined {
/*@internal*/
findConfiguredProjectByProjectName(configFileName: NormalizedPath): ConfiguredProject | undefined {
// make sure that casing of config file name is consistent
const canonicalConfigFilePath = asNormalizedPath(this.toCanonicalFileName(configFileName));
return this.getConfiguredProjectByCanonicalConfigFilePath(canonicalConfigFilePath);
Expand Down Expand Up @@ -1871,7 +1923,8 @@ namespace ts.server {
project.setTypeAcquisition(typeAcquisition);
}

private createConfiguredProject(configFileName: NormalizedPath) {
/* @internal */
createConfiguredProject(configFileName: NormalizedPath) {
const cachedDirectoryStructureHost = createCachedDirectoryStructureHost(this.host, this.host.getCurrentDirectory(), this.host.useCaseSensitiveFileNames)!; // TODO: GH#18217
this.logger.info(`Opened configuration file ${configFileName}`);
const project = new ConfiguredProject(
Expand All @@ -1895,7 +1948,7 @@ namespace ts.server {
}

/* @internal */
private createAndLoadConfiguredProject(configFileName: NormalizedPath, reason: string) {
createAndLoadConfiguredProject(configFileName: NormalizedPath, reason: string) {
const project = this.createConfiguredProject(configFileName);
this.loadConfiguredProject(project, reason);
return project;
Expand Down Expand Up @@ -2713,7 +2766,8 @@ namespace ts.server {
const configFileName = this.getConfigFileNameForFile(info);
if (configFileName) {
const project = this.findConfiguredProjectByProjectName(configFileName) || this.createConfiguredProject(configFileName);
if (!updatedProjects.has(configFileName)) {
if (!updatedProjects.has(project.canonicalConfigFilePath)) {
updatedProjects.set(project.canonicalConfigFilePath, true);
if (delayReload) {
project.pendingReload = ConfigFileProgramReloadLevel.Full;
project.pendingReloadReason = reason;
Expand All @@ -2722,8 +2776,20 @@ namespace ts.server {
else {
// reload from the disk
this.reloadConfiguredProject(project, reason);
if (!project.containsScriptInfo(info) && project.isSolution()) {
forEachResolvedProjectReferenceProject(
project,
child => {
if (!updatedProjects.has(child.canonicalConfigFilePath)) {
updatedProjects.set(child.canonicalConfigFilePath, true);
this.reloadConfiguredProject(child, reason);
}
return projectContainsInfoDirectly(child, info);
},
ProjectReferenceProjectLoadKind.FindCreate
);
}
}
updatedProjects.set(configFileName, true);
}
}
});
Expand Down Expand Up @@ -2860,6 +2926,7 @@ namespace ts.server {
let configFileErrors: readonly Diagnostic[] | undefined;
let project: ConfiguredProject | ExternalProject | undefined = this.findExternalProjectContainingOpenScriptInfo(info);
let defaultConfigProject: ConfiguredProject | undefined;
let retainProjects: ConfiguredProject[] | ConfiguredProject | undefined;
if (!project && !this.syntaxOnly) { // Checking syntaxOnly is an optimization
configFileName = this.getConfigFileNameForFile(info);
if (configFileName) {
Expand All @@ -2880,9 +2947,41 @@ namespace ts.server {
// Ensure project is ready to check if it contains opened script info
updateProjectIfDirty(project);
}

defaultConfigProject = project;
// Create ancestor configured project
this.createAncestorProjects(info, defaultConfigProject);
retainProjects = defaultConfigProject;

// If this configured project doesnt contain script info but
// it is solution with project references, try those project references
if (!project.containsScriptInfo(info) && project.isSolution()) {
forEachResolvedProjectReferenceProject(
project,
(child, childConfigFileName) => {
updateProjectIfDirty(child);
// Retain these projects
if (!isArray(retainProjects)) {
retainProjects = [project as ConfiguredProject, child];
}
else {
retainProjects.push(child);
}

// If script info belongs to this child project, use this as default config project
if (projectContainsInfoDirectly(child, info)) {
configFileName = childConfigFileName;
configFileErrors = child.getAllProjectErrors();
this.sendConfigFileDiagEvent(child, info.fileName);
return child;
}
},
ProjectReferenceProjectLoadKind.FindCreateLoad,
`Creating project referenced in solution ${project.projectName} to find possible configured project for ${info.fileName} to open`
);
}
else {
// Create ancestor configured project
this.createAncestorProjects(info, defaultConfigProject || project);
}
}
}

Expand All @@ -2902,7 +3001,7 @@ namespace ts.server {
this.assignOrphanScriptInfoToInferredProject(info, this.openFiles.get(info.path));
}
Debug.assert(!info.isOrphan());
return { configFileName, configFileErrors, defaultConfigProject };
return { configFileName, configFileErrors, retainProjects };
}

private createAncestorProjects(info: ScriptInfo, project: ConfiguredProject) {
Expand Down Expand Up @@ -2953,7 +3052,10 @@ namespace ts.server {
if (forEachPotentialProjectReference(
project,
potentialRefPath => forProjects!.has(potentialRefPath)
)) {
) || (project.isSolution() && forEachResolvedProjectReference(
project,
(_ref, resolvedPath) => forProjects!.has(resolvedPath)
))) {
// Load children
this.ensureProjectChildren(project, seenProjects);
}
Expand All @@ -2966,19 +3068,15 @@ namespace ts.server {
updateProjectIfDirty(project);

// Create tree because project is uptodate we only care of resolved references
forEachResolvedProjectReference(
forEachResolvedProjectReferenceProject(
project,
ref => {
if (!ref) return;
const configFileName = toNormalizedPath(ref.sourceFile.fileName);
const child = this.findConfiguredProjectByProjectName(configFileName) ||
this.createAndLoadConfiguredProject(configFileName, `Creating project for reference of project: ${project.projectName}`);
this.ensureProjectChildren(child, seenProjects);
}
child => this.ensureProjectChildren(child, seenProjects),
ProjectReferenceProjectLoadKind.FindCreateLoad,
`Creating project for reference of project: ${project.projectName}`
);
}

private cleanupAfterOpeningFile(toRetainConfigProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
private cleanupAfterOpeningFile(toRetainConfigProjects: readonly ConfiguredProject[] | ConfiguredProject | undefined) {
// This was postponed from closeOpenFile to after opening next file,
// so that we can reuse the project if we need to right away
this.removeOrphanConfiguredProjects(toRetainConfigProjects);
Expand All @@ -3000,14 +3098,14 @@ namespace ts.server {

openClientFileWithNormalizedPath(fileName: NormalizedPath, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean, projectRootPath?: NormalizedPath): OpenConfiguredProjectResult {
const info = this.getOrCreateOpenScriptInfo(fileName, fileContent, scriptKind, hasMixedContent, projectRootPath);
const { defaultConfigProject, ...result } = this.assignProjectToOpenedScriptInfo(info);
this.cleanupAfterOpeningFile(defaultConfigProject);
const { retainProjects, ...result } = this.assignProjectToOpenedScriptInfo(info);
this.cleanupAfterOpeningFile(retainProjects);
this.telemetryOnOpenFile(info);
this.printProjects();
return result;
}

private removeOrphanConfiguredProjects(toRetainConfiguredProjects: ConfiguredProject[] | ConfiguredProject | undefined) {
private removeOrphanConfiguredProjects(toRetainConfiguredProjects: readonly ConfiguredProject[] | ConfiguredProject | undefined) {
const toRemoveConfiguredProjects = cloneMap(this.configuredProjects);
const markOriginalProjectsAsUsed = (project: Project) => {
if (!project.isOrphan() && project.originalConfiguredProjects) {
Expand Down Expand Up @@ -3206,9 +3304,9 @@ namespace ts.server {
}

// All the script infos now exist, so ok to go update projects for open files
let defaultConfigProjects: ConfiguredProject[] | undefined;
let retainProjects: readonly ConfiguredProject[] | undefined;
if (openScriptInfos) {
defaultConfigProjects = mapDefined(openScriptInfos, info => this.assignProjectToOpenedScriptInfo(info).defaultConfigProject);
retainProjects = flatMap(openScriptInfos, info => this.assignProjectToOpenedScriptInfo(info).retainProjects);
}

// While closing files there could be open files that needed assigning new inferred projects, do it now
Expand All @@ -3218,7 +3316,7 @@ namespace ts.server {

if (openScriptInfos) {
// Cleanup projects
this.cleanupAfterOpeningFile(defaultConfigProjects);
this.cleanupAfterOpeningFile(retainProjects);
// Telemetry
openScriptInfos.forEach(info => this.telemetryOnOpenFile(info));
this.printProjects();
Expand Down
24 changes: 23 additions & 1 deletion src/server/project.ts
View file
Open in desktop
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) {
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
8 changes: 4 additions & 4 deletions src/testRunner/unittests/publicApi.ts
View file
Open in desktop
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
Loading