From 5e79538722f9425f2c6a6ba1f9d90bb7ed073247 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Mon, 23 Jul 2018 13:49:41 -0700 Subject: [PATCH 01/11] fix unit test launch.json target We were passing the root of the test folder instead of the unitTests specific folder into the runner, so it wasn't finding unit tests. --- Extension/.vscode/launch.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Extension/.vscode/launch.json b/Extension/.vscode/launch.json index 838793545..022eaed81 100644 --- a/Extension/.vscode/launch.json +++ b/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, From 5ff8006a3dd434245182ef359a0fcbf660e4a367 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Mon, 23 Jul 2018 13:50:30 -0700 Subject: [PATCH 02/11] fix res. of vars with env,config,workspaceFolder If a variable had env, config, or workspaceFolder as substring, it was being treated as an environment variable lookup on the reminder of the string minus the character immediately following the substring. For instance: envRoot was being looked up as "oot" in the configuarion environment block. The issue is that the regex to match the env., config., and workspaceFolder. patterns used . instead of \. --- Extension/src/common.ts | 2 +- Extension/test/unitTests/common.test.ts | 49 +++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 Extension/test/unitTests/common.test.ts diff --git a/Extension/src/common.ts b/Extension/src/common.ts index 4a7c613d5..570a9cc85 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -178,7 +178,7 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st } // Replace environment and configuration variables. - let regexp: RegExp = /\$\{((env|config|workspaceFolder)(.|:))?(.*?)\}/g; + 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 diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts new file mode 100644 index 000000000..5a4ade546 --- /dev/null +++ b/Extension/test/unitTests/common.test.ts @@ -0,0 +1,49 @@ +import * as assert from "assert"; +import * as vscode from "vscode"; +import { resolveVariables } from "../../src/common"; + +suite("Common Utility validation", () => { + suite("resolveVariables", () => { + test("raw input", () => { + const input = "test"; + assert.equal(resolveVariables(input, {}), input); + }); + + test("env input", () => { + shouldSuccessfullyLookupInEnv("${test}", "test"); + }); + + test("env input not in env", () => { + const input = "${test}"; + assert.equal(resolveVariables(input, {}), input); + }); + + test("env input contains env", () => { + shouldSuccessfullyLookupInEnv("${envRoot}", "envRoot"); + }); + + test("env input contains config", () => { + shouldSuccessfullyLookupInEnv("${configRoot}", "configRoot"); + }); + + test("env input contains workspaceFolder", () => { + shouldSuccessfullyLookupInEnv("${workspaceFolderRoot}", "workspaceFolderRoot"); + }); + + test("input contains env.", () => { + shouldSuccessfullyLookupInEnv("${env.Root}", "Root"); + }); + + test("input contains env:", () => { + shouldSuccessfullyLookupInEnv("${env:Root}", "Root"); + }); + + const shouldSuccessfullyLookupInEnv = (input: string, expectedResolvedKey: string) => { + const success = "success"; + const env = {}; + env[expectedResolvedKey] = success; + const result = resolveVariables(input, env); + assert.equal(result, success); + } + }); +}); \ No newline at end of file From 8736bb8bed6ac1ed872cc338eff7207c42cc65d8 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Mon, 23 Jul 2018 21:59:31 -0700 Subject: [PATCH 03/11] tests existing tag resolution functionality --- Extension/test/unitTests/common.test.ts | 69 +++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index 5a4ade546..f3f541b97 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -1,18 +1,81 @@ import * as assert from "assert"; -import * as vscode from "vscode"; -import { resolveVariables } from "../../src/common"; +import { resolveVariables } from "../../src/common"; suite("Common Utility validation", () => { suite("resolveVariables", () => { + const success = "success"; + const home = process.env.HOME; + test("raw input", () => { const input = "test"; assert.equal(resolveVariables(input, {}), input); }); + test("raw input with tilde", () => { + const input = "~/test"; + assert.equal(resolveVariables(input, {}), `${home}/test`) + }); + + test("env input with tilde", () => { + const input = "${path}/test"; + const actual = resolveVariables(input, { + path: home + }); + assert.equal(actual, `${home}/test`) + }); + + test("solo env input resulting in array", () => { + const input = "${test}"; + const actual = resolveVariables(input, { + test: ["foo", "bar"] + }); + assert.equal(actual, "foo;bar") + }); + + test("mixed raw and env input resulting in array", () => { + const input = "baz${test}"; + const actual = resolveVariables(input, { + test: ["foo", "bar"] + }); + assert.equal(actual, input) + }); + + test("solo env input not in env config finds process env", () => { + const processKey = `cpptoolstests_${Date.now()}`; + const input = "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", () => { shouldSuccessfullyLookupInEnv("${test}", "test"); }); + test("env input mixed with plain text", () => { + const input = "${test}bar"; + const env = { + test: "foo" + }; + const result = resolveVariables(input, env); + assert.equal(result, "foobar"); + }); + + test("env input with two variables", () => { + const input = "f${a}${b}r"; + const env = { + a: "oo", + b: "ba" + }; + const result = resolveVariables(input, env); + assert.equal(result, "foobar"); + }); + test("env input not in env", () => { const input = "${test}"; assert.equal(resolveVariables(input, {}), input); @@ -38,8 +101,8 @@ suite("Common Utility validation", () => { shouldSuccessfullyLookupInEnv("${env:Root}", "Root"); }); + const shouldSuccessfullyLookupInEnv = (input: string, expectedResolvedKey: string) => { - const success = "success"; const env = {}; env[expectedResolvedKey] = success; const result = resolveVariables(input, env); From 419afd00a27e4f4e8119bd9bc2f35187c09608ab Mon Sep 17 00:00:00 2001 From: John Patterson Date: Mon, 23 Jul 2018 23:41:20 -0700 Subject: [PATCH 04/11] rough draft --- Extension/src/common.ts | 152 +++++++++++++++++------- Extension/test/unitTests/common.test.ts | 41 +++++++ 2 files changed, 149 insertions(+), 44 deletions(-) diff --git a/Extension/src/common.ts b/Extension/src/common.ts index 570a9cc85..632fc697e 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -169,6 +169,104 @@ export function showReleaseNotes(): void { vscode.commands.executeCommand('vscode.previewHtml', vscode.Uri.file(getExtensionFilePath("ReleaseNotes.html")), vscode.ViewColumn.One, "C/C++ Extension Release Notes"); } +export function resolveInnerVariable(originalInput: string, wholeVariable: string, innerVariable: string, additionalEnvironment: {[key: string]: string | string[]}): string { + let varType: string; + let name: string = innerVariable.replace(/(env|config|workspaceFolder)[\.|:](.*)/, (ignored: string, type: string, rest: string) => { + varType = type; + return rest; + }); + + // 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 (originalInput === wholeVariable && v instanceof Array) { + newValue = v.join(";"); + } + if (!newValue) { + newValue = process.env[name]; + } + break; + } + case "config": { + let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(); + if (config) { + newValue = config.get(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; + } + default: { assert.fail("unknown varType matched"); } + } + return (newValue) ? newValue : wholeVariable; +} + +function indexOfMatchingCloseBrace(startingPos: number, input: string) { + let balanceFactor = 0; + for (let i = startingPos; i < input.length; i++) { + if (input[i] === "$" && i + 1 < input.length && input[i+1] === "{") { + balanceFactor += 1; + } else if (input[i] === "}") { + if (balanceFactor === 0) { + return i; + } else { + balanceFactor -= 1; + } + } + } + return -1; +} + +interface StackEntry { + prefix: string; + postfix: string; +} + +function expandVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { + let stack: StackEntry[] = []; + let stackInput = input; + while (stackInput !== "") { + const openIndex = stackInput.indexOf("${"); + if (-1 < openIndex) { + const closeIndex = indexOfMatchingCloseBrace(openIndex+2, stackInput); + stack.push({ + prefix: stackInput.substring(0, openIndex), + postfix: stackInput.substring(closeIndex + 1) + }); + stackInput = stackInput.substring(openIndex + 2, closeIndex); + } else { + break; + } + } + + let ret: string = stackInput; + while (stack.length !== 0) { + let stackEntry = stack.pop(); + let resolvedItem = resolveInnerVariable(input, "${" + ret + "}", ret, additionalEnvironment); + ret = stackEntry.prefix + resolvedItem + stackEntry.postfix; + } + + return ret; +} + export function resolveVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { if (!input) { return ""; @@ -178,53 +276,19 @@ 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 ret: string = input; + let openTagRegex = /\$\{/; + while (openTagRegex.test(ret)) { + let expansion = expandVariables(ret, additionalEnvironment); + // Needed to prevent infinite loop in the failed lookup case + if (expansion === ret) { + break; } - 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; - } - case "config": { - let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(); - if (config) { - newValue = config.get(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; - } - default: { assert.fail("unknown varType matched"); } - } - return (newValue) ? newValue : match; - }); + ret = expansion; + } // Resolve '~' at the start of the path. - regexp = /^\~/g; + let regexp: RegExp = /^\~/g; ret = ret.replace(regexp, (match: string, name: string) => { let newValue: string = process.env.HOME; return (newValue) ? newValue : match; diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index f3f541b97..12d21f27d 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -81,6 +81,47 @@ suite("Common Utility validation", () => { assert.equal(resolveVariables(input, {}), input); }); + test("env input with 1 level of nested variables anchored at end", () => { + const input = "${foo${test}}"; + + const env = { + "foobar": success, + "test": "bar" + }; + assert.equal(resolveVariables(input, env), success); + }); + + test("env input with 1 level of nested variables anchored in the middle", () => { + const input = "${f${test}r}"; + + const env = { + "foobar": success, + "test": "ooba" + }; + assert.equal(resolveVariables(input, env), success); + }); + + test("env input with 1 level of nested variable anchored at front", () => { + const input = "${${test}bar}"; + + const env = { + "foobar": success, + "test": "foo" + }; + assert.equal(resolveVariables(input, env), success); + }); + + test("env input with 3 levels of nested variables", () => { + const input = "${foo${a${b${c}}}}"; + const env = { + "foobar": success, + "a1": "bar", + "b2": "1", + "c": "2" + }; + assert.equal(resolveVariables(input, env), success); + }); + test("env input contains env", () => { shouldSuccessfullyLookupInEnv("${envRoot}", "envRoot"); }); From 012e2df7ccff8c3d292e982ddd1aee188eac7819 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 08:27:10 -0700 Subject: [PATCH 05/11] refactors common test file This gets rid of linting errors and adopts a more BDD literate assert style. --- Extension/test/unitTests/common.test.ts | 192 ++++++++++++++---------- 1 file changed, 114 insertions(+), 78 deletions(-) diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index 12d21f27d..c106be66d 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -1,48 +1,53 @@ +/* -------------------------------------------------------------------------------------------- + * 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 = "success"; - const home = process.env.HOME; + const success: string = "success"; + const home: string = process.env.HOME; test("raw input", () => { - const input = "test"; - assert.equal(resolveVariables(input, {}), input); + const input: string = "test"; + inputAndEnvironment(input, {}) + .shouldResolveTo(input); }); test("raw input with tilde", () => { - const input = "~/test"; - assert.equal(resolveVariables(input, {}), `${home}/test`) + inputAndEnvironment("~/test", {}) + .shouldResolveTo(`${home}/test`); }); test("env input with tilde", () => { - const input = "${path}/test"; - const actual = resolveVariables(input, { - path: home - }); - assert.equal(actual, `${home}/test`) + inputAndEnvironment("${path}/test", { + path: home + }) + .shouldResolveTo(`${home}/test`); }); test("solo env input resulting in array", () => { - const input = "${test}"; - const actual = resolveVariables(input, { - test: ["foo", "bar"] - }); - assert.equal(actual, "foo;bar") + inputAndEnvironment("${test}", { + test: ["foo", "bar"] + }) + .shouldResolveTo("foo;bar"); }); test("mixed raw and env input resulting in array", () => { - const input = "baz${test}"; - const actual = resolveVariables(input, { - test: ["foo", "bar"] - }); - assert.equal(actual, input) + 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 = `cpptoolstests_${Date.now()}`; - const input = "foo${" + processKey + "}"; + const processKey: string = `cpptoolstests_${Date.now()}`; + const input: string = "foo${" + processKey + "}"; let actual: string; try { process.env[processKey] = "bar"; @@ -54,100 +59,131 @@ suite("Common Utility validation", () => { }); test("env input", () => { - shouldSuccessfullyLookupInEnv("${test}", "test"); + resolveVariablesWithInput("${test}") + .withEnvironment({ + "test": success + }) + .shouldResolveTo(success); }); test("env input mixed with plain text", () => { - const input = "${test}bar"; - const env = { - test: "foo" - }; - const result = resolveVariables(input, env); - assert.equal(result, "foobar"); + resolveVariablesWithInput("${test}bar") + .withEnvironment({ + "test": "foo" + }) + .shouldResolveTo("foobar"); }); test("env input with two variables", () => { - const input = "f${a}${b}r"; - const env = { - a: "oo", - b: "ba" - }; - const result = resolveVariables(input, env); - assert.equal(result, "foobar"); + resolveVariablesWithInput("f${a}${b}r") + .withEnvironment({ + a: "oo", + b: "ba" + }) + .shouldResolveTo("foobar"); }); test("env input not in env", () => { - const input = "${test}"; - assert.equal(resolveVariables(input, {}), input); + const input: string = "${test}"; + resolveVariablesWithInput(input) + .withEnvironment({}) + .shouldResolveTo(input); }); test("env input with 1 level of nested variables anchored at end", () => { - const input = "${foo${test}}"; - - const env = { - "foobar": success, - "test": "bar" - }; - assert.equal(resolveVariables(input, env), success); + resolveVariablesWithInput("${foo${test}}") + .withEnvironment({ + "foobar": success, + "test": "bar" + }) + .shouldResolveTo(success); }); test("env input with 1 level of nested variables anchored in the middle", () => { - const input = "${f${test}r}"; - - const env = { - "foobar": success, - "test": "ooba" - }; - assert.equal(resolveVariables(input, env), success); + resolveVariablesWithInput("${f${test}r}") + .withEnvironment({ + "foobar": success, + "test": "ooba" + }) + .shouldResolveTo(success); }); test("env input with 1 level of nested variable anchored at front", () => { - const input = "${${test}bar}"; - - const env = { - "foobar": success, - "test": "foo" - }; - assert.equal(resolveVariables(input, env), success); + resolveVariablesWithInput("${${test}bar}") + .withEnvironment({ + "foobar": success, + "test": "foo" + }) + .shouldResolveTo(success); }); test("env input with 3 levels of nested variables", () => { - const input = "${foo${a${b${c}}}}"; - const env = { - "foobar": success, - "a1": "bar", - "b2": "1", - "c": "2" - }; - assert.equal(resolveVariables(input, env), success); + resolveVariablesWithInput("${foo${a${b${c}}}}") + .withEnvironment({ + "foobar": success, + "a1": "bar", + "b2": "1", + "c": "2" + }) + .shouldResolveTo(success); }); test("env input contains env", () => { - shouldSuccessfullyLookupInEnv("${envRoot}", "envRoot"); + resolveVariablesWithInput("${envRoot}") + .shouldLookupSymbol("envRoot"); }); test("env input contains config", () => { - shouldSuccessfullyLookupInEnv("${configRoot}", "configRoot"); + resolveVariablesWithInput("${configRoot}") + .shouldLookupSymbol("configRoot"); }); test("env input contains workspaceFolder", () => { - shouldSuccessfullyLookupInEnv("${workspaceFolderRoot}", "workspaceFolderRoot"); + resolveVariablesWithInput("${workspaceFolderRoot}") + .shouldLookupSymbol("workspaceFolderRoot"); }); test("input contains env.", () => { - shouldSuccessfullyLookupInEnv("${env.Root}", "Root"); + resolveVariablesWithInput("${env.Root}") + .shouldLookupSymbol("Root"); }); test("input contains env:", () => { - shouldSuccessfullyLookupInEnv("${env:Root}", "Root"); + 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); + } + }; + } - const shouldSuccessfullyLookupInEnv = (input: string, expectedResolvedKey: string) => { - const env = {}; - env[expectedResolvedKey] = success; - const result = resolveVariables(input, env); - assert.equal(result, 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); + } + }; } + }); }); \ No newline at end of file From 91e4dcf47793133813fe179f27f0764a296461b8 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 08:27:44 -0700 Subject: [PATCH 06/11] fixes linting errors in common.ts --- Extension/src/common.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Extension/src/common.ts b/Extension/src/common.ts index 632fc697e..43d8e5e8d 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -219,10 +219,10 @@ export function resolveInnerVariable(originalInput: string, wholeVariable: strin return (newValue) ? newValue : wholeVariable; } -function indexOfMatchingCloseBrace(startingPos: number, input: string) { - let balanceFactor = 0; - for (let i = startingPos; i < input.length; i++) { - if (input[i] === "$" && i + 1 < input.length && input[i+1] === "{") { +function indexOfMatchingCloseBrace(startingPos: number, input: string): number { + let balanceFactor: number = 0; + for (let i: number = startingPos; i < input.length; i++) { + if (input[i] === "$" && i + 1 < input.length && input[i + 1] === "{") { balanceFactor += 1; } else if (input[i] === "}") { if (balanceFactor === 0) { @@ -242,11 +242,11 @@ interface StackEntry { function expandVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { let stack: StackEntry[] = []; - let stackInput = input; + let stackInput: string = input; while (stackInput !== "") { - const openIndex = stackInput.indexOf("${"); + const openIndex: number = stackInput.indexOf("${"); if (-1 < openIndex) { - const closeIndex = indexOfMatchingCloseBrace(openIndex+2, stackInput); + const closeIndex: number = indexOfMatchingCloseBrace(openIndex + 2, stackInput); stack.push({ prefix: stackInput.substring(0, openIndex), postfix: stackInput.substring(closeIndex + 1) @@ -259,8 +259,8 @@ function expandVariables(input: string, additionalEnvironment: {[key: string]: s let ret: string = stackInput; while (stack.length !== 0) { - let stackEntry = stack.pop(); - let resolvedItem = resolveInnerVariable(input, "${" + ret + "}", ret, additionalEnvironment); + let stackEntry: StackEntry = stack.pop(); + let resolvedItem: string = resolveInnerVariable(input, "${" + ret + "}", ret, additionalEnvironment); ret = stackEntry.prefix + resolvedItem + stackEntry.postfix; } @@ -277,9 +277,9 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st // Replace environment and configuration variables. let ret: string = input; - let openTagRegex = /\$\{/; + const openTagRegex: RegExp = /\$\{/; while (openTagRegex.test(ret)) { - let expansion = expandVariables(ret, additionalEnvironment); + const expansion: string = expandVariables(ret, additionalEnvironment); // Needed to prevent infinite loop in the failed lookup case if (expansion === ret) { break; From 06ad729c217f802168b9598b8d535f52205245dd Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 09:38:28 -0700 Subject: [PATCH 07/11] refactors resolveVariables implementation --- Extension/src/common.ts | 103 ++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 52 deletions(-) diff --git a/Extension/src/common.ts b/Extension/src/common.ts index 43d8e5e8d..b242fe0ed 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -169,7 +169,54 @@ export function showReleaseNotes(): void { vscode.commands.executeCommand('vscode.previewHtml', vscode.Uri.file(getExtensionFilePath("ReleaseNotes.html")), vscode.ViewColumn.One, "C/C++ Extension Release Notes"); } -export function resolveInnerVariable(originalInput: string, wholeVariable: string, innerVariable: string, additionalEnvironment: {[key: string]: string | string[]}): string { +function indexOfMatchingCloseBrace(startingPos: number, input: string): number { + let closeBracesToFind: number = 1; + for (let i: number = startingPos; i < input.length; i++) { + if (input[i] === "$" && i + 1 < input.length && input[i + 1] === "{") { + closeBracesToFind += 1; + } else if (input[i] === "}") { + closeBracesToFind -= 1; + if (closeBracesToFind < 1) { + return i; + } + } + } + return -1; +} + +interface StackEntry { + prefix: string; + postfix: string; +} + +function expandNestedVariable(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { + let stack: StackEntry[] = []; + let stackInput: string = input; + while (stackInput !== "") { + const openIndex: number = stackInput.indexOf("${"); + if (openIndex < 0) { + break; + } + + const closeIndex: number = indexOfMatchingCloseBrace(openIndex + 2, stackInput); + stack.push({ + prefix: stackInput.substring(0, openIndex), + postfix: stackInput.substring(closeIndex + 1) + }); + stackInput = stackInput.substring(openIndex + 2, closeIndex); + } + + let ret: string = stackInput; + while (stack.length !== 0) { + let stackEntry: StackEntry = stack.pop(); + let resolvedItem: string = resolveSingleVariableBlock(input, "${" + ret + "}", ret, additionalEnvironment); + ret = stackEntry.prefix + resolvedItem + stackEntry.postfix; + } + + return ret; +} + +function resolveSingleVariableBlock(originalInput: string, wholeVariable: string, innerVariable: string, additionalEnvironment: {[key: string]: string | string[]}): string { let varType: string; let name: string = innerVariable.replace(/(env|config|workspaceFolder)[\.|:](.*)/, (ignored: string, type: string, rest: string) => { varType = type; @@ -219,54 +266,6 @@ export function resolveInnerVariable(originalInput: string, wholeVariable: strin return (newValue) ? newValue : wholeVariable; } -function indexOfMatchingCloseBrace(startingPos: number, input: string): number { - let balanceFactor: number = 0; - for (let i: number = startingPos; i < input.length; i++) { - if (input[i] === "$" && i + 1 < input.length && input[i + 1] === "{") { - balanceFactor += 1; - } else if (input[i] === "}") { - if (balanceFactor === 0) { - return i; - } else { - balanceFactor -= 1; - } - } - } - return -1; -} - -interface StackEntry { - prefix: string; - postfix: string; -} - -function expandVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { - let stack: StackEntry[] = []; - let stackInput: string = input; - while (stackInput !== "") { - const openIndex: number = stackInput.indexOf("${"); - if (-1 < openIndex) { - const closeIndex: number = indexOfMatchingCloseBrace(openIndex + 2, stackInput); - stack.push({ - prefix: stackInput.substring(0, openIndex), - postfix: stackInput.substring(closeIndex + 1) - }); - stackInput = stackInput.substring(openIndex + 2, closeIndex); - } else { - break; - } - } - - let ret: string = stackInput; - while (stack.length !== 0) { - let stackEntry: StackEntry = stack.pop(); - let resolvedItem: string = resolveInnerVariable(input, "${" + ret + "}", ret, additionalEnvironment); - ret = stackEntry.prefix + resolvedItem + stackEntry.postfix; - } - - return ret; -} - export function resolveVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { if (!input) { return ""; @@ -279,9 +278,9 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st let ret: string = input; const openTagRegex: RegExp = /\$\{/; while (openTagRegex.test(ret)) { - const expansion: string = expandVariables(ret, additionalEnvironment); - // Needed to prevent infinite loop in the failed lookup case - if (expansion === ret) { + const expansion: string = expandNestedVariable(ret, additionalEnvironment); + const doneExpanding: boolean = expansion === ret; + if (doneExpanding) { break; } ret = expansion; From 6568cef27c7aeec3d659b98cfa5c3103d37ddd59 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 09:43:26 -0700 Subject: [PATCH 08/11] adds specific test for user scenario --- Extension/test/unitTests/common.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index c106be66d..97e01bea9 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -90,6 +90,15 @@ suite("Common Utility validation", () => { .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 input with 1 level of nested variables anchored at end", () => { resolveVariablesWithInput("${foo${test}}") .withEnvironment({ From d220d5d65a55a4757b519470a4ed3af8baedaa0d Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 10:26:28 -0700 Subject: [PATCH 09/11] fix cycle detection and half open detection --- Extension/src/common.ts | 8 ++++++- Extension/test/unitTests/common.test.ts | 28 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Extension/src/common.ts b/Extension/src/common.ts index b242fe0ed..0dafa9898 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -199,6 +199,10 @@ function expandNestedVariable(input: string, additionalEnvironment: {[key: strin } const closeIndex: number = indexOfMatchingCloseBrace(openIndex + 2, stackInput); + if (closeIndex < 0) { + break; + } + stack.push({ prefix: stackInput.substring(0, openIndex), postfix: stackInput.substring(closeIndex + 1) @@ -276,13 +280,15 @@ export function resolveVariables(input: string, additionalEnvironment: {[key: st // Replace environment and configuration variables. let ret: string = input; + const cycleCache: Set = new Set([ ret ]); const openTagRegex: RegExp = /\$\{/; while (openTagRegex.test(ret)) { const expansion: string = expandNestedVariable(ret, additionalEnvironment); - const doneExpanding: boolean = expansion === ret; + const doneExpanding: boolean = cycleCache.has(expansion); if (doneExpanding) { break; } + cycleCache.add(expansion); ret = expansion; } diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index 97e01bea9..43776f961 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -99,6 +99,34 @@ suite("Common Utility validation", () => { .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("${c}"); + }); + test("env input with 1 level of nested variables anchored at end", () => { resolveVariablesWithInput("${foo${test}}") .withEnvironment({ From 735c1f3a54b462a493990cb23586a5e89bda1f99 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 12:36:18 -0700 Subject: [PATCH 10/11] simplifies parsing into single pass --- Extension/src/common.ts | 162 +++++++----------------- Extension/test/unitTests/common.test.ts | 10 +- 2 files changed, 54 insertions(+), 118 deletions(-) diff --git a/Extension/src/common.ts b/Extension/src/common.ts index 0dafa9898..9b4cd444e 100644 --- a/Extension/src/common.ts +++ b/Extension/src/common.ts @@ -169,107 +169,6 @@ export function showReleaseNotes(): void { vscode.commands.executeCommand('vscode.previewHtml', vscode.Uri.file(getExtensionFilePath("ReleaseNotes.html")), vscode.ViewColumn.One, "C/C++ Extension Release Notes"); } -function indexOfMatchingCloseBrace(startingPos: number, input: string): number { - let closeBracesToFind: number = 1; - for (let i: number = startingPos; i < input.length; i++) { - if (input[i] === "$" && i + 1 < input.length && input[i + 1] === "{") { - closeBracesToFind += 1; - } else if (input[i] === "}") { - closeBracesToFind -= 1; - if (closeBracesToFind < 1) { - return i; - } - } - } - return -1; -} - -interface StackEntry { - prefix: string; - postfix: string; -} - -function expandNestedVariable(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { - let stack: StackEntry[] = []; - let stackInput: string = input; - while (stackInput !== "") { - const openIndex: number = stackInput.indexOf("${"); - if (openIndex < 0) { - break; - } - - const closeIndex: number = indexOfMatchingCloseBrace(openIndex + 2, stackInput); - if (closeIndex < 0) { - break; - } - - stack.push({ - prefix: stackInput.substring(0, openIndex), - postfix: stackInput.substring(closeIndex + 1) - }); - stackInput = stackInput.substring(openIndex + 2, closeIndex); - } - - let ret: string = stackInput; - while (stack.length !== 0) { - let stackEntry: StackEntry = stack.pop(); - let resolvedItem: string = resolveSingleVariableBlock(input, "${" + ret + "}", ret, additionalEnvironment); - ret = stackEntry.prefix + resolvedItem + stackEntry.postfix; - } - - return ret; -} - -function resolveSingleVariableBlock(originalInput: string, wholeVariable: string, innerVariable: string, additionalEnvironment: {[key: string]: string | string[]}): string { - let varType: string; - let name: string = innerVariable.replace(/(env|config|workspaceFolder)[\.|:](.*)/, (ignored: string, type: string, rest: string) => { - varType = type; - return rest; - }); - - // 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 (originalInput === wholeVariable && v instanceof Array) { - newValue = v.join(";"); - } - if (!newValue) { - newValue = process.env[name]; - } - break; - } - case "config": { - let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(); - if (config) { - newValue = config.get(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; - } - default: { assert.fail("unknown varType matched"); } - } - return (newValue) ? newValue : wholeVariable; -} - export function resolveVariables(input: string, additionalEnvironment: {[key: string]: string | string[]}): string { if (!input) { return ""; @@ -279,22 +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; - const cycleCache: Set = new Set([ ret ]); - const openTagRegex: RegExp = /\$\{/; - while (openTagRegex.test(ret)) { - const expansion: string = expandNestedVariable(ret, additionalEnvironment); - const doneExpanding: boolean = cycleCache.has(expansion); - if (doneExpanding) { - break; - } - cycleCache.add(expansion); - ret = expansion; + let cycleCache: Set = 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"; + } + 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; + } + case "config": { + let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(); + if (config) { + newValue = config.get(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; + } + default: { assert.fail("unknown varType matched"); } + } + return (newValue) ? newValue : match; + }); } // Resolve '~' at the start of the path. - let regexp: 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; }); diff --git a/Extension/test/unitTests/common.test.ts b/Extension/test/unitTests/common.test.ts index 43776f961..1b7324716 100644 --- a/Extension/test/unitTests/common.test.ts +++ b/Extension/test/unitTests/common.test.ts @@ -124,7 +124,7 @@ suite("Common Utility validation", () => { "b": "${c}", "c": "${a}" }) - .shouldResolveTo("${c}"); + .shouldResolveTo("${a}"); }); test("env input with 1 level of nested variables anchored at end", () => { @@ -133,7 +133,7 @@ suite("Common Utility validation", () => { "foobar": success, "test": "bar" }) - .shouldResolveTo(success); + .shouldResolveTo("${foo${test}}"); }); test("env input with 1 level of nested variables anchored in the middle", () => { @@ -142,7 +142,7 @@ suite("Common Utility validation", () => { "foobar": success, "test": "ooba" }) - .shouldResolveTo(success); + .shouldResolveTo("${f${test}r}"); }); test("env input with 1 level of nested variable anchored at front", () => { @@ -151,7 +151,7 @@ suite("Common Utility validation", () => { "foobar": success, "test": "foo" }) - .shouldResolveTo(success); + .shouldResolveTo("${${test}bar}"); }); test("env input with 3 levels of nested variables", () => { @@ -162,7 +162,7 @@ suite("Common Utility validation", () => { "b2": "1", "c": "2" }) - .shouldResolveTo(success); + .shouldResolveTo("${foo${a${b${c}}}}"); }); test("env input contains env", () => { From ebf9a0bc3e6602c07312b94514460bf1f3065802 Mon Sep 17 00:00:00 2001 From: John Patterson Date: Tue, 24 Jul 2018 14:22:46 -0700 Subject: [PATCH 11/11] fixes Travis CI failing unitTests As per https://github.com/Microsoft/vscode-wordcount/issues/5, running tests from CLI isn't supported with vscode except through the [workaround listed for Travis CI](https://code.visualstudio.com/docs/extensions/testing-extensions#_running-tests-automatically-on-travis-ci-build-machines). --- Extension/gulpfile.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Extension/gulpfile.js b/Extension/gulpfile.js index 03ff32cb7..405960d2c 100644 --- a/Extension/gulpfile.js +++ b/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" })