Add multi-pass environment variable resolution

Extension/.vscode/launch.json

@@ -24,7 +24,7 @@
 			"runtimeExecutable": "${execPath}",
 			"args": [
 				"--extensionDevelopmentPath=${workspaceFolder}",
-				"--extensionTestsPath=${workspaceFolder}/out/test"
+				"--extensionTestsPath=${workspaceFolder}/out/test/unitTests"
 			],
 			"stopOnEntry": false,
 			"sourceMaps": true,

Extension/gulpfile.js

@@ -18,7 +18,11 @@ gulp.task('allTests', () => {
 });
 
 gulp.task('unitTests', () => {
-    gulp.src('./out/test/unitTests', {read: false}).pipe(
+    env.set({
+            CODE_TESTS_PATH: "./out/test/unitTests",
+        }
+    );
+    gulp.src('./test/runVsCodeTestsWithAbsolutePaths.js', {read: false}).pipe(
         mocha({
             ui: "tdd"
         })

Extension/src/common.ts

@@ -178,54 +178,59 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st
     }
 
     // Replace environment and configuration variables.
-    let regexp: RegExp = /\$\{((env|config|workspaceFolder)(.|:))?(.*?)\}/g;
-    let ret: string = input.replace(regexp, (match: string, ignored1: string, varType: string, ignored2: string, name: string) => {
-        // Historically, if the variable didn't have anything before the "." or ":"
-        // it was assumed to be an environment variable
-        if (varType === undefined) {
-            varType = "env";
-        }
-        let newValue: string = undefined;
-        switch (varType) {
-            case "env": {
-                let v: string | string[] = additionalEnvironment[name];
-                if (typeof v === "string") {
-                    newValue = v;
-                } else if (input === match && v instanceof Array) {
-                    newValue = v.join(";");
-                }
-                if (!newValue) {
-                    newValue = process.env[name];
-                }
-                break;
+    let regexp: () => RegExp = () => /\$\{((env|config|workspaceFolder)(\.|:))?(.*?)\}/g;
+    let ret: string = input;
+    let cycleCache: Set<string> = new Set();
+    while (!cycleCache.has(ret)) {
+        cycleCache.add(ret);
+        ret = ret.replace(regexp(), (match: string, ignored1: string, varType: string, ignored2: string, name: string) => {
+            // Historically, if the variable didn't have anything before the "." or ":"
+            // it was assumed to be an environment variable
+            if (varType === undefined) {
+                varType = "env";
             }
-            case "config": {
-                let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration();
-                if (config) {
-                    newValue = config.get<string>(name);
+            let newValue: string = undefined;
+            switch (varType) {
+                case "env": {
+                    let v: string | string[] = additionalEnvironment[name];
+                    if (typeof v === "string") {
+                        newValue = v;
+                    } else if (input === match && v instanceof Array) {
+                        newValue = v.join(";");
+                    }
+                    if (!newValue) {
+                        newValue = process.env[name];
+                    }
+                    break;
                 }
-                break;
-            }
-            case "workspaceFolder": {
-                // Only replace ${workspaceFolder:name} variables for now.
-                // We may consider doing replacement of ${workspaceFolder} here later, but we would have to update the language server and also
-                // intercept messages with paths in them and add the ${workspaceFolder} variable back in (e.g. for light bulb suggestions)
-                if (name && vscode.workspace && vscode.workspace.workspaceFolders) {
-                    let folder: vscode.WorkspaceFolder = vscode.workspace.workspaceFolders.find(folder => folder.name.toLocaleLowerCase() === name.toLocaleLowerCase());
-                    if (folder) {
-                        newValue = folder.uri.fsPath;
+                case "config": {
+                    let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration();
+                    if (config) {
+                        newValue = config.get<string>(name);
+                    }
+                    break;
+                }
+                case "workspaceFolder": {
+                    // Only replace ${workspaceFolder:name} variables for now.
+                    // We may consider doing replacement of ${workspaceFolder} here later, but we would have to update the language server and also
+                    // intercept messages with paths in them and add the ${workspaceFolder} variable back in (e.g. for light bulb suggestions)
+                    if (name && vscode.workspace && vscode.workspace.workspaceFolders) {
+                        let folder: vscode.WorkspaceFolder = vscode.workspace.workspaceFolders.find(folder => folder.name.toLocaleLowerCase() === name.toLocaleLowerCase());
+                        if (folder) {
+                            newValue = folder.uri.fsPath;
+                        }
                     }
+                    break;
                 }
-                break;
+                default: { assert.fail("unknown varType matched"); }
             }
-            default: { assert.fail("unknown varType matched"); }
-        }
-        return (newValue) ? newValue : match;
-    });
+            return (newValue) ? newValue : match;
+        });
+    }
 
     // Resolve '~' at the start of the path.
-    regexp = /^\~/g;
-    ret = ret.replace(regexp, (match: string, name: string) => {
+    regexp = () => /^\~/g;
+    ret = ret.replace(regexp(), (match: string, name: string) => {
         let newValue: string = process.env.HOME;
         return (newValue) ? newValue : match;
     });

Extension/test/unitTests/common.test.ts

@@ -0,0 +1,226 @@
+/* --------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All Rights Reserved.
+ * See 'LICENSE' in the project root for license information.
+ * ------------------------------------------------------------------------------------------ */
+
+import * as assert from "assert";
+import {  resolveVariables } from "../../src/common";
+
+suite("Common Utility validation", () => {
+    suite("resolveVariables", () => {
+        const success: string = "success";
+        const home: string = process.env.HOME;
+
+        test("raw input", () => {
+            const input: string = "test";
+            inputAndEnvironment(input, {})
+                .shouldResolveTo(input);
+        });
+
+        test("raw input with tilde", () => {
+            inputAndEnvironment("~/test", {})
+                .shouldResolveTo(`${home}/test`);
+        });
+
+        test("env input with tilde", () => {
+            inputAndEnvironment("${path}/test", {
+                    path: home
+                })
+                .shouldResolveTo(`${home}/test`);
+        });
+
+        test("solo env input resulting in array", () => {
+            inputAndEnvironment("${test}", {
+                    test: ["foo", "bar"]
+                })
+                .shouldResolveTo("foo;bar");
+        });
+
+        test("mixed raw and env input resulting in array", () => {
+            const input: string = "baz${test}";
+            resolveVariablesWithInput(input)
+                .withEnvironment({
+                    test: ["foo", "bar"]
+                })
+                .shouldResolveTo(input);
+        });
+
+        test("solo env input not in env config finds process env", () => {
+            const processKey: string = `cpptoolstests_${Date.now()}`;
+            const input: string = "foo${" + processKey + "}";
+            let actual: string;
+            try {
+                process.env[processKey] = "bar";
+                actual = resolveVariables(input, {});
+            } finally {
+                delete process.env[processKey];
+            }
+            assert.equal(actual, "foobar");
+        });
+
+        test("env input", () => {
+            resolveVariablesWithInput("${test}")
+                .withEnvironment({
+                    "test": success
+                })
+                .shouldResolveTo(success);
+        });
+
+        test("env input mixed with plain text", () => {
+            resolveVariablesWithInput("${test}bar")
+                .withEnvironment({
+                    "test": "foo"
+                })
+                .shouldResolveTo("foobar");
+        });
+
+        test("env input with two variables", () => {
+            resolveVariablesWithInput("f${a}${b}r")
+                .withEnvironment({
+                    a: "oo",
+                    b: "ba"
+                })
+                .shouldResolveTo("foobar");
+        });
+
+        test("env input not in env", () => {
+            const input: string = "${test}";
+            resolveVariablesWithInput(input)
+                .withEnvironment({})
+                .shouldResolveTo(input);
+        });
+
+        test("env with macro inside environment definition", () => {
+            resolveVariablesWithInput("${arm6.include}")
+                .withEnvironment({
+                    "envRoot": "apps/tool/buildenv",
+                    "arm6.include": "${envRoot}/arm6/include"
+                })
+                .shouldResolveTo("apps/tool/buildenv/arm6/include");
+        });
+
+        test("env nested with half open variable", () => {
+            resolveVariablesWithInput("${arm6.include}")
+                .withEnvironment({
+                    "envRoot": "apps/tool/buildenv",
+                    "arm6.include": "${envRoot/arm6/include"
+                })
+                .shouldResolveTo("${envRoot/arm6/include");
+        });
+
+        test("env nested with half closed variable", () => {
+            resolveVariablesWithInput("${arm6.include}")
+                .withEnvironment({
+                    "envRoot": "apps/tool/buildenv",
+                    "arm6.include": "envRoot}/arm6/include"
+                })
+                .shouldResolveTo("envRoot}/arm6/include");
+        });
+
+        test("env nested with a cycle", () => {
+            resolveVariablesWithInput("${a}")
+                .withEnvironment({
+                    "a": "${b}",
+                    "b": "${c}",
+                    "c": "${a}"
+                })
+                .shouldResolveTo("${a}");
+        });
+
+        test("env input with 1 level of nested variables anchored at end", () => {
+            resolveVariablesWithInput("${foo${test}}")
+                .withEnvironment({
+                    "foobar": success,
+                    "test": "bar"
+                })
+                .shouldResolveTo("${foo${test}}");
+        });
+
+        test("env input with 1 level of nested variables anchored in the middle", () => {
+            resolveVariablesWithInput("${f${test}r}")
+                .withEnvironment({
+                    "foobar": success,
+                    "test": "ooba"
+                })
+                .shouldResolveTo("${f${test}r}");
+        });
+
+        test("env input with 1 level of nested variable anchored at front", () => {
+            resolveVariablesWithInput("${${test}bar}")
+                .withEnvironment({
+                    "foobar": success,
+                    "test": "foo"
+                })
+                .shouldResolveTo("${${test}bar}");
+        });
+
+        test("env input with 3 levels of nested variables", () => {
+            resolveVariablesWithInput("${foo${a${b${c}}}}")
+                .withEnvironment({
+                    "foobar": success,
+                    "a1": "bar",
+                    "b2": "1",
+                    "c": "2"
+                })
+                .shouldResolveTo("${foo${a${b${c}}}}");
+        });
+
+        test("env input contains env", () => {
+            resolveVariablesWithInput("${envRoot}")
+                .shouldLookupSymbol("envRoot");
+        });
+
+        test("env input contains config", () => {
+            resolveVariablesWithInput("${configRoot}")
+                .shouldLookupSymbol("configRoot");
+        });
+
+        test("env input contains workspaceFolder", () => {
+            resolveVariablesWithInput("${workspaceFolderRoot}")
+                .shouldLookupSymbol("workspaceFolderRoot");
+        });
+
+        test("input contains env.", () => {
+            resolveVariablesWithInput("${env.Root}")
+                .shouldLookupSymbol("Root");
+        });
+
+        test("input contains env:", () => {
+            resolveVariablesWithInput("${env:Root}")
+                .shouldLookupSymbol("Root");
+        });
+
+        interface ResolveTestFlowEnvironment {
+            withEnvironment(additionalEnvironment: {[key: string]: string | string[]}): ResolveTestFlowAssert;
+            shouldLookupSymbol: (key: string) => void;
+        }
+        interface ResolveTestFlowAssert {
+            shouldResolveTo: (x: string) => void;
+        }
+
+        function resolveVariablesWithInput(input: string): ResolveTestFlowEnvironment {
+            return {
+                withEnvironment: (additionalEnvironment: {[key: string]: string | string[]}) => {
+                    return inputAndEnvironment(input, additionalEnvironment);
+                },
+                shouldLookupSymbol: (symbol: string) => {
+                    const environment: {[key: string]: string | string[]} = {};
+                    environment[symbol] = success;
+                    return inputAndEnvironment(input, environment)
+                        .shouldResolveTo(success);
+                }
+            };
+        }
+
+        function inputAndEnvironment(input: string, additionalEnvironment: {[key: string]: string | string[]}): ResolveTestFlowAssert {
+            return {
+                    shouldResolveTo: (expected: string) => {
+                    const actual: string = resolveVariables(input, additionalEnvironment);
+                    const msg: string = `Expected ${expected}. Got ${actual} with input ${input} and environment ${JSON.stringify(additionalEnvironment)}.`;
+                    assert.equal(actual, expected, msg);
+                }
+            };
+        }
+
+    });
+});