Auto merge of #14019 - Veykril:ts-bin-og, r=Veykril

Substitute VSCode variables more generally

editors/code/src/bootstrap.ts

@@ -1,6 +1,6 @@
 import * as vscode from "vscode";
 import * as os from "os";
-import { Config, substituteVSCodeVariables } from "./config";
+import { Config } from "./config";
 import { log, isValidExecutable } from "./util";
 import { PersistentState } from "./persistent_state";
 import { exec } from "child_process";
@@ -31,58 +31,12 @@ export async function bootstrap(
 
     return path;
 }
-
-async function patchelf(dest: vscode.Uri): Promise<void> {
-    await vscode.window.withProgress(
-        {
-            location: vscode.ProgressLocation.Notification,
-            title: "Patching rust-analyzer for NixOS",
-        },
-        async (progress, _) => {
-            const expression = `
-            {srcStr, pkgs ? import <nixpkgs> {}}:
-                pkgs.stdenv.mkDerivation {
-                    name = "rust-analyzer";
-                    src = /. + srcStr;
-                    phases = [ "installPhase" "fixupPhase" ];
-                    installPhase = "cp $src $out";
-                    fixupPhase = ''
-                    chmod 755 $out
-                    patchelf --set-interpreter "$(cat $NIX_CC/nix-support/dynamic-linker)" $out
-                    '';
-                }
-            `;
-            const origFile = vscode.Uri.file(dest.fsPath + "-orig");
-            await vscode.workspace.fs.rename(dest, origFile, { overwrite: true });
-            try {
-                progress.report({ message: "Patching executable", increment: 20 });
-                await new Promise((resolve, reject) => {
-                    const handle = exec(
-                        `nix-build -E - --argstr srcStr '${origFile.fsPath}' -o '${dest.fsPath}'`,
-                        (err, stdout, stderr) => {
-                            if (err != null) {
-                                reject(Error(stderr));
-                            } else {
-                                resolve(stdout);
-                            }
-                        }
-                    );
-                    handle.stdin?.write(expression);
-                    handle.stdin?.end();
-                });
-            } finally {
-                await vscode.workspace.fs.delete(origFile);
-            }
-        }
-    );
-}
-
 async function getServer(
     context: vscode.ExtensionContext,
     config: Config,
     state: PersistentState
 ): Promise<string | undefined> {
-    const explicitPath = serverPath(config);
+    const explicitPath = process.env.__RA_LSP_SERVER_DEBUG ?? config.serverPath;
     if (explicitPath) {
         if (explicitPath.startsWith("~/")) {
             return os.homedir() + explicitPath.slice("~".length);
@@ -131,9 +85,6 @@ async function getServer(
     );
     return undefined;
 }
-function serverPath(config: Config): string | null {
-    return process.env.__RA_LSP_SERVER_DEBUG ?? substituteVSCodeVariables(config.serverPath);
-}
 
 async function isNixOs(): Promise<boolean> {
     try {
@@ -146,3 +97,48 @@ async function isNixOs(): Promise<boolean> {
         return false;
     }
 }
+
+async function patchelf(dest: vscode.Uri): Promise<void> {
+    await vscode.window.withProgress(
+        {
+            location: vscode.ProgressLocation.Notification,
+            title: "Patching rust-analyzer for NixOS",
+        },
+        async (progress, _) => {
+            const expression = `
+            {srcStr, pkgs ? import <nixpkgs> {}}:
+                pkgs.stdenv.mkDerivation {
+                    name = "rust-analyzer";
+                    src = /. + srcStr;
+                    phases = [ "installPhase" "fixupPhase" ];
+                    installPhase = "cp $src $out";
+                    fixupPhase = ''
+                    chmod 755 $out
+                    patchelf --set-interpreter "$(cat $NIX_CC/nix-support/dynamic-linker)" $out
+                    '';
+                }
+            `;
+            const origFile = vscode.Uri.file(dest.fsPath + "-orig");
+            await vscode.workspace.fs.rename(dest, origFile, { overwrite: true });
+            try {
+                progress.report({ message: "Patching executable", increment: 20 });
+                await new Promise((resolve, reject) => {
+                    const handle = exec(
+                        `nix-build -E - --argstr srcStr '${origFile.fsPath}' -o '${dest.fsPath}'`,
+                        (err, stdout, stderr) => {
+                            if (err != null) {
+                                reject(Error(stderr));
+                            } else {
+                                resolve(stdout);
+                            }
+                        }
+                    );
+                    handle.stdin?.write(expression);
+                    handle.stdin?.end();
+                });
+            } finally {
+                await vscode.workspace.fs.delete(origFile);
+            }
+        }
+    );
+}

editors/code/src/config.ts

@@ -1,5 +1,6 @@
-import * as path from "path";
+import * as Is from "vscode-languageclient/lib/common/utils/is";
 import * as os from "os";
+import * as path from "path";
 import * as vscode from "vscode";
 import { Env } from "./client";
 import { log } from "./util";
@@ -47,7 +48,7 @@ export class Config {
     }
 
     private refreshLogging() {
-        log.setEnabled(this.traceExtension);
+        log.setEnabled(this.traceExtension ?? false);
         log.info("Extension version:", this.package.version);
 
         const cfg = Object.entries(this.cfg).filter(([_, val]) => !(val instanceof Function));
@@ -163,18 +164,24 @@ export class Config {
      * ```
      * So this getter handles this quirk by not requiring the caller to use postfix `!`
      */
-    private get<T>(path: string): T {
+    private get<T>(path: string): T | undefined {
-        return this.cfg.get<T>(path)!;
+        return substituteVSCodeVariables(this.cfg.get<T>(path));
     }
 
     get serverPath() {
         return this.get<null | string>("server.path") ?? this.get<null | string>("serverPath");
     }
+
     get serverExtraEnv(): Env {
         const extraEnv =
             this.get<{ [key: string]: string | number } | null>("server.extraEnv") ?? {};
-        return Object.fromEntries(
+        return substituteVariablesInEnv(
-            Object.entries(extraEnv).map(([k, v]) => [k, typeof v !== "string" ? v.toString() : v])
+            Object.fromEntries(
+                Object.entries(extraEnv).map(([k, v]) => [
+                    k,
+                    typeof v !== "string" ? v.toString() : v,
+                ])
+            )
         );
     }
     get traceExtension() {
@@ -216,13 +223,13 @@ export class Config {
         if (sourceFileMap !== "auto") {
             // "/rustc/<id>" used by suggestions only.
             const { ["/rustc/<id>"]: _, ...trimmed } =
-                this.get<Record<string, string>>("debug.sourceFileMap");
+                this.get<Record<string, string>>("debug.sourceFileMap") ?? {};
             sourceFileMap = trimmed;
         }
 
         return {
             engine: this.get<string>("debug.engine"),
-            engineSettings: this.get<object>("debug.engineSettings"),
+            engineSettings: this.get<object>("debug.engineSettings") ?? {},
             openDebugPane: this.get<boolean>("debug.openDebugPane"),
             sourceFileMap: sourceFileMap,
         };
@@ -247,37 +254,27 @@ export class Config {
     }
 }
 
-const VarRegex = new RegExp(/\$\{(.+?)\}/g);
+export function substituteVSCodeVariables<T>(resp: T): T {
-
+    if (Is.string(resp)) {
-export function substituteVSCodeVariableInString(val: string): string {
+        return substituteVSCodeVariableInString(resp) as T;
-    return val.replace(VarRegex, (substring: string, varName) => {
+    } else if (resp && Is.array<any>(resp)) {
-        if (typeof varName === "string") {
-            return computeVscodeVar(varName) || substring;
-        } else {
-            return substring;
-        }
-    });
-}
-
-export function substituteVSCodeVariables(resp: any): any {
-    if (typeof resp === "string") {
-        return substituteVSCodeVariableInString(resp);
-    } else if (resp && Array.isArray(resp)) {
         return resp.map((val) => {
             return substituteVSCodeVariables(val);
-        });
+        }) as T;
     } else if (resp && typeof resp === "object") {
         const res: { [key: string]: any } = {};
         for (const key in resp) {
             const val = resp[key];
             res[key] = substituteVSCodeVariables(val);
         }
-        return res;
+        return res as T;
-    } else if (typeof resp === "function") {
+    } else if (Is.func(resp)) {
-        return null;
+        throw new Error("Unexpected function type in substitution");
     }
     return resp;
 }
+
+// FIXME: Merge this with `substituteVSCodeVariables` above
 export function substituteVariablesInEnv(env: Env): Env {
     const missingDeps = new Set<string>();
     // vscode uses `env:ENV_NAME` for env vars resolution, and it's easier
@@ -355,6 +352,17 @@ export function substituteVariablesInEnv(env: Env): Env {
     return resolvedEnv;
 }
 
+const VarRegex = new RegExp(/\$\{(.+?)\}/g);
+function substituteVSCodeVariableInString(val: string): string {
+    return val.replace(VarRegex, (substring: string, varName) => {
+        if (Is.string(varName)) {
+            return computeVscodeVar(varName) || substring;
+        } else {
+            return substring;
+        }
+    });
+}
+
 function computeVscodeVar(varName: string): string | null {
     const workspaceFolder = () => {
         const folders = vscode.workspace.workspaceFolders ?? [];

editors/code/src/ctx.ts

@@ -2,7 +2,7 @@ import * as vscode from "vscode";
 import * as lc from "vscode-languageclient/node";
 import * as ra from "./lsp_ext";
 
-import { Config, substituteVariablesInEnv, substituteVSCodeVariables } from "./config";
+import { Config, substituteVSCodeVariables } from "./config";
 import { createClient } from "./client";
 import { isRustDocument, isRustEditor, log, RustEditor } from "./util";
 import { ServerStatusParams } from "./lsp_ext";
@@ -152,9 +152,7 @@ export class Ctx {
                     throw new Error(message);
                 }
             );
-            const newEnv = substituteVariablesInEnv(
+            const newEnv = Object.assign({}, process.env, this.config.serverExtraEnv);
-                Object.assign({}, process.env, this.config.serverExtraEnv)
-            );
             const run: lc.Executable = {
                 command: this._serverPath,
                 options: { env: newEnv },

editors/code/src/debug.ts

@@ -84,7 +84,7 @@ async function getDebugConfiguration(
             debugEngine = vscode.extensions.getExtension(engineId);
             if (debugEngine) break;
         }
-    } else {
+    } else if (debugOptions.engine) {
         debugEngine = vscode.extensions.getExtension(debugOptions.engine);
     }
 

editors/code/src/util.ts

@@ -117,7 +117,7 @@ export function isValidExecutable(path: string): boolean {
 
     const res = spawnSync(path, ["--version"], { encoding: "utf8" });
 
-    const printOutput = res.error && (res.error as any).code !== "ENOENT" ? log.warn : log.debug;
+    const printOutput = res.error ? log.warn : log.info;
     printOutput(path, "--version:", res);
 
     return res.status === 0;