Skip to content

Handle getScriptVersion correctly to ensure program structure is checked correctly #36808

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

Merged
merged 6 commits into from
Feb 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
12 changes: 7 additions & 5 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 @@ -1033,7 +1035,7 @@ namespace ts.server {
// file has been changed which might affect the set of referenced files in projects that include
// this file and set of inferred projects
info.delayReloadNonMixedContentFile();
this.delayUpdateProjectGraphs(info.containingProjects);
this.delayUpdateProjectGraphs(info.containingProjects, /*clearSourceMapperCache*/ false);
this.handleSourceMapProjects(info);
}
}
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 All @@ -1082,7 +1084,7 @@ namespace ts.server {
info.detachAllProjects();

// update projects to make sure that set of referenced files is correct
this.delayUpdateProjectGraphs(containingProjects);
this.delayUpdateProjectGraphs(containingProjects, /*clearSourceMapperCache*/ false);
this.handleSourceMapProjects(info);
info.closeSourceMapFileWatcher();
// need to recalculate source map from declaration file
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
14 changes: 12 additions & 2 deletions src/server/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,9 @@ namespace ts.server {
}

getScriptVersion(filename: string) {
const info = this.getOrCreateScriptInfoAndAttachToProject(filename);
// Don't attach 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 +560,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 +1231,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);
});
});
});
}
Loading