Skip to content

Commit

Permalink
Handle getScriptVersion correctly to ensure program structure is chec…
Browse files Browse the repository at this point in the history
…ked correctly

Fixes #36748
  • Loading branch information
sheetalkamat committed Feb 14, 2020
1 parent 793d398 commit cdb2871
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ namespace ts {
program: Program | undefined,
rootFileNames: string[],
newOptions: CompilerOptions,
getSourceVersion: (path: Path) => string | undefined,
getSourceVersion: (path: Path, fileName: string) => string | undefined,
fileExists: (fileName: string) => boolean,
hasInvalidatedResolution: HasInvalidatedResolution,
hasChangedAutomaticTypeDirectiveNames: boolean,
Expand Down Expand Up @@ -613,7 +613,7 @@ namespace ts {
}

function sourceFileVersionUptoDate(sourceFile: SourceFile) {
return sourceFile.version === getSourceVersion(sourceFile.resolvedPath);
return sourceFile.version === getSourceVersion(sourceFile.resolvedPath, sourceFile.fileName);
}

function projectReferenceUptoDate(oldRef: ProjectReference, newRef: ProjectReference, index: number) {
Expand Down
4 changes: 4 additions & 0 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,10 @@ namespace ts.server {
return notImplemented();
}

clearSourceMapperCache(): never {
return notImplemented();
}

dispose(): void {
throw new Error("dispose is not available through the server layer.");
}
Expand Down
3 changes: 3 additions & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,9 @@ namespace Harness.LanguageService {
getSourceMapper(): never {
return ts.notImplemented();
}
clearSourceMapperCache(): never {
return ts.notImplemented();
}
dispose(): void { this.shim.dispose({}); }
}

Expand Down
8 changes: 5 additions & 3 deletions src/server/editorServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -873,9 +873,11 @@ namespace ts.server {
this.delayEnsureProjectForOpenFiles();
}

private delayUpdateProjectGraphs(projects: readonly Project[]) {
private delayUpdateProjectGraphs(projects: readonly Project[], clearSourceMapperCache?: boolean) {
if (projects.length) {
for (const project of projects) {
// Even if program doesnt change, clear the source mapper cache
if (clearSourceMapperCache) project.clearSourceMapperCache();
this.delayUpdateProjectGraph(project);
}
this.delayEnsureProjectForOpenFiles();
Expand Down Expand Up @@ -1066,7 +1068,7 @@ namespace ts.server {
private delayUpdateProjectsOfScriptInfoPath(path: Path) {
const info = this.getScriptInfoForPath(path);
if (info) {
this.delayUpdateProjectGraphs(info.containingProjects);
this.delayUpdateProjectGraphs(info.containingProjects, /*clearSourceMapperCache*/ true);
}
}

Expand Down Expand Up @@ -2537,7 +2539,7 @@ namespace ts.server {
const declarationInfo = this.getScriptInfoForPath(declarationInfoPath);
if (declarationInfo && declarationInfo.sourceMapFilePath && !isString(declarationInfo.sourceMapFilePath)) {
// Update declaration and source projects
this.delayUpdateProjectGraphs(declarationInfo.containingProjects);
this.delayUpdateProjectGraphs(declarationInfo.containingProjects, /*clearSourceMapperCache*/ true);
this.delayUpdateSourceInfoProjects(declarationInfo.sourceMapFilePath.sourceInfos);
declarationInfo.closeSourceMapFileWatcher();
}
Expand Down
15 changes: 12 additions & 3 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,9 @@ namespace ts.server {
return (info && info.scriptKind)!; // TODO: GH#18217
}

getScriptVersion(filename: string) {
const info = this.getOrCreateScriptInfoAndAttachToProject(filename);
getScriptVersion(fileName: string) {
// Dont attache to the project if version is asked
const info = this.projectService.getOrCreateScriptInfoNotOpenedByClient(fileName, this.currentDirectory, this.directoryStructureHost);
return (info && info.getLatestVersion())!; // TODO: GH#18217
}

Expand Down Expand Up @@ -558,6 +559,11 @@ namespace ts.server {
return this.getLanguageService().getSourceMapper();
}

/** @internal */
clearSourceMapperCache() {
this.languageService.clearSourceMapperCache();
}

/*@internal*/
getDocumentPositionMapper(generatedFileName: string, sourceFileName?: string): DocumentPositionMapper | undefined {
return this.projectService.getDocumentPositionMapper(this, generatedFileName, sourceFileName);
Expand Down Expand Up @@ -1224,7 +1230,10 @@ namespace ts.server {
watcher: this.projectService.watchFactory.watchFile(
this.projectService.host,
generatedFile,
() => this.projectService.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(this),
() => {
this.clearSourceMapperCache();
this.projectService.delayUpdateProjectGraphAndEnsureProjectStructureForOpenFiles(this);
},
PollingInterval.High,
this.projectService.getWatchOptions(this),
WatchType.MissingGeneratedFile,
Expand Down
2 changes: 2 additions & 0 deletions src/server/scriptInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,8 @@ namespace ts.server {
}

getLatestVersion() {
// Ensure we have updated snapshot to give back latest version
this.textStorage.getSnapshot();
return this.textStorage.getVersion();
}

Expand Down
8 changes: 2 additions & 6 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -980,11 +980,6 @@ namespace ts {
return names;
}

public getVersion(path: Path): string {
const file = this.getHostFileInformation(path);
return (file && file.version)!; // TODO: GH#18217
}

public getScriptSnapshot(path: Path): IScriptSnapshot {
const file = this.getHostFileInformation(path);
return (file && file.scriptSnapshot)!; // TODO: GH#18217
Expand Down Expand Up @@ -1228,7 +1223,7 @@ namespace ts {
const projectReferences = hostCache.getProjectReferences();

// If the program is already up-to-date, we can reuse it
if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), path => hostCache!.getVersion(path), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames, projectReferences)) {
if (isProgramUptoDate(program, rootFileNames, hostCache.compilationSettings(), (_path, fileName) => host.getScriptVersion(fileName), fileExists, hasInvalidatedResolution, !!host.hasChangedAutomaticTypeDirectiveNames, projectReferences)) {
return;
}

Expand Down Expand Up @@ -2227,6 +2222,7 @@ namespace ts {
getEditsForRefactor,
toLineColumnOffset: sourceMapper.toLineColumnOffset,
getSourceMapper: () => sourceMapper,
clearSourceMapperCache: () => sourceMapper.clearCache(),
prepareCallHierarchy,
provideCallHierarchyIncomingCalls,
provideCallHierarchyOutgoingCalls
Expand Down
2 changes: 2 additions & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,8 @@ namespace ts {
toLineColumnOffset?(fileName: string, position: number): LineAndCharacter;
/** @internal */
getSourceMapper(): SourceMapper;
/** @internal */
clearSourceMapperCache(): void;

getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: readonly number[], formatOptions: FormatCodeSettings, preferences: UserPreferences): readonly CodeFixAction[];
getCombinedCodeFix(scope: CombinedCodeFixScope, fixId: {}, formatOptions: FormatCodeSettings, preferences: UserPreferences): CombinedCodeActions;
Expand Down
56 changes: 56 additions & 0 deletions src/testRunner/unittests/services/languageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,61 @@ export function Component(x: Config): any;`
}
);
});

describe("detects program upto date correctly", () => {
function verifyProgramUptoDate(useProjectVersion: boolean) {
let projectVersion = "1";
const files = createMap<{ version: string, text: string; }>();
files.set("/project/root.ts", { version: "1", text: `import { foo } from "./other"` });
files.set("/project/other.ts", { version: "1", text: `export function foo() { }` });
files.set("/lib/lib.d.ts", { version: "1", text: projectSystem.libFile.content });
const host: LanguageServiceHost = {
useCaseSensitiveFileNames: returnTrue,
getCompilationSettings: getDefaultCompilerOptions,
fileExists: path => files.has(path),
getProjectVersion: !useProjectVersion ? undefined : () => projectVersion,
getScriptFileNames: () => ["/project/root.ts"],
getScriptVersion: path => files.get(path)?.version || "",
getScriptSnapshot: path => {
const text = files.get(path)?.text;
return text ? ScriptSnapshot.fromString(text) : undefined;
},
getCurrentDirectory: () => "/project",
getDefaultLibFileName: () => "/lib/lib.d.ts"
};
const ls = ts.createLanguageService(host);
const program1 = ls.getProgram()!;
const program2 = ls.getProgram()!;
assert.strictEqual(program1, program2);
verifyProgramFiles(program1);

// Change other
projectVersion = "2";
files.set("/project/other.ts", { version: "2", text: `export function foo() { } export function bar() { }` });
const program3 = ls.getProgram()!;
assert.notStrictEqual(program2, program3);
verifyProgramFiles(program3);

// change root
projectVersion = "3";
files.set("/project/root.ts", { version: "2", text: `import { foo, bar } from "./other"` });
const program4 = ls.getProgram()!;
assert.notStrictEqual(program3, program4);
verifyProgramFiles(program4);

function verifyProgramFiles(program: Program) {
assert.deepEqual(
program.getSourceFiles().map(f => f.fileName),
["/lib/lib.d.ts", "/project/other.ts", "/project/root.ts"]
);
}
}
it("when host implements getProjectVersion", () => {
verifyProgramUptoDate(/*useProjectVersion*/ true);
});
it("when host does not implement getProjectVersion", () => {
verifyProgramUptoDate(/*useProjectVersion*/ false);
});
});
});
}

0 comments on commit cdb2871

Please sign in to comment.