Merge pull request microsoft#27440 from Microsoft/invalidTypes

Only copy non error values in array when converting the json

Author: sheetalkamat

src/compiler/commandLineParser.ts

@@ -1529,7 +1529,12 @@ namespace ts {
             elements: NodeArray<Expression>,
             elementOption: CommandLineOption | undefined
         ): any[] | void {
-            return (returnValue ? elements.map : elements.forEach).call(elements, (element: Expression) => convertPropertyValueToJson(element, elementOption));
+            if (!returnValue) {
+                return elements.forEach(element => convertPropertyValueToJson(element, elementOption));
+            }
+
+            // Filter out invalid values
+            return filter(elements.map(element => convertPropertyValueToJson(element, elementOption)), v => v !== undefined);
         }
 
         function convertPropertyValueToJson(valueExpression: Expression, option: CommandLineOption | undefined): any {

src/testRunner/unittests/convertCompilerOptionsFromJson.ts

@@ -1,33 +1,50 @@
 namespace ts {
     describe("convertCompilerOptionsFromJson", () => {
-        function assertCompilerOptions(json: any, configFileName: string, expectedResult: { compilerOptions: CompilerOptions, errors: Diagnostic[] }) {
+        const formatDiagnosticHost: FormatDiagnosticsHost = {
+            getCurrentDirectory: () => "/apath/",
+            getCanonicalFileName: createGetCanonicalFileName(/*useCaseSensitiveFileNames*/ true),
+            getNewLine: () => "\n"
+        };
+
+        interface ExpectedResultWithParsingSuccess {
+            compilerOptions: CompilerOptions;
+            errors: ReadonlyArray<Diagnostic>;
+        }
+
+        interface ExpectedResultWithParsingFailure {
+            compilerOptions: CompilerOptions;
+            hasParseErrors: true;
+        }
+
+        type ExpectedResult = ExpectedResultWithParsingSuccess | ExpectedResultWithParsingFailure;
+
+        function isExpectedResultWithParsingFailure(expectedResult: ExpectedResult): expectedResult is ExpectedResultWithParsingFailure {
+            return !!(expectedResult as ExpectedResultWithParsingFailure).hasParseErrors;
+        }
+
+        function assertCompilerOptions(json: any, configFileName: string, expectedResult: ExpectedResultWithParsingSuccess) {
             assertCompilerOptionsWithJson(json, configFileName, expectedResult);
             assertCompilerOptionsWithJsonNode(json, configFileName, expectedResult);
         }
 
-        function assertCompilerOptionsWithJson(json: any, configFileName: string, expectedResult: { compilerOptions: CompilerOptions, errors: Diagnostic[] }) {
-            const { options: actualCompilerOptions, errors: actualErrors} = convertCompilerOptionsFromJson(json.compilerOptions, "/apath/", configFileName);
+        function assertCompilerOptionsWithJson(json: any, configFileName: string, expectedResult: ExpectedResultWithParsingSuccess) {
+            const { options: actualCompilerOptions, errors: actualErrors } = convertCompilerOptionsFromJson(json.compilerOptions, "/apath/", configFileName);
 
             const parsedCompilerOptions = JSON.stringify(actualCompilerOptions);
             const expectedCompilerOptions = JSON.stringify({ ...expectedResult.compilerOptions, configFilePath: configFileName });
             assert.equal(parsedCompilerOptions, expectedCompilerOptions);
 
-            const expectedErrors = expectedResult.errors;
-            assert.isTrue(expectedResult.errors.length === actualErrors.length, `Expected error: ${JSON.stringify(expectedResult.errors)}. Actual error: ${JSON.stringify(actualErrors)}.`);
-            for (let i = 0; i < actualErrors.length; i++) {
-                const actualError = actualErrors[i];
-                const expectedError = expectedErrors[i];
-                assert.equal(actualError.code, expectedError.code);
-                assert.equal(actualError.category, expectedError.category);
-                assert.equal(actualError.messageText, expectedError.messageText);
-            }
+            verifyErrors(actualErrors, expectedResult.errors, /*ignoreLocation*/ true);
         }
 
-        function assertCompilerOptionsWithJsonNode(json: any, configFileName: string, expectedResult: { compilerOptions: CompilerOptions, errors: Diagnostic[] }) {
-            const fileText = JSON.stringify(json);
+        function assertCompilerOptionsWithJsonNode(json: any, configFileName: string, expectedResult: ExpectedResultWithParsingSuccess) {
+            assertCompilerOptionsWithJsonText(JSON.stringify(json), configFileName, expectedResult);
+        }
+
+        function assertCompilerOptionsWithJsonText(fileText: string, configFileName: string, expectedResult: ExpectedResult) {
             const result = parseJsonText(configFileName, fileText);
-            assert(!result.parseDiagnostics.length);
             assert(!!result.endOfFileToken);
+            assert.equal(!!result.parseDiagnostics.length, isExpectedResultWithParsingFailure(expectedResult));
             const host: ParseConfigHost = new fakes.ParseConfigHost(new vfs.FileSystem(/*ignoreCase*/ false, { cwd: "/apath/" }));
             const { options: actualCompilerOptions, errors: actualParseErrors } = parseJsonSourceFileConfigFileContent(result, host, "/apath/", /*existingOptions*/ undefined, configFileName);
             expectedResult.compilerOptions.configFilePath = configFileName;
@@ -37,17 +54,32 @@ namespace ts {
             assert.equal(parsedCompilerOptions, expectedCompilerOptions);
             assert.equal(actualCompilerOptions.configFile, result);
 
-            const actualErrors = filter(actualParseErrors, error => error.code !== Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2.code);
-            const expectedErrors = expectedResult.errors;
-            assert.isTrue(expectedResult.errors.length === actualErrors.length, `Expected error: ${JSON.stringify(expectedResult.errors)}. Actual error: ${JSON.stringify(actualErrors)}.`);
+            if (!isExpectedResultWithParsingFailure(expectedResult)) {
+                verifyErrors(actualParseErrors.filter(error => error.code !== Diagnostics.No_inputs_were_found_in_config_file_0_Specified_include_paths_were_1_and_exclude_paths_were_2.code), expectedResult.errors);
+            }
+        }
+
+        function verifyErrors(actualErrors: Diagnostic[], expectedErrors: ReadonlyArray<Diagnostic>, ignoreLocation?: boolean) {
+            assert.isTrue(expectedErrors.length === actualErrors.length, `Expected error: ${JSON.stringify(expectedErrors.map(getDiagnosticString), undefined, " ")}. Actual error: ${JSON.stringify(actualErrors.map(getDiagnosticString), undefined, " ")}.`);
             for (let i = 0; i < actualErrors.length; i++) {
                 const actualError = actualErrors[i];
                 const expectedError = expectedErrors[i];
+
                 assert.equal(actualError.code, expectedError.code, `Expected error-code: ${JSON.stringify(expectedError.code)}. Actual error-code: ${JSON.stringify(actualError.code)}.`);
                 assert.equal(actualError.category, expectedError.category, `Expected error-category: ${JSON.stringify(expectedError.category)}. Actual error-category: ${JSON.stringify(actualError.category)}.`);
-                assert(actualError.file);
-                assert(actualError.start);
-                assert(actualError.length);
+                if (!ignoreLocation) {
+                    assert(actualError.file);
+                    assert(actualError.start);
+                    assert(actualError.length);
+                }
+            }
+
+            function getDiagnosticString(diagnostic: Diagnostic) {
+                if (ignoreLocation) {
+                    const { file, ...rest } = diagnostic;
+                    diagnostic = { file: undefined, ...rest };
+                }
+                return formatDiagnostic(diagnostic, formatDiagnosticHost);
             }
         }
 
@@ -539,5 +571,37 @@ namespace ts {
                 }
             );
         });
+
+        it("Convert tsconfig options when there are multiple invalid strings", () => {
+            assertCompilerOptionsWithJsonText(`{
+  "compilerOptions": {
+    "target": "<%- options.useTsWithBabel ? 'esnext' : 'es5' %>",
+    "module": "esnext",
+    <%_ if (options.classComponent) { _%>
+    "experimentalDecorators": true,
+    <%_ } _%>
+    "sourceMap": true,
+    "types": [
+      "webpack-env"<% if (hasMocha || hasJest) { %>,<% } %>
+      <%_ if (hasMocha) { _%>
+      "mocha",
+      "chai"
+      <%_ } else if (hasJest) { _%>
+      "jest"
+      <%_ } _%>
+    ]
+  }
+}
+`, "tsconfig.json",
+                {
+                    compilerOptions: {
+                        target: undefined,
+                        module: ModuleKind.ESNext,
+                        types: []
+                    },
+                    hasParseErrors: true
+                }
+            );
+        });
     });
 }