Respond to code review comments

mhegazy · mhegazy · commit c735a98523e1 · 2015-09-15T12:47:11.000-07:00
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -125,7 +125,7 @@ module FourSlash {
     // List of allowed metadata names
     let fileMetadataNames = [metadataOptionNames.fileName, metadataOptionNames.emitThisFile, metadataOptionNames.resolveReference];
 
-    function convertGlobalOptionsToCompilerOptions(globalOptions: { [idx: string]: string }): ts.CompilerOptions {
+    function convertGlobalOptionsToCompilerOptions(globalOptions: Harness.TestCaseParser.CompilerSettings): ts.CompilerOptions {
         let settings: ts.CompilerOptions = { target: ts.ScriptTarget.ES5 };
         Harness.Compiler.setCompilerOptionsFromHarnessSetting(globalOptions, settings);
         return settings;
diff --git a/src/harness/harness.ts b/src/harness/harness.ts
@@ -949,7 +949,7 @@ module Harness {
             };
         }
 
-        interface HarnesOptions {
+        interface HarnessOptions {
             useCaseSensitiveFileNames?: boolean;
             includeBuiltFile?: string;
             baselineFile?: string;
@@ -977,10 +977,13 @@ module Harness {
             return ts.lookUp(optionsIndex, name.toLowerCase());
         }
 
-        export function setCompilerOptionsFromHarnessSetting(settings: Harness.TestCaseParser.CompilerSettings, options: ts.CompilerOptions & HarnesOptions): void {
+        export function setCompilerOptionsFromHarnessSetting(settings: Harness.TestCaseParser.CompilerSettings, options: ts.CompilerOptions & HarnessOptions): void {
             for (let name in settings) {
                 if (settings.hasOwnProperty(name)) {
                     let value = settings[name];
+                    if (value === undefined) {
+                        throw new Error(`Cannot have undefined value for compiler option '${name}'.`);
+                    }
                     let option = getCommandLineOption(name);
                     if (option) {
                         switch (option.type) {
@@ -993,17 +996,17 @@ module Harness {
                             // If not a primitive, the possible types are specified in what is effectively a map of options.
                             default:
                                 let map = <ts.Map<number>>option.type;
-                                let key = (value).toLowerCase();
+                                let key = value.toLowerCase();
                                 if (ts.hasProperty(map, key)) {
                                     options[option.name] = map[key];
                                 }
                                 else {
-                                    throw new Error(`Unkown value '${value}' for compiler option '${name}'.`);
+                                    throw new Error(`Unknown value '${value}' for compiler option '${name}'.`);
                                 }
                         }
                     }
                     else {
-                        throw new Error(`Unkown compiler option '${name}'.`);
+                        throw new Error(`Unknown compiler option '${name}'.`);
                     }
                 }
             }
@@ -1062,7 +1065,7 @@ module Harness {
                 otherFiles: { unitName: string; content: string }[],
                 onComplete: (result: CompilerResult, program: ts.Program) => void,
                 settingsCallback?: (settings: ts.CompilerOptions) => void,
-                options?: ts.CompilerOptions & HarnesOptions,
+                options?: ts.CompilerOptions & HarnessOptions,
                 // Current directory is needed for rwcRunner to be able to use currentDirectory defined in json file
                 currentDirectory?: string) {
 

Original file line number	Diff line number	Diff line change
`@@ -949,7 +949,7 @@ module Harness {`
`949`	`949`	`};`
`950`	`950`	`}`
`951`	`951`
`952`		`- interface HarnesOptions {`
	`952`	`+ interface HarnessOptions {`
`953`	`953`	`useCaseSensitiveFileNames?: boolean;`
`954`	`954`	`includeBuiltFile?: string;`
`955`	`955`	`baselineFile?: string;`
`@@ -977,10 +977,13 @@ module Harness {`
`977`	`977`	`return ts.lookUp(optionsIndex, name.toLowerCase());`
`978`	`978`	`}`
`979`	`979`
`980`		`- export function setCompilerOptionsFromHarnessSetting(settings: Harness.TestCaseParser.CompilerSettings, options: ts.CompilerOptions & HarnesOptions): void {`
	`980`	`+ export function setCompilerOptionsFromHarnessSetting(settings: Harness.TestCaseParser.CompilerSettings, options: ts.CompilerOptions & HarnessOptions): void {`
`981`	`981`	`for (let name in settings) {`
`982`	`982`	`if (settings.hasOwnProperty(name)) {`
`983`	`983`	`let value = settings[name];`
	`984`	`+ if (value === undefined) {`
	`985`	+ throw new Error(`Cannot have undefined value for compiler option '${name}'.`);
	`986`	`+ }`
`984`	`987`	`let option = getCommandLineOption(name);`
`985`	`988`	`if (option) {`
`986`	`989`	`switch (option.type) {`
`@@ -993,17 +996,17 @@ module Harness {`
`993`	`996`	`// If not a primitive, the possible types are specified in what is effectively a map of options.`
`994`	`997`	`default:`
`995`	`998`	`let map = <ts.Map<number>>option.type;`
`996`		`- let key = (value).toLowerCase();`
	`999`	`+ let key = value.toLowerCase();`
`997`	`1000`	`if (ts.hasProperty(map, key)) {`
`998`	`1001`	`options[option.name] = map[key];`
`999`	`1002`	`}`
`1000`	`1003`	`else {`
`1001`		- throw new Error(`Unkown value '${value}' for compiler option '${name}'.`);
	`1004`	+ throw new Error(`Unknown value '${value}' for compiler option '${name}'.`);
`1002`	`1005`	`}`
`1003`	`1006`	`}`
`1004`	`1007`	`}`
`1005`	`1008`	`else {`
`1006`		- throw new Error(`Unkown compiler option '${name}'.`);
	`1009`	+ throw new Error(`Unknown compiler option '${name}'.`);
`1007`	`1010`	`}`
`1008`	`1011`	`}`
`1009`	`1012`	`}`
`@@ -1062,7 +1065,7 @@ module Harness {`
`1062`	`1065`	`otherFiles: { unitName: string; content: string }[],`
`1063`	`1066`	`onComplete: (result: CompilerResult, program: ts.Program) => void,`
`1064`	`1067`	`settingsCallback?: (settings: ts.CompilerOptions) => void,`
`1065`		`- options?: ts.CompilerOptions & HarnesOptions,`
	`1068`	`+ options?: ts.CompilerOptions & HarnessOptions,`
`1066`	`1069`	`// Current directory is needed for rwcRunner to be able to use currentDirectory defined in json file`
`1067`	`1070`	`currentDirectory?: string) {`
`1068`	`1071`