Add discoverConfig Support for rust-analyzer#4075
Conversation
UebelAndre
force-pushed
the
rust-analyzer
branch
7 times, most recently
from
cc07124 to
6b5e5e9
Compare
UebelAndre
force-pushed
the
rust-analyzer
branch
from
6b5e5e9 to
9641096
Compare
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
I'm still reviewing, but leaving a few comments in case you want to start considering them
| don't thrash the user's primary `bazel build` analysis cache. The two | ||
| Bazel servers — yours and the flycheck one — are fully isolated. | ||
|
|
||
| Failed actions are deliberately supported: rustc still emits its |
There was a problem hiding this comment.
But presumably anything that depends on the failed action won't attempt to build? Does that impact anything worth noting? Or even worth doing a query/cquery/aquery for if any actions failed?
| `rust-analyzer` will switch workspaces whenever an out-of-tree file gets opened, essentially indexing that | ||
| crate and its dependencies separately. A caveat of this is that _dependents_ of the crate currently being | ||
| worked on are not indexed and won't be tracked by `rust-analyzer`. | ||
| To force whole-workspace mode instead, drop the `{arg}` from the |
There was a problem hiding this comment.
I'm not sure about this as a default, it feels useful in a large monorepo, but probably quite annoying in a smaller repo? Do we want to make this a flag (maybe even a required flag, for discoverability) to setup?
| # labels against BEP labels to find each action's stderr for | ||
| # diagnostics. Without the leading `//`, the match silently | ||
| # fails and the wrapper emits no diagnostics for the crate. | ||
| "label": "//" + ctx.label.package + ":" + ctx.label.name, |
There was a problem hiding this comment.
Is there an issue here with deps from foreign workspaces?
| .map(Utf8PathBuf::from) | ||
| }) | ||
| .unwrap_or_else(|| workspace.join(".vscode/.rules_rust_analyzer/output_user_root")); | ||
| let _ = std::fs::create_dir_all(&output_user_root); |
There was a problem hiding this comment.
Panic on failure instead of ignoring?
| let _ = serde_json::to_writer(&mut out, &value); | ||
| let _ = out.write_all(b"\n"); | ||
| } | ||
| Err(_) => { | ||
| // Not strict JSON — pass through unmodified so we don't | ||
| // silently drop a diagnostic format we don't recognize. | ||
| let _ = out.write_all(line.as_bytes()); | ||
| let _ = out.write_all(b"\n"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| let _ = out.flush(); |
There was a problem hiding this comment.
Panics rather than swallow on failures?
|
|
||
| /// See `rust_analyzer.rs` for the rationale; mirrors the same fallback so | ||
| /// editors that spawn the wrapper directly don't trip RunfilesDirNotFound. | ||
| fn ensure_runfiles_env() { |
There was a problem hiding this comment.
Worth extracting into a lib? Or even our runfiles lib as an optional unofficial thing?
| /// function returning the findings; callers are responsible for surfacing | ||
| /// them (`log::warn!`, persistent log file, etc). | ||
| pub fn diagnose(crate_specs: &BTreeSet<CrateSpec>) -> AssemblyDiagnostics { | ||
| use std::collections::BTreeSet as Set; |
2 participants
This change improves rust-analyzer support by moving away from
rust-project.jsonand promoting a more automated approach. Documentation has been updated to reflect the recommended integration.Changes:
setuptool that users can invoke to configure their IDE for Bazel-backed rust-analyzer support.closes #2488
closes #3762