Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #17275 - roife:fix-issue-17012, r=Veykril
Fix inconsistent cwd of `run` and `debug` command in client
Fix #17012. Also related to #13022 and #15993.
When the `kind` of runnable is `bin`, Cargo would use the workspace root as the cwd for the `run` command; otherwise, Cargo defaults to the package root as the cwd for `run`.
Initially, r-a assumed the workspace root as the cwd for all runnables in `debug` command, which led to issue #13022. In this case, during unit testing, the `run` command would use the package root while `debug` would use the workspace root, causing inconsistency.
PR #15993 addressed this problem by using the package root as the cwd for `debug` command. However, it also resulted in an inconsistency: when executing the `run` command within the main fn of a package (whose target is `bin`), Cargo would use the workspace root, whereas `debug` would use the package root, leading to issue #17012.
The preferable approach is to determine the cwd based on the runnable's type. To resolve this, this PR introduces a new `cwd` field within `CargoRunnable`, allowing r-a to decide the appropriate cwd depending on the specific kind of the runnable.
| -rw-r--r-- | crates/rust-analyzer/src/handlers/request.rs | 7 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/ext.rs | 2 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/to_proto.rs | 5 | ||||
| -rw-r--r-- | crates/rust-analyzer/tests/slow-tests/main.rs | 93 | ||||
| -rw-r--r-- | docs/dev/lsp-extensions.md | 3 | ||||
| -rw-r--r-- | editors/code/src/debug.ts | 30 | ||||
| -rw-r--r-- | editors/code/src/lsp_ext.ts | 1 | ||||
| -rw-r--r-- | editors/code/src/toolchain.ts | 14 |
8 files changed, 122 insertions, 33 deletions
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 41996db32e..1e24bf3aae 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -860,6 +860,11 @@ pub(crate) fn handle_runnables( if cmd == "run" && spec.target_kind != TargetKind::Bin { continue; } + let cwd = if cmd != "test" || spec.target_kind == TargetKind::Bin { + spec.workspace_root.clone() + } else { + spec.cargo_toml.parent().to_path_buf() + }; let mut cargo_args = vec![cmd.to_owned(), "--package".to_owned(), spec.package.clone()]; let all_targets = cmd != "run" && !is_crate_no_std; @@ -876,6 +881,7 @@ pub(crate) fn handle_runnables( kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::CargoRunnable { workspace_root: Some(spec.workspace_root.clone().into()), + cwd: Some(cwd.into()), override_cargo: config.override_cargo.clone(), cargo_args, cargo_extra_args: config.cargo_extra_args.clone(), @@ -893,6 +899,7 @@ pub(crate) fn handle_runnables( kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::CargoRunnable { workspace_root: None, + cwd: None, override_cargo: config.override_cargo, cargo_args: vec!["check".to_owned(), "--workspace".to_owned()], cargo_extra_args: config.cargo_extra_args, diff --git a/crates/rust-analyzer/src/lsp/ext.rs b/crates/rust-analyzer/src/lsp/ext.rs index 03bd83aab5..aa75633ac3 100644 --- a/crates/rust-analyzer/src/lsp/ext.rs +++ b/crates/rust-analyzer/src/lsp/ext.rs @@ -441,6 +441,8 @@ pub struct CargoRunnable { pub override_cargo: Option<String>, #[serde(skip_serializing_if = "Option::is_none")] pub workspace_root: Option<PathBuf>, + #[serde(skip_serializing_if = "Option::is_none")] + pub cwd: Option<PathBuf>, // command, --package and --lib stuff pub cargo_args: Vec<String>, // user-specified additional cargo args, like `--release`. diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 03daccc99c..1d7062c989 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -1360,6 +1360,10 @@ pub(crate) fn runnable( let config = snap.config.runnables(); let spec = CargoTargetSpec::for_file(snap, runnable.nav.file_id)?; let workspace_root = spec.as_ref().map(|it| it.workspace_root.clone()); + let cwd = match runnable.kind { + ide::RunnableKind::Bin { .. } => workspace_root.clone().map(|it| it.into()), + _ => spec.as_ref().map(|it| it.cargo_toml.parent().into()), + }; let target = spec.as_ref().map(|s| s.target.clone()); let (cargo_args, executable_args) = CargoTargetSpec::runnable_args(snap, spec, &runnable.kind, &runnable.cfg); @@ -1372,6 +1376,7 @@ pub(crate) fn runnable( kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::CargoRunnable { workspace_root: workspace_root.map(|it| it.into()), + cwd, override_cargo: config.override_cargo, cargo_args, cargo_extra_args: config.cargo_extra_args, diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs index d886e405e1..43a8305010 100644 --- a/crates/rust-analyzer/tests/slow-tests/main.rs +++ b/crates/rust-analyzer/tests/slow-tests/main.rs @@ -260,6 +260,7 @@ fn main() {} "executableArgs": ["test_eggs", "--exact", "--show-output"], "cargoExtraArgs": [], "overrideCargo": null, + "cwd": server.path().join("foo"), "workspaceRoot": server.path().join("foo") }, "kind": "cargo", @@ -279,6 +280,7 @@ fn main() {} { "args": { "overrideCargo": null, + "cwd": server.path().join("foo"), "workspaceRoot": server.path().join("foo"), "cargoArgs": [ "test", @@ -325,6 +327,7 @@ fn main() {} "executableArgs": [], "cargoExtraArgs": [], "overrideCargo": null, + "cwd": server.path().join("foo"), "workspaceRoot": server.path().join("foo") }, "kind": "cargo", @@ -336,6 +339,7 @@ fn main() {} "executableArgs": [], "cargoExtraArgs": [], "overrideCargo": null, + "cwd": server.path().join("foo"), "workspaceRoot": server.path().join("foo") }, "kind": "cargo", @@ -415,6 +419,7 @@ mod tests { "args": { "overrideCargo": null, "workspaceRoot": server.path().join(runnable), + "cwd": server.path().join(runnable), "cargoArgs": [ "test", "--package", @@ -432,6 +437,94 @@ mod tests { } } +// The main fn in packages should be run from the workspace root +#[test] +fn test_runnables_cwd() { + if skip_slow_tests() { + return; + } + + let server = Project::with_fixture( + r#" +//- /foo/Cargo.toml +[workspace] +members = ["mainpkg", "otherpkg"] + +//- /foo/mainpkg/Cargo.toml +[package] +name = "mainpkg" +version = "0.1.0" + +//- /foo/mainpkg/src/main.rs +fn main() {} + +//- /foo/otherpkg/Cargo.toml +[package] +name = "otherpkg" +version = "0.1.0" + +//- /foo/otherpkg/src/lib.rs +#[test] +fn otherpkg() {} +"#, + ) + .root("foo") + .server() + .wait_until_workspace_is_loaded(); + + server.request::<Runnables>( + RunnablesParams { text_document: server.doc_id("foo/mainpkg/src/main.rs"), position: None }, + json!([ + "{...}", + { + "label": "cargo test -p mainpkg --all-targets", + "kind": "cargo", + "args": { + "overrideCargo": null, + "workspaceRoot": server.path().join("foo"), + "cwd": server.path().join("foo"), + "cargoArgs": [ + "test", + "--package", + "mainpkg", + "--all-targets" + ], + "cargoExtraArgs": [], + "executableArgs": [] + }, + }, + "{...}", + "{...}" + ]), + ); + + server.request::<Runnables>( + RunnablesParams { text_document: server.doc_id("foo/otherpkg/src/lib.rs"), position: None }, + json!([ + "{...}", + { + "label": "cargo test -p otherpkg --all-targets", + "kind": "cargo", + "args": { + "overrideCargo": null, + "workspaceRoot": server.path().join("foo"), + "cwd": server.path().join("foo").join("otherpkg"), + "cargoArgs": [ + "test", + "--package", + "otherpkg", + "--all-targets" + ], + "cargoExtraArgs": [], + "executableArgs": [] + }, + }, + "{...}", + "{...}" + ]), + ); +} + #[test] fn test_format_document() { if skip_slow_tests() { diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md index c8728c33ea..1c91e856e7 100644 --- a/docs/dev/lsp-extensions.md +++ b/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@ <!--- -lsp/ext.rs hash: 422dcc22c2e56166 +lsp/ext.rs hash: 1babf76a3c2cef3b 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: @@ -377,6 +377,7 @@ rust-analyzer supports only one `kind`, `"cargo"`. The `args` for `"cargo"` look ```typescript { workspaceRoot?: string; + cwd?: string; cargoArgs: string[]; cargoExtraArgs: string[]; executableArgs: string[]; diff --git a/editors/code/src/debug.ts b/editors/code/src/debug.ts index bad1f48de8..855e8b082a 100644 --- a/editors/code/src/debug.ts +++ b/editors/code/src/debug.ts @@ -3,7 +3,7 @@ import * as vscode from "vscode"; import * as path from "path"; import type * as ra from "./lsp_ext"; -import { Cargo, type ExecutableInfo, getRustcId, getSysroot } from "./toolchain"; +import { Cargo, getRustcId, getSysroot } from "./toolchain"; import type { Ctx } from "./ctx"; import { prepareEnv } from "./run"; import { unwrapUndefinable } from "./undefinable"; @@ -12,7 +12,6 @@ const debugOutput = vscode.window.createOutputChannel("Debug"); type DebugConfigProvider = ( config: ra.Runnable, executable: string, - cargoWorkspace: string, env: Record<string, string>, sourceFileMap?: Record<string, string>, ) => vscode.DebugConfiguration; @@ -134,7 +133,7 @@ async function getDebugConfiguration( } const env = prepareEnv(runnable, ctx.config.runnablesExtraEnv); - const { executable, workspace: cargoWorkspace } = await getDebugExecutableInfo(runnable, env); + const executable = await getDebugExecutable(runnable, env); let sourceFileMap = debugOptions.sourceFileMap; if (sourceFileMap === "auto") { // let's try to use the default toolchain @@ -148,13 +147,7 @@ async function getDebugConfiguration( } const provider = unwrapUndefinable(knownEngines[debugEngine.id]); - const debugConfig = provider( - runnable, - simplifyPath(executable), - cargoWorkspace, - env, - sourceFileMap, - ); + const debugConfig = provider(runnable, simplifyPath(executable), env, sourceFileMap); if (debugConfig.type in debugOptions.engineSettings) { const settingsMap = (debugOptions.engineSettings as any)[debugConfig.type]; for (var key in settingsMap) { @@ -176,21 +169,20 @@ async function getDebugConfiguration( return debugConfig; } -async function getDebugExecutableInfo( +async function getDebugExecutable( runnable: ra.Runnable, env: Record<string, string>, -): Promise<ExecutableInfo> { +): Promise<string> { const cargo = new Cargo(runnable.args.workspaceRoot || ".", debugOutput, env); - const executableInfo = await cargo.executableInfoFromArgs(runnable.args.cargoArgs); + const executable = await cargo.executableFromArgs(runnable.args.cargoArgs); // if we are here, there were no compilation errors. - return executableInfo; + return executable; } function getCCppDebugConfig( runnable: ra.Runnable, executable: string, - cargoWorkspace: string, env: Record<string, string>, sourceFileMap?: Record<string, string>, ): vscode.DebugConfiguration { @@ -200,7 +192,7 @@ function getCCppDebugConfig( name: runnable.label, program: executable, args: runnable.args.executableArgs, - cwd: cargoWorkspace || runnable.args.workspaceRoot, + cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".", sourceFileMap, env, // See https://github.com/rust-lang/rust-analyzer/issues/16901#issuecomment-2024486941 @@ -213,7 +205,6 @@ function getCCppDebugConfig( function getCodeLldbDebugConfig( runnable: ra.Runnable, executable: string, - cargoWorkspace: string, env: Record<string, string>, sourceFileMap?: Record<string, string>, ): vscode.DebugConfiguration { @@ -223,7 +214,7 @@ function getCodeLldbDebugConfig( name: runnable.label, program: executable, args: runnable.args.executableArgs, - cwd: cargoWorkspace || runnable.args.workspaceRoot, + cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".", sourceMap: sourceFileMap, sourceLanguages: ["rust"], env, @@ -233,7 +224,6 @@ function getCodeLldbDebugConfig( function getNativeDebugConfig( runnable: ra.Runnable, executable: string, - cargoWorkspace: string, env: Record<string, string>, _sourceFileMap?: Record<string, string>, ): vscode.DebugConfiguration { @@ -244,7 +234,7 @@ function getNativeDebugConfig( target: executable, // See https://github.com/WebFreak001/code-debug/issues/359 arguments: quote(runnable.args.executableArgs), - cwd: cargoWorkspace || runnable.args.workspaceRoot, + cwd: runnable.args.cwd || runnable.args.workspaceRoot || ".", env, valuesFormatting: "prettyPrinters", }; diff --git a/editors/code/src/lsp_ext.ts b/editors/code/src/lsp_ext.ts index 9a7a4aae95..8e48aeef15 100644 --- a/editors/code/src/lsp_ext.ts +++ b/editors/code/src/lsp_ext.ts @@ -226,6 +226,7 @@ export type Runnable = { kind: "cargo"; args: { workspaceRoot?: string; + cwd?: string; cargoArgs: string[]; cargoExtraArgs: string[]; executableArgs: string[]; diff --git a/editors/code/src/toolchain.ts b/editors/code/src/toolchain.ts index a0b34406c1..58e5fc747a 100644 --- a/editors/code/src/toolchain.ts +++ b/editors/code/src/toolchain.ts @@ -9,17 +9,11 @@ import { unwrapUndefinable } from "./undefinable"; interface CompilationArtifact { fileName: string; - workspace: string; name: string; kind: string; isTest: boolean; } -export interface ExecutableInfo { - executable: string; - workspace: string; -} - export interface ArtifactSpec { cargoArgs: string[]; filter?: (artifacts: CompilationArtifact[]) => CompilationArtifact[]; @@ -74,7 +68,6 @@ export class Cargo { artifacts.push({ fileName: message.executable, name: message.target.name, - workspace: path.dirname(message.manifest_path), kind: message.target.kind[0], isTest: message.profile.test, }); @@ -93,7 +86,7 @@ export class Cargo { return spec.filter?.(artifacts) ?? artifacts; } - async executableInfoFromArgs(args: readonly string[]): Promise<ExecutableInfo> { + async executableFromArgs(args: readonly string[]): Promise<string> { const artifacts = await this.getArtifacts(Cargo.artifactSpec(args)); if (artifacts.length === 0) { @@ -103,10 +96,7 @@ export class Cargo { } const artifact = unwrapUndefinable(artifacts[0]); - return { - executable: artifact.fileName, - workspace: artifact.workspace, - }; + return artifact.fileName; } private async runCargo( |