Auto merge of #17547 - Veykril:runnables-env, r=Veykril

internal: Clean up runnable lsp extension

This feels like a natural addition to me, and also allows us to drop the expect-test hardcoding from the extension. Additionally, `cargoExtraArgs` is pointless, all the client will do is merge it with `cargoArgs` so the server can do that instead of delegating that to the client.

crates/rust-analyzer/src/handlers/request.rs

@@ -847,7 +847,7 @@ pub(crate) fn handle_runnables(
             if expect_test {
                 if let lsp_ext::RunnableArgs::Cargo(r) = &mut runnable.args {
                     runnable.label = format!("{} + expect", runnable.label);
-                    r.expect_test = Some(true);
+                    r.environment.insert("UPDATE_EXPECT".to_owned(), "1".to_owned());
                 }
             }
             res.push(runnable);
@@ -874,6 +874,7 @@ pub(crate) fn handle_runnables(
                 if all_targets {
                     cargo_args.push("--all-targets".to_owned());
                 }
+                cargo_args.extend(config.cargo_extra_args.iter().cloned());
                 res.push(lsp_ext::Runnable {
                     label: format!(
                         "cargo {cmd} -p {}{all_targets}",
@@ -884,12 +885,11 @@ pub(crate) fn handle_runnables(
                     kind: lsp_ext::RunnableKind::Cargo,
                     args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs {
                         workspace_root: Some(spec.workspace_root.clone().into()),
-                        cwd: Some(cwd.into()),
+                        cwd: cwd.into(),
                         override_cargo: config.override_cargo.clone(),
                         cargo_args,
-                        cargo_extra_args: config.cargo_extra_args.clone(),
                         executable_args: Vec::new(),
-                        expect_test: None,
+                        environment: Default::default(),
                     }),
                 })
             }
@@ -897,20 +897,23 @@ pub(crate) fn handle_runnables(
         Some(TargetSpec::ProjectJson(_)) => {}
         None => {
             if !snap.config.linked_or_discovered_projects().is_empty() {
-                res.push(lsp_ext::Runnable {
+                if let Some(path) = snap.file_id_to_file_path(file_id).parent() {
-                    label: "cargo check --workspace".to_owned(),
+                    let mut cargo_args = vec!["check".to_owned(), "--workspace".to_owned()];
-                    location: None,
+                    cargo_args.extend(config.cargo_extra_args.iter().cloned());
-                    kind: lsp_ext::RunnableKind::Cargo,
+                    res.push(lsp_ext::Runnable {
-                    args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs {
+                        label: "cargo check --workspace".to_owned(),
-                        workspace_root: None,
+                        location: None,
-                        cwd: None,
+                        kind: lsp_ext::RunnableKind::Cargo,
-                        override_cargo: config.override_cargo,
+                        args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs {
-                        cargo_args: vec!["check".to_owned(), "--workspace".to_owned()],
+                            workspace_root: None,
-                        cargo_extra_args: config.cargo_extra_args,
+                            cwd: path.as_path().unwrap().to_path_buf().into(),
-                        executable_args: Vec::new(),
+                            override_cargo: config.override_cargo,
-                        expect_test: None,
+                            cargo_args,
-                    }),
+                            executable_args: Vec::new(),
-                });
+                            environment: Default::default(),
+                        }),
+                    });
+                };
             }
         }
     }

crates/rust-analyzer/src/lsp/ext.rs

@@ -460,28 +460,27 @@ pub enum RunnableKind {
 #[derive(Deserialize, Serialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct CargoRunnableArgs {
-    // command to be executed instead of cargo
+    #[serde(skip_serializing_if = "FxHashMap::is_empty")]
+    pub environment: FxHashMap<String, String>,
+    pub cwd: Utf8PathBuf,
+    /// Command to be executed instead of cargo
     pub override_cargo: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub workspace_root: Option<Utf8PathBuf>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub cwd: Option<Utf8PathBuf>,
     // command, --package and --lib stuff
     pub cargo_args: Vec<String>,
-    // user-specified additional cargo args, like `--release`.
-    pub cargo_extra_args: Vec<String>,
     // stuff after --
     pub executable_args: Vec<String>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub expect_test: Option<bool>,
 }
 
 #[derive(Deserialize, Serialize, Debug)]
 #[serde(rename_all = "camelCase")]
 pub struct ShellRunnableArgs {
+    #[serde(skip_serializing_if = "FxHashMap::is_empty")]
+    pub environment: FxHashMap<String, String>,
+    pub cwd: Utf8PathBuf,
     pub program: String,
     pub args: Vec<String>,
-    pub cwd: Utf8PathBuf,
 }
 
 pub enum RelatedTests {}

