From dec9b741fea03b6ef398a29b9d8cf2dc0ed6e246 Mon Sep 17 00:00:00 2001 From: Orta Date: Fri, 9 Jul 2021 16:53:15 +0100 Subject: [PATCH 1/5] Add support for raising if you set a tsconfig entry of target/module with the right setting in the root --- src/compiler/commandLineParser.ts | 15 ++++++- src/compiler/diagnosticMessages.json | 4 ++ .../config/convertCompilerOptionsFromJson.ts | 41 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 09107e83df8f8..f2e24ed66d46e 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2890,10 +2890,23 @@ namespace ts { return; } }, - onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, _value: CompilerOptionsValue, _valueNode: Expression) { + onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, value: CompilerOptionsValue, _valueNode: Expression) { if (key === "excludes") { errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics.Unknown_option_excludes_Did_you_mean_exclude)); } + if (key === "target") { + const possibleValidEntries = arrayFrom(targetOptionDeclaration.type.keys()); + if (contains(possibleValidEntries, value)) { + errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, "target")); + } + } + if (key === "module") { + const moduleOption = commandOptionsWithoutBuild.find(opt => opt.name === "module")!; + const possibleValidEntries = arrayFrom((moduleOption.type as Map).keys()); + if (contains(possibleValidEntries, value)) { + errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, "module")); + } + } } }; const json = convertConfigFileToObject(sourceFile, errors, /*reportOptionsErrors*/ true, optionsIterator); diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 134588f89fe57..4315157f2a236 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4918,6 +4918,10 @@ "category": "Message", "code": 6257 }, + "'{0}' should be set inside the 'compilerOptions' object of the tsconfig.json": { + "category": "Error", + "code": 6258 + }, "Projects to reference": { "category": "Message", diff --git a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts index 71c826351d05a..0af7526f187b2 100644 --- a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts +++ b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts @@ -663,6 +663,47 @@ namespace ts { }); }); + + it("raises an error if you've set module to a correct string in the root", () => { + assertCompilerOptionsWithJsonText(`{ + "module": "esnext", + "compilerOptions": { + "target": "esnext" + } + }`, "tsconfig.json", { + compilerOptions: { + target: ScriptTarget.ESNext + }, + errors: [{ + ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, + messageText: "'module' should be set inside the 'compilerOptions' object of the tsconfig.json.", + file: undefined, + start: 0, + length: 0 + }] + }); + }); + + it("raises an error if you've set target to a correct string in the root", () => { + assertCompilerOptionsWithJsonText(`{ + "target": "esnext", + "compilerOptions": { + "module": "esnext" + } + }`, "tsconfig.json", { + compilerOptions: { + module: ModuleKind.ESNext + }, + errors: [{ + ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, + messageText: "'target' should be set inside the 'compilerOptions' object of the tsconfig.json.", + file: undefined, + start: 0, + length: 0 + }] + }); + }); + it("Don't crash when root expression is not objecty at all", () => { assertCompilerOptionsWithJsonText(`42`, "tsconfig.json", { compilerOptions: {}, From b0c822c009a8c5f927a870f2be5b95e17787c971 Mon Sep 17 00:00:00 2001 From: Orta Date: Tue, 20 Jul 2021 08:59:31 +0100 Subject: [PATCH 2/5] Switch to check for any compiler option in the root which doesn't include compilerOptions --- src/compiler/commandLineParser.ts | 22 ++++++++----------- src/compiler/diagnosticMessages.json | 2 +- .../config/convertCompilerOptionsFromJson.ts | 22 +++++-------------- 3 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index f2e24ed66d46e..68a021774b710 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2852,6 +2852,7 @@ namespace ts { let typeAcquisition: TypeAcquisition | undefined, typingOptionstypeAcquisition: TypeAcquisition | undefined; let watchOptions: WatchOptions | undefined; let extendedConfigPath: string | undefined; + const rootCompilerOptions: PropertyName[] = []; const optionsIterator: JsonConversionNotifier = { onSetValidOptionKeyValueInParent(parentOption: string, option: CommandLineOption, value: CompilerOptionsValue) { @@ -2890,22 +2891,12 @@ namespace ts { return; } }, - onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, value: CompilerOptionsValue, _valueNode: Expression) { + onSetUnknownOptionKeyValueInRoot(key: string, keyNode: PropertyName, _value: CompilerOptionsValue, _valueNode: Expression) { if (key === "excludes") { errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics.Unknown_option_excludes_Did_you_mean_exclude)); } - if (key === "target") { - const possibleValidEntries = arrayFrom(targetOptionDeclaration.type.keys()); - if (contains(possibleValidEntries, value)) { - errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, "target")); - } - } - if (key === "module") { - const moduleOption = commandOptionsWithoutBuild.find(opt => opt.name === "module")!; - const possibleValidEntries = arrayFrom((moduleOption.type as Map).keys()); - if (contains(possibleValidEntries, value)) { - errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, "module")); - } + if (find(commandOptionsWithoutBuild, (opt) => opt.name === key)) { + rootCompilerOptions.push(keyNode); } } }; @@ -2926,6 +2917,11 @@ namespace ts { } } + // eslint-disable-next-line no-in-operator + if (rootCompilerOptions.length && json && !("compilerOptions" in json)) { + errors.push(createDiagnosticForNodeInSourceFile(sourceFile, rootCompilerOptions[0], Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_config_json_file, getTextOfPropertyName(rootCompilerOptions[0]) as string)); + } + return { raw: json, options, watchOptions, typeAcquisition, extendedConfigPath }; } diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 4315157f2a236..a25220c48b4b3 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4918,7 +4918,7 @@ "category": "Message", "code": 6257 }, - "'{0}' should be set inside the 'compilerOptions' object of the tsconfig.json": { + "'{0}' should be set inside the 'compilerOptions' object of the config json file": { "category": "Error", "code": 6258 }, diff --git a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts index 0af7526f187b2..eec49764b13a2 100644 --- a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts +++ b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts @@ -663,20 +663,16 @@ namespace ts { }); }); - - it("raises an error if you've set module to a correct string in the root", () => { + it("raises an error if you've set a compiler flag in the root without including 'compilerOptions'", () => { assertCompilerOptionsWithJsonText(`{ "module": "esnext", - "compilerOptions": { - "target": "esnext" - } }`, "tsconfig.json", { compilerOptions: { target: ScriptTarget.ESNext }, errors: [{ - ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, - messageText: "'module' should be set inside the 'compilerOptions' object of the tsconfig.json.", + ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_config_json_file, + messageText: "'module' should be set inside the 'compilerOptions' object of the config json file.", file: undefined, start: 0, length: 0 @@ -684,7 +680,7 @@ namespace ts { }); }); - it("raises an error if you've set target to a correct string in the root", () => { + it("does not raise an error if you've set a compiler flag in the root when you have included 'compilerOptions'", () => { assertCompilerOptionsWithJsonText(`{ "target": "esnext", "compilerOptions": { @@ -694,17 +690,11 @@ namespace ts { compilerOptions: { module: ModuleKind.ESNext }, - errors: [{ - ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_tsconfig_json, - messageText: "'target' should be set inside the 'compilerOptions' object of the tsconfig.json.", - file: undefined, - start: 0, - length: 0 - }] + errors: [] }); }); - it("Don't crash when root expression is not objecty at all", () => { + it("Don't crash when root expression is not object at all", () => { assertCompilerOptionsWithJsonText(`42`, "tsconfig.json", { compilerOptions: {}, errors: [{ From db1c5d92c5b9ad40d04328595489b26aae808f0e Mon Sep 17 00:00:00 2001 From: Orta Date: Tue, 20 Jul 2021 14:51:42 +0100 Subject: [PATCH 3/5] Get green --- .../config/convertCompilerOptionsFromJson.ts | 4 +-- .../unittests/tsbuildWatch/programUpdates.ts | 8 ++++-- ...project-with-extended-config-is-removed.js | 25 +++++++++++-------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts index eec49764b13a2..f388f7e2b5d99 100644 --- a/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts +++ b/src/testRunner/unittests/config/convertCompilerOptionsFromJson.ts @@ -667,9 +667,7 @@ namespace ts { assertCompilerOptionsWithJsonText(`{ "module": "esnext", }`, "tsconfig.json", { - compilerOptions: { - target: ScriptTarget.ESNext - }, + compilerOptions: {}, errors: [{ ...Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_config_json_file, messageText: "'module' should be set inside the 'compilerOptions' object of the config json file.", diff --git a/src/testRunner/unittests/tsbuildWatch/programUpdates.ts b/src/testRunner/unittests/tsbuildWatch/programUpdates.ts index 011bebf2e66a1..5d63b9b47e157 100644 --- a/src/testRunner/unittests/tsbuildWatch/programUpdates.ts +++ b/src/testRunner/unittests/tsbuildWatch/programUpdates.ts @@ -718,7 +718,9 @@ export function someFn() { }`), const alphaExtendedConfigFile: File = { path: "/a/b/alpha.tsconfig.json", content: JSON.stringify({ - strict: true + compilerOptions: { + strict: true + } }) }; const project1Config: File = { @@ -734,7 +736,9 @@ export function someFn() { }`), const bravoExtendedConfigFile: File = { path: "/a/b/bravo.tsconfig.json", content: JSON.stringify({ - strict: true + compilerOptions: { + strict: true + } }) }; const otherFile: File = { diff --git a/tests/baselines/reference/tsbuild/watchMode/programUpdates/works-correctly-when-project-with-extended-config-is-removed.js b/tests/baselines/reference/tsbuild/watchMode/programUpdates/works-correctly-when-project-with-extended-config-is-removed.js index 16d987b8ad248..b389fa1e55e38 100644 --- a/tests/baselines/reference/tsbuild/watchMode/programUpdates/works-correctly-when-project-with-extended-config-is-removed.js +++ b/tests/baselines/reference/tsbuild/watchMode/programUpdates/works-correctly-when-project-with-extended-config-is-removed.js @@ -16,7 +16,7 @@ interface Array { length: number; [n: number]: T; } {"references":[{"path":"./project1.tsconfig.json"},{"path":"./project2.tsconfig.json"}],"files":[]} //// [/a/b/alpha.tsconfig.json] -{"strict":true} +{"compilerOptions":{"strict":true}} //// [/a/b/project1.tsconfig.json] {"extends":"./alpha.tsconfig.json","compilerOptions":{"composite":true},"files":["/a/b/commonFile1.ts","/a/b/commonFile2.ts"]} @@ -28,7 +28,7 @@ let x = 1 let y = 1 //// [/a/b/bravo.tsconfig.json] -{"strict":true} +{"compilerOptions":{"strict":true}} //// [/a/b/project2.tsconfig.json] {"extends":"./bravo.tsconfig.json","compilerOptions":{"composite":true},"files":["/a/b/other.ts"]} @@ -60,7 +60,7 @@ Output:: Program root files: ["/a/b/commonFile1.ts","/a/b/commonFile2.ts"] -Program options: {"composite":true,"watch":true,"configFilePath":"/a/b/project1.tsconfig.json"} +Program options: {"strict":true,"composite":true,"watch":true,"configFilePath":"/a/b/project1.tsconfig.json"} Program structureReused: Not Program files:: /a/lib/lib.d.ts @@ -78,7 +78,7 @@ Shape signatures in builder refreshed for:: /a/b/commonfile2.ts (used version) Program root files: ["/a/b/other.ts"] -Program options: {"composite":true,"watch":true,"configFilePath":"/a/b/project2.tsconfig.json"} +Program options: {"strict":true,"composite":true,"watch":true,"configFilePath":"/a/b/project2.tsconfig.json"} Program structureReused: Not Program files:: /a/lib/lib.d.ts @@ -117,6 +117,7 @@ FsWatchesRecursive:: exitCode:: ExitStatus.undefined //// [/a/b/commonFile1.js] +"use strict"; var x = 1; @@ -125,6 +126,7 @@ declare let x: number; //// [/a/b/commonFile2.js] +"use strict"; var y = 1; @@ -133,7 +135,7 @@ declare let y: number; //// [/a/b/project1.tsconfig.tsbuildinfo] -{"program":{"fileNames":["../lib/lib.d.ts","./commonfile1.ts","./commonfile2.ts"],"fileInfos":[{"version":"-7698705165-/// \ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array { length: number; [n: number]: T; }","affectsGlobalScope":true},{"version":"2167136208-let x = 1","affectsGlobalScope":true},{"version":"2168322129-let y = 1","affectsGlobalScope":true}],"options":{"composite":true},"referencedMap":[],"exportedModulesMap":[],"semanticDiagnosticsPerFile":[2,3,1]},"version":"FakeTSVersion"} +{"program":{"fileNames":["../lib/lib.d.ts","./commonfile1.ts","./commonfile2.ts"],"fileInfos":[{"version":"-7698705165-/// \ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array { length: number; [n: number]: T; }","affectsGlobalScope":true},{"version":"2167136208-let x = 1","affectsGlobalScope":true},{"version":"2168322129-let y = 1","affectsGlobalScope":true}],"options":{"composite":true,"strict":true},"referencedMap":[],"exportedModulesMap":[],"semanticDiagnosticsPerFile":[2,3,1]},"version":"FakeTSVersion"} //// [/a/b/project1.tsconfig.tsbuildinfo.readable.baseline.txt] { @@ -161,7 +163,8 @@ declare let y: number; } }, "options": { - "composite": true + "composite": true, + "strict": true }, "referencedMap": {}, "exportedModulesMap": {}, @@ -172,10 +175,11 @@ declare let y: number; ] }, "version": "FakeTSVersion", - "size": 753 + "size": 767 } //// [/a/b/other.js] +"use strict"; var z = 0; @@ -184,7 +188,7 @@ declare let z: number; //// [/a/b/project2.tsconfig.tsbuildinfo] -{"program":{"fileNames":["../lib/lib.d.ts","./other.ts"],"fileInfos":[{"version":"-7698705165-/// \ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array { length: number; [n: number]: T; }","affectsGlobalScope":true},{"version":"2874288940-let z = 0;","affectsGlobalScope":true}],"options":{"composite":true},"referencedMap":[],"exportedModulesMap":[],"semanticDiagnosticsPerFile":[2,1]},"version":"FakeTSVersion"} +{"program":{"fileNames":["../lib/lib.d.ts","./other.ts"],"fileInfos":[{"version":"-7698705165-/// \ninterface Boolean {}\ninterface Function {}\ninterface CallableFunction {}\ninterface NewableFunction {}\ninterface IArguments {}\ninterface Number { toExponential: any; }\ninterface Object {}\ninterface RegExp {}\ninterface String { charAt: any; }\ninterface Array { length: number; [n: number]: T; }","affectsGlobalScope":true},{"version":"2874288940-let z = 0;","affectsGlobalScope":true}],"options":{"composite":true,"strict":true},"referencedMap":[],"exportedModulesMap":[],"semanticDiagnosticsPerFile":[2,1]},"version":"FakeTSVersion"} //// [/a/b/project2.tsconfig.tsbuildinfo.readable.baseline.txt] { @@ -206,7 +210,8 @@ declare let z: number; } }, "options": { - "composite": true + "composite": true, + "strict": true }, "referencedMap": {}, "exportedModulesMap": {}, @@ -216,7 +221,7 @@ declare let z: number; ] }, "version": "FakeTSVersion", - "size": 666 + "size": 680 } From ebb5ed7df2b8e8745b0dbc2a99de8e29fc2071ad Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Fri, 23 Jul 2021 07:54:05 +0100 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Andrew Branch --- src/compiler/commandLineParser.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 68a021774b710..0796938c98c1f 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2852,7 +2852,7 @@ namespace ts { let typeAcquisition: TypeAcquisition | undefined, typingOptionstypeAcquisition: TypeAcquisition | undefined; let watchOptions: WatchOptions | undefined; let extendedConfigPath: string | undefined; - const rootCompilerOptions: PropertyName[] = []; + let rootCompilerOptions: PropertyName[] | undefined; const optionsIterator: JsonConversionNotifier = { onSetValidOptionKeyValueInParent(parentOption: string, option: CommandLineOption, value: CompilerOptionsValue) { @@ -2896,7 +2896,7 @@ namespace ts { errors.push(createDiagnosticForNodeInSourceFile(sourceFile, keyNode, Diagnostics.Unknown_option_excludes_Did_you_mean_exclude)); } if (find(commandOptionsWithoutBuild, (opt) => opt.name === key)) { - rootCompilerOptions.push(keyNode); + rootCompilerOptions = append(rootCompilerOptions, keyNode); } } }; @@ -2918,7 +2918,7 @@ namespace ts { } // eslint-disable-next-line no-in-operator - if (rootCompilerOptions.length && json && !("compilerOptions" in json)) { + if (rootCompilerOptions && json && !("compilerOptions" in json)) { errors.push(createDiagnosticForNodeInSourceFile(sourceFile, rootCompilerOptions[0], Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_config_json_file, getTextOfPropertyName(rootCompilerOptions[0]) as string)); } From bc0898c1bc62ef19a64b507bcc4b1be500ae0aa6 Mon Sep 17 00:00:00 2001 From: Orta Therox Date: Wed, 4 Aug 2021 09:56:01 +0100 Subject: [PATCH 5/5] Update src/compiler/commandLineParser.ts Co-authored-by: Daniel Rosenwasser --- src/compiler/commandLineParser.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 0796938c98c1f..233f034118668 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2917,8 +2917,7 @@ namespace ts { } } - // eslint-disable-next-line no-in-operator - if (rootCompilerOptions && json && !("compilerOptions" in json)) { + if (rootCompilerOptions && json && json.compilerOptions === undefined) { errors.push(createDiagnosticForNodeInSourceFile(sourceFile, rootCompilerOptions[0], Diagnostics._0_should_be_set_inside_the_compilerOptions_object_of_the_config_json_file, getTextOfPropertyName(rootCompilerOptions[0]) as string)); }