about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2022-05-12 11:05:21 +0000
committerbors <bors@rust-lang.org>2022-05-12 11:05:21 +0000
commit927ef0ce7ed557187738e6a2f99e22f0e4628d1c (patch)
treef7fcda01eb6ac8d92575a1704f4b75ecbcbbfd92
parent7a55863c3dfda75dd51d2940b737818ba2fa2fcc (diff)
parent33d2c8a3f9530d193bb3563ae9d11d329e8a4c62 (diff)
downloadrust-927ef0ce7ed557187738e6a2f99e22f0e4628d1c.tar.gz
rust-927ef0ce7ed557187738e6a2f99e22f0e4628d1c.zip
Auto merge of #12215 - listochkin:Support-variable-substitution-in-vscode-settings, r=Veykril
feat: Support variable substitution in VSCode settings

Currently support a subset of [variables provided by VSCode](https://code.visualstudio.com/docs/editor/variables-reference) in `server.extraEnv` section of Rust-Analyzer settings:

  * `workspaceFolder`
  * `workspaceFolderBasename`
  * `cwd`
  * `execPath`
  * `pathSeparator`

Also, this PR adds support for general environment variables resolution. You can declare environment variables and reference them from other variables like this:

```JSON
"rust-analyzer.server.extraEnv": {
    "RUSTFLAGS": "-L${env:OPEN_XR_SDK_PATH}",
    "OPEN_XR_SDK_PATH": "${workspaceFolder}\\..\\OpenXR-SDK\\build\\src\\loader\\Release"
},
```
The order of variable declaration doesn't matter, you can reference variables before defining them. If the variable is not present in `extraEnv` section, VSCode will search for them in your environment. Missing variables will be replaced with empty string. Circular references won't be resolved and will be passed to rust-analyzer server process as is.

Closes #9626, but doesn't address use cases where people want to use values provided by `rustc` or `cargo`, such as `${targetTriple}` proposal #11649
-rw-r--r--editors/code/package-lock.json30
-rw-r--r--editors/code/package.json7
-rw-r--r--editors/code/src/client.ts7
-rw-r--r--editors/code/src/config.ts123
-rw-r--r--editors/code/tests/runTests.ts2
-rw-r--r--editors/code/tests/unit/index.ts4
-rw-r--r--editors/code/tests/unit/settings.test.ts61
7 files changed, 225 insertions, 9 deletions
diff --git a/editors/code/package-lock.json b/editors/code/package-lock.json
index 3c7c643798e..74cda037d8b 100644
--- a/editors/code/package-lock.json
+++ b/editors/code/package-lock.json
@@ -19,6 +19,7 @@
                 "@typescript-eslint/eslint-plugin": "^5.16.0",
                 "@typescript-eslint/parser": "^5.16.0",
                 "@vscode/test-electron": "^2.1.3",
+                "cross-env": "^7.0.3",
                 "esbuild": "^0.14.27",
                 "eslint": "^8.11.0",
                 "tslib": "^2.3.0",
@@ -27,7 +28,7 @@
                 "vsce": "^2.7.0"
             },
             "engines": {
-                "vscode": "^1.65.0"
+                "vscode": "^1.66.0"
             }
         },
         "node_modules/@eslint/eslintrc": {
@@ -790,6 +791,24 @@
             "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==",
             "dev": true
         },
+        "node_modules/cross-env": {
+            "version": "7.0.3",
+            "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz",
+            "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==",
+            "dev": true,
+            "dependencies": {
+                "cross-spawn": "^7.0.1"
+            },
+            "bin": {
+                "cross-env": "src/bin/cross-env.js",
+                "cross-env-shell": "src/bin/cross-env-shell.js"
+            },
+            "engines": {
+                "node": ">=10.14",
+                "npm": ">=6",
+                "yarn": ">=1"
+            }
+        },
         "node_modules/cross-spawn": {
             "version": "7.0.3",
             "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz",
@@ -4663,6 +4682,15 @@
             "integrity": "sha512-ZQBvi1DcpJ4GDqanjucZ2Hj3wEO5pZDS89BWbkcrvdxksJorwUDDZamX9ldFkp9aw2lmBDLgkObEA4DWNJ9FYQ==",
             "dev": true
         },