crates/rust-analyzer/src/lsp/to_proto.rs

@@ -1390,10 +1390,9 @@ pub(crate) fn runnable(
                     workspace_root: Some(workspace_root.into()),
                     override_cargo: config.override_cargo,
                     cargo_args,
-                    cwd: Some(cwd.into()),
+                    cwd: cwd.into(),
-                    cargo_extra_args: config.cargo_extra_args,
                     executable_args,
-                    expect_test: None,
+                    environment: Default::default(),
                 }),
             }))
         }
@@ -1407,6 +1406,7 @@ pub(crate) fn runnable(
                         program: json_shell_runnable_args.program,
                         args: json_shell_runnable_args.args,
                         cwd: json_shell_runnable_args.cwd,
+                        environment: Default::default(),
                     };
                     Ok(Some(lsp_ext::Runnable {
                         label,
@@ -1419,6 +1419,9 @@ pub(crate) fn runnable(
             }
         }
         None => {
+            let Some(path) = snap.file_id_to_file_path(runnable.nav.file_id).parent() else {
+                return Ok(None);
+            };
             let (cargo_args, executable_args) =
                 CargoTargetSpec::runnable_args(snap, None, &runnable.kind, &runnable.cfg);
 
@@ -1433,10 +1436,9 @@ pub(crate) fn runnable(
                     workspace_root: None,
                     override_cargo: config.override_cargo,
                     cargo_args,
-                    cwd: None,
+                    cwd: path.as_path().unwrap().to_path_buf().into(),
-                    cargo_extra_args: config.cargo_extra_args,
                     executable_args,
-                    expect_test: None,
+                    environment: Default::default(),
                 }),
             }))
         }

crates/rust-analyzer/src/target_spec.rs

@@ -110,7 +110,8 @@ impl CargoTargetSpec {
         kind: &RunnableKind,
         cfg: &Option<CfgExpr>,
     ) -> (Vec<String>, Vec<String>) {
-        let extra_test_binary_args = snap.config.runnables().extra_test_binary_args;
+        let config = snap.config.runnables();
+        let extra_test_binary_args = config.extra_test_binary_args;
 
         let mut cargo_args = Vec::new();
         let mut executable_args = Vec::new();
@@ -196,6 +197,7 @@ impl CargoTargetSpec {
                 }
             }
         }
+        cargo_args.extend(config.cargo_extra_args.iter().cloned());
         (cargo_args, executable_args)
     }
 

crates/rust-analyzer/tests/slow-tests/main.rs

@@ -258,7 +258,6 @@ fn main() {}
             "args": {
               "cargoArgs": ["test", "--package", "foo", "--test", "spam"],
               "executableArgs": ["test_eggs", "--exact", "--show-output"],
-              "cargoExtraArgs": [],
               "overrideCargo": null,
               "cwd": server.path().join("foo"),
               "workspaceRoot": server.path().join("foo")
@@ -289,7 +288,6 @@ fn main() {}
                 "--test",
                 "spam"
               ],
-              "cargoExtraArgs": [],
               "executableArgs": [
                 "",
                 "--show-output"
@@ -325,7 +323,6 @@ fn main() {}
             "args": {
               "cargoArgs": ["check", "--package", "foo", "--all-targets"],
               "executableArgs": [],
-              "cargoExtraArgs": [],
               "overrideCargo": null,
               "cwd": server.path().join("foo"),
               "workspaceRoot": server.path().join("foo")
@@ -337,7 +334,6 @@ fn main() {}
             "args": {
               "cargoArgs": ["test", "--package", "foo", "--all-targets"],
               "executableArgs": [],
-              "cargoExtraArgs": [],
               "overrideCargo": null,
               "cwd": server.path().join("foo"),
               "workspaceRoot": server.path().join("foo")
@@ -426,7 +422,6 @@ mod tests {
                             runnable,
                             "--all-targets"
                         ],
-                        "cargoExtraArgs": [],
                         "executableArgs": []
                     },
                 },
@@ -489,7 +484,6 @@ fn otherpkg() {}
                         "mainpkg",
                         "--all-targets"
                     ],
-                    "cargoExtraArgs": [],
                     "executableArgs": []
                 },
             },
@@ -515,7 +509,6 @@ fn otherpkg() {}
                         "otherpkg",
                         "--all-targets"
                     ],
-                    "cargoExtraArgs": [],
                     "executableArgs": []
                 },
             },

docs/dev/lsp-extensions.md

@@ -1,5 +1,5 @@
 <!---
