Fixed loss of data in gen_rust_project for duplicate crate ids (#3317)

UebelAndre · web-flow · commit 8f1424c9cf09 · 2025-03-10T14:52:00.000Z
Previously `aliases` and `source` were susceptible to being dropped from
`rust-project.json` in the event a crate without either of these fields
was processed before another crate with them. These are now aggregated
and `gen_rust_project` should be more resilient to the ordering targets
are listed in aquery outputs.
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
@@ -27,7 +27,6 @@ mod tests {
         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));
-        println!("{}", content);
         let project: Project =
             serde_json::from_str(&content).expect("Failed to deserialize project JSON");
 
@@ -42,15 +41,15 @@ mod tests {
             .unwrap();
         println!("output_base: {output_base}");
 
-        let gen = project
+        let with_gen = project
             .crates
             .iter()
             .find(|c| &c.display_name == "generated_srcs")
             .unwrap();
-        assert!(gen.root_module.starts_with("/"));
-        assert!(gen.root_module.ends_with("/lib.rs"));
+        assert!(with_gen.root_module.starts_with("/"));
+        assert!(with_gen.root_module.ends_with("/lib.rs"));
 
-        let include_dirs = &gen.source.as_ref().unwrap().include_dirs;
+        let include_dirs = &with_gen.source.as_ref().unwrap().include_dirs;
         assert!(include_dirs.len() == 1);
         assert!(include_dirs[0].starts_with(output_base));
     }
diff --git a/test/rust_analyzer/rust_analyzer_test_runner.sh b/test/rust_analyzer/rust_analyzer_test_runner.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 
 # This script creates temporary workspaces and generates `rust-project.json`
 # files unique to the set of targets defined in the generated workspace.
@@ -77,6 +77,10 @@ function rust_analyzer_test() {
     local source_dir="$1"
     local workspace="$2"
     local generator_arg="$3"
+    local rust_log="info"
+    if [[ -n "${RUST_ANALYZER_TEST_DEBUG:-}" ]]; then
+        rust_log="debug"
+    fi
 
     echo "Testing '$(basename "${source_dir}")'"
     rm -f "${workspace}"/*.rs "${workspace}"/*.json "${workspace}"/*.bzl "${workspace}/BUILD.bazel" "${workspace}/BUILD.bazel-e"
@@ -92,10 +96,11 @@ function rust_analyzer_test() {
 
     pushd "${workspace}" &>/dev/null
     echo "Generating rust-project.json..."
+
     if [[ -n "${generator_arg}" ]]; then
-        bazel run "@rules_rust//tools/rust_analyzer:gen_rust_project" -- "${generator_arg}"
+        RUST_LOG="${rust_log}" bazel run "@rules_rust//tools/rust_analyzer:gen_rust_project" -- "${generator_arg}"
     else
-        bazel run "@rules_rust//tools/rust_analyzer:gen_rust_project"
+        RUST_LOG="${rust_log}" bazel run "@rules_rust//tools/rust_analyzer:gen_rust_project"
     fi
     echo "Building..."
     bazel build //...
@@ -109,9 +114,11 @@ function rust_analyzer_test() {
 function cleanup() {
     local workspace="$1"
     pushd "${workspace}" &>/dev/null
-    bazel clean --async
+    bazel clean --expunge --async
     popd &>/dev/null
-    rm -rf "${workspace}"
+    if [[ -z "${RUST_ANALYZER_TEST_DEBUG:-}" ]]; then
+        rm -rf "${workspace}"
+    fi
 }
 
 function run_test_suite() {
diff --git a/tools/rust_analyzer/aquery.rs b/tools/rust_analyzer/aquery.rs
@@ -1,5 +1,4 @@
 use std::collections::{BTreeMap, BTreeSet};
-use std::fs::File;
 use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command;
@@ -71,7 +70,8 @@ pub fn get_crate_specs(
     log::debug!("Get crate specs with targets: {:?}", targets);
     let target_pattern = format!("deps({})", targets.join("+"));
 
-    let aquery_output = Command::new(bazel)
+    let mut aquery_command = Command::new(bazel);
+    aquery_command
         .current_dir(workspace)
         .env_remove("BAZELISK_SKIP_WRAPPER")
         .env_remove("BUILD_WORKING_DIRECTORY")
@@ -86,18 +86,26 @@ pub fn get_crate_specs(
         .arg(format!(
             r#"outputs(".*\.rust_analyzer_crate_spec\.json",{target_pattern})"#
         ))
-        .arg("--output=jsonproto")
-        .output()?;
+        .arg("--output=jsonproto");
+    log::trace!("Running aquery: {:#?}", aquery_command);
+    let aquery_output = aquery_command
+        .output()
+        .context("Failed to spawn aquery command")?;
 
-    let crate_spec_files =
-        parse_aquery_output_files(execution_root, &String::from_utf8(aquery_output.stdout)?)?;
+    let aquery_results = String::from_utf8(aquery_output.stdout)
+        .context("Failed to decode aquery results as utf-8.")?;
+
+    log::trace!("Aquery results: {}", &aquery_results);
+
+    let crate_spec_files = parse_aquery_output_files(execution_root, &aquery_results)?;
 
     let crate_specs = crate_spec_files
         .into_iter()
         .map(|file| {
-            let f = File::open(&file)
-                .with_context(|| format!("Failed to open file: {}", file.display()))?;
-            serde_json::from_reader(f)
+            let content = std::fs::read_to_string(&file)
+                .with_context(|| format!("Failed to read file: {}", file.display()))?;
+            log::trace!("{}\n{}", file.display(), content);
+            serde_json::from_str(&content)
                 .with_context(|| format!("Failed to deserialize file: {}", file.display()))
         })
         .collect::<anyhow::Result<Vec<CrateSpec>>>()?;
@@ -177,6 +185,22 @@ fn consolidate_crate_specs(crate_specs: Vec<CrateSpec>) -> anyhow::Result<BTreeS
         log::debug!("{:?}", spec);
         if let Some(existing) = consolidated_specs.get_mut(&spec.crate_id) {
             existing.deps.extend(spec.deps);
+            existing.aliases.extend(spec.aliases);
+
+            if let Some(source) = &mut existing.source {
+                if let Some(mut new_source) = spec.source {
+                    new_source
+                        .exclude_dirs
+                        .retain(|src| !source.exclude_dirs.contains(src));
+                    new_source
+                        .include_dirs
+                        .retain(|src| !source.include_dirs.contains(src));
+                    source.exclude_dirs.extend(new_source.exclude_dirs);
+                    source.include_dirs.extend(new_source.include_dirs);
+                }
+            } else {
+                existing.source = spec.source;
+            }
 
             spec.cfg.retain(|cfg| !existing.cfg.contains(cfg));
             existing.cfg.extend(spec.cfg);
@@ -622,4 +646,190 @@ mod test {
             );
         }
     }
+
+    #[test]
+    fn consolidate_spec_with_aliases() {
+        let crate_specs = vec![
+            CrateSpec {
+                aliases: BTreeMap::new(),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: None,
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "rlib".into(),
+            },
+            CrateSpec {
+                aliases: BTreeMap::from([("ID-mylib_dep.rs".into(), "aliased_name".into())]),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib_test".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: None,
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "bin".into(),
+            },
+        ];
+
+        for perm in crate_specs.into_iter().permutations(2) {
+            assert_eq!(
+                consolidate_crate_specs(perm).unwrap(),
+                BTreeSet::from([CrateSpec {
+                    aliases: BTreeMap::from([("ID-mylib_dep.rs".into(), "aliased_name".into())]),
+                    crate_id: "ID-mylib.rs".into(),
+                    display_name: "mylib".into(),
+                    edition: "2018".into(),
+                    root_module: "mylib.rs".into(),
+                    is_workspace_member: true,
+                    deps: BTreeSet::from([]),
+                    proc_macro_dylib_path: None,
+                    source: None,
+                    cfg: vec!["test".into(), "debug_assertions".into()],
+                    env: BTreeMap::new(),
+                    target: "x86_64-unknown-linux-gnu".into(),
+                    crate_type: "rlib".into(),
+                }])
+            );
+        }
+    }
+
+    #[test]
+    fn consolidate_spec_with_sources() {
+        let crate_specs = vec![
+            CrateSpec {
+                aliases: BTreeMap::new(),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: None,
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "rlib".into(),
+            },
+            CrateSpec {
+                aliases: BTreeMap::new(),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib_test".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: Some(CrateSpecSource {
+                    exclude_dirs: vec!["exclude".into()],
+                    include_dirs: vec!["include".into()],
+                }),
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "bin".into(),
+            },
+        ];
+
+        for perm in crate_specs.into_iter().permutations(2) {
+            assert_eq!(
+                consolidate_crate_specs(perm).unwrap(),
+                BTreeSet::from([CrateSpec {
+                    aliases: BTreeMap::new(),
+                    crate_id: "ID-mylib.rs".into(),
+                    display_name: "mylib".into(),
+                    edition: "2018".into(),
+                    root_module: "mylib.rs".into(),
+                    is_workspace_member: true,
+                    deps: BTreeSet::from([]),
+                    proc_macro_dylib_path: None,
+                    source: Some(CrateSpecSource {
+                        exclude_dirs: vec!["exclude".into()],
+                        include_dirs: vec!["include".into()],
+                    }),
+                    cfg: vec!["test".into(), "debug_assertions".into()],
+                    env: BTreeMap::new(),
+                    target: "x86_64-unknown-linux-gnu".into(),
+                    crate_type: "rlib".into(),
+                }])
+            );
+        }
+    }
+
+    #[test]
+    fn consolidate_spec_with_duplicate_sources() {
+        let crate_specs = vec![
+            CrateSpec {
+                aliases: BTreeMap::new(),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: Some(CrateSpecSource {
+                    exclude_dirs: vec!["exclude".into()],
+                    include_dirs: vec!["include".into()],
+                }),
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "rlib".into(),
+            },
+            CrateSpec {
+                aliases: BTreeMap::new(),
+                crate_id: "ID-mylib.rs".into(),
+                display_name: "mylib_test".into(),
+                edition: "2018".into(),
+                root_module: "mylib.rs".into(),
+                is_workspace_member: true,
+                deps: BTreeSet::new(),
+                proc_macro_dylib_path: None,
+                source: Some(CrateSpecSource {
+                    exclude_dirs: vec!["exclude".into()],
+                    include_dirs: vec!["include".into()],
+                }),
+                cfg: vec!["test".into(), "debug_assertions".into()],
+                env: BTreeMap::new(),
+                target: "x86_64-unknown-linux-gnu".into(),
+                crate_type: "bin".into(),
+            },
+        ];
+
+        for perm in crate_specs.into_iter().permutations(2) {
+            assert_eq!(
+                consolidate_crate_specs(perm).unwrap(),
+                BTreeSet::from([CrateSpec {
+                    aliases: BTreeMap::new(),
+                    crate_id: "ID-mylib.rs".into(),
+                    display_name: "mylib".into(),
+                    edition: "2018".into(),
+                    root_module: "mylib.rs".into(),
+                    is_workspace_member: true,
+                    deps: BTreeSet::from([]),
+                    proc_macro_dylib_path: None,
+                    source: Some(CrateSpecSource {
+                        exclude_dirs: vec!["exclude".into()],
+                        include_dirs: vec!["include".into()],
+                    }),
+                    cfg: vec!["test".into(), "debug_assertions".into()],
+                    env: BTreeMap::new(),
+                    target: "x86_64-unknown-linux-gnu".into(),
+                    crate_type: "rlib".into(),
+                }])
+            );
+        }
+    }
 }
