Workspace crate graph duplication breaks "find all references"

[LHolten]

When two workspaces share a dependency, rust-analyzer is unable to find all uses of functions defined in the dependency and used in both workspaces.
This happens because the dependency is duplicated in the crate graph. When rust-analyzer goes to find in which crate the function is defined, it looks at the crate graph and just picks the first crate that matches.
Then it looks at the dependants of that crate and finds only one of the workspaces (because the crate was not deduplicated).

Deduplication was initially introduced here #14476 as an optimization and then later removed again #18080 because it broke uses of CARGO_RUSTC_CURRENT_DIR.

It looks like a difficult problem to solve. The two solutions that I can think of are these:

• Make sure that all analyses work correctly with duplicate crates in the crate graph.
• Deduplicate the crates anyway and generate the CARGO_RUSTC_CURRENT_DIR env var only at macro expansions?

[Veykril]

Make sure that all analyses work correctly with duplicate crates in the crate graph.

Isn't really feasible, if the crates are duplicated they are inherently different and they themselves as well as their definitions can't be treated the same.

Deduplicate the crates anyway and generate the CARGO_RUSTC_CURRENT_DIR env var only at macro expansions?

Isn't an option as cargo is likely to introduce an env var that can be observed outside of proc-macros. Quoting myself from #18080

Cargo also will expose this in an env var in the future (if it doesn't already on stable)

I don't recall which one it was though. Would be good to double check which one that was but I reckon its not the only reason why we can't do so.

[Veykril]

When rust-analyzer goes to find in which crate the function is defined, it looks at the crate graph and just picks the first crate that matches.
Then it looks at the dependants of that crate and finds only one of the workspaces (because the crate was not deduplicated).

I assume this means no matter from which workspace usage you trigger the search, you always only find usages in the first workspace (and not the one you triggered it from)?

[LHolten]

When rust-analyzer goes to find in which crate the function is defined, it looks at the crate graph and just picks the first crate that matches.
Then it looks at the dependants of that crate and finds only one of the workspaces (because the crate was not deduplicated).

I assume this means no matter from which workspace usage you trigger the search, you always only find usages in the first workspace (and not the one you triggered it from)?

Yes that is exactly what happens

[LHolten]

Deduplicate the crates anyway and generate the CARGO_RUSTC_CURRENT_DIR env var only at macro expansions?

Isn't an option as cargo is likely to introduce an env var that can be observed outside of proc-macros. Quoting myself from #18080

Cargo also will expose this in an env var in the future (if it doesn't already on stable)

I don't recall which one it was though. Would be good to double check which one that was but I reckon its not the only reason why we can't do so.

I think it is unlikely for dependencies to ever use CARGO_RUSTC_CURRENT_DIR (or similar) to change their public API in a way that affects rust-analyzer.
This is not true for macro expansions in tests, which is why I think those should be provided with CARGO_RUSTC_CURRENT_DIR (or similar).

Btw, env!("CARGO_RUSTC_CURRENT_DIR") does not compile on nightly or stable rust. It seems it was removed here rust-lang/cargo#14799.

[LHolten]

I can not reproduce #17748 even if I remove these lines

rust-analyzer/crates/project-model/src/env.rs

Lines 67 to 69 in add0963

	if kind.is_proc_macro() {
	env.set("CARGO_RUSTC_CURRENT_DIR", cargo.manifest_path().parent().to_string());
	}

[LHolten]

Yeah I think the CARGO_RUSTC_CURRENT_DIR thing is unrelated to #17748.
Changing the cwd (also in #18080) that was used while invoking the macro is probably what fixed it.

So my proposal is to add back deduplication and remove the CARGO_RUSTC_CURRENT_DIR env var completely.

I have the changes locally, so I can just make a PR

[Veykril]

Well thats nice to hear that they are likely rolling back CARGO_RUSTC_CURRENT_DIR. I just noticed we haven't even removed it from r-a yet or made it available to all crates, the comment there even talks about the non-existent deduplicationg logic.

In that case, yes, we can consider bringing the dedup logic back. (though I think my PR also cleaned up some stuff in general, so a full backout of that PR is probably not what we want).

This topic did raise another thing to keep in mind though, cargo configs that specifiy relative env vars will thrash the dedup logic as well. (as that is effectively what that env var was as well) Now this is a user opt-in, so we could probably diagnose and warn when this happens.