-lsp/ext.rs hash: 8e6e340f2899b5e9
+lsp/ext.rs hash: a0867710490bf8da
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:
@@ -376,12 +376,29 @@ rust-analyzer supports two `kind`s of runnables, `"cargo"` and `"shell"`. The `a
 
 ```typescript
 {
+    /**
+     * Environment variables to set before running the command.
+     */
+    environment?: Record<string, string>;
+    /**
+     * The working directory to run the command in.
+     */
+    cwd: string;
+    /**
+     * The workspace root directory of the cargo project.
+     */
     workspaceRoot?: string;
-    cwd?: string;
+    /**
+     * The cargo command to run.
+     */
     cargoArgs: string[];
-    cargoExtraArgs: string[];
+    /**
+     * Arguments to pass to the executable, these will be passed to the command after a `--` argument.
+     */
     executableArgs: string[];
-    expectTest?: boolean;
+    /**
+     * Command to execute instead of `cargo`.
+     */
     overrideCargo?: string;
 }
 ```
@@ -390,10 +407,17 @@ The args for `"shell"` look like this:
 
 ```typescript
 {
+    /**
+     * Environment variables to set before running the command.
+     */
+    environment?: Record<string, string>;
+    /**
+     * The working directory to run the command in.
+     */
+    cwd: string;
     kind: string;
     program: string;
     args: string[];
-    cwd: string;
 }
 ```
 

editors/code/src/lsp_ext.ts

@@ -235,22 +235,43 @@ type RunnableShell = {
     args: ShellRunnableArgs;
 };
 
+export type CommonRunnableArgs = {
+    /**
+     * Environment variables to set before running the command.
+     */
+    environment?: Record<string, string>;
+    /**
+     * The working directory to run the command in.
+     */
+    cwd: string;
+};
+
 export type ShellRunnableArgs = {
     kind: string;
     program: string;
     args: string[];
-    cwd: string;
+} & CommonRunnableArgs;
-};
 
 export type CargoRunnableArgs = {
+    /**
+     * The workspace root directory of the cargo project.
+     */
     workspaceRoot?: string;
-    cargoArgs: string[];
+    /**
-    cwd: string;
+     * Arguments to pass to the executable, these will be passed to the command after a `--` argument.
-    cargoExtraArgs: string[];
+     */
     executableArgs: string[];
-    expectTest?: boolean;
+    /**
+     * Arguments to pass to cargo.
+     */
+    cargoArgs: string[];
+    /**
+     * Command to execute instead of `cargo`.
+     */
+    // This is supplied by the user via config. We could pull this through the client config in the
+    // extension directly, but that would prevent us from honoring the rust-analyzer.toml for it.
     overrideCargo?: string;
-};
+} & CommonRunnableArgs;
 
 export type RunnablesParams = {
     textDocument: lc.TextDocumentIdentifier;

editors/code/src/run.ts

@@ -66,9 +66,12 @@ export class RunnableQuickPick implements vscode.QuickPickItem {
     }
 }
 
-export function prepareBaseEnv(): Record<string, string> {
+export function prepareBaseEnv(base?: Record<string, string>): Record<string, string> {
     const env: Record<string, string> = { RUST_BACKTRACE: "short" };
-    Object.assign(env, process.env as { [key: string]: string });
+    Object.assign(env, process.env);
+    if (base) {
+        Object.assign(env, base);
+    }
     return env;
 }
 
@@ -77,12 +80,7 @@ export function prepareEnv(
     runnableArgs: ra.CargoRunnableArgs,
     runnableEnvCfg: RunnableEnvCfg,
 ): Record<string, string> {
-    const env = prepareBaseEnv();
+    const env = prepareBaseEnv(runnableArgs.environment);
-
-    if (runnableArgs.expectTest) {
-        env["UPDATE_EXPECT"] = "1";
-    }
-
     const platform = process.platform;
 
     const checkPlatform = (it: RunnableEnvCfgItem) => {
@@ -167,9 +165,6 @@ export async function createTaskFromRunnable(
 
 export function createCargoArgs(runnableArgs: ra.CargoRunnableArgs): string[] {
     const args = [...runnableArgs.cargoArgs]; // should be a copy!
-    if (runnableArgs.cargoExtraArgs) {
-        args.push(...runnableArgs.cargoExtraArgs); // Append user-specified cargo options.
-    }
     if (runnableArgs.executableArgs.length > 0) {
         args.push("--", ...runnableArgs.executableArgs);
     }

editors/code/tests/unit/runnable_env.test.ts

@@ -12,7 +12,6 @@ function makeRunnable(label: string): ra.Runnable {
             cargoArgs: [],
             cwd: ".",
             executableArgs: [],
-            cargoExtraArgs: [],
         },
     };
 }