Unnamed repository; edit this file 'description' to name the repository.
Use runnable kind to disambiguate between cargo and shell commands
Found this bug while investigating why configuring nextest based on the instructions at https://github.com/rust-lang/rust-analyzer/issues/21137#issuecomment-4254611341 wasn't working within Zed.
Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:
```json
{
"label": "test my_test",
"kind": "shell",
"args": {
"environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
"cwd": "/project",
"program": "cargo",
"args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
}
}
```
would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.
With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.
See https://github.com/zed-industries/zed/pull/54011 for the original fix to Zed.
| -rw-r--r-- | crates/rust-analyzer/src/handlers/request.rs | 2 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/ext.rs | 103 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/to_proto.rs | 4 | ||||
| -rw-r--r-- | docs/book/src/contributing/lsp-extensions.md | 2 |
4 files changed, 94 insertions, 17 deletions
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 0c1c067ffa..ca6a2e70b0 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -1049,7 +1049,6 @@ pub(crate) fn handle_runnables( all_targets = if all_targets { " --all-targets" } else { "" } ), location: None, - kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs { workspace_root: Some(spec.workspace_root.clone().into()), cwd: cwd.into(), @@ -1076,7 +1075,6 @@ pub(crate) fn handle_runnables( res.push(lsp_ext::Runnable { label: "cargo check --workspace".to_owned(), location: None, - kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs { workspace_root: None, cwd: path.as_path().unwrap().to_path_buf().into(), diff --git a/crates/rust-analyzer/src/lsp/ext.rs b/crates/rust-analyzer/src/lsp/ext.rs index 91e065251d..754d6e65fe 100644 --- a/crates/rust-analyzer/src/lsp/ext.rs +++ b/crates/rust-analyzer/src/lsp/ext.rs @@ -451,25 +451,17 @@ pub struct Runnable { pub label: String, #[serde(skip_serializing_if = "Option::is_none")] pub location: Option<lsp_types::LocationLink>, - pub kind: RunnableKind, + #[serde(flatten)] pub args: RunnableArgs, } #[derive(Deserialize, Serialize, Debug, Clone)] -#[serde(rename_all = "camelCase")] -#[serde(untagged)] +#[serde(tag = "kind", content = "args", rename_all = "lowercase")] pub enum RunnableArgs { Cargo(CargoRunnableArgs), Shell(ShellRunnableArgs), } -#[derive(Serialize, Deserialize, Debug, Clone)] -#[serde(rename_all = "lowercase")] -pub enum RunnableKind { - Cargo, - Shell, -} - #[derive(Deserialize, Serialize, Debug, Clone)] #[serde(rename_all = "camelCase")] pub struct CargoRunnableArgs { @@ -881,3 +873,94 @@ impl Request for GetFailedObligations { type Result = String; const METHOD: &'static str = "rust-analyzer/getFailedObligations"; } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn cargo_runnable_round_trips() { + let runnable = Runnable { + label: "cargo test -p my-crate".to_owned(), + location: None, + args: RunnableArgs::Cargo(CargoRunnableArgs { + environment: [("RUSTC_TOOLCHAIN".to_owned(), "/toolchain".to_owned())] + .into_iter() + .collect(), + cwd: "/project".into(), + override_cargo: None, + workspace_root: Some("/project".into()), + cargo_args: vec![ + "test".into(), + "--package".into(), + "my-crate".into(), + "--lib".into(), + ], + executable_args: vec!["my_test".into(), "--exact".into()], + }), + }; + let expected = json!({ + "label": "cargo test -p my-crate", + "kind": "cargo", + "args": { + "environment": {"RUSTC_TOOLCHAIN": "/toolchain"}, + "cwd": "/project", + "overrideCargo": null, + "workspaceRoot": "/project", + "cargoArgs": ["test", "--package", "my-crate", "--lib"], + "executableArgs": ["my_test", "--exact"], + } + }); + + let serialized = serde_json::to_value(&runnable).expect("serialized runnable"); + assert_eq!(serialized, expected); + + let deserialized: Runnable = + serde_json::from_value(expected).expect("cargo runnable should deserialize"); + let RunnableArgs::Cargo(cargo) = &deserialized.args else { + panic!("expected Cargo variant, got {:?}", deserialized.args); + }; + assert_eq!(cargo.cargo_args, vec!["test", "--package", "my-crate", "--lib"]); + assert_eq!(cargo.executable_args, vec!["my_test", "--exact"]); + } + + #[test] + fn shell_runnable_round_trips() { + let runnable = Runnable { + label: "nextest test_one".to_owned(), + location: None, + args: RunnableArgs::Shell(ShellRunnableArgs { + environment: [("RUSTC_TOOLCHAIN".to_owned(), "/toolchain".to_owned())] + .into_iter() + .collect(), + cwd: "/project".into(), + program: "cargo".into(), + args: vec!["nextest".into(), "run".into(), "--package".into(), "my-crate".into()], + }), + }; + let expected = json!({ + "label": "nextest test_one", + "kind": "shell", + "args": { + "environment": {"RUSTC_TOOLCHAIN": "/toolchain"}, + "cwd": "/project", + "program": "cargo", + "args": ["nextest", "run", "--package", "my-crate"], + } + }); + + let serialized = serde_json::to_value(&runnable).expect("serialized runnable"); + assert_eq!(serialized, expected); + + // Every shell runnable is a structurally valid cargo runnable if the `kind` tag isn't + // used. This test ensures that the `kind` tag is used. + let deserialized: Runnable = + serde_json::from_value(expected).expect("shell runnable should deserialize"); + let RunnableArgs::Shell(shell) = &deserialized.args else { + panic!("expected Shell variant, got {:?}", deserialized.args); + }; + assert_eq!(shell.program, "cargo"); + assert_eq!(shell.args, vec!["nextest", "run", "--package", "my-crate"]); + } +} diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index d59e9d8434..eff5477969 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -1608,7 +1608,6 @@ pub(crate) fn runnable( Some((program, args)) => Some(lsp_ext::Runnable { label, location: Some(location), - kind: lsp_ext::RunnableKind::Shell, args: lsp_ext::RunnableArgs::Shell(lsp_ext::ShellRunnableArgs { environment, cwd: cwd.into(), @@ -1621,7 +1620,6 @@ pub(crate) fn runnable( None => Some(lsp_ext::Runnable { label, location: Some(location), - kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs { workspace_root: Some(workspace_root.into()), override_cargo: config.override_cargo, @@ -1648,7 +1646,6 @@ pub(crate) fn runnable( Ok(Some(lsp_ext::Runnable { label, location: Some(location), - kind: lsp_ext::RunnableKind::Shell, args: lsp_ext::RunnableArgs::Shell(runnable_args), })) } @@ -1668,7 +1665,6 @@ pub(crate) fn runnable( Ok(Some(lsp_ext::Runnable { label, location: Some(location), - kind: lsp_ext::RunnableKind::Cargo, args: lsp_ext::RunnableArgs::Cargo(lsp_ext::CargoRunnableArgs { workspace_root: None, override_cargo: config.override_cargo, diff --git a/docs/book/src/contributing/lsp-extensions.md b/docs/book/src/contributing/lsp-extensions.md index 9bb10aac3f..b74c40c422 100644 --- a/docs/book/src/contributing/lsp-extensions.md +++ b/docs/book/src/contributing/lsp-extensions.md @@ -1,5 +1,5 @@ <!--- -lsp/ext.rs hash: edab2f3c124dc28e +lsp/ext.rs hash: 57ae57d2a5c65b14 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: |