Mitigate & document problems with mixed generated sources

- deterministically prefer rust_library's crate spec over rust_test's.
  This means root_module gets remapped to a workspace path more often,
  and fixes flakiness of generated_srcs_test.
- document the reasons and tradeoffs for this remapping.
- stop adding `include_dirs` when remapping, it doesn't do anything to
  help with this problem, and is confusing.
- fix test to strictly assert the path, which I broke in
  74f164b

Partially fixes bazelbuild#3126

rust/private/rust_analyzer.bzl

@@ -230,12 +230,24 @@ def _create_single_crate(ctx, attrs, info):
     crate["root_module"] = path_prefix + info.crate.root.path
     crate["source"] = {"exclude_dirs": [], "include_dirs": []}
 
+    # If some sources are generated and others are not, paths are complicated.
+    #
+    # bazel splits the files across bazel-out/package/* and workspace/package/*,
+    # but rustc requires the sources to all be in one directory.
+    # To address this, rules_rust adds symlinks: bazel-out/package/* => workspace/package/*
+    #
+    # If we specify one of the symlinks as our root_module, then rust-analyzer can index the crate,
+    # but won't recognize files in the workspace as being part of it, so features won't work.
+    # If we specify the path within the workspace, it won't manage to resolve the generated modules.
+    #
+    # There's no perfect solution here, for now we choose to try to find a workspace path.
+    # https://github.com/bazelbuild/rules_rust/issues/3126
     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"] = _WORKSPACE_TEMPLATE + src_map[info.crate.root.short_path].path
-            crate["source"]["include_dirs"].append(path_prefix + info.crate.root.dirname)
+        # 'root' is a transformed source, likely a symlink.
+        # 'srcs' are what the user originally specified, likely workspace paths.
+        for src in getattr(ctx.rule.files, "srcs", []):
+          if src.is_source and info.crate.root.short_path == src.short_path:
+            crate["root_module"] = _WORKSPACE_TEMPLATE + src.path
 
     if info.build_info != None and info.build_info.out_dir != None:
         out_dir_path = info.build_info.out_dir.path

test/rust_analyzer/generated_srcs_test/rust_project_json_test.rs

@@ -6,20 +6,13 @@ mod tests {
 
     #[derive(Deserialize)]
     struct Project {
-        sysroot_src: String,
         crates: Vec<Crate>,
     }
 
     #[derive(Deserialize)]
     struct Crate {
         display_name: String,
         root_module: String,
-        source: Option<Source>,
-    }
-
-    #[derive(Deserialize)]
-    struct Source {
-        include_dirs: Vec<String>,
     }
 
     #[test]
@@ -31,27 +24,20 @@ mod tests {
         let project: Project =
             serde_json::from_str(&content).expect("Failed to deserialize project JSON");
 
-        // /tmp/_bazel/12345678/external/tools/rustlib/library => /tmp/_bazel
-        let output_base = project
-            .sysroot_src
-            .rsplitn(2, "/external/")
-            .last()
-            .unwrap()
-            .rsplitn(2, '/')
-            .last()
-            .unwrap();
-        println!("output_base: {output_base}");
-
         let 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"));
 
-        let include_dirs = &gen.source.as_ref().unwrap().include_dirs;
-        assert!(include_dirs.len() == 1);
-        assert!(include_dirs[0].starts_with(output_base));
+        // This target has mixed generated+plain sources, so rules_rust provides a
+        // directory where the root module is a symlink.
+        // However in the crate spec, we want the root_module to be a workspace path
+        // when possible.
+        let workspace_path = PathBuf::from(env::var("WORKSPACE").unwrap());
+        assert_eq!(
+            gen.root_module,
+            workspace_path.join("lib.rs").to_string_lossy()
+        );
     }
 }

test/rust_analyzer/rust_analyzer_test_runner.sh

@@ -101,7 +101,7 @@ function rust_analyzer_test() {
     echo "Building..."
     bazel build //...
     echo "Testing..."
-    bazel test //...
+    bazel test --test_env=WORKSPACE="${workspace}" //...
     echo "Building with Aspects..."
     bazel build //... --config=strict
     popd &>/dev/null

tools/rust_analyzer/aquery.rs

@@ -176,20 +176,23 @@ fn consolidate_crate_specs(crate_specs: Vec<CrateSpec>) -> anyhow::Result<BTreeS
     for mut spec in crate_specs.into_iter() {
         log::debug!("{:?}", spec);
         if let Some(existing) = consolidated_specs.get_mut(&spec.crate_id) {
+            // Prefer rlib crates over bin crates (typically tests):
+            // - the display name that matches the crate name (foo vs foo_test)
+            //   Rust Analyzer seems to use display_name for matching crate entries
+            //   in rust-project.json against symbols in source files.
+            //   https://github.com/bazelbuild/rules_rust/issues/1032
+            // - test crates may report a genfiles path as the root_module, rather
+            //   than the original source file in the workspace.
+            //   This causes rust-analyzer to not recognize the workspace file.
+            //   https://github.com/bazelbuild/rules_rust/issues/3126
+            if spec.crate_type == "rlib" {
+                std::mem::swap(&mut spec, existing);
+            }
             existing.deps.extend(spec.deps);
 
             spec.cfg.retain(|cfg| !existing.cfg.contains(cfg));
             existing.cfg.extend(spec.cfg);
 
-            // display_name should match the library's crate name because Rust Analyzer
-            // seems to use display_name for matching crate entries in rust-project.json
-            // against symbols in source files. For more details, see
-            // https://github.com/bazelbuild/rules_rust/issues/1032
-            if spec.crate_type == "rlib" {
-                existing.display_name = spec.display_name;
-                existing.crate_type = "rlib".into();
-            }
-
             // For proc-macro crates that exist within the workspace, there will be a
             // generated crate-spec in both the fastbuild and opt-exec configuration.
             // Prefer proc macro paths with an opt-exec component in the path.