Resolve empty array environment variables microsoft#2532 (microsoft#2534)

bobbrow · web-flow · commit ad451bb438ab · 2018-09-20T16:15:32.000-07:00
diff --git a/Extension/CHANGELOG.md b/Extension/CHANGELOG.md
@@ -1,5 +1,10 @@
 # C/C++ for Visual Studio Code Change Log
 
+## Version 0.19.0: September 26, 2018
+* Add support for v2 of the configuration provider API. [#2237](https://github.com/Microsoft/vscode-cpptools/issues/2237)
+* Add `C_Cpp.updateChannel` setting for easier access to Insider builds of the extension. [#1526](https://github.com/Microsoft/vscode-cpptools/issues/1526)
+* Fix bug with variable resolution. [#2532](https://github.com/Microsoft/vscode-cpptools/issues/2532)
+
 ## Version 0.18.1: August 17, 2018
 * Fix 0.18.0 regression causing non-MinGW compilers to use `-fms-extensions` on Windows. [#2424](https://github.com/Microsoft/vscode-cpptools/issues/2424), [#2425](https://github.com/Microsoft/vscode-cpptools/issues/2425)
 
diff --git a/Extension/src/common.ts b/Extension/src/common.ts
@@ -176,19 +176,19 @@ export function isUri(input: any): input is vscode.Uri {
 }
 
 export function isString(input: any): input is string {
-    return input && typeof(input) === "string";
+    return typeof(input) === "string";
 }
 
 export function isOptionalString(input: any): input is string|undefined {
-    return input === undefined || typeof(input) === "string";
+    return input === undefined || isString(input);
 }
 
 export function isArrayOfString(input: any): input is string[] {
-    return input && (input instanceof Array) && input.every(item => typeof(item) === "string");
+    return (input instanceof Array) && input.every(item => isString(item));
 }
 
 export function isOptionalArrayOfString(input: any): input is string[]|undefined {
-    return input === undefined || ((input instanceof Array) && input.every(item => typeof(item) === "string"));
+    return input === undefined || isArrayOfString(input);
 }
 
 export function resolveVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string {
@@ -211,7 +211,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st
             if (varType === undefined) {
                 varType = "env";
             }
-            let newValue: string = undefined;
+            let newValue: string;
             switch (varType) {
                 case "env": {
                     let v: string | string[] = additionalEnvironment[name];
@@ -220,7 +220,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st
                     } else if (input === match && isArrayOfString(v)) {
                         newValue = v.join(";");
                     }
-                    if (!newValue) {
+                    if (!isString(newValue)) {
                         newValue = process.env[name];
                     }
                     break;
@@ -246,7 +246,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st
                 }
                 default: { assert.fail("unknown varType matched"); }
             }
-            return (newValue) ? newValue : match;
+            return (isString(newValue)) ? newValue : match;
         });
     }
 
diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts
@@ -36,6 +36,14 @@ suite("Common Utility validation", () => {
                 .shouldResolveTo("foo;bar");
         });
 
+        test("solo env input with empty array env value", () => {
+            resolveVariablesWithInput("${empty}")
+                .withEnvironment({
+                    "empty": []
+                })
+                .shouldResolveTo("");
+        });
+
         test("mixed raw and env input resulting in array", () => {
             const input: string = "baz${test}";
             resolveVariablesWithInput(input)

Original file line number	Diff line number	Diff line change
`@@ -176,19 +176,19 @@ export function isUri(input: any): input is vscode.Uri {`
`176`	`176`	`}`
`177`	`177`
`178`	`178`	`export function isString(input: any): input is string {`
`179`		`- return input && typeof(input) === "string";`
	`179`	`+ return typeof(input) === "string";`
`180`	`180`	`}`
`181`	`181`
`182`	`182`	`export function isOptionalString(input: any): input is string\|undefined {`
`183`		`- return input === undefined \|\| typeof(input) === "string";`
	`183`	`+ return input === undefined \|\| isString(input);`
`184`	`184`	`}`
`185`	`185`
`186`	`186`	`export function isArrayOfString(input: any): input is string[] {`
`187`		`- return input && (input instanceof Array) && input.every(item => typeof(item) === "string");`
	`187`	`+ return (input instanceof Array) && input.every(item => isString(item));`
`188`	`188`	`}`
`189`	`189`
`190`	`190`	`export function isOptionalArrayOfString(input: any): input is string[]\|undefined {`
`191`		`- return input === undefined \|\| ((input instanceof Array) && input.every(item => typeof(item) === "string"));`
	`191`	`+ return input === undefined \|\| isArrayOfString(input);`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`export function resolveVariables(input: string, additionalEnvironment: {[key: string]: string \| string[]}): string {`
`@@ -211,7 +211,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st`
`211`	`211`	`if (varType === undefined) {`
`212`	`212`	`varType = "env";`
`213`	`213`	`}`
`214`		`- let newValue: string = undefined;`
	`214`	`+ let newValue: string;`
`215`	`215`	`switch (varType) {`
`216`	`216`	`case "env": {`
`217`	`217`	`let v: string \| string[] = additionalEnvironment[name];`
`@@ -220,7 +220,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st`
`220`	`220`	`} else if (input === match && isArrayOfString(v)) {`
`221`	`221`	`newValue = v.join(";");`
`222`	`222`	`}`
`223`		`- if (!newValue) {`
	`223`	`+ if (!isString(newValue)) {`
`224`	`224`	`newValue = process.env[name];`
`225`	`225`	`}`
`226`	`226`	`break;`
`@@ -246,7 +246,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st`
`246`	`246`	`}`
`247`	`247`	`default: { assert.fail("unknown varType matched"); }`
`248`	`248`	`}`
`249`		`- return (newValue) ? newValue : match;`
	`249`	`+ return (isString(newValue)) ? newValue : match;`
`250`	`250`	`});`
`251`	`251`	`}`
`252`	`252`