From c197bae99071fa52fde27de341b436f1194e0c05 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 1 Oct 2018 11:44:53 -0700 Subject: [PATCH 1/3] Add tests for failing redirect reuse of program when host implements getSourceFileByPath Test for #27207 --- .../unittests/reuseProgramStructure.ts | 143 +++++++++--------- 1 file changed, 72 insertions(+), 71 deletions(-) diff --git a/src/testRunner/unittests/reuseProgramStructure.ts b/src/testRunner/unittests/reuseProgramStructure.ts index e9c5989de07..0bae9651309 100644 --- a/src/testRunner/unittests/reuseProgramStructure.ts +++ b/src/testRunner/unittests/reuseProgramStructure.ts @@ -104,7 +104,7 @@ namespace ts { return file; } - export function createTestCompilerHost(texts: ReadonlyArray, target: ScriptTarget, oldProgram?: ProgramWithSourceTexts): TestCompilerHost { + export function createTestCompilerHost(texts: ReadonlyArray, target: ScriptTarget, oldProgram?: ProgramWithSourceTexts, useGetSourceFileByPath?: boolean) { const files = arrayToMap(texts, t => t.name, t => { if (oldProgram) { let oldFile = oldProgram.getSourceFile(t.name); @@ -117,55 +117,47 @@ namespace ts { } return createSourceFileWithText(t.name, t.text, target); }); + const useCaseSensitiveFileNames = sys && sys.useCaseSensitiveFileNames; + const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); + const filesByPath = useGetSourceFileByPath ? mapEntries(files, (fileName, file) => [toPath(fileName, "", getCanonicalFileName), file]) : undefined; const trace: string[] = []; - - return { + const result: TestCompilerHost = { trace: s => trace.push(s), getTrace: () => trace, - getSourceFile(fileName): SourceFile { - return files.get(fileName)!; - }, - getDefaultLibFileName(): string { - return "lib.d.ts"; - }, + getSourceFile: fileName => files.get(fileName), + getDefaultLibFileName: () => "lib.d.ts", writeFile: notImplemented, - getCurrentDirectory(): string { - return ""; - }, - getDirectories(): string[] { - return []; - }, - getCanonicalFileName(fileName): string { - return sys && sys.useCaseSensitiveFileNames ? fileName : fileName.toLowerCase(); - }, - useCaseSensitiveFileNames(): boolean { - return sys && sys.useCaseSensitiveFileNames; - }, - getNewLine(): string { - return sys ? sys.newLine : newLine; - }, + getCurrentDirectory: () => "", + getDirectories: () => [], + getCanonicalFileName, + useCaseSensitiveFileNames: () => useCaseSensitiveFileNames, + getNewLine: () => sys ? sys.newLine : newLine, fileExists: fileName => files.has(fileName), readFile: fileName => { const file = files.get(fileName); return file && file.text; }, }; + if (useGetSourceFileByPath) { + result.getSourceFileByPath = (_fileName, path) => filesByPath!.get(path); + } + return result; } - export function newProgram(texts: NamedSourceText[], rootNames: string[], options: CompilerOptions): ProgramWithSourceTexts { - const host = createTestCompilerHost(texts, options.target!); + export function newProgram(texts: NamedSourceText[], rootNames: string[], options: CompilerOptions, useGetSourceFileByPath?: boolean): ProgramWithSourceTexts { + const host = createTestCompilerHost(texts, options.target!, /*oldProgram*/ undefined, useGetSourceFileByPath); const program = createProgram(rootNames, options, host); program.sourceTexts = texts; program.host = host; return program; } - export function updateProgram(oldProgram: ProgramWithSourceTexts, rootNames: ReadonlyArray, options: CompilerOptions, updater: (files: NamedSourceText[]) => void, newTexts?: NamedSourceText[]) { + export function updateProgram(oldProgram: ProgramWithSourceTexts, rootNames: ReadonlyArray, options: CompilerOptions, updater: (files: NamedSourceText[]) => void, newTexts?: NamedSourceText[], useGetSourceFileByPath?: boolean) { if (!newTexts) { newTexts = oldProgram.sourceTexts!.slice(0); } updater(newTexts); - const host = createTestCompilerHost(newTexts, options.target!, oldProgram); + const host = createTestCompilerHost(newTexts, options.target!, oldProgram, useGetSourceFileByPath); const program = createProgram(rootNames, options, host, oldProgram); program.sourceTexts = newTexts; program.host = host; @@ -809,7 +801,7 @@ namespace ts { const root = "/a.ts"; const compilerOptions = { target, moduleResolution: ModuleResolutionKind.NodeJs }; - function createRedirectProgram(options?: { bText: string, bVersion: string }): ProgramWithSourceTexts { + function createRedirectProgram(useGetSourceFileByPath: boolean, options?: { bText: string, bVersion: string }): ProgramWithSourceTexts { const files: NamedSourceText[] = [ { name: "/node_modules/a/index.d.ts", @@ -841,55 +833,64 @@ namespace ts { }, ]; - return newProgram(files, [root], compilerOptions); + return newProgram(files, [root], compilerOptions, useGetSourceFileByPath); } - function updateRedirectProgram(program: ProgramWithSourceTexts, updater: (files: NamedSourceText[]) => void): ProgramWithSourceTexts { - return updateProgram(program, [root], compilerOptions, updater); + function updateRedirectProgram(program: ProgramWithSourceTexts, updater: (files: NamedSourceText[]) => void, useGetSourceFileByPath: boolean): ProgramWithSourceTexts { + return updateProgram(program, [root], compilerOptions, updater, /*newTexts*/ undefined, useGetSourceFileByPath); } - it("No changes -> redirect not broken", () => { - const program1 = createRedirectProgram(); + function verifyRedirects(useGetSourceFileByPath: boolean) { + it("No changes -> redirect not broken", () => { + const program1 = createRedirectProgram(useGetSourceFileByPath); - const program2 = updateRedirectProgram(program1, files => { - updateProgramText(files, root, "const x = 1;"); + const program2 = updateRedirectProgram(program1, files => { + updateProgramText(files, root, "const x = 1;"); + }, useGetSourceFileByPath); + assert.equal(program1.structureIsReused, StructureIsReused.Completely); + assert.lengthOf(program2.getSemanticDiagnostics(), 0); }); - assert.equal(program1.structureIsReused, StructureIsReused.Completely); - assert.lengthOf(program2.getSemanticDiagnostics(), 0); + + it("Target changes -> redirect broken", () => { + const program1 = createRedirectProgram(useGetSourceFileByPath); + assert.lengthOf(program1.getSemanticDiagnostics(), 0); + + const program2 = updateRedirectProgram(program1, files => { + updateProgramText(files, axIndex, "export default class X { private x: number; private y: number; }"); + updateProgramText(files, axPackage, JSON.stringify('{ name: "x", version: "1.2.4" }')); + }, useGetSourceFileByPath); + assert.equal(program1.structureIsReused, StructureIsReused.Not); + assert.lengthOf(program2.getSemanticDiagnostics(), 1); + }); + + it("Underlying changes -> redirect broken", () => { + const program1 = createRedirectProgram(useGetSourceFileByPath); + + const program2 = updateRedirectProgram(program1, files => { + updateProgramText(files, bxIndex, "export default class X { private x: number; private y: number; }"); + updateProgramText(files, bxPackage, JSON.stringify({ name: "x", version: "1.2.4" })); + }, useGetSourceFileByPath); + assert.equal(program1.structureIsReused, StructureIsReused.Not); + assert.lengthOf(program2.getSemanticDiagnostics(), 1); + }); + + it("Previously duplicate packages -> program structure not reused", () => { + const program1 = createRedirectProgram(useGetSourceFileByPath, { bVersion: "1.2.4", bText: "export = class X { private x: number; }" }); + + const program2 = updateRedirectProgram(program1, files => { + updateProgramText(files, bxIndex, "export default class X { private x: number; }"); + updateProgramText(files, bxPackage, JSON.stringify({ name: "x", version: "1.2.3" })); + }, useGetSourceFileByPath); + assert.equal(program1.structureIsReused, StructureIsReused.Not); + assert.deepEqual(program2.getSemanticDiagnostics(), []); + }); + } + + describe("when host implements getSourceFile", () => { + verifyRedirects(/*useGetSourceFileByPath*/ false); }); - - it("Target changes -> redirect broken", () => { - const program1 = createRedirectProgram(); - assert.lengthOf(program1.getSemanticDiagnostics(), 0); - - const program2 = updateRedirectProgram(program1, files => { - updateProgramText(files, axIndex, "export default class X { private x: number; private y: number; }"); - updateProgramText(files, axPackage, JSON.stringify('{ name: "x", version: "1.2.4" }')); - }); - assert.equal(program1.structureIsReused, StructureIsReused.Not); - assert.lengthOf(program2.getSemanticDiagnostics(), 1); - }); - - it("Underlying changes -> redirect broken", () => { - const program1 = createRedirectProgram(); - - const program2 = updateRedirectProgram(program1, files => { - updateProgramText(files, bxIndex, "export default class X { private x: number; private y: number; }"); - updateProgramText(files, bxPackage, JSON.stringify({ name: "x", version: "1.2.4" })); - }); - assert.equal(program1.structureIsReused, StructureIsReused.Not); - assert.lengthOf(program2.getSemanticDiagnostics(), 1); - }); - - it("Previously duplicate packages -> program structure not reused", () => { - const program1 = createRedirectProgram({ bVersion: "1.2.4", bText: "export = class X { private x: number; }" }); - - const program2 = updateRedirectProgram(program1, files => { - updateProgramText(files, bxIndex, "export default class X { private x: number; }"); - updateProgramText(files, bxPackage, JSON.stringify({ name: "x", version: "1.2.3" })); - }); - assert.equal(program1.structureIsReused, StructureIsReused.Not); - assert.deepEqual(program2.getSemanticDiagnostics(), []); + describe("when host implements getSourceFileByPath", () => { + verifyRedirects(/*useGetSourceFileByPath*/ true); }); }); }); From 1c2f2555ec3fd611210b69cc071f26e9bbdd0634 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 1 Oct 2018 11:49:43 -0700 Subject: [PATCH 2/3] Add resolvedPath and originalFileName to redirected file Fixes #27207 --- src/compiler/program.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 44eae36d654..25f354e8190 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -2024,10 +2024,12 @@ namespace ts { } } - function createRedirectSourceFile(redirectTarget: SourceFile, unredirected: SourceFile, fileName: string, path: Path): SourceFile { + function createRedirectSourceFile(redirectTarget: SourceFile, unredirected: SourceFile, fileName: string, path: Path, resolvedPath: Path, originalFileName: string): SourceFile { const redirect: SourceFile = Object.create(redirectTarget); redirect.fileName = fileName; redirect.path = path; + redirect.resolvedPath = resolvedPath; + redirect.originalFileName = originalFileName; redirect.redirectInfo = { redirectTarget, unredirected }; Object.defineProperties(redirect, { id: { @@ -2118,7 +2120,7 @@ namespace ts { if (fileFromPackageId) { // Some other SourceFile already exists with this package name and version. // Instead of creating a duplicate, just redirect to the existing one. - const dupFile = createRedirectSourceFile(fileFromPackageId, file!, fileName, path); // TODO: GH#18217 + const dupFile = createRedirectSourceFile(fileFromPackageId, file!, fileName, path, toPath(fileName), originalFileName); // TODO: GH#18217 redirectTargetsMap.add(fileFromPackageId.path, fileName); filesByName.set(path, dupFile); sourceFileToPackageName.set(path, packageId.name); From dd3277c21997082e1511508bbbab0939edf2b1c8 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 1 Oct 2018 12:12:22 -0700 Subject: [PATCH 3/3] PR feedback --- src/testRunner/unittests/reuseProgramStructure.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/testRunner/unittests/reuseProgramStructure.ts b/src/testRunner/unittests/reuseProgramStructure.ts index 0bae9651309..0ab50a65105 100644 --- a/src/testRunner/unittests/reuseProgramStructure.ts +++ b/src/testRunner/unittests/reuseProgramStructure.ts @@ -119,7 +119,6 @@ namespace ts { }); const useCaseSensitiveFileNames = sys && sys.useCaseSensitiveFileNames; const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); - const filesByPath = useGetSourceFileByPath ? mapEntries(files, (fileName, file) => [toPath(fileName, "", getCanonicalFileName), file]) : undefined; const trace: string[] = []; const result: TestCompilerHost = { trace: s => trace.push(s), @@ -139,7 +138,8 @@ namespace ts { }, }; if (useGetSourceFileByPath) { - result.getSourceFileByPath = (_fileName, path) => filesByPath!.get(path); + const filesByPath = mapEntries(files, (fileName, file) => [toPath(fileName, "", getCanonicalFileName), file]); + result.getSourceFileByPath = (_fileName, path) => filesByPath.get(path); } return result; }