From c87ca2f1ab8caab05c6085c0135bdcf7a61ac042 Mon Sep 17 00:00:00 2001 From: christian Date: Mon, 3 Sep 2018 22:57:26 -0400 Subject: [PATCH 1/5] Fix diagnostic reporting for empty files in tsconfig --- src/compiler/commandLineParser.ts | 16 ++++--- src/compiler/tsbuild.ts | 16 +++++-- src/testRunner/unittests/tsbuild.ts | 42 +++++++++++++++++++ src/testRunner/unittests/tsconfigParsing.ts | 39 +++++++++++++++++ tests/projects/empty-files/core/index.ts | 1 + tests/projects/empty-files/core/tsconfig.json | 7 ++++ .../empty-files/no-references/tsconfig.json | 9 ++++ .../empty-files/with-references/tsconfig.json | 11 +++++ 8 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 tests/projects/empty-files/core/index.ts create mode 100644 tests/projects/empty-files/core/tsconfig.json create mode 100644 tests/projects/empty-files/no-references/tsconfig.json create mode 100644 tests/projects/empty-files/with-references/tsconfig.json diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index e4c4edcb4db..5d87525073f 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1843,7 +1843,9 @@ namespace ts { if (hasProperty(raw, "files") && !isNullOrUndefined(raw.files)) { if (isArray(raw.files)) { filesSpecs = >raw.files; - if (filesSpecs.length === 0) { + const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); + const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; + if (filesSpecs.length === 0 && hasZeroOrNoReferences) { createCompilerDiagnosticOnlyIfJson(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); } } @@ -2067,11 +2069,6 @@ namespace ts { createDiagnosticForNodeInSourceFile(sourceFile, valueNode, message, arg0) ); return; - case "files": - if ((>value).length === 0) { - errors.push(createDiagnosticForNodeInSourceFile(sourceFile, valueNode, Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json")); - } - return; } }, onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, _value: CompilerOptionsValue, _valueNode: Expression) { @@ -2081,6 +2078,13 @@ namespace ts { } }; const json = convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, getTsconfigRootOptionsMap(), optionsIterator); + const hasZeroFiles = json && json.files && json.files.length === 0; + const hasZeroOrNoReferences = !(json && json.references) || json.references.length === 0; + + if (hasZeroFiles && hasZeroOrNoReferences) { + errors.push(createCompilerDiagnostic(Diagnostics.The_files_list_in_config_file_0_is_empty, sourceFile.fileName)); + } + if (!typeAcquisition) { if (typingOptionstypeAcquisition) { typeAcquisition = (typingOptionstypeAcquisition.enableAutoDiscovery !== undefined) ? diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 4ddda98a41f..418185b4c16 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -946,7 +946,6 @@ namespace ts { context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Config file errors" }); return resultFlags; } - if (configFile.fileNames.length === 0) { // Nothing to build - must be a solution file, basically return BuildResultFlags.None; @@ -956,7 +955,8 @@ namespace ts { projectReferences: configFile.projectReferences, host, rootNames: configFile.fileNames, - options: configFile.options + options: configFile.options, + configFileParsingDiagnostics: configFile.errors, }; const program = createProgram(programOptions); @@ -1149,7 +1149,6 @@ namespace ts { const queue = graph.buildQueue; reportBuildQueue(graph); - let anyFailed = false; for (const next of queue) { const proj = configFileCache.parseConfigFile(next); @@ -1157,11 +1156,15 @@ namespace ts { anyFailed = true; break; } + + // report errors early when using continue or break statements + const errors = proj.errors; const status = getUpToDateStatus(proj); verboseReportProjectStatus(next, status); const projName = proj.options.configFilePath!; if (status.type === UpToDateStatusType.UpToDate && !context.options.force) { + reportErrors(errors); // Up to date, skip if (defaultOptions.dry) { // In a dry build, inform the user of this fact @@ -1171,17 +1174,20 @@ namespace ts { } if (status.type === UpToDateStatusType.UpToDateWithUpstreamTypes && !context.options.force) { + reportErrors(errors); // Fake build updateOutputTimestamps(proj); continue; } if (status.type === UpToDateStatusType.UpstreamBlocked) { + reportErrors(errors); if (context.options.verbose) reportStatus(Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, projName, status.upstreamProjectName); continue; } if (status.type === UpToDateStatusType.ContainerOnly) { + reportErrors(errors); // Do nothing continue; } @@ -1193,6 +1199,10 @@ namespace ts { return anyFailed ? ExitStatus.DiagnosticsPresent_OutputsSkipped : ExitStatus.Success; } + function reportErrors(errors: Diagnostic[]) { + errors.forEach((err) => host.reportDiagnostic(err)); + } + /** * Report the build ordering inferred from the current project graph if we're in verbose mode */ diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts index ea99ee92457..6d6f95ce19c 100644 --- a/src/testRunner/unittests/tsbuild.ts +++ b/src/testRunner/unittests/tsbuild.ts @@ -292,6 +292,48 @@ namespace ts { }); } + export namespace EmptyFiles { + const projFs = loadProjectFromDisk("tests/projects/empty-files"); + + const allExpectedOutputs = [ + "/src/core/index.js", + "/src/core/index.d.ts", + "/src/core/index.d.ts.map", + ]; + + describe("tsbuild - empty files option in tsconfig", () => { + it("has empty files diagnostic when files is empty and no references are provided", () => { + const fs = projFs.shadow(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/no-references"], { dry: false, force: false, verbose: false }); + + host.clearDiagnostics(); + builder.buildAllProjects(); + host.assertDiagnosticMessages(Diagnostics.The_files_list_in_config_file_0_is_empty); + + // Check for outputs to not be written. + for (const output of allExpectedOutputs) { + assert(!fs.existsSync(output), `Expect file ${output} to not exist`); + } + }); + + it("does not have empty files diagnostic when files is empty and references are provided", () => { + const fs = projFs.shadow(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/with-references"], { dry: false, force: false, verbose: false }); + + host.clearDiagnostics(); + builder.buildAllProjects(); + host.assertDiagnosticMessages(/*empty*/); + + // Check for outputs to be written. + for (const output of allExpectedOutputs) { + assert(fs.existsSync(output), `Expect file ${output} to exist`); + } + }); + }); + } + describe("tsbuild - graph-ordering", () => { let host: fakes.SolutionBuilderHost | undefined; const deps: [string, string][] = [ diff --git a/src/testRunner/unittests/tsconfigParsing.ts b/src/testRunner/unittests/tsconfigParsing.ts index 6ef5697046f..c2e8f0eb10d 100644 --- a/src/testRunner/unittests/tsconfigParsing.ts +++ b/src/testRunner/unittests/tsconfigParsing.ts @@ -61,6 +61,19 @@ namespace ts { } } + function assertParseFileDiagnosticsExclusion(jsonText: string, configFileName: string, basePath: string, allFileList: string[], expectedExcludedDiagnosticCode: number) { + { + const parsed = getParsedCommandJson(jsonText, configFileName, basePath, allFileList); + assert.isTrue(parsed.errors.length >= 0); + assert.isTrue(parsed.errors.findIndex(e => e.code === expectedExcludedDiagnosticCode) === -1, `Expected error code ${expectedExcludedDiagnosticCode} to not be in ${JSON.stringify(parsed.errors)}`); + } + { + const parsed = getParsedCommandJsonNode(jsonText, configFileName, basePath, allFileList); + assert.isTrue(parsed.errors.length >= 0); + assert.isTrue(parsed.errors.findIndex(e => e.code === expectedExcludedDiagnosticCode) === -1, `Expected error code ${expectedExcludedDiagnosticCode} to not be in ${JSON.stringify(parsed.errors)}`); + } + } + it("returns empty config for file with only whitespaces", () => { assertParseResult("", { config : {} }); assertParseResult(" ", { config : {} }); @@ -274,6 +287,32 @@ namespace ts { "files": [] }`; assertParseFileDiagnostics(content, + "/apath/tsconfig.json", + "tests/cases/unittests", + ["/apath/a.ts"], + Diagnostics.The_files_list_in_config_file_0_is_empty.code, + /*noLocation*/ true); + }); + + it("generates errors for empty files list when no references are provided", () => { + const content = `{ + "files": [], + "references": [] + }`; + assertParseFileDiagnostics(content, + "/apath/tsconfig.json", + "tests/cases/unittests", + ["/apath/a.ts"], + Diagnostics.The_files_list_in_config_file_0_is_empty.code, + /*noLocation*/ true); + }); + + it("does not generate errors for empty files list when one or more references are provided", () => { + const content = `{ + "files": [], + "references": [{ "path": "/apath" }] + }`; + assertParseFileDiagnosticsExclusion(content, "/apath/tsconfig.json", "tests/cases/unittests", ["/apath/a.ts"], diff --git a/tests/projects/empty-files/core/index.ts b/tests/projects/empty-files/core/index.ts new file mode 100644 index 00000000000..3da69271e97 --- /dev/null +++ b/tests/projects/empty-files/core/index.ts @@ -0,0 +1 @@ +export function multiply(a: number, b: number) { return a * b; } diff --git a/tests/projects/empty-files/core/tsconfig.json b/tests/projects/empty-files/core/tsconfig.json new file mode 100644 index 00000000000..24b64bc7b2c --- /dev/null +++ b/tests/projects/empty-files/core/tsconfig.json @@ -0,0 +1,7 @@ +{ + "compilerOptions": { + "composite": true, + "declaration": true, + "declarationMap": true + } +} \ No newline at end of file diff --git a/tests/projects/empty-files/no-references/tsconfig.json b/tests/projects/empty-files/no-references/tsconfig.json new file mode 100644 index 00000000000..9b02f3654e3 --- /dev/null +++ b/tests/projects/empty-files/no-references/tsconfig.json @@ -0,0 +1,9 @@ +{ + "references": [], + "files": [], + "compilerOptions": { + "composite": true, + "declaration": true, + "forceConsistentCasingInFileNames": true + } +} \ No newline at end of file diff --git a/tests/projects/empty-files/with-references/tsconfig.json b/tests/projects/empty-files/with-references/tsconfig.json new file mode 100644 index 00000000000..bf5e2690064 --- /dev/null +++ b/tests/projects/empty-files/with-references/tsconfig.json @@ -0,0 +1,11 @@ +{ + "references": [ + { "path": "../core" }, + ], + "files": [], + "compilerOptions": { + "composite": true, + "declaration": true, + "forceConsistentCasingInFileNames": true + } +} \ No newline at end of file From 959dbbba2821c63bcb1de9d4422a80aeab6b5156 Mon Sep 17 00:00:00 2001 From: christian Date: Mon, 3 Sep 2018 23:16:53 -0400 Subject: [PATCH 2/5] Add newline to bottom of tsconfig files --- tests/projects/empty-files/core/tsconfig.json | 2 +- tests/projects/empty-files/no-references/tsconfig.json | 2 +- tests/projects/empty-files/with-references/tsconfig.json | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/projects/empty-files/core/tsconfig.json b/tests/projects/empty-files/core/tsconfig.json index 24b64bc7b2c..0e8205977dd 100644 --- a/tests/projects/empty-files/core/tsconfig.json +++ b/tests/projects/empty-files/core/tsconfig.json @@ -4,4 +4,4 @@ "declaration": true, "declarationMap": true } -} \ No newline at end of file +} diff --git a/tests/projects/empty-files/no-references/tsconfig.json b/tests/projects/empty-files/no-references/tsconfig.json index 9b02f3654e3..c6b8f1a43d7 100644 --- a/tests/projects/empty-files/no-references/tsconfig.json +++ b/tests/projects/empty-files/no-references/tsconfig.json @@ -6,4 +6,4 @@ "declaration": true, "forceConsistentCasingInFileNames": true } -} \ No newline at end of file +} diff --git a/tests/projects/empty-files/with-references/tsconfig.json b/tests/projects/empty-files/with-references/tsconfig.json index bf5e2690064..3a55cad1b1c 100644 --- a/tests/projects/empty-files/with-references/tsconfig.json +++ b/tests/projects/empty-files/with-references/tsconfig.json @@ -8,4 +8,4 @@ "declaration": true, "forceConsistentCasingInFileNames": true } -} \ No newline at end of file +} From ea984d7b64fc7b57d643ffc2792eac01baccddc2 Mon Sep 17 00:00:00 2001 From: christian Date: Wed, 5 Sep 2018 23:18:39 -0400 Subject: [PATCH 3/5] Centralize diagnostic reporting for empty files diagnostic --- src/compiler/commandLineParser.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 5d87525073f..a2dac4e7d33 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1846,7 +1846,7 @@ namespace ts { const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; if (filesSpecs.length === 0 && hasZeroOrNoReferences) { - createCompilerDiagnosticOnlyIfJson(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); + errors.push(createCompilerDiagnostic(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json")); } } else { @@ -2078,12 +2078,6 @@ namespace ts { } }; const json = convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, getTsconfigRootOptionsMap(), optionsIterator); - const hasZeroFiles = json && json.files && json.files.length === 0; - const hasZeroOrNoReferences = !(json && json.references) || json.references.length === 0; - - if (hasZeroFiles && hasZeroOrNoReferences) { - errors.push(createCompilerDiagnostic(Diagnostics.The_files_list_in_config_file_0_is_empty, sourceFile.fileName)); - } if (!typeAcquisition) { if (typingOptionstypeAcquisition) { From ec72f4751d0986690440dad7be2d6b8d86a65da9 Mon Sep 17 00:00:00 2001 From: christian Date: Thu, 6 Sep 2018 20:40:02 -0400 Subject: [PATCH 4/5] Add location info to empty lists diagnostics when tsconfig file exists --- src/compiler/commandLineParser.ts | 9 ++++++++- src/testRunner/unittests/tsconfigParsing.ts | 6 ++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index a2dac4e7d33..ecc69fccf86 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1846,7 +1846,14 @@ namespace ts { const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; if (filesSpecs.length === 0 && hasZeroOrNoReferences) { - errors.push(createCompilerDiagnostic(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json")); + if (sourceFile) { + const nodeValue = firstDefined(getTsConfigPropArray(sourceFile, "files"), property => property.initializer); + const error = createDiagnosticForNodeInSourceFile(sourceFile, nodeValue!, Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); + errors.push(error); + } + else { + createCompilerDiagnosticOnlyIfJson(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); + } } } else { diff --git a/src/testRunner/unittests/tsconfigParsing.ts b/src/testRunner/unittests/tsconfigParsing.ts index c2e8f0eb10d..6909e260d2a 100644 --- a/src/testRunner/unittests/tsconfigParsing.ts +++ b/src/testRunner/unittests/tsconfigParsing.ts @@ -290,8 +290,7 @@ namespace ts { "/apath/tsconfig.json", "tests/cases/unittests", ["/apath/a.ts"], - Diagnostics.The_files_list_in_config_file_0_is_empty.code, - /*noLocation*/ true); + Diagnostics.The_files_list_in_config_file_0_is_empty.code); }); it("generates errors for empty files list when no references are provided", () => { @@ -303,8 +302,7 @@ namespace ts { "/apath/tsconfig.json", "tests/cases/unittests", ["/apath/a.ts"], - Diagnostics.The_files_list_in_config_file_0_is_empty.code, - /*noLocation*/ true); + Diagnostics.The_files_list_in_config_file_0_is_empty.code); }); it("does not generate errors for empty files list when one or more references are provided", () => { From 16477b65067d7e4b3d71eeb1300ae03d7723fe2d Mon Sep 17 00:00:00 2001 From: christian Date: Sat, 8 Sep 2018 00:06:07 -0400 Subject: [PATCH 5/5] Take into account undefined nodeValue when recording diagnostic --- src/compiler/commandLineParser.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index ecc69fccf86..edee847019a 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1847,8 +1847,12 @@ namespace ts { const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; if (filesSpecs.length === 0 && hasZeroOrNoReferences) { if (sourceFile) { + const fileName = configFileName || "tsconfig.json"; + const diagnosticMessage = Diagnostics.The_files_list_in_config_file_0_is_empty; const nodeValue = firstDefined(getTsConfigPropArray(sourceFile, "files"), property => property.initializer); - const error = createDiagnosticForNodeInSourceFile(sourceFile, nodeValue!, Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); + const error = nodeValue + ? createDiagnosticForNodeInSourceFile(sourceFile, nodeValue, diagnosticMessage, fileName) + : createCompilerDiagnostic(diagnosticMessage, fileName); errors.push(error); } else {