Skip to content

Commit 0736459

Browse files
authored
Handle consistent file name during editor and tsc --watch scenarios (microsoft#34622)
* Add isInferredProject, isConfiguredProject and isExternalProject * Add test for rename on file with case change that fails Test for microsoft#25460 * Dont store fileName on text storage * Store root file names in the root file map to reflect their name * Delay open file triggering watches * Correct the name of source file as we query it (eg. it could be same source file returned in old program with different casing on case insensitive file name) * More tests * Refactoring * Cache bind And check diagnostics and always get program diagnostics from the program * Another test * Try to report conflicting file error on file instead of global diagnostics * Create better tests for module resolution diagnostics check * Fix lint errors
1 parent 50603ed commit 0736459

18 files changed

+653
-178
lines changed

src/compiler/builder.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace ts {
2020

2121
export interface ReusableBuilderProgramState extends ReusableBuilderState {
2222
/**
23-
* Cache of semantic diagnostics for files with their Path being the key
23+
* Cache of bind and check diagnostics for files with their Path being the key
2424
*/
2525
semanticDiagnosticsPerFile?: ReadonlyMap<readonly ReusableDiagnostic[] | readonly Diagnostic[]> | undefined;
2626
/**
@@ -85,7 +85,7 @@ namespace ts {
8585
// TODO: GH#18217 Properties of this interface are frequently asserted to be defined.
8686
export interface BuilderProgramState extends BuilderState {
8787
/**
88-
* Cache of semantic diagnostics for files with their Path being the key
88+
* Cache of bind and check diagnostics for files with their Path being the key
8989
*/
9090
semanticDiagnosticsPerFile: Map<readonly Diagnostic[]> | undefined;
9191
/**
@@ -647,21 +647,32 @@ namespace ts {
647647
}
648648

649649
/**
650-
* Gets the semantic diagnostics either from cache if present, or otherwise from program and caches it
651-
* Note that it is assumed that the when asked about semantic diagnostics, the file has been taken out of affected files/changed file set
650+
* Gets semantic diagnostics for the file which are
651+
* bindAndCheckDiagnostics (from cache) and program diagnostics
652652
*/
653653
function getSemanticDiagnosticsOfFile(state: BuilderProgramState, sourceFile: SourceFile, cancellationToken?: CancellationToken): readonly Diagnostic[] {
654+
return concatenate(
655+
getBinderAndCheckerDiagnosticsOfFile(state, sourceFile, cancellationToken),
656+
Debug.assertDefined(state.program).getProgramDiagnostics(sourceFile)
657+
);
658+
}
659+
660+
/**
661+
* Gets the binder and checker diagnostics either from cache if present, or otherwise from program and caches it
662+
* Note that it is assumed that when asked about binder and checker diagnostics, the file has been taken out of affected files/changed file set
663+
*/
664+
function getBinderAndCheckerDiagnosticsOfFile(state: BuilderProgramState, sourceFile: SourceFile, cancellationToken?: CancellationToken): readonly Diagnostic[] {
654665
const path = sourceFile.path;
655666
if (state.semanticDiagnosticsPerFile) {
656667
const cachedDiagnostics = state.semanticDiagnosticsPerFile.get(path);
657-
// Report the semantic diagnostics from the cache if we already have those diagnostics present
668+
// Report the bind and check diagnostics from the cache if we already have those diagnostics present
658669
if (cachedDiagnostics) {
659670
return cachedDiagnostics;
660671
}
661672
}
662673

663674
// Diagnostics werent cached, get them from program, and cache the result
664-
const diagnostics = Debug.assertDefined(state.program).getSemanticDiagnostics(sourceFile, cancellationToken);
675+
const diagnostics = Debug.assertDefined(state.program).getBindAndCheckDiagnostics(sourceFile, cancellationToken);
665676
if (state.semanticDiagnosticsPerFile) {
666677
state.semanticDiagnosticsPerFile.set(path, diagnostics);
667678
}

src/compiler/diagnosticMessages.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,10 @@
855855
"category": "Error",
856856
"code": 1260
857857
},
858+
"Already included file name '{0}' differs from file name '{1}' only in casing.": {
859+
"category": "Error",
860+
"code": 1261
861+
},
858862
"'with' statements are not allowed in an async function block.": {
859863
"category": "Error",
860864
"code": 1300
@@ -4064,7 +4068,7 @@
40644068
"category": "Message",
40654069
"code": 6201
40664070
},
4067-
"Project references may not form a circular graph. Cycle detected: {0}": {
4071+
"Project references may not form a circular graph. Cycle detected: {0}": {
40684072
"category": "Error",
40694073
"code": 6202
40704074
},

src/compiler/program.ts

Lines changed: 142 additions & 84 deletions
Large diffs are not rendered by default.

src/compiler/types.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3082,6 +3082,7 @@ namespace ts {
30823082

30833083
/*@internal*/
30843084
export interface RefFile {
3085+
referencedFileName: string;
30853086
kind: RefFileKind;
30863087
index: number;
30873088
file: Path;
@@ -3132,6 +3133,9 @@ namespace ts {
31323133
getConfigFileParsingDiagnostics(): readonly Diagnostic[];
31333134
/* @internal */ getSuggestionDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): readonly DiagnosticWithLocation[];
31343135

3136+
/* @internal */ getBindAndCheckDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): readonly Diagnostic[];
3137+
/* @internal */ getProgramDiagnostics(sourceFile: SourceFile, cancellationToken?: CancellationToken): readonly Diagnostic[];
3138+
31353139
/**
31363140
* Gets a type checker that can be used to semantically analyze source files in the program.
31373141
*/

src/compiler/watchPublic.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,8 +585,8 @@ namespace ts {
585585

586586
function reloadFileNamesFromConfigFile() {
587587
writeLog("Reloading new file names and options");
588-
const result = getFileNamesFromConfigSpecs(configFileSpecs, getDirectoryPath(configFileName), compilerOptions, parseConfigFileHost);
589-
if (updateErrorForNoInputFiles(result, configFileName, configFileSpecs, configFileParsingDiagnostics!, canConfigFileJsonReportNoInputFiles)) {
588+
const result = getFileNamesFromConfigSpecs(configFileSpecs, getNormalizedAbsolutePath(getDirectoryPath(configFileName), currentDirectory), compilerOptions, parseConfigFileHost);
589+
if (updateErrorForNoInputFiles(result, getNormalizedAbsolutePath(configFileName, currentDirectory), configFileSpecs, configFileParsingDiagnostics!, canConfigFileJsonReportNoInputFiles)) {
590590
hasChangedConfigFileParsingErrors = true;
591591
}
592592
rootFileNames = result.fileNames;
@@ -713,4 +713,4 @@ namespace ts {
713713
);
714714
}
715715
}
716-
}
716+
}

src/harness/virtualFileSystemWithWatch.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,26 @@ interface Array<T> { length: number; [n: number]: T; }`
518518
}
519519
}
520520

521+
renameFile(fileName: string, newFileName: string) {
522+
const fullPath = getNormalizedAbsolutePath(fileName, this.currentDirectory);
523+
const path = this.toPath(fullPath);
524+
const file = this.fs.get(path) as FsFile;
525+
Debug.assert(!!file);
526+
527+
// Only remove the file
528+
this.removeFileOrFolder(file, returnFalse, /*isRenaming*/ true);
529+
530+
// Add updated folder with new folder name
531+
const newFullPath = getNormalizedAbsolutePath(newFileName, this.currentDirectory);
532+
const newFile = this.toFsFile({ path: newFullPath, content: file.content });
533+
const newPath = newFile.path;
534+
const basePath = getDirectoryPath(path);
535+
Debug.assert(basePath !== path);
536+
Debug.assert(basePath === getDirectoryPath(newPath));
537+
const baseFolder = this.fs.get(basePath) as FsFolder;
538+
this.addFileOrFolderInFolder(baseFolder, newFile);
539+
}
540+
521541
renameFolder(folderName: string, newFolderName: string) {
522542
const fullPath = getNormalizedAbsolutePath(folderName, this.currentDirectory);
523543
const path = this.toPath(fullPath);

src/server/editorServices.ts

Lines changed: 54 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,8 @@ namespace ts.server {
407407
}
408408

409409
function setProjectOptionsUsed(project: ConfiguredProject | ExternalProject) {
410-
if (project.projectKind === ProjectKind.Configured) {
411-
(project as ConfiguredProject).projectOptions = true;
410+
if (isConfiguredProject(project)) {
411+
project.projectOptions = true;
412412
}
413413
}
414414

@@ -1009,6 +1009,9 @@ namespace ts.server {
10091009

10101010
// don't trigger callback on open, existing files
10111011
if (project.fileIsOpen(fileOrDirectoryPath)) {
1012+
if (project.pendingReload !== ConfigFileProgramReloadLevel.Full) {
1013+
project.openFileWatchTriggered.set(fileOrDirectoryPath, true);
1014+
}
10121015
return;
10131016
}
10141017

@@ -1208,16 +1211,26 @@ namespace ts.server {
12081211
// collect all projects that should be removed
12091212
let ensureProjectsForOpenFiles = false;
12101213
for (const p of info.containingProjects) {
1211-
if (p.projectKind === ProjectKind.Configured) {
1214+
if (isConfiguredProject(p)) {
12121215
if (info.hasMixedContent) {
12131216
info.registerFileUpdate();
12141217
}
12151218
// Do not remove the project so that we can reuse this project
12161219
// if it would need to be re-created with next file open
1220+
1221+
// If project had open file affecting
1222+
// Reload the root Files from config if its not already scheduled
1223+
if (p.openFileWatchTriggered.has(info.path)) {
1224+
p.openFileWatchTriggered.delete(info.path);
1225+
if (!p.pendingReload) {
1226+
p.pendingReload = ConfigFileProgramReloadLevel.Partial;
1227+
p.markFileAsDirty(info.path);
1228+
}
1229+
}
12171230
}
1218-
else if (p.projectKind === ProjectKind.Inferred && p.isRoot(info)) {
1231+
else if (isInferredProject(p) && p.isRoot(info)) {
12191232
// If this was the last open root file of inferred project
1220-
if ((p as InferredProject).isProjectWithSingleRoot()) {
1233+
if (p.isProjectWithSingleRoot()) {
12211234
ensureProjectsForOpenFiles = true;
12221235
}
12231236

@@ -1679,7 +1692,7 @@ namespace ts.server {
16791692
return;
16801693
}
16811694

1682-
const projectOptions = project.projectKind === ProjectKind.Configured ? (project as ConfiguredProject).projectOptions as ProjectOptions : undefined;
1695+
const projectOptions = isConfiguredProject(project) ? project.projectOptions as ProjectOptions : undefined;
16831696
setProjectOptionsUsed(project);
16841697
const data: ProjectInfoTelemetryEventData = {
16851698
projectId: this.host.createSHA256Hash(project.projectName),
@@ -1699,7 +1712,7 @@ namespace ts.server {
16991712
this.eventHandler({ eventName: ProjectInfoTelemetryEvent, data });
17001713

17011714
function configFileName(): ProjectInfoTelemetryEventData["configFileName"] {
1702-
if (!(project instanceof ConfiguredProject)) {
1715+
if (!isConfiguredProject(project)) {
17031716
return "other";
17041717
}
17051718

@@ -1832,49 +1845,64 @@ namespace ts.server {
18321845

18331846
private updateNonInferredProjectFiles<T>(project: ExternalProject | ConfiguredProject, files: T[], propertyReader: FilePropertyReader<T>) {
18341847
const projectRootFilesMap = project.getRootFilesMap();
1835-
const newRootScriptInfoMap = createMap<ProjectRoot>();
1848+
const newRootScriptInfoMap = createMap<true>();
18361849

18371850
for (const f of files) {
18381851
const newRootFile = propertyReader.getFileName(f);
1839-
const normalizedPath = toNormalizedPath(newRootFile);
1840-
const isDynamic = isDynamicFileName(normalizedPath);
1841-
let scriptInfo: ScriptInfo | NormalizedPath;
1852+
const fileName = toNormalizedPath(newRootFile);
1853+
const isDynamic = isDynamicFileName(fileName);
18421854
let path: Path;
18431855
// Use the project's fileExists so that it can use caching instead of reaching to disk for the query
18441856
if (!isDynamic && !project.fileExistsWithCache(newRootFile)) {
1845-
path = normalizedPathToPath(normalizedPath, this.currentDirectory, this.toCanonicalFileName);
1846-
const existingValue = projectRootFilesMap.get(path)!;
1847-
if (isScriptInfo(existingValue)) {
1848-
project.removeFile(existingValue, /*fileExists*/ false, /*detachFromProject*/ true);
1857+
path = normalizedPathToPath(fileName, this.currentDirectory, this.toCanonicalFileName);
1858+
const existingValue = projectRootFilesMap.get(path);
1859+
if (existingValue) {
1860+
if (existingValue.info) {
1861+
project.removeFile(existingValue.info, /*fileExists*/ false, /*detachFromProject*/ true);
1862+
existingValue.info = undefined;
1863+
}
1864+
existingValue.fileName = fileName;
1865+
}
1866+
else {
1867+
projectRootFilesMap.set(path, { fileName });
18491868
}
1850-
projectRootFilesMap.set(path, normalizedPath);
1851-
scriptInfo = normalizedPath;
18521869
}
18531870
else {
18541871
const scriptKind = propertyReader.getScriptKind(f, this.hostConfiguration.extraFileExtensions);
18551872
const hasMixedContent = propertyReader.hasMixedContent(f, this.hostConfiguration.extraFileExtensions);
1856-
scriptInfo = this.getOrCreateScriptInfoNotOpenedByClientForNormalizedPath(normalizedPath, project.currentDirectory, scriptKind, hasMixedContent, project.directoryStructureHost)!; // TODO: GH#18217
1873+
const scriptInfo = Debug.assertDefined(this.getOrCreateScriptInfoNotOpenedByClientForNormalizedPath(
1874+
fileName,
1875+
project.currentDirectory,
1876+
scriptKind,
1877+
hasMixedContent,
1878+
project.directoryStructureHost
1879+
));
18571880
path = scriptInfo.path;
1881+
const existingValue = projectRootFilesMap.get(path);
18581882
// If this script info is not already a root add it
1859-
if (!project.isRoot(scriptInfo)) {
1860-
project.addRoot(scriptInfo);
1883+
if (!existingValue || existingValue.info !== scriptInfo) {
1884+
project.addRoot(scriptInfo, fileName);
18611885
if (scriptInfo.isScriptOpen()) {
18621886
// if file is already root in some inferred project
18631887
// - remove the file from that project and delete the project if necessary
18641888
this.removeRootOfInferredProjectIfNowPartOfOtherProject(scriptInfo);
18651889
}
18661890
}
1891+
else {
1892+
// Already root update the fileName
1893+
existingValue.fileName = fileName;
1894+
}
18671895
}
1868-
newRootScriptInfoMap.set(path, scriptInfo);
1896+
newRootScriptInfoMap.set(path, true);
18691897
}
18701898

18711899
// project's root file map size is always going to be same or larger than new roots map
18721900
// as we have already all the new files to the project
18731901
if (projectRootFilesMap.size > newRootScriptInfoMap.size) {
18741902
projectRootFilesMap.forEach((value, path) => {
18751903
if (!newRootScriptInfoMap.has(path)) {
1876-
if (isScriptInfo(value)) {
1877-
project.removeFile(value, project.fileExistsWithCache(path), /*detachFromProject*/ true);
1904+
if (value.info) {
1905+
project.removeFile(value.info, project.fileExistsWithCache(path), /*detachFromProject*/ true);
18781906
}
18791907
else {
18801908
projectRootFilesMap.delete(path);
@@ -2562,7 +2590,7 @@ namespace ts.server {
25622590
const firstProject = info.containingProjects[0];
25632591

25642592
if (!firstProject.isOrphan() &&
2565-
firstProject.projectKind === ProjectKind.Inferred &&
2593+
isInferredProject(firstProject) &&
25662594
firstProject.isRoot(info) &&
25672595
forEach(info.containingProjects, p => p !== firstProject && !p.isOrphan())) {
25682596
firstProject.removeFile(info, /*fileExists*/ true, /*detachFromProject*/ true);
@@ -2633,8 +2661,8 @@ namespace ts.server {
26332661

26342662
// Add configured projects as referenced
26352663
originalScriptInfo.containingProjects.forEach(project => {
2636-
if (project.projectKind === ProjectKind.Configured) {
2637-
addOriginalConfiguredProject(project as ConfiguredProject);
2664+
if (isConfiguredProject(project)) {
2665+
addOriginalConfiguredProject(project);
26382666
}
26392667
});
26402668
return originalLocation;

0 commit comments

Comments
 (0)