Merge pull request #19634 from Veykril/push-mnpmxxrprymo

feat: Allow unsetting env vars in `server.extraEnv` config

editors/code/package.json

@@ -543,7 +543,8 @@
                         "additionalProperties": {
                             "type": [
                                 "string",
-                                "number"
+                                "number",
+                                "null"
                             ]
                         },
                         "default": null,

editors/code/src/bootstrap.ts

@@ -187,8 +187,16 @@ async function hasToolchainFileWithRaDeclared(uri: vscode.Uri): Promise<boolean>
 export async function isValidExecutable(path: string, extraEnv: Env): Promise<boolean> {
     log.debug("Checking availability of a binary at", path);
 
+    const newEnv = { ...process.env };
+    for (const [k, v] of Object.entries(extraEnv)) {
+        if (v) {
+            newEnv[k] = v;
+        } else if (k in newEnv) {
+            delete newEnv[k];
+        }
+    }
     const res = await spawnAsync(path, ["--version"], {
-        env: { ...process.env, ...extraEnv },
+        env: newEnv,
     });
 
     if (res.error) {

editors/code/src/config.ts

@@ -213,12 +213,13 @@ export class Config {
 
     get serverExtraEnv(): Env {
         const extraEnv =
-            this.get<{ [key: string]: string | number } | null>("server.extraEnv") ?? {};
+            this.get<{ [key: string]: { toString(): string } | null } | null>("server.extraEnv") ??
+            {};
         return substituteVariablesInEnv(
             Object.fromEntries(
                 Object.entries(extraEnv).map(([k, v]) => [
                     k,
-                    typeof v !== "string" ? v.toString() : v,
+                    typeof v === "string" ? v : v?.toString(),
                 ]),
             ),
         );
@@ -398,6 +399,7 @@ export function prepareVSCodeConfig<T>(resp: T): T {
 
 // FIXME: Merge this with `substituteVSCodeVariables` above
 export function substituteVariablesInEnv(env: Env): Env {
+    const depRe = new RegExp(/\${(?<depName>.+?)}/g);
     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
@@ -405,7 +407,7 @@ export function substituteVariablesInEnv(env: Env): Env {
     const envWithDeps = Object.fromEntries(
         Object.entries(env).map(([key, value]) => {
             const deps = new Set<string>();
-            const depRe = new RegExp(/\${(?<depName>.+?)}/g);
+            if (value) {
                 let match = undefined;
                 while ((match = depRe.exec(value))) {
                     const depName = unwrapUndefinable(match.groups?.["depName"]);
@@ -416,6 +418,7 @@ export function substituteVariablesInEnv(env: Env): Env {
                         missingDeps.add(depName);
                     }
                 }
+            }
             return [`env:${key}`, { deps: [...deps], value }];
         }),
     );
@@ -454,11 +457,10 @@ export function substituteVariablesInEnv(env: Env): Env {
     do {
         leftToResolveSize = toResolve.size;
         for (const key of toResolve) {
-            const item = unwrapUndefinable(envWithDeps[key]);
+            const item = envWithDeps[key];
-            if (item.deps.every((dep) => resolved.has(dep))) {
+            if (item && item.deps.every((dep) => resolved.has(dep))) {
-                item.value = item.value.replace(/\${(?<depName>.+?)}/g, (_wholeMatch, depName) => {
+                item.value = item.value?.replace(/\${(?<depName>.+?)}/g, (_wholeMatch, depName) => {
-                    const item = unwrapUndefinable(envWithDeps[depName]);
+                    return envWithDeps[depName]?.value ?? "";
-                    return item.value;
                 });
                 resolved.add(key);
                 toResolve.delete(key);

editors/code/src/ctx.ts

@@ -213,7 +213,14 @@ export class Ctx implements RustAnalyzerExtensionApi {
                     this.refreshServerStatus();
                 },
             );
-            const newEnv = Object.assign({}, process.env, this.config.serverExtraEnv);
+            const newEnv = { ...process.env };
+            for (const [k, v] of Object.entries(this.config.serverExtraEnv)) {
+                if (v) {
+                    newEnv[k] = v;
+                } else if (k in newEnv) {
+                    delete newEnv[k];
+                }
+            }
             const run: lc.Executable = {
                 command: this._serverPath,
                 options: { env: newEnv },

editors/code/src/util.ts

@@ -14,7 +14,7 @@ export function assert(condition: boolean, explanation: string): asserts conditi
 }
 
 export type Env = {
-    [name: string]: string;
+    [name: string]: string | undefined;
 };
 
 class Log {