From 2685d409d577549f12d83420919cc442ecce13c2 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 9 Jul 2015 14:40:33 -0700 Subject: [PATCH] addressed PR feedback --- src/compiler/program.ts | 10 +- src/compiler/types.ts | 2 + src/compiler/utilities.ts | 2 +- .../cases/unittests/reuseProgramStructure.ts | 502 +++++++++--------- 4 files changed, 263 insertions(+), 253 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 27154a6610a..13c49989347 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -164,6 +164,8 @@ namespace ts { let filesByName = createFileMap(fileName => host.getCanonicalFileName(fileName)); if (oldProgram) { + // check properties that can affect structure of the program or module resolution strategy + // if any of these properties has changed - structure cannot be reused let oldOptions = oldProgram.getCompilerOptions(); if ((oldOptions.module !== options.module) || (oldOptions.noResolve !== options.noResolve) || @@ -246,7 +248,13 @@ namespace ts { return false; } - if (oldSourceFile !== newSourceFile) { + if (oldSourceFile !== newSourceFile) { + if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { + // value of no-default-lib has changed + // this will affect if default library is injected into the list of files + return false; + } + // check tripleslash references if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { // tripleslash references has changed diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 79ac4e85dd8..0fbbf0053ff 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1244,6 +1244,8 @@ namespace ts { /* @internal */ getIdentifierCount(): number; /* @internal */ getSymbolCount(): number; /* @internal */ getTypeCount(): number; + + // For testing purposes only. /* @internal */ structureIsReused?: boolean; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 150101b641b..bb5112c5808 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -102,7 +102,7 @@ namespace ts { } export function getResolvedModuleFileName(sourceFile: SourceFile, moduleNameText: string): string { - return hasResolvedModuleName(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText]: undefined; + return hasResolvedModuleName(sourceFile, moduleNameText) ? sourceFile.resolvedModules[moduleNameText] : undefined; } export function setResolvedModuleName(sourceFile: SourceFile, moduleNameText: string, resolvedFileName: string): void { diff --git a/tests/cases/unittests/reuseProgramStructure.ts b/tests/cases/unittests/reuseProgramStructure.ts index 996f6bb316b..1cb389f7717 100644 --- a/tests/cases/unittests/reuseProgramStructure.ts +++ b/tests/cases/unittests/reuseProgramStructure.ts @@ -3,259 +3,259 @@ /// module ts { - - const enum ChangedPart { - references = 1 << 0, - importsAndExports = 1 << 1, - program = 1 << 2 - } - - let newLine = "\r\n"; - - interface SourceFileWithText extends SourceFile { - sourceText?: SourceText; - } - interface NamedSourceText { - name: string; - text: SourceText - } - - interface ProgramWithSourceTexts extends Program { - sourceTexts?: NamedSourceText[]; - } - - class SourceText implements IScriptSnapshot { - private fullText: string; - - constructor(private references: string, - private importsAndExports: string, - private program: string, - private changedPart: ChangedPart = 0, - private version = 0) { - } - - static New(references: string, importsAndExports: string, program: string): SourceText { - Debug.assert(references !== undefined); - Debug.assert(importsAndExports !== undefined); - Debug.assert(program !== undefined); - return new SourceText(references + newLine, importsAndExports + newLine, program || ""); - } - - public getVersion(): number { - return this.version; - } - - public updateReferences(newReferences: string): SourceText { - Debug.assert(newReferences !== undefined); - return new SourceText(newReferences + newLine, this.importsAndExports, this.program, this.changedPart | ChangedPart.references, this.version + 1); - } - public updateImportsAndExports(newImportsAndExports: string): SourceText { - Debug.assert(newImportsAndExports !== undefined); - return new SourceText(this.references, newImportsAndExports + newLine, this.program, this.changedPart | ChangedPart.importsAndExports, this.version + 1); - } - public updateProgram(newProgram: string): SourceText { - Debug.assert(newProgram !== undefined); - return new SourceText(this.references, this.importsAndExports, newProgram, this.changedPart | ChangedPart.program, this.version + 1); - } - - public getFullText() { - return this.fullText || (this.fullText = this.references + this.importsAndExports + this.program); - } - - public getText(start: number, end: number): string { - return this.getFullText().substring(start, end); - } - - getLength(): number { - return this.getFullText().length; - } - - getChangeRange(oldSnapshot: IScriptSnapshot): TextChangeRange { - var oldText = oldSnapshot; - var oldSpan: TextSpan; - var newLength: number; - switch(oldText.changedPart ^ this.changedPart){ - case ChangedPart.references: - oldSpan = createTextSpan(0, oldText.references.length); - newLength = this.references.length; - break; - case ChangedPart.importsAndExports: - oldSpan = createTextSpan(oldText.references.length, oldText.importsAndExports.length); - newLength = this.importsAndExports.length - break; - case ChangedPart.program: - oldSpan = createTextSpan(oldText.references.length + oldText.importsAndExports.length, oldText.program.length); - newLength = this.program.length; - break; - default: - Debug.assert(false, "Unexpected change"); - } - - return createTextChangeRange(oldSpan, newLength); - } - } - - function createTestCompilerHost(texts: NamedSourceText[], target: ScriptTarget): CompilerHost { - let files: Map = {}; - for (let t of texts) { - let file = createSourceFile(t.name, t.text.getFullText(), target); - file.sourceText = t.text; - files[t.name] = file; - } - return { - getSourceFile(fileName): SourceFile { - return files[fileName]; - }, - getDefaultLibFileName(): string { - return "lib.d.ts" - }, - writeFile(file, text) { - throw new Error("NYI"); - }, - getCurrentDirectory(): string { - return ""; - }, - getCanonicalFileName(fileName): string { - return sys.useCaseSensitiveFileNames ? fileName: fileName.toLowerCase(); - }, - useCaseSensitiveFileNames(): boolean { - return sys.useCaseSensitiveFileNames; - }, - getNewLine() : string { - return sys.newLine; - }, - hasChanges(oldFile: SourceFileWithText): boolean { - let current = files[oldFile.fileName]; - return !current || oldFile.sourceText.getVersion() !== current.sourceText.getVersion(); - } - - } - } - - function newProgram(texts: NamedSourceText[], rootNames: string[], options: CompilerOptions): Program { - var host = createTestCompilerHost(texts, options.target); - let program = createProgram(rootNames, options, host); - program.sourceTexts = texts; - return program; - } - - function updateProgram(oldProgram: Program, rootNames: string[], options: CompilerOptions, updater: (files: NamedSourceText[]) => void) { - var texts: NamedSourceText[] = (oldProgram).sourceTexts.slice(0); - updater(texts); - var host = createTestCompilerHost(texts, options.target); - var program = createProgram(rootNames, options, host, oldProgram); - program.sourceTexts = texts; - return program; - } - - function getSizeOfMap(map: Map): number { - let size = 0; - for (let id in map) { - if (hasProperty(map, id)) { - size++; - } - } - return size; - } - - function checkResolvedModulesCache(program: Program, fileName: string, expectedContent: Map): void { - let file = program.getSourceFile(fileName); - assert.isTrue(file !==undefined, `cannot find file ${fileName}`); - if (expectedContent === undefined) { - assert.isTrue(file.resolvedModules === undefined, "expected resolvedModules to be undefined"); - } - else { - assert.isTrue(file.resolvedModules !== undefined, "expected resolvedModuled to be set"); - let actualCacheSize = getSizeOfMap(file.resolvedModules); - let expectedSize = getSizeOfMap(expectedContent); - assert.isTrue(actualCacheSize === expectedSize, `expected actual size: ${actualCacheSize} to be equal to ${expectedSize}`); - - for (let id in expectedContent) { - if (hasProperty(expectedContent, id)) { - assert.isTrue(hasProperty(file.resolvedModules, id), `expected ${id} to be found in resolved modules`); - assert.isTrue(expectedContent[id] === file.resolvedModules[id], `expected '${expectedContent[id]}' to be equal to '${file.resolvedModules[id]}'`); - } - } - } - } - - describe("Reuse program structure", () => { - let target = ScriptTarget.Latest; - let files = [ - {name: "a.ts", text: SourceText.New(`/// `, "", `var x = 1`)}, - {name: "b.ts", text: SourceText.New(`/// `, "", `var y = 2`)}, - {name: "c.ts", text: SourceText.New("", "", `var z = 1;`)}, - ] - - it("successful if change does not affect imports", () => { - var program_1 = newProgram(files, ["a.ts"], {target}); - var program_2 = updateProgram(program_1, ["a.ts"], {target}, files => { - files[0].text = files[0].text.updateProgram("var x = 100"); - }); - assert.isTrue(program_1.structureIsReused); - }); - - it("fails if change affects tripleslash references", () => { - var program_1 = newProgram(files, ["a.ts"], {target}); - var program_2 = updateProgram(program_1, ["a.ts"], {target}, files => { - let newReferences = `/// - /// - `; - files[0].text = files[0].text.updateReferences(newReferences); - }); - assert.isTrue(!program_1.structureIsReused); - }); + const enum ChangedPart { + references = 1 << 0, + importsAndExports = 1 << 1, + program = 1 << 2 + } - it("fails if change affects imports", () => { - var program_1 = newProgram(files, ["a.ts"], {target}); - var program_2 = updateProgram(program_1, ["a.ts"], {target}, files => { - files[2].text = files[2].text.updateImportsAndExports("import x from 'b'"); - }); - assert.isTrue(!program_1.structureIsReused); - }); + let newLine = "\r\n"; - it("fails if module kind changes", () => { - var program_1 = newProgram(files, ["a.ts"], {target, module: ModuleKind.CommonJS}); - var program_2 = updateProgram(program_1, ["a.ts"], {target, module: ModuleKind.AMD}, files => void 0); - assert.isTrue(!program_1.structureIsReused); - }); - - it("resolution cache follows imports", () => { - let files = [ - { name: "a.ts", text: SourceText.New("", "import {_} from 'b'", "var x = 1") }, - { name: "b.ts", text: SourceText.New("", "", "var y = 2") }, - ]; - var options: CompilerOptions = {target}; - - var program_1 = newProgram(files, ["a.ts"], options); - checkResolvedModulesCache(program_1, "a.ts", { "b": "b.ts" }); - checkResolvedModulesCache(program_1, "b.ts", undefined); - - var program_2 = updateProgram(program_1, ["a.ts"], options, files => { - files[0].text = files[0].text.updateProgram("var x = 2"); - }); - assert.isTrue(program_1.structureIsReused); - - // content of resolution cache should not change - checkResolvedModulesCache(program_1, "a.ts", { "b": "b.ts" }); - checkResolvedModulesCache(program_1, "b.ts", undefined); - - // imports has changed - program is not reused - var program_3 = updateProgram(program_2, ["a.ts"], options, files => { - files[0].text = files[0].text.updateImportsAndExports(""); - }); - assert.isTrue(!program_2.structureIsReused); - checkResolvedModulesCache(program_3, "a.ts", undefined); + interface SourceFileWithText extends SourceFile { + sourceText?: SourceText; + } - var program_4 = updateProgram(program_3, ["a.ts"], options, files => { - let newImports = `import x from 'b' - import y from 'c' - `; - files[0].text = files[0].text.updateImportsAndExports(newImports); - }); - assert.isTrue(!program_3.structureIsReused); - checkResolvedModulesCache(program_4, "a.ts", {"b": "b.ts", "c": undefined}); - }); - }) + interface NamedSourceText { + name: string; + text: SourceText + } + + interface ProgramWithSourceTexts extends Program { + sourceTexts?: NamedSourceText[]; + } + + class SourceText implements IScriptSnapshot { + private fullText: string; + + constructor(private references: string, + private importsAndExports: string, + private program: string, + private changedPart: ChangedPart = 0, + private version = 0) { + } + + static New(references: string, importsAndExports: string, program: string): SourceText { + Debug.assert(references !== undefined); + Debug.assert(importsAndExports !== undefined); + Debug.assert(program !== undefined); + return new SourceText(references + newLine, importsAndExports + newLine, program || ""); + } + + public getVersion(): number { + return this.version; + } + + public updateReferences(newReferences: string): SourceText { + Debug.assert(newReferences !== undefined); + return new SourceText(newReferences + newLine, this.importsAndExports, this.program, this.changedPart | ChangedPart.references, this.version + 1); + } + public updateImportsAndExports(newImportsAndExports: string): SourceText { + Debug.assert(newImportsAndExports !== undefined); + return new SourceText(this.references, newImportsAndExports + newLine, this.program, this.changedPart | ChangedPart.importsAndExports, this.version + 1); + } + public updateProgram(newProgram: string): SourceText { + Debug.assert(newProgram !== undefined); + return new SourceText(this.references, this.importsAndExports, newProgram, this.changedPart | ChangedPart.program, this.version + 1); + } + + public getFullText() { + return this.fullText || (this.fullText = this.references + this.importsAndExports + this.program); + } + + public getText(start: number, end: number): string { + return this.getFullText().substring(start, end); + } + + getLength(): number { + return this.getFullText().length; + } + + getChangeRange(oldSnapshot: IScriptSnapshot): TextChangeRange { + var oldText = oldSnapshot; + var oldSpan: TextSpan; + var newLength: number; + switch (oldText.changedPart ^ this.changedPart) { + case ChangedPart.references: + oldSpan = createTextSpan(0, oldText.references.length); + newLength = this.references.length; + break; + case ChangedPart.importsAndExports: + oldSpan = createTextSpan(oldText.references.length, oldText.importsAndExports.length); + newLength = this.importsAndExports.length + break; + case ChangedPart.program: + oldSpan = createTextSpan(oldText.references.length + oldText.importsAndExports.length, oldText.program.length); + newLength = this.program.length; + break; + default: + Debug.assert(false, "Unexpected change"); + } + + return createTextChangeRange(oldSpan, newLength); + } + } + + function createTestCompilerHost(texts: NamedSourceText[], target: ScriptTarget): CompilerHost { + let files: Map = {}; + for (let t of texts) { + let file = createSourceFile(t.name, t.text.getFullText(), target); + file.sourceText = t.text; + files[t.name] = file; + } + return { + getSourceFile(fileName): SourceFile { + return files[fileName]; + }, + getDefaultLibFileName(): string { + return "lib.d.ts" + }, + writeFile(file, text) { + throw new Error("NYI"); + }, + getCurrentDirectory(): string { + return ""; + }, + getCanonicalFileName(fileName): string { + return sys.useCaseSensitiveFileNames ? fileName : fileName.toLowerCase(); + }, + useCaseSensitiveFileNames(): boolean { + return sys.useCaseSensitiveFileNames; + }, + getNewLine(): string { + return sys.newLine; + }, + hasChanges(oldFile: SourceFileWithText): boolean { + let current = files[oldFile.fileName]; + return !current || oldFile.sourceText.getVersion() !== current.sourceText.getVersion(); + } + + } + } + + function newProgram(texts: NamedSourceText[], rootNames: string[], options: CompilerOptions): Program { + var host = createTestCompilerHost(texts, options.target); + let program = createProgram(rootNames, options, host); + program.sourceTexts = texts; + return program; + } + + function updateProgram(oldProgram: Program, rootNames: string[], options: CompilerOptions, updater: (files: NamedSourceText[]) => void) { + var texts: NamedSourceText[] = (oldProgram).sourceTexts.slice(0); + updater(texts); + var host = createTestCompilerHost(texts, options.target); + var program = createProgram(rootNames, options, host, oldProgram); + program.sourceTexts = texts; + return program; + } + + function getSizeOfMap(map: Map): number { + let size = 0; + for (let id in map) { + if (hasProperty(map, id)) { + size++; + } + } + return size; + } + + function checkResolvedModulesCache(program: Program, fileName: string, expectedContent: Map): void { + let file = program.getSourceFile(fileName); + assert.isTrue(file !== undefined, `cannot find file ${fileName}`); + if (expectedContent === undefined) { + assert.isTrue(file.resolvedModules === undefined, "expected resolvedModules to be undefined"); + } + else { + assert.isTrue(file.resolvedModules !== undefined, "expected resolvedModuled to be set"); + let actualCacheSize = getSizeOfMap(file.resolvedModules); + let expectedSize = getSizeOfMap(expectedContent); + assert.isTrue(actualCacheSize === expectedSize, `expected actual size: ${actualCacheSize} to be equal to ${expectedSize}`); + + for (let id in expectedContent) { + if (hasProperty(expectedContent, id)) { + assert.isTrue(hasProperty(file.resolvedModules, id), `expected ${id} to be found in resolved modules`); + assert.isTrue(expectedContent[id] === file.resolvedModules[id], `expected '${expectedContent[id]}' to be equal to '${file.resolvedModules[id]}'`); + } + } + } + } + + describe("Reuse program structure", () => { + let target = ScriptTarget.Latest; + let files = [ + { name: "a.ts", text: SourceText.New(`/// `, "", `var x = 1`) }, + { name: "b.ts", text: SourceText.New(`/// `, "", `var y = 2`) }, + { name: "c.ts", text: SourceText.New("", "", `var z = 1;`) }, + ] + + it("successful if change does not affect imports", () => { + var program_1 = newProgram(files, ["a.ts"], { target }); + var program_2 = updateProgram(program_1, ["a.ts"], { target }, files => { + files[0].text = files[0].text.updateProgram("var x = 100"); + }); + assert.isTrue(program_1.structureIsReused); + }); + + it("fails if change affects tripleslash references", () => { + var program_1 = newProgram(files, ["a.ts"], { target }); + var program_2 = updateProgram(program_1, ["a.ts"], { target }, files => { + let newReferences = `/// + /// + `; + files[0].text = files[0].text.updateReferences(newReferences); + }); + assert.isTrue(!program_1.structureIsReused); + }); + + it("fails if change affects imports", () => { + var program_1 = newProgram(files, ["a.ts"], { target }); + var program_2 = updateProgram(program_1, ["a.ts"], { target }, files => { + files[2].text = files[2].text.updateImportsAndExports("import x from 'b'"); + }); + assert.isTrue(!program_1.structureIsReused); + }); + + it("fails if module kind changes", () => { + var program_1 = newProgram(files, ["a.ts"], { target, module: ModuleKind.CommonJS }); + var program_2 = updateProgram(program_1, ["a.ts"], { target, module: ModuleKind.AMD }, files => void 0); + assert.isTrue(!program_1.structureIsReused); + }); + + it("resolution cache follows imports", () => { + let files = [ + { name: "a.ts", text: SourceText.New("", "import {_} from 'b'", "var x = 1") }, + { name: "b.ts", text: SourceText.New("", "", "var y = 2") }, + ]; + var options: CompilerOptions = { target }; + + var program_1 = newProgram(files, ["a.ts"], options); + checkResolvedModulesCache(program_1, "a.ts", { "b": "b.ts" }); + checkResolvedModulesCache(program_1, "b.ts", undefined); + + var program_2 = updateProgram(program_1, ["a.ts"], options, files => { + files[0].text = files[0].text.updateProgram("var x = 2"); + }); + assert.isTrue(program_1.structureIsReused); + + // content of resolution cache should not change + checkResolvedModulesCache(program_1, "a.ts", { "b": "b.ts" }); + checkResolvedModulesCache(program_1, "b.ts", undefined); + + // imports has changed - program is not reused + var program_3 = updateProgram(program_2, ["a.ts"], options, files => { + files[0].text = files[0].text.updateImportsAndExports(""); + }); + assert.isTrue(!program_2.structureIsReused); + checkResolvedModulesCache(program_3, "a.ts", undefined); + + var program_4 = updateProgram(program_3, ["a.ts"], options, files => { + let newImports = `import x from 'b' + import y from 'c' + `; + files[0].text = files[0].text.updateImportsAndExports(newImports); + }); + assert.isTrue(!program_3.structureIsReused); + checkResolvedModulesCache(program_4, "a.ts", { "b": "b.ts", "c": undefined }); + }); + }) } \ No newline at end of file