Skip to content

setting rust-analyzer.cargo.extraEnv.key = null has no effect #19626

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

Closed
jyn514 opened this issue Apr 19, 2025 · 6 comments · Fixed by #19629
Closed

setting rust-analyzer.cargo.extraEnv.key = null has no effect #19626

jyn514 opened this issue Apr 19, 2025 · 6 comments · Fixed by #19629
Labels
A-config configuration C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started

Comments

@jyn514
Copy link
Member

jyn514 commented Apr 19, 2025

rust-analyzer version: rust-analyzer version: 0.3.2379-standalone (8365cf8 2025-04-13) [/home/jyn/.vscode/extensions/rust-lang.rust-analyzer-0.3.2379-linux-x64/server/rust-analyzer]

rustc version (eg. output of rustc -V): rustc 1.88.0-nightly (78f2104e3 2025-04-16)

editor or extension: VSCode 1.96.2 (fabdb6a30b49f79a7aba0f2ad9df9b399473380f), extension version 0.3.2379

relevant settings:

$ cat .vscode/settings.json
{
    "rust-analyzer.cargo.extraEnv": {
        "ENV": null
    }
}

repository link (if public, optional): N/A; happens on every project i've tried

code snippet to reproduce: N/A

if i run strace on rust-analyzer, i see this output:

execve("/home/jyn/.local/lib/cargo/bin/cargo", ["/home/jyn/.local/lib/cargo/bin/cargo", "check", "--quiet", "--workspace", "--message-format=json", "--manifest-path", "/home/jyn/src/example/Cargo.toml", "--all-targets", "--all-features", "--keep-going"], [/*...*/ "ENV=/home/jyn/.profile])

i expect ENV to be unset here, since i've explicitly unset it in the config file.

i tracked through a bunch of the RA codebase but couldn't figure out why this happens. the typescript extension passes it through unchanged, i see extraEnv: { ENV: null } in the typescript output. the "rust-analyzer Language Server" "output" panel shows Server process exited with code 0., which is odd because the LSP is working fine, goto-definition etc work fine.

i found that this config is parsed into cargo_extraEnv: FxHashMap<String, String> using get_field_json, and i would have expected that function to return an error given that serde_json::from_value::<String>(null) returns an error. but i don't see that anywhere in the logs.

@Veykril
Copy link
Member

Veykril commented Apr 19, 2025

and i would have expected that function to return an error given

it likely does, and as such, the environment passed to cargo is unchanged (that is whatever is inherited). The config only adds to that environment. We should support setting env keys to null to explicitly unset them though, since overwriting it already possible.

@Veykril Veykril added C-feature Category: feature request A-config configuration and removed C-bug Category: bug labels Apr 19, 2025
@Veykril
Copy link
Member

Veykril commented Apr 19, 2025

So,

cargo_extraEnv: FxHashMap<String, String> = FxHashMap::default(),
and
check_extraEnv | checkOnSave_extraEnv: FxHashMap<String, String> = FxHashMap::default(),
need their types changed to FxHashMap<String, Option<String>>

Here we need to also insert an extra entry for that type (but keep the previous)

"FxHashMap<String, String>" => set! {
"type": "object",
},

And the rest the compiler should guide one through via diagnostics.

@Veykril Veykril added the E-has-instructions Issue has some instructions and pointers to code to get started label Apr 19, 2025
@jyn514
Copy link
Member Author

jyn514 commented Apr 19, 2025

@Veykril should we also improve the config parsing to log an error when it can’t deserialize the environment?

@Veykril
Copy link
Member

Veykril commented Apr 19, 2025

Well we should already either log them or show them in the status bar item, so if we don't that's a bug somewhere.

@jyn514
Copy link
Member Author

jyn514 commented Apr 19, 2025

i think the bug is that .find(Result::is_ok).and_then(/* handle_error */) doesn't work, the value in handle_error will always be an Ok value because it's filtered out everything else:

.find(Result::is_ok)
.and_then(|res| match res {
Ok(it) => Some(it),
Err((e, pointer)) => {
tracing::warn!("Failed to deserialize config field at {}: {:?}", pointer, e);

@jyn514
Copy link
Member Author

jyn514 commented Apr 19, 2025

ok yeah that fixed it, i'll make a pr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config configuration C-feature Category: feature request E-has-instructions Issue has some instructions and pointers to code to get started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants