From b2eb0c862873c25944535f081a6307456762c126 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Tue, 22 Jun 2021 09:22:57 +0200 Subject: [PATCH] Fix rust analyzer --- rust/private/rust_analyzer.bzl | 106 ++++++++++++------ .../merging_crates_test/BUILD.bazel | 8 +- .../rust_project_json_test.rs | 8 +- .../merging_crates_test_reversed/BUILD.bazel | 41 +++++++ .../extra_test_dep.rs | 1 + .../merging_crates_test_reversed/lib_dep.rs | 1 + .../merging_crates_test_reversed/mylib.rs | 1 + .../rust_project_json_test.rs | 19 ++++ 8 files changed, 142 insertions(+), 43 deletions(-) create mode 100644 test/rust_analyzer/merging_crates_test_reversed/BUILD.bazel create mode 100644 test/rust_analyzer/merging_crates_test_reversed/extra_test_dep.rs create mode 100644 test/rust_analyzer/merging_crates_test_reversed/lib_dep.rs create mode 100644 test/rust_analyzer/merging_crates_test_reversed/mylib.rs create mode 100644 test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs diff --git a/rust/private/rust_analyzer.bzl b/rust/private/rust_analyzer.bzl index 3b16dcd574..22ceff7af6 100644 --- a/rust/private/rust_analyzer.bzl +++ b/rust/private/rust_analyzer.bzl @@ -20,6 +20,7 @@ given targets. This file can be consumed by rust-analyzer as an alternative to Cargo.toml files. """ +load("@bazel_skylib//lib:collections.bzl", "collections") load("//rust/platform:triple_mappings.bzl", "system_to_dylib_ext", "triple_to_system") load("//rust/private:common.bzl", "rust_common") load("//rust/private:rustc.bzl", "BuildInfo") @@ -50,7 +51,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] @@ -122,51 +124,79 @@ 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 +_DepRefInfo = provider( + doc = "RustAnalyzerInfo holds rust crate metadata for targets", + fields = { + "crate": "BuildInfo: build info for this crate if present", + "name": "List[String]: features or other compilation --cfg settings", + }, +) + +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 has the same crate name as the + the crate being processed, we don't add it as a dependency to itself. This is + the 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 """ + 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) + crate["env"].update(canonical_info.env) + + duplicated_deps = [] + for info in infos: + for dep in info.deps: + if dep.crate.name == crate_name: + continue + duplicated_deps.append(_DepRefInfo(crate = crate_mapping[_crate_id(dep.crate)], name = dep.crate.name)) + + deps = [{"crate": d.crate, "name": d.name} for d in collections.uniq(duplicated_deps)] - 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["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 @@ -187,29 +217,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( diff --git a/test/rust_analyzer/merging_crates_test/BUILD.bazel b/test/rust_analyzer/merging_crates_test/BUILD.bazel index 2f93da81ec..a54e1932e0 100644 --- a/test/rust_analyzer/merging_crates_test/BUILD.bazel +++ b/test/rust_analyzer/merging_crates_test/BUILD.bazel @@ -1,4 +1,4 @@ -load("//rust:defs.bzl", "rust_analyzer", "rust_library", "rust_proc_macro", "rust_test") +load("//rust:defs.bzl", "rust_analyzer", "rust_library", "rust_test") rust_library( name = "mylib", @@ -25,7 +25,11 @@ rust_library( rust_analyzer( name = "rust_analyzer", testonly = True, - targets = [":mylib_test", ":mylib", ], + targets = [ + # it's significant that `mylib` goes before `mylib_test`. + ":mylib", + ":mylib_test", + ], ) rust_test( 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 c57e01c7ab..2be7caa0ae 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 @@ -12,10 +12,8 @@ mod tests { let content = std::fs::read_to_string(&rust_project_path) .expect(&format!("couldn't open {:?}", &rust_project_path)); - for dep in &["lib_dep","", "extra_test_dep"] { - if !content.contains(dep) { - panic!("expected rust-project.json to contain {}.", dep); - } - } + 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."); } } diff --git a/test/rust_analyzer/merging_crates_test_reversed/BUILD.bazel b/test/rust_analyzer/merging_crates_test_reversed/BUILD.bazel new file mode 100644 index 0000000000..8cdd7f567d --- /dev/null +++ b/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"], +) diff --git a/test/rust_analyzer/merging_crates_test_reversed/extra_test_dep.rs b/test/rust_analyzer/merging_crates_test_reversed/extra_test_dep.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/test/rust_analyzer/merging_crates_test_reversed/extra_test_dep.rs @@ -0,0 +1 @@ + diff --git a/test/rust_analyzer/merging_crates_test_reversed/lib_dep.rs b/test/rust_analyzer/merging_crates_test_reversed/lib_dep.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/test/rust_analyzer/merging_crates_test_reversed/lib_dep.rs @@ -0,0 +1 @@ + diff --git a/test/rust_analyzer/merging_crates_test_reversed/mylib.rs b/test/rust_analyzer/merging_crates_test_reversed/mylib.rs new file mode 100644 index 0000000000..8b13789179 --- /dev/null +++ b/test/rust_analyzer/merging_crates_test_reversed/mylib.rs @@ -0,0 +1 @@ + diff --git a/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs new file mode 100644 index 0000000000..dc5d71975a --- /dev/null +++ b/test/rust_analyzer/merging_crates_test_reversed/rust_project_json_test.rs @@ -0,0 +1,19 @@ +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."); + } +}