Merge deps of related crates in Rust Analyzer support (#781)

It can happen a single source file is present in multiple crates - there can
  be a `rust_library` with a `lib.rs` file, and a `rust_test` for the `test`
  module in that file. Tests can declare more dependencies than what library
  had. Therefore we had to collect all `RustAnalyzerInfo`s for a given crate
  and take deps from all of them.

  There's one exception - if the dependency is the same crate name as the
  the crate being processed, we don't add it as a dependency to itself. This is
  common and expected - `rust_test.crate` pointing to the `rust_library`.

rust/private/rust_analyzer.bzl

@@ -44,7 +44,8 @@ def _rust_analyzer_aspect_impl(target, ctx):
 
     toolchain = find_toolchain(ctx)
 
-    # Always add test & debug_assertions (like here: https://github.com/rust-analyzer/rust-analyzer/blob/505ff4070a3de962dbde66f08b6550cda2eb4eab/crates/project_model/src/lib.rs#L379-L381)
+    # Always add test & debug_assertions (like here:
+    # https://github.com/rust-analyzer/rust-analyzer/blob/505ff4070a3de962dbde66f08b6550cda2eb4eab/crates/project_model/src/lib.rs#L379-L381)
     cfgs = ["test", "debug_assertions"]
     if hasattr(ctx.rule.attr, "crate_features"):
         cfgs += ['feature="{}"'.format(f) for f in ctx.rule.attr.crate_features]
@@ -116,51 +117,73 @@ def _crate_id(crate_info):
     """
     return "ID-" + crate_info.root.path
 
-def create_crate(ctx, info, crate_mapping):
-    """Creates a crate in the rust-project.json format
+def _create_crate(ctx, infos, crate_mapping):
+    """Creates a crate in the rust-project.json format.
+
+    It can happen a single source file is present in multiple crates - there can
+    be a `rust_library` with a `lib.rs` file, and a `rust_test` for the `test`
+    module in that file. Tests can declare more dependencies than what library
+    had. Therefore we had to collect all RustAnalyzerInfos for a given crate
+    and take deps from all of them.
+
+    There's one exception - if the dependency is the same crate name as the
+    the crate being processed, we don't add it as a dependency to itself. This is
+    common and expected - `rust_test.crate` pointing to the `rust_library`.
 
     Args:
         ctx (ctx): The rule context
-        info (RustAnalyzerInfo): The crate RustAnalyzerInfo for the current crate
+        infos (list of RustAnalyzerInfos): RustAnalyzerInfos for the current crate
         crate_mapping (dict): A dict of {String:Int} that memoizes crates for deps.
 
     Returns:
         (dict) The crate rust-project.json representation
     """
+    if len(infos) == 0:
+        fail("Expected to receive at least one crate to serialize to json, got 0.")
+    canonical_info = infos[0]
+    crate_name = canonical_info.crate.name
     crate = dict()
-    crate["display_name"] = info.crate.name
-    crate["edition"] = info.crate.edition
+    crate["display_name"] = crate_name
+    crate["edition"] = canonical_info.crate.edition
     crate["env"] = {}
 
     # Switch on external/ to determine if crates are in the workspace or remote.
     # TODO: Some folks may want to override this for vendored dependencies.
-    if info.crate.root.path.startswith("external/"):
+    root_path = canonical_info.crate.root.path
+    root_dirname = canonical_info.crate.root.dirname
+    if root_path.startswith("external/"):
         crate["is_workspace_member"] = False
-        crate["root_module"] = _exec_root_tmpl + info.crate.root.path
-        crate_root = _exec_root_tmpl + info.crate.root.dirname
+        crate["root_module"] = _exec_root_tmpl + root_path
+        crate_root = _exec_root_tmpl + root_dirname
     else:
         crate["is_workspace_member"] = True
-        crate["root_module"] = info.crate.root.path
-        crate_root = info.crate.root.dirname
+        crate["root_module"] = root_path
+        crate_root = root_dirname
 
-    if info.build_info != None:
-        crate["env"].update({"OUT_DIR": _exec_root_tmpl + info.build_info.out_dir.path})
+    if canonical_info.build_info != None:
+        out_dir_path = canonical_info.build_info.out_dir.path
+        crate["env"].update({"OUT_DIR": _exec_root_tmpl + 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_tmpl + info.build_info.out_dir.path],
+            "include_dirs": [crate_root, _exec_root_tmpl + out_dir_path],
         }
-    crate["env"].update(info.env)
-
-    deps = [
-        {"crate": crate_mapping[_crate_id(d.crate)], "name": d.crate.name}
-        for d in info.deps
-    ]
-    crate["deps"] = deps
-    crate["cfg"] = info.cfgs
+    crate["env"].update(canonical_info.env)
+
+    # Collect deduplicated pairs of (crate idx from crate_mapping, crate name).
+    # Using dict because we don't have sets in Starlark.
+    deps = {
+        (crate_mapping[_crate_id(dep.crate)], dep.crate.name): None
+        for info in infos
+        for dep in info.deps
+        if dep.crate.name != crate_name
+    }.keys()
+
+    crate["deps"] = [{"crate": d[0], "name": d[1]} for d in deps]
+    crate["cfg"] = canonical_info.cfgs
     crate["target"] = find_toolchain(ctx).target_triple
-    if info.proc_macro_dylib_path != None:
-        crate["proc_macro_dylib_path"] = _exec_root_tmpl + info.proc_macro_dylib_path
+    if canonical_info.proc_macro_dylib_path != None:
+        crate["proc_macro_dylib_path"] = _exec_root_tmpl + canonical_info.proc_macro_dylib_path
     return crate
 
 # This implementation is incomplete because in order to get rustc env vars we
@@ -181,29 +204,33 @@ def _rust_analyzer_impl(ctx):
     if rust_toolchain.rustc_srcs.label.workspace_root:
         sysroot_src = _exec_root_tmpl + rust_toolchain.rustc_srcs.label.workspace_root + "/" + sysroot_src
 
-    # Gather all crates and their dependencies into an array.
-    # Dependencies are referenced by index, so leaves should come first.
-    crates = []
+    # Groups of RustAnalyzerInfos with the same _crate_id().
+    rust_analyzer_info_groups = []
+
+    # Dict from _crate_id() to the index of a RustAnalyzerInfo group in `rust_analyzer_info_groups`.
     crate_mapping = dict()
+
+    # Dependencies are referenced by index, so leaves should come first.
     idx = 0
     for target in ctx.attr.targets:
         if RustAnalyzerInfo not in target:
             continue
 
-        # Add this crate's transitive deps to the crate mapping and output.
-        for dep_info in target[RustAnalyzerInfo].transitive_deps.to_list():
-            crate_id = _crate_id(dep_info.crate)
+        for info in depset(
+            direct = [target[RustAnalyzerInfo]],
+            transitive = [target[RustAnalyzerInfo].transitive_deps],
+            order = "postorder",
+        ).to_list():
+            crate_id = _crate_id(info.crate)
             if crate_id not in crate_mapping:
                 crate_mapping[crate_id] = idx
+                rust_analyzer_info_groups.append([])
                 idx += 1
-                crates.append(create_crate(ctx, dep_info, crate_mapping))
-
-        # Add this crate to the crate mapping and output.
-        crate_id = _crate_id(target[RustAnalyzerInfo].crate)
-        if crate_id not in crate_mapping:
-            crate_mapping[crate_id] = idx
-            idx += 1
-            crates.append(create_crate(ctx, target[RustAnalyzerInfo], crate_mapping))
+            rust_analyzer_info_groups[crate_mapping[crate_id]].append(info)
+
+    crates = []
+    for group in rust_analyzer_info_groups:
+        crates.append(_create_crate(ctx, group, crate_mapping))
 
     # TODO(djmarcin): Use json module once bazel 4.0 is released.
     ctx.actions.write(output = ctx.outputs.filename, content = struct(

test/rust_analyzer/rust_project_json_test.rs → test/rust_analyzer/aspect_traversal_test/rust_project_json_test.rs

@@ -2,7 +2,8 @@ use runfiles::Runfiles;
 
 fn main() {
     let r = Runfiles::create().unwrap();
-    let rust_project_path = r.rlocation("rules_rust/test/rust_analyzer/rust-project.json");
+    let rust_project_path =
+        r.rlocation("rules_rust/test/rust_analyzer/aspect_traversal_test/rust-project.json");
 
     let content = std::fs::read_to_string(&rust_project_path)
         .expect(&format!("couldn't open {:?}", &rust_project_path));

test/rust_analyzer/merging_crates_test/BUILD.bazel

@@ -0,0 +1,41 @@
+load("//rust:defs.bzl", "rust_analyzer", "rust_library", "rust_test")
+
+rust_library(
+    name = "mylib",
+    srcs = ["mylib.rs"],
+    deps = [":lib_dep"],
+)
+
+rust_library(
+    name = "lib_dep",
+    srcs = ["lib_dep.rs"],
+)
+
+rust_test(
+    name = "mylib_test",
+    crate = ":mylib",
+    deps = [":extra_test_dep"],
+)
+
+rust_library(
+    name = "extra_test_dep",
+    srcs = ["extra_test_dep.rs"],
+)
+
+rust_analyzer(
+    name = "rust_analyzer",
+    testonly = True,
+    targets = [
+        # it's significant that `mylib` goes before `mylib_test`.
+        ":mylib",
+        ":mylib_test",
+    ],
+)
+
+rust_test(
+    name = "rust_project_json_test",
+    srcs = ["rust_project_json_test.rs"],
+    data = [":rust-project.json"],
+    edition = "2018",
+    deps = ["//tools/runfiles"],
+)

test/rust_analyzer/merging_crates_test/extra_test_dep.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test/lib_dep.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test/mylib.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test/rust_project_json_test.rs

@@ -0,0 +1,20 @@
+use runfiles::Runfiles;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_deps_of_crate_and_its_test_are_merged() {
+        let r = Runfiles::create().unwrap();
+        let rust_project_path =
+            r.rlocation("rules_rust/test/rust_analyzer/merging_crates_test/rust-project.json");
+
+        let content = std::fs::read_to_string(&rust_project_path)
+            .expect(&format!("couldn't open {:?}", &rust_project_path));
+
+        assert!(
+            content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":2,"name":"extra_test_dep"}]"#),
+            "expected rust-project.json to contain both lib_dep and extra_test_dep in deps of mylib.rs.");
+    }
+}

test/rust_analyzer/merging_crates_test_reversed/BUILD.bazel

@@ -0,0 +1,41 @@
+load("//rust:defs.bzl", "rust_analyzer", "rust_library", "rust_test")
+
+rust_library(
+    name = "mylib",
+    srcs = ["mylib.rs"],
+    deps = [":lib_dep"],
+)
+
+rust_library(
+    name = "lib_dep",
+    srcs = ["lib_dep.rs"],
+)
+
+rust_test(
+    name = "mylib_test",
+    crate = ":mylib",
+    deps = [":extra_test_dep"],
+)
+
+rust_library(
+    name = "extra_test_dep",
+    srcs = ["extra_test_dep.rs"],
+)
+
+rust_analyzer(
+    name = "rust_analyzer",
+    testonly = True,
+    targets = [
+        # it's significant that `mylib_test` goes before `mylib`.
+        ":mylib_test",
+        ":mylib",
+    ],
+)
+
+rust_test(
+    name = "rust_project_json_test",
+    srcs = ["rust_project_json_test.rs"],
+    data = [":rust-project.json"],
+    edition = "2018",
+    deps = ["//tools/runfiles"],
+)

test/rust_analyzer/merging_crates_test_reversed/extra_test_dep.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test_reversed/lib_dep.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test_reversed/mylib.rs

@@ -0,0 +1 @@
+

test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs

@@ -0,0 +1,21 @@
+use runfiles::Runfiles;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_deps_of_crate_and_its_test_are_merged() {
+        let r = Runfiles::create().unwrap();
+        let rust_project_path = r.rlocation(
+            "rules_rust/test/rust_analyzer/merging_crates_test_reversed/rust-project.json",
+        );
+
+        let content = std::fs::read_to_string(&rust_project_path)
+            .expect(&format!("couldn't open {:?}", &rust_project_path));
+
+        assert!(
+            content.contains(r#""root_module":"test/rust_analyzer/merging_crates_test_reversed/mylib.rs","deps":[{"crate":0,"name":"lib_dep"},{"crate":1,"name":"extra_test_dep"}]"#),
+            "expected rust-project.json to contain both lib_dep and extra_test_dep in deps of mylib.rs.");
+    }
+}