Addressing CR comments

jramsay · jramsay · commit cc3d4d3291e2 · 2016-03-08T15:17:09.000-08:00
- Adding check to ensure TypingOptions 'include' and 'exclude' arrays are composed of strings
- Allow leading whitespace when removing comments from json
diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -590,10 +590,10 @@ namespace ts {
                         }
                     }
                     else if (id === "include") {
-                        options.include = isArray(jsonTypingOptions[id]) ? <string[]>jsonTypingOptions[id] : [];
+                        options.include = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
                     }
                     else if (id === "exclude") {
-                        options.exclude = isArray(jsonTypingOptions[id]) ? <string[]>jsonTypingOptions[id] : [];
+                        options.exclude = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);
                     }
                     else {
                         errors.push(createCompilerDiagnostic(Diagnostics.Unknown_typing_option_0, id));
@@ -636,8 +636,8 @@ namespace ts {
                         }
                     }
                     if (opt.isFilePath) {
-                        value = normalizePath(combinePaths(basePath, value));
-                        if (value === "") {
+                       value = normalizePath(combinePaths(basePath, value));
+                       if (value === "") {
                             value = ".";
                         }
                     }
@@ -654,4 +654,28 @@ namespace ts {
 
         return { options, errors };
     }
+
+    function ConvertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {
+        let items: string[] = [];
+        let invalidOptionType = false;
+        if (!isArray(optionJson)) {
+            invalidOptionType = true;
+        }
+        else {
+            for (const element of <any[]>optionJson) {
+                if (typeof element === "string") {
+                    const item = func ? func(element) : element;
+                    items.push(item);
+                }
+                else {
+                    invalidOptionType = true;
+                    break;
+                }
+            }
+        }
+        if (invalidOptionType) {
+            errors.push(createCompilerDiagnostic(Diagnostics.Option_0_should_have_array_of_strings_as_a_value, optionName));
+        }
+        return items;
+    }
 }
diff --git a/src/services/jsTyping.ts b/src/services/jsTyping.ts
@@ -22,7 +22,7 @@ namespace ts.JsTyping {
         if (host.fileExists(jsonPath)) {
             try {
                 // Strip out single-line comments
-                const contents = host.readFile(jsonPath).replace(/^\/\/(.*)$/gm, "");
+                const contents = host.readFile(jsonPath).replace(/^\s*\/\/(.*)$/gm, "");
                 return JSON.parse(contents);
             }
             catch (e) { }
@@ -65,7 +65,7 @@ namespace ts.JsTyping {
             return { cachedTypingPaths: [], newTypingNames: [], filesToWatch: [] };
         }
 
-        const cachePath = projectRootPath ? projectRootPath : globalCachePath;
+        const cachePath = projectRootPath || globalCachePath;
         // Only infer typings for .js and .jsx files
         fileNames = fileNames
             .map(ts.normalizePath)
@@ -82,7 +82,7 @@ namespace ts.JsTyping {
         let exclude: string[] = [];
 
         mergeTypings(typingOptions.include);
-        exclude = typingOptions.exclude ? typingOptions.exclude : [];
+        exclude = typingOptions.exclude || [];
 
         if (typingOptions.enableAutoDiscovery) {
             const possibleSearchDirs = fileNames.map(ts.getDirectoryPath);

Original file line number	Diff line number	Diff line change
`@@ -590,10 +590,10 @@ namespace ts {`
`590`	`590`	`}`
`591`	`591`	`}`
`592`	`592`	`else if (id === "include") {`
`593`		`- options.include = isArray(jsonTypingOptions[id]) ? <string[]>jsonTypingOptions[id] : [];`
	`593`	`+ options.include = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`594`	`594`	`}`
`595`	`595`	`else if (id === "exclude") {`
`596`		`- options.exclude = isArray(jsonTypingOptions[id]) ? <string[]>jsonTypingOptions[id] : [];`
	`596`	`+ options.exclude = ConvertJsonOptionToStringArray(id, jsonTypingOptions[id], errors);`
`597`	`597`	`}`
`598`	`598`	`else {`
`599`	`599`	`errors.push(createCompilerDiagnostic(Diagnostics.Unknown_typing_option_0, id));`
`@@ -636,8 +636,8 @@ namespace ts {`
`636`	`636`	`}`
`637`	`637`	`}`
`638`	`638`	`if (opt.isFilePath) {`
`639`		`- value = normalizePath(combinePaths(basePath, value));`
`640`		`- if (value === "") {`
	`639`	`+ value = normalizePath(combinePaths(basePath, value));`
	`640`	`+ if (value === "") {`
`641`	`641`	`value = ".";`
`642`	`642`	`}`
`643`	`643`	`}`
`@@ -654,4 +654,28 @@ namespace ts {`
`654`	`654`
`655`	`655`	`return { options, errors };`
`656`	`656`	`}`
	`657`	`+`
	`658`	`+ function ConvertJsonOptionToStringArray(optionName: string, optionJson: any, errors: Diagnostic[], func?: (element: string) => string): string[] {`
	`659`	`+ let items: string[] = [];`
	`660`	`+ let invalidOptionType = false;`
	`661`	`+ if (!isArray(optionJson)) {`
	`662`	`+ invalidOptionType = true;`
	`663`	`+ }`
	`664`	`+ else {`
	`665`	`+ for (const element of <any[]>optionJson) {`
	`666`	`+ if (typeof element === "string") {`
	`667`	`+ const item = func ? func(element) : element;`
	`668`	`+ items.push(item);`
	`669`	`+ }`
	`670`	`+ else {`
	`671`	`+ invalidOptionType = true;`
	`672`	`+ break;`
	`673`	`+ }`
	`674`	`+ }`
	`675`	`+ }`
	`676`	`+ if (invalidOptionType) {`
	`677`	`+ errors.push(createCompilerDiagnostic(Diagnostics.Option_0_should_have_array_of_strings_as_a_value, optionName));`
	`678`	`+ }`
	`679`	`+ return items;`
	`680`	`+ }`
`657`	`681`	`}`