Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inconsistent cwd of run and debug command in client #17275

Merged
merged 5 commits into from
May 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(),
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions crates/rust-analyzer/src/lsp/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
5 changes: 5 additions & 0 deletions crates/rust-analyzer/src/lsp/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down
93 changes: 93 additions & 0 deletions crates/rust-analyzer/tests/slow-tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -279,6 +280,7 @@ fn main() {}
{
"args": {
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo"),
"cargoArgs": [
"test",
Expand Down Expand Up @@ -325,6 +327,7 @@ fn main() {}
"executableArgs": [],
"cargoExtraArgs": [],
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo")
},
"kind": "cargo",
Expand All @@ -336,6 +339,7 @@ fn main() {}
"executableArgs": [],
"cargoExtraArgs": [],
"overrideCargo": null,
"cwd": server.path().join("foo"),
"workspaceRoot": server.path().join("foo")
},
"kind": "cargo",
Expand Down Expand Up @@ -415,6 +419,7 @@ mod tests {
"args": {
"overrideCargo": null,
"workspaceRoot": server.path().join(runnable),
"cwd": server.path().join(runnable),
"cargoArgs": [
"test",
"--package",
Expand All @@ -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() {
Expand Down
3 changes: 2 additions & 1 deletion docs/dev/lsp-extensions.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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[];
Expand Down
30 changes: 10 additions & 20 deletions editors/code/src/debug.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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",
};
Expand Down
1 change: 1 addition & 0 deletions editors/code/src/lsp_ext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ export type Runnable = {
kind: "cargo";
args: {
workspaceRoot?: string;
cwd?: string;
cargoArgs: string[];
cargoExtraArgs: string[];
executableArgs: string[];
Expand Down
14 changes: 2 additions & 12 deletions editors/code/src/toolchain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -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,
});
Expand All @@ -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) {
Expand All @@ -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(
Expand Down