From 3877078566241d2ecbed108750e7f8a8f40d3fbb Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Thu, 4 Jul 2024 08:35:09 -0700 Subject: [PATCH] Updated rust-analyzer to use repo vs generated files as crate roots (#2717) This change updates the `rust_analyzer_aspect` to attempt to match generated crate root sources to `srcs` inputs and use the the path of the source file in place of the generated path. The generated path is then added to `sources.include_dirs` for compatibility. The impact of this can be seen in the following diff of the `@rules_rust//:rust-project.json` file: ```diff --- rust-project.old.json 2024-06-28 09:00:51 +++ rust-project.json 2024-06-28 09:04:43 @@ -821,10 +821,16 @@ }, { "display_name": "libgensrc_with_crate_root", - "root_module": "/private/var/tmp/_bazel_user/76282c66b0dfe3c5cb9a230bdc913a52/execroot/rules_rust/bazel-out/darwin_arm64-fastbuild/bin/test/generated_inputs/lib.rs", + "root_module": "test/generated_inputs/lib.rs", "edition": "2018", "deps": [], "is_workspace_member": true, + "source": { + "include_dirs": [ + "/private/var/tmp/_bazel_user/76282c66b0dfe3c5cb9a230bdc913a52/execroot/rules_rust/bazel-out/darwin_arm64-fastbuild/bin/test/generated_inputs" + ], + "exclude_dirs": [] + }, "cfg": [ "test", "debug_assertions" @@ -850,10 +856,16 @@ ... ... ... ``` closes https://github.com/bazelbuild/rules_rust/issues/2716 --- rust/private/rust_analyzer.bzl | 21 ++++++--- .../generated_srcs_test/BUILD.bazel | 37 ++++++++++++++++ test/rust_analyzer/generated_srcs_test/lib.rs | 9 ++++ .../rust_project_json_test.rs | 43 +++++++++++++++++++ .../rust_project_json_test.rs | 18 +++++++- .../rust_project_json_test.rs | 13 +++++- tools/rust_analyzer/rust_project.rs | 27 ++++++++---- 7 files changed, 151 insertions(+), 17 deletions(-) create mode 100644 test/rust_analyzer/generated_srcs_test/BUILD.bazel create mode 100644 test/rust_analyzer/generated_srcs_test/lib.rs create mode 100644 test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs diff --git a/rust/private/rust_analyzer.bzl b/rust/private/rust_analyzer.bzl index b972206f60..dfccf5a051 100644 --- a/rust/private/rust_analyzer.bzl +++ b/rust/private/rust_analyzer.bzl @@ -46,6 +46,7 @@ def write_rust_analyzer_spec_file(ctx, attrs, owner, base_info): """ crate_spec = ctx.actions.declare_file("{}.rust_analyzer_crate_spec.json".format(owner.name)) + # Recreate the provider with the spec file embedded in it. rust_analyzer_info = RustAnalyzerInfo( aliases = base_info.aliases, crate = base_info.crate, @@ -224,16 +225,24 @@ def _create_single_crate(ctx, attrs, info): path_prefix = _EXEC_ROOT_TEMPLATE if is_external or is_generated else "" crate["is_workspace_member"] = not is_external crate["root_module"] = path_prefix + info.crate.root.path - crate_root = path_prefix + info.crate.root.dirname + crate["source"] = {"exclude_dirs": [], "include_dirs": []} + + if is_generated: + srcs = getattr(ctx.rule.files, "srcs", []) + src_map = {src.short_path: src for src in srcs if src.is_source} + if info.crate.root.short_path in src_map: + crate["root_module"] = src_map[info.crate.root.short_path].path + crate["source"]["include_dirs"].append(path_prefix + info.crate.root.dirname) if info.build_info != None and info.build_info.out_dir != None: out_dir_path = info.build_info.out_dir.path crate["env"].update({"OUT_DIR": _EXEC_ROOT_TEMPLATE + out_dir_path}) - crate["source"] = { - # We have to tell rust-analyzer about our out_dir since it's not under the crate root. - "exclude_dirs": [], - "include_dirs": [crate_root, _EXEC_ROOT_TEMPLATE + out_dir_path], - } + + # We have to tell rust-analyzer about our out_dir since it's not under the crate root. + crate["source"]["include_dirs"].extend([ + path_prefix + info.crate.root.dirname, + _EXEC_ROOT_TEMPLATE + out_dir_path, + ]) # TODO: The only imagined use case is an env var holding a filename in the workspace passed to a # macro like include_bytes!. Other use cases might exist that require more complex logic. diff --git a/test/rust_analyzer/generated_srcs_test/BUILD.bazel b/test/rust_analyzer/generated_srcs_test/BUILD.bazel new file mode 100644 index 0000000000..96045899d8 --- /dev/null +++ b/test/rust_analyzer/generated_srcs_test/BUILD.bazel @@ -0,0 +1,37 @@ +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") + +write_file( + name = "generated_rs", + out = "generated.rs", + content = [ + "pub fn forty_two() -> i32 { 42 }", + "", + ], +) + +rust_library( + name = "generated_srcs", + srcs = [ + "lib.rs", + ":generated.rs", + ], + edition = "2021", +) + +rust_test( + name = "generated_srcs_test", + crate = ":generated_srcs", +) + +rust_test( + name = "rust_project_json_test", + srcs = ["rust_project_json_test.rs"], + data = [":rust-project.json"], + edition = "2021", + env = {"RUST_PROJECT_JSON": "$(rootpath :rust-project.json)"}, + # This target is tagged as manual since it's not expected to pass in + # contexts outside of `//test/rust_analyzer:rust_analyzer_test`. Run + # that target to execute this test. + tags = ["manual"], +) diff --git a/test/rust_analyzer/generated_srcs_test/lib.rs b/test/rust_analyzer/generated_srcs_test/lib.rs new file mode 100644 index 0000000000..eb0e121823 --- /dev/null +++ b/test/rust_analyzer/generated_srcs_test/lib.rs @@ -0,0 +1,9 @@ +pub mod generated; + +#[cfg(test)] +mod tests { + #[test] + fn test_fourty_two() { + assert_eq!(super::generated::forty_two(), 42); + } +} diff --git a/test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs b/test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs new file mode 100644 index 0000000000..8ba6bb4cd7 --- /dev/null +++ b/test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs @@ -0,0 +1,43 @@ +#[cfg(test)] +mod tests { + use std::env; + use std::path::PathBuf; + + #[test] + fn test_deps_of_crate_and_its_test_are_merged() { + let rust_project_path = PathBuf::from(env::var("RUST_PROJECT_JSON").unwrap()); + + let content = std::fs::read_to_string(&rust_project_path) + .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); + + let output_base = content + .lines() + .find(|text| text.trim_start().starts_with("\"sysroot_src\":")) + .map(|text| { + let mut split = text.splitn(2, "\"sysroot_src\": "); + let mut with_hash = split.nth(1).unwrap().trim().splitn(2, "/external/"); + let mut output = with_hash.next().unwrap().rsplitn(2, '/'); + output.nth(1).unwrap() + }) + .expect("Failed to find sysroot entry."); + + let expected = r#"{ + "display_name": "generated_srcs", + "root_module": "lib.rs", + "edition": "2021", + "deps": [], + "is_workspace_member": true, + "source": { + "include_dirs": [ + "# + .to_owned() + + output_base; + + println!("{}", content); + assert!( + content.contains(&expected), + "expected rust-project.json to contain the following block:\n{}", + expected + ); + } +} diff --git a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs index 8f0db407f1..6e476b9651 100644 --- a/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs +++ b/test/rust_analyzer/merging_crates_test/rust_project_json_test.rs @@ -10,8 +10,24 @@ mod tests { let content = std::fs::read_to_string(&rust_project_path) .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); + let expected = r#"{ + "display_name": "mylib", + "root_module": "mylib.rs", + "edition": "2018", + "deps": [ + { + "crate": 0, + "name": "extra_test_dep" + }, + { + "crate": 1, + "name": "lib_dep" + } + ],"#; + + println!("{}", content); assert!( - content.contains(r#""root_module":"mylib.rs","edition":"2018","deps":[{"crate":0,"name":"extra_test_dep"},{"crate":1,"name":"lib_dep"}]"#), + content.contains(expected), "expected rust-project.json to contain both lib_dep and extra_test_dep in deps of mylib.rs."); } } diff --git a/test/rust_analyzer/static_and_shared_lib_test/rust_project_json_test.rs b/test/rust_analyzer/static_and_shared_lib_test/rust_project_json_test.rs index fc573d6493..6bff4fcd36 100644 --- a/test/rust_analyzer/static_and_shared_lib_test/rust_project_json_test.rs +++ b/test/rust_analyzer/static_and_shared_lib_test/rust_project_json_test.rs @@ -10,12 +10,21 @@ mod tests { let content = std::fs::read_to_string(&rust_project_path) .unwrap_or_else(|_| panic!("couldn't open {:?}", &rust_project_path)); + println!("{}", content); + + let expected_cdylib = r#"{ + "display_name": "greeter_cdylib", + "root_module": "shared_lib.rs","#; assert!( - content.contains(r#"{"display_name":"greeter_cdylib","root_module":"shared_lib.rs"#), + content.contains(expected_cdylib), "expected rust-project.json to contain a rust_shared_library target." ); + + let expected_staticlib = r#"{ + "display_name": "greeter_staticlib", + "root_module": "static_lib.rs","#; assert!( - content.contains(r#"{"display_name":"greeter_staticlib","root_module":"static_lib.rs"#), + content.contains(expected_staticlib), "expected rust-project.json to contain a rust_static_library target." ); } diff --git a/tools/rust_analyzer/rust_project.rs b/tools/rust_analyzer/rust_project.rs index 09ad72385e..248a196fd6 100644 --- a/tools/rust_analyzer/rust_project.rs +++ b/tools/rust_analyzer/rust_project.rs @@ -33,6 +33,7 @@ pub struct RustProject { /// [rust-analyzer documentation][rd] for a thorough description of this interface. /// [rd]: https://rust-analyzer.github.io/manual.html#non-cargo-based-projects #[derive(Debug, Serialize)] +#[serde(default)] pub struct Crate { /// A name used in the package's project declaration #[serde(skip_serializing_if = "Option::is_none")] @@ -52,8 +53,8 @@ pub struct Crate { is_workspace_member: Option, /// Optionally specify the (super)set of `.rs` files comprising this crate. - #[serde(skip_serializing_if = "Option::is_none")] - source: Option, + #[serde(skip_serializing_if = "Source::is_empty")] + source: Source, /// The set of cfgs activated for a given crate, like /// `["unix", "feature=\"foo\"", "feature=\"bar\""]`. @@ -75,12 +76,19 @@ pub struct Crate { proc_macro_dylib_path: Option, } -#[derive(Debug, Serialize)] +#[derive(Debug, Default, Serialize)] pub struct Source { include_dirs: Vec, exclude_dirs: Vec, } +impl Source { + /// Returns true if no include information has been added. + fn is_empty(&self) -> bool { + self.include_dirs.is_empty() && self.exclude_dirs.is_empty() + } +} + #[derive(Debug, Serialize)] pub struct Dependency { /// Index of a crate in the `crates` array. @@ -150,10 +158,13 @@ pub fn generate_rust_project( }) .collect(), is_workspace_member: Some(c.is_workspace_member), - source: c.source.as_ref().map(|s| Source { - exclude_dirs: s.exclude_dirs.clone(), - include_dirs: s.include_dirs.clone(), - }), + source: match &c.source { + Some(s) => Source { + exclude_dirs: s.exclude_dirs.clone(), + include_dirs: s.include_dirs.clone(), + }, + None => Source::default(), + }, cfg: c.cfg.clone(), target: Some(c.target.clone()), env: Some(c.env.clone()), @@ -258,7 +269,7 @@ pub fn write_rust_project( // Render the `rust-project.json` file and replace the exec root // placeholders with the path to the local exec root. - let rust_project_content = serde_json::to_string(rust_project)? + let rust_project_content = serde_json::to_string_pretty(rust_project)? .replace("${pwd}", execution_root) .replace("__EXEC_ROOT__", execution_root) .replace("__OUTPUT_BASE__", output_base);