From 604af5c621e67ab558b8c6f7b7594b73170ad79e Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 28 Jan 2019 12:04:39 -0800 Subject: [PATCH 1/2] Handle get from readFileCache correctly. Fixes #29623 --- src/compiler/program.ts | 2 +- src/harness/fakes.ts | 23 ++-- src/testRunner/unittests/tsbuild.ts | 123 ++++++++++++------ .../missingExtendedConfig/tsconfig.first.json | 6 + .../missingExtendedConfig/tsconfig.json | 9 ++ .../tsconfig.second.json | 6 + 6 files changed, 116 insertions(+), 53 deletions(-) create mode 100644 tests/projects/missingExtendedConfig/tsconfig.first.json create mode 100644 tests/projects/missingExtendedConfig/tsconfig.json create mode 100644 tests/projects/missingExtendedConfig/tsconfig.second.json diff --git a/src/compiler/program.ts b/src/compiler/program.ts index e7c2295a0ca..fdb3676f964 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -241,7 +241,7 @@ namespace ts { host.readFile = fileName => { const key = toPath(fileName); const value = readFileCache.get(key); - if (value !== undefined) return value; // could be .d.ts from output + if (value !== undefined) return value || undefined; // could be .d.ts from output if (!fileExtensionIs(fileName, Extension.Json)) { return originalReadFile.call(host, fileName); } diff --git a/src/harness/fakes.ts b/src/harness/fakes.ts index e8c81617052..093f57a112b 100644 --- a/src/harness/fakes.ts +++ b/src/harness/fakes.ts @@ -375,6 +375,15 @@ namespace fakes { } } + export type ExpectedDiagnostic = [ts.DiagnosticMessage, ...(string | number)[]]; + function expectedDiagnosticToText([message, ...args]: ExpectedDiagnostic) { + let text = ts.getLocaleSpecificMessage(message); + if (args.length) { + text = ts.formatStringFromArgs(text, args); + } + return text; + } + export class SolutionBuilderHost extends CompilerHost implements ts.SolutionBuilderHost { createProgram = ts.createAbstractBuilder; now() { @@ -395,16 +404,10 @@ namespace fakes { this.diagnostics.length = 0; } - assertDiagnosticMessages(...expected: ts.DiagnosticMessage[]) { - const actual = this.diagnostics.slice(); - if (actual.length !== expected.length) { - assert.fail(actual, expected, `Diagnostic arrays did not match - got\r\n${actual.map(a => " " + a.messageText).join("\r\n")}\r\nexpected\r\n${expected.map(e => " " + e.message).join("\r\n")}`); - } - for (let i = 0; i < actual.length; i++) { - if (actual[i].code !== expected[i].code) { - assert.fail(actual[i].messageText, expected[i].message, `Mismatched error code - expected diagnostic ${i} "${actual[i].messageText}" to match ${expected[i].message}`); - } - } + assertDiagnosticMessages(...expectedDiagnostics: ExpectedDiagnostic[]) { + const actual = this.diagnostics.slice().map(d => d.messageText as string); + const expected = expectedDiagnostics.map(expectedDiagnosticToText); + assert.deepEqual(actual, expected, "Diagnostic arrays did not match"); } printDiagnostics(header = "== Diagnostics ==") { diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts index e1bc78c712b..707a8534004 100644 --- a/src/testRunner/unittests/tsbuild.ts +++ b/src/testRunner/unittests/tsbuild.ts @@ -1,5 +1,10 @@ namespace ts { let currentTime = 100; + + function getExpectedDiagnosticForProjectsInBuild(...projects: string[]): fakes.ExpectedDiagnostic { + return [Diagnostics.Projects_in_this_build_Colon_0, projects.map(p => "\r\n * " + p).join("")]; + } + export namespace Sample1 { tick(); const projFs = loadProjectFromDisk("tests/projects/sample1"); @@ -67,7 +72,11 @@ namespace ts { const host = new fakes.SolutionBuilderHost(fs); const builder = createSolutionBuilder(host, ["/src/tests"], { dry: true, force: false, verbose: false }); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0); + host.assertDiagnosticMessages( + [Diagnostics.A_non_dry_build_would_build_project_0, "/src/core/tsconfig.json"], + [Diagnostics.A_non_dry_build_would_build_project_0, "/src/logic/tsconfig.json"], + [Diagnostics.A_non_dry_build_would_build_project_0, "/src/tests/tsconfig.json"] + ); // Check for outputs to not be written. Not an exhaustive list for (const output of allExpectedOutputs) { @@ -86,7 +95,11 @@ namespace ts { host.clearDiagnostics(); builder = createSolutionBuilder(host, ["/src/tests"], { dry: true, force: false, verbose: false }); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date); + host.assertDiagnosticMessages( + [Diagnostics.Project_0_is_up_to_date, "/src/core/tsconfig.json"], + [Diagnostics.Project_0_is_up_to_date, "/src/logic/tsconfig.json"], + [Diagnostics.Project_0_is_up_to_date, "/src/tests/tsconfig.json"] + ); }); }); @@ -146,13 +159,15 @@ namespace ts { host.clearDiagnostics(); builder.resetBuildContext(); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, - Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, - Diagnostics.Building_project_0, - Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, - Diagnostics.Building_project_0, - Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, - Diagnostics.Building_project_0); + host.assertDiagnosticMessages( + getExpectedDiagnosticForProjectsInBuild("src/core/tsconfig.json", "src/logic/tsconfig.json", "src/tests/tsconfig.json"), + [Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/core/tsconfig.json", "src/core/anotherModule.js"], + [Diagnostics.Building_project_0, "/src/core/tsconfig.json"], + [Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/logic/tsconfig.json", "src/logic/index.js"], + [Diagnostics.Building_project_0, "/src/logic/tsconfig.json"], + [Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/tests/tsconfig.json", "src/tests/index.js"], + [Diagnostics.Building_project_0, "/src/tests/tsconfig.json"] + ); tick(); }); @@ -161,10 +176,12 @@ namespace ts { host.clearDiagnostics(); builder.resetBuildContext(); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, - Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, - Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, - Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2); + host.assertDiagnosticMessages( + getExpectedDiagnosticForProjectsInBuild("src/core/tsconfig.json", "src/logic/tsconfig.json", "src/tests/tsconfig.json"), + [Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, "src/core/tsconfig.json", "src/core/anotherModule.ts", "src/core/anotherModule.js"], + [Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, "src/logic/tsconfig.json", "src/logic/index.ts", "src/logic/index.js"], + [Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, "src/tests/tsconfig.json", "src/tests/index.ts", "src/tests/index.js"] + ); tick(); }); @@ -175,11 +192,13 @@ namespace ts { builder.resetBuildContext(); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, - Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, - Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, - Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, - Diagnostics.Building_project_0); + host.assertDiagnosticMessages( + getExpectedDiagnosticForProjectsInBuild("src/core/tsconfig.json", "src/logic/tsconfig.json", "src/tests/tsconfig.json"), + [Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, "src/core/tsconfig.json", "src/core/anotherModule.ts", "src/core/anotherModule.js"], + [Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, "src/logic/tsconfig.json", "src/logic/index.ts", "src/logic/index.js"], + [Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/tests/tsconfig.json", "src/tests/index.js", "src/tests/index.ts"], + [Diagnostics.Building_project_0, "/src/tests/tsconfig.json"] + ); tick(); }); @@ -190,13 +209,15 @@ namespace ts { builder.resetBuildContext(); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, - Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, - Diagnostics.Building_project_0, - Diagnostics.Project_0_is_up_to_date_with_d_ts_files_from_its_dependencies, - Diagnostics.Updating_output_timestamps_of_project_0, - Diagnostics.Project_0_is_up_to_date_with_d_ts_files_from_its_dependencies, - Diagnostics.Updating_output_timestamps_of_project_0); + host.assertDiagnosticMessages( + getExpectedDiagnosticForProjectsInBuild("src/core/tsconfig.json", "src/logic/tsconfig.json", "src/tests/tsconfig.json"), + [Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, "src/core/tsconfig.json", "src/core/anotherModule.js", "src/core/index.ts"], + [Diagnostics.Building_project_0, "/src/core/tsconfig.json"], + [Diagnostics.Project_0_is_up_to_date_with_d_ts_files_from_its_dependencies, "src/logic/tsconfig.json"], + [Diagnostics.Updating_output_timestamps_of_project_0, "/src/logic/tsconfig.json"], + [Diagnostics.Project_0_is_up_to_date_with_d_ts_files_from_its_dependencies, "src/tests/tsconfig.json"], + [Diagnostics.Updating_output_timestamps_of_project_0, "/src/tests/tsconfig.json"] + ); }); }); @@ -210,14 +231,14 @@ namespace ts { replaceText(fs, "/src/logic/index.ts", "c.multiply(10, 15)", `c.muitply()`); builder.buildAllProjects(); host.assertDiagnosticMessages( - Diagnostics.Projects_in_this_build_Colon_0, - Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, - Diagnostics.Building_project_0, - Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, - Diagnostics.Building_project_0, - Diagnostics.Property_0_does_not_exist_on_type_1, - Diagnostics.Project_0_can_t_be_built_because_its_dependency_1_has_errors, - Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors + getExpectedDiagnosticForProjectsInBuild("src/core/tsconfig.json", "src/logic/tsconfig.json", "src/tests/tsconfig.json"), + [Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/core/tsconfig.json", "src/core/anotherModule.js"], + [Diagnostics.Building_project_0, "/src/core/tsconfig.json"], + [Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, "src/logic/tsconfig.json", "src/logic/index.js"], + [Diagnostics.Building_project_0, "/src/logic/tsconfig.json"], + [Diagnostics.Property_0_does_not_exist_on_type_1, "muitply", `typeof import("/src/core/index")`], + [Diagnostics.Project_0_can_t_be_built_because_its_dependency_1_has_errors, "src/tests/tsconfig.json", "src/logic"], + [Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, "/src/tests/tsconfig.json", "/src/logic"] ); }); }); @@ -281,12 +302,12 @@ export class cNew {}`); const projFs = loadProjectFromDisk("tests/projects/resolveJsonModuleAndComposite"); const allExpectedOutputs = ["/src/tests/dist/src/index.js", "/src/tests/dist/src/index.d.ts", "/src/tests/dist/src/hello.json"]; - function verifyProjectWithResolveJsonModule(configFile: string, ...expectedDiagnosticMessages: DiagnosticMessage[]) { + function verifyProjectWithResolveJsonModule(configFile: string, ...expectedDiagnosticMessages: fakes.ExpectedDiagnostic[]) { const fs = projFs.shadow(); verifyProjectWithResolveJsonModuleWithFs(fs, configFile, allExpectedOutputs, ...expectedDiagnosticMessages); } - function verifyProjectWithResolveJsonModuleWithFs(fs: vfs.FileSystem, configFile: string, allExpectedOutputs: ReadonlyArray, ...expectedDiagnosticMessages: DiagnosticMessage[]) { + function verifyProjectWithResolveJsonModuleWithFs(fs: vfs.FileSystem, configFile: string, allExpectedOutputs: ReadonlyArray, ...expectedDiagnosticMessages: fakes.ExpectedDiagnostic[]) { const host = new fakes.SolutionBuilderHost(fs); const builder = createSolutionBuilder(host, [configFile], { dry: false, force: false, verbose: false }); builder.buildAllProjects(); @@ -300,7 +321,10 @@ export class cNew {}`); } it("with resolveJsonModule and include only", () => { - verifyProjectWithResolveJsonModule("/src/tests/tsconfig_withInclude.json", Diagnostics.File_0_is_not_in_project_file_list_Projects_must_list_all_files_or_use_an_include_pattern); + verifyProjectWithResolveJsonModule("/src/tests/tsconfig_withInclude.json", [ + Diagnostics.File_0_is_not_in_project_file_list_Projects_must_list_all_files_or_use_an_include_pattern, + "/src/tests/src/hello.json" + ]); }); it("with resolveJsonModule and include of *.json along with other include", () => { @@ -415,7 +439,7 @@ export default hello.hello`); "/src/c.ts" ]; - function verifyBuild(modifyDiskLayout: (fs: vfs.FileSystem) => void, allExpectedOutputs: ReadonlyArray, expectedDiagnostics: DiagnosticMessage[], expectedFileTraces: ReadonlyArray) { + function verifyBuild(modifyDiskLayout: (fs: vfs.FileSystem) => void, allExpectedOutputs: ReadonlyArray, expectedFileTraces: ReadonlyArray, ...expectedDiagnostics: fakes.ExpectedDiagnostic[]) { const fs = projFs.shadow(); const host = new fakes.SolutionBuilderHost(fs); modifyDiskLayout(fs); @@ -442,11 +466,11 @@ export const b = new A();`); } it("verify that it builds correctly", () => { - verifyBuild(noop, allExpectedOutputs, emptyArray, expectedFileTraces); + verifyBuild(noop, allExpectedOutputs, expectedFileTraces); }); it("verify that it builds correctly when the referenced project uses different module resolution", () => { - verifyBuild(fs => modifyFsBTsToNonRelativeImport(fs, "classic"), allExpectedOutputs, emptyArray, expectedFileTraces); + verifyBuild(fs => modifyFsBTsToNonRelativeImport(fs, "classic"), allExpectedOutputs, expectedFileTraces); }); it("verify that it build reports error about module not found with node resolution with external module name", () => { @@ -458,10 +482,25 @@ export const b = new A();`); ]; verifyBuild(fs => modifyFsBTsToNonRelativeImport(fs, "node"), allExpectedOutputs, - [Diagnostics.Cannot_find_module_0], - expectedFileTraces); + expectedFileTraces, + [Diagnostics.Cannot_find_module_0, "a"], + ); }); }); + + it("unittests:: tsbuild - when tsconfig extends the missing file", () => { + const projFs = loadProjectFromDisk("tests/projects/missingExtendedConfig"); + const fs = projFs.shadow(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tsconfig.json"], {}); + builder.buildAllProjects(); + host.assertDiagnosticMessages( + [Diagnostics.The_specified_path_does_not_exist_Colon_0, "/src/foobar.json"], + [Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2, "/src/tsconfig.first.json", "[\"**/*\"]", "[]"], + [Diagnostics.The_specified_path_does_not_exist_Colon_0, "/src/foobar.json"], + [Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2, "/src/tsconfig.second.json", "[\"**/*\"]", "[]"] + ); + }); } export namespace OutFile { @@ -564,7 +603,7 @@ export const b = new A();`); host.clearDiagnostics(); builder.buildAllProjects(); - host.assertDiagnosticMessages(Diagnostics.The_files_list_in_config_file_0_is_empty); + host.assertDiagnosticMessages([Diagnostics.The_files_list_in_config_file_0_is_empty, "/src/no-references/tsconfig.json"]); // Check for outputs to not be written. for (const output of allExpectedOutputs) { diff --git a/tests/projects/missingExtendedConfig/tsconfig.first.json b/tests/projects/missingExtendedConfig/tsconfig.first.json new file mode 100644 index 00000000000..98a964017d4 --- /dev/null +++ b/tests/projects/missingExtendedConfig/tsconfig.first.json @@ -0,0 +1,6 @@ +{ + "extends": "./foobar.json", + "compilerOptions": { + "composite": true + } +} \ No newline at end of file diff --git a/tests/projects/missingExtendedConfig/tsconfig.json b/tests/projects/missingExtendedConfig/tsconfig.json new file mode 100644 index 00000000000..a14558f41e4 --- /dev/null +++ b/tests/projects/missingExtendedConfig/tsconfig.json @@ -0,0 +1,9 @@ +{ + "compilerOptions": { + "composite": true + }, + "references": [ + { "path": "./tsconfig.first.json" }, + { "path": "./tsconfig.second.json" } + ] +} \ No newline at end of file diff --git a/tests/projects/missingExtendedConfig/tsconfig.second.json b/tests/projects/missingExtendedConfig/tsconfig.second.json new file mode 100644 index 00000000000..98a964017d4 --- /dev/null +++ b/tests/projects/missingExtendedConfig/tsconfig.second.json @@ -0,0 +1,6 @@ +{ + "extends": "./foobar.json", + "compilerOptions": { + "composite": true + } +} \ No newline at end of file From 55b8e4fd294d7b6e24c77ce4b7f426c99afd3ab9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 30 Jan 2019 12:32:08 -0800 Subject: [PATCH 2/2] Handle "" correctly when cacheing file text --- src/compiler/program.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index fdb3676f964..280f46fa445 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -230,18 +230,18 @@ namespace ts { const readFileWithCache = (fileName: string): string | undefined => { const key = toPath(fileName); const value = readFileCache.get(key); - if (value !== undefined) return value || undefined; + if (value !== undefined) return value !== false ? value : undefined; return setReadFileCache(key, fileName); }; const setReadFileCache = (key: Path, fileName: string) => { const newValue = originalReadFile.call(host, fileName); - readFileCache.set(key, newValue || false); + readFileCache.set(key, newValue !== undefined ? newValue : false); return newValue; }; host.readFile = fileName => { const key = toPath(fileName); const value = readFileCache.get(key); - if (value !== undefined) return value || undefined; // could be .d.ts from output + if (value !== undefined) return value !== false ? value : undefined; // could be .d.ts from output if (!fileExtensionIs(fileName, Extension.Json)) { return originalReadFile.call(host, fileName); }