+        "cross-env": {
+            "version": "7.0.3",
+            "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz",
+            "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==",
+            "dev": true,
+            "requires": {
+                "cross-spawn": "^7.0.1"
+            }
+        },
         "cross-spawn": {
             "version": "7.0.3",
             "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.3.tgz",
diff --git a/editors/code/package.json b/editors/code/package.json
index 86766872a32..c6f4bc6ce30 100644
--- a/editors/code/package.json
+++ b/editors/code/package.json
@@ -33,12 +33,12 @@
         "lint": "tsfmt --verify && eslint -c .eslintrc.js --ext ts ./src ./tests",
         "fix": " tsfmt -r       && eslint -c .eslintrc.js --ext ts ./src ./tests --fix",
         "pretest": "tsc && npm run build",
-        "test": "node ./out/tests/runTests.js"
+        "test": "cross-env TEST_VARIABLE=test node ./out/tests/runTests.js"
     },
     "dependencies": {
-        "vscode-languageclient": "8.0.0-next.14",
         "d3": "^7.3.0",
-        "d3-graphviz": "^4.1.0"
+        "d3-graphviz": "^4.1.0",
+        "vscode-languageclient": "8.0.0-next.14"
     },
     "devDependencies": {
         "@types/node": "~14.17.5",
@@ -46,6 +46,7 @@
         "@typescript-eslint/eslint-plugin": "^5.16.0",
         "@typescript-eslint/parser": "^5.16.0",
         "@vscode/test-electron": "^2.1.3",
+        "cross-env": "^7.0.3",
         "esbuild": "^0.14.27",
         "eslint": "^8.11.0",
         "tslib": "^2.3.0",
diff --git a/editors/code/src/client.ts b/editors/code/src/client.ts
index 99b72635d18..d28c20aa08f 100644
--- a/editors/code/src/client.ts
+++ b/editors/code/src/client.ts
@@ -6,6 +6,7 @@ import { assert } from './util';
 import { WorkspaceEdit } from 'vscode';
 import { Workspace } from './ctx';
 import { updateConfig } from './config';
+import { substituteVariablesInEnv } from './config';
 
 export interface Env {
     [name: string]: string;
@@ -30,9 +31,9 @@ export async function createClient(serverPath: string, workspace: Workspace, ext
     // TODO?: Workspace folders support Uri's (eg: file://test.txt).
     // It might be a good idea to test if the uri points to a file.
 
-    const newEnv = Object.assign({}, process.env);
-    Object.assign(newEnv, extraEnv);
-
+    const newEnv = substituteVariablesInEnv(Object.assign(
+        {}, process.env, extraEnv
+    ));
     const run: lc.Executable = {
         command: serverPath,
         options: { env: newEnv },
diff --git a/editors/code/src/config.ts b/editors/code/src/config.ts
index 7e79eaab8e9..bf4572fcf61 100644
--- a/editors/code/src/config.ts
+++ b/editors/code/src/config.ts
@@ -1,3 +1,4 @@
+import path = require('path');
 import * as vscode from 'vscode';
 import { Env } from './client';
 import { log } from "./util";
@@ -210,3 +211,125 @@ export async function updateConfig(config: vscode.WorkspaceConfiguration) {
         }
     }
 }
+
+export function substituteVariablesInEnv(env: Env): Env {
+    const missingDeps = new Set<string>();
+    // vscode uses `env:ENV_NAME` for env vars resolution, and it's easier
+    // to follow the same convention for our dependency tracking
+    const definedEnvKeys = new Set(Object.keys(env).map(key => `env:${key}`));
+    const envWithDeps = Object.fromEntries(Object.entries(env).map(([key, value]) => {
+        const deps = new Set<string>();
+        const depRe = new RegExp(/\${(?<depName>.+?)}/g);
+        let match = undefined;
+        while ((match = depRe.exec(value))) {
+            const depName = match.groups!.depName;
+            deps.add(depName);
+            // `depName` at this point can have a form of `expression` or
+            // `prefix:expression`
+            if (!definedEnvKeys.has(depName)) {
+                missingDeps.add(depName);
+            }
+        }
+        return [`env:${key}`, { deps: [...deps], value }];
+    }));
+
+    const resolved = new Set<string>();
+    for (const dep of missingDeps) {
+        const match = /(?<prefix>.*?):(?<body>.+)/.exec(dep);
+        if (match) {
+            const { prefix, body } = match.groups!;
+            if (prefix === 'env') {
+                const envName = body;
+                envWithDeps[dep] = {
+                    value: process.env[envName] ?? '',
+                    deps: []
+                };
+                resolved.add(dep);
+            } else {
+                // we can't handle other prefixes at the moment
+                // leave values as is, but still mark them as resolved
+                envWithDeps[dep] = {
+                    value: '${' + dep + '}',
+                    deps: []
+                };
+                resolved.add(dep);
+            }
+        } else {
+            envWithDeps[dep] = {
+                value: computeVscodeVar(dep),
+                deps: []
+            };
+        }
+    }
+    const toResolve = new Set(Object.keys(envWithDeps));
+
+    let leftToResolveSize;
+    do {
+        leftToResolveSize = toResolve.size;
+        for (const key of toResolve) {
+            if (envWithDeps[key].deps.every(dep => resolved.has(dep))) {
+                envWithDeps[key].value = envWithDeps[key].value.replace(
+                    /\${(?<depName>.+?)}/g, (_wholeMatch, depName) => {
+                        return envWithDeps[depName].value;
+                    });
+                resolved.add(key);
+                toResolve.delete(key);
+            }
+        }
+    } while (toResolve.size > 0 && toResolve.size < leftToResolveSize);
+
+    const resolvedEnv: Env = {};
+    for (const key of Object.keys(env)) {
+        resolvedEnv[key] = envWithDeps[`env:${key}`].value;
+    }
+    return resolvedEnv;
+}
+
+function computeVscodeVar(varName: string): string {
+    // https://code.visualstudio.com/docs/editor/variables-reference
+    const supportedVariables: { [k: string]: () => string } = {
+        workspaceFolder: () => {
+            const folders = vscode.workspace.workspaceFolders ?? [];
+            if (folders.length === 1) {
+                // TODO: support for remote workspaces?
+                return folders[0].uri.fsPath;
+            } else if (folders.length > 1) {
+                // could use currently opened document to detect the correct
+                // workspace. However, that would be determined by the document
+                // user has opened on Editor startup. Could lead to
+                // unpredictable workspace selection in practice.
+                // It's better to pick the first one
+                return folders[0].uri.fsPath;
+            } else {
+                // no workspace opened
+                return '';
+            }
+        },
+
+        workspaceFolderBasename: () => {
+            const workspaceFolder = computeVscodeVar('workspaceFolder');
+            if (workspaceFolder) {
+                return path.basename(workspaceFolder);
+            } else {
+                return '';
+            }
+        },
+
+        cwd: () => process.cwd(),
+
+        // see
+        // https://github.com/microsoft/vscode/blob/08ac1bb67ca2459496b272d8f4a908757f24f56f/src/vs/workbench/api/common/extHostVariableResolverService.ts#L81
+        // or
+        // https://github.com/microsoft/vscode/blob/29eb316bb9f154b7870eb5204ec7f2e7cf649bec/src/vs/server/node/remoteTerminalChannel.ts#L56
+        execPath: () => process.env.VSCODE_EXEC_PATH ?? process.execPath,
+
+        pathSeparator: () => path.sep
+    };
+
+    if (varName in supportedVariables) {
+        return supportedVariables[varName]();
+    } else {
+        // can't resolve, keep the expression as is
+        return '${' + varName + '}';
+    }
+}
diff --git a/editors/code/tests/runTests.ts b/editors/code/tests/runTests.ts
index 7a8f3ef698b..6172cc7d5f9 100644
--- a/editors/code/tests/runTests.ts
+++ b/editors/code/tests/runTests.ts
@@ -14,7 +14,7 @@ async function main() {
     let minimalVersion: string = json.engines.vscode;
     if (minimalVersion.startsWith('^')) minimalVersion = minimalVersion.slice(1);
 
-    const launchArgs = ["--disable-extensions"];
+    const launchArgs = ["--disable-extensions", extensionDevelopmentPath];
 
     // All test suites (either unit tests or integration tests) should be in subfolders.
     const extensionTestsPath = path.resolve(__dirname, './unit/index');
diff --git a/editors/code/tests/unit/index.ts b/editors/code/tests/unit/index.ts
index 39ff36707f9..288bd60326c 100644
--- a/editors/code/tests/unit/index.ts
+++ b/editors/code/tests/unit/index.ts
@@ -1,3 +1,4 @@
+import { readdir } from 'fs/promises';
 import * as path from 'path';
 
 class Test {
@@ -57,7 +58,8 @@ export class Context {
 
 export async function run(): Promise<void> {
     const context = new Context();
-    const testFiles = ["launch_config.test.js", "runnable_env.test.js"];
+
+    const testFiles = (await readdir(path.resolve(__dirname))).filter(name => name.endsWith('.test.js'));
     for (const testFile of testFiles) {
         try {
             const testModule = require(path.resolve(__dirname, testFile));
diff --git a/editors/code/tests/unit/settings.test.ts b/editors/code/tests/unit/settings.test.ts
new file mode 100644
index 00000000000..dca4e38d138
--- /dev/null
+++ b/editors/code/tests/unit/settings.test.ts
@@ -0,0 +1,61 @@
+import * as assert from 'assert';
+import { Context } from '.';
+import { substituteVariablesInEnv } from '../../src/config';
+
+export async function getTests(ctx: Context) {
+    await ctx.suite('Server Env Settings', suite => {
+        suite.addTest('Replacing Env Variables', async () => {
+            const envJson = {
+                USING_MY_VAR: "${env:MY_VAR} test ${env:MY_VAR}",
+                MY_VAR: "test"
+            };
+            const expectedEnv = {
+                USING_MY_VAR: "test test test",
+                MY_VAR: "test"
+            };
+            const actualEnv = await substituteVariablesInEnv(envJson);
+            assert.deepStrictEqual(actualEnv, expectedEnv);
+        });
+
+        suite.addTest('Circular dependencies remain as is', async () => {
+            const envJson = {
+                A_USES_B: "${env:B_USES_A}",
+                B_USES_A: "${env:A_USES_B}",
+                C_USES_ITSELF: "${env:C_USES_ITSELF}",
+                D_USES_C: "${env:C_USES_ITSELF}",
+                E_IS_ISOLATED: "test",
+                F_USES_E: "${env:E_IS_ISOLATED}"
+            };
+            const expectedEnv = {
+                A_USES_B: "${env:B_USES_A}",
+                B_USES_A: "${env:A_USES_B}",
+                C_USES_ITSELF: "${env:C_USES_ITSELF}",
+                D_USES_C: "${env:C_USES_ITSELF}",
+                E_IS_ISOLATED: "test",
+                F_USES_E: "test"
+            };
+            const actualEnv = await substituteVariablesInEnv(envJson);
+            assert.deepStrictEqual(actualEnv, expectedEnv);
+        });
+
+        suite.addTest('Should support external variables', async () => {
+            const envJson = {
+                USING_EXTERNAL_VAR: "${env:TEST_VARIABLE} test ${env:TEST_VARIABLE}"
+            };
+            const expectedEnv = {
+                USING_EXTERNAL_VAR: "test test test"
+            };
+
+            const actualEnv = await substituteVariablesInEnv(envJson);
+            assert.deepStrictEqual(actualEnv, expectedEnv);
+        });
+
+        suite.addTest('should support VSCode variables', async () => {
+            const envJson = {
+                USING_VSCODE_VAR: "${workspaceFolderBasename}"
+            };
+            const actualEnv = await substituteVariablesInEnv(envJson);
+            assert.deepStrictEqual(actualEnv.USING_VSCODE_VAR, 'code');
+        });
+    });
+}