Stream: t-compiler/wg-rls-2.0

Topic: Support for #[cfg(test)]


Thiébaud Weksteen (Aug 07 2020 at 07:26, on Zulip):

I've noticed that r-a was not resolving names when inside a tests mod (that is, preceded with the attribute #[cfg(test)]). This is for a rust-project.json type project. This can be fixed by adding "test" to the cfg attribute but I've noticed that many projects uses this as a fake/mocking mechanism (with #[cfg(not(test))] for the real module). In this case, we would lose the analysis of the real module.
Is there anything I'm missing here? Is there any solution to have both parsed?

Thiébaud Weksteen (Aug 10 2020 at 09:04, on Zulip):

@Paul Faria , you've submitted https://github.com/rust-analyzer/rust-analyzer/pull/4784/commits/9c35f135b9c872e904ee1e838cfa69fc5745c45f#diff-1c8e6b45624e6ce512961af52e9a1aa3R396 which explicitly adds "test" cfg. Any input on my question?

Paul Faria (Aug 10 2020 at 11:33, on Zulip):

As I understand it, this isn't currently possible. It would break goto def functionality (which item do you route to?). It would also not be clear what functionality is available. What happens when the types, functions or fields conflict with each other?

I had proposed configuring cfg items with some kind of UI, and @matklad had brought up an example of a checkbox UI that could appear on Cargo.toml files. Basically having the ability to toggle the features on and off.

I've definitely run into your issue before in my own crates and at work, so I understand the frustration. I could also be wrong, and the above might be possible, but I don't have enough experience with all the details to say for sure.

Florian Diebold (Aug 10 2020 at 12:15, on Zulip):

it is kind of possible, functionality like goto def is already built with this in mind. We can have multiple configurations of a crate in the crate graph, and a file can belong to any number of them. Actually implementing that would probably not be completely trivial anyway though, and would have a performance impact since we'd have to do a lot of analysis twice.

Florian Diebold (Aug 10 2020 at 12:18, on Zulip):

it would be interesting to experiment with though

Florian Diebold (Aug 10 2020 at 12:24, on Zulip):

(basically, the way it works for e.g. goto def all goes back to this, where it tries to find a crate that actually contains the file. I'm not 100% sure it'll deal well with the case where the crate contains the file, but the actual code we're interested in is cfg'd out though)

Paul Faria (Aug 10 2020 at 13:06, on Zulip):

TIL. I assume this means we'd need to keep track of two trees? One with test on and one off? If not, how would it interact with chalk (I assume that's what's used to do the type analysis, I've never delved that deep into the RA codebase before).

Florian Diebold (Aug 10 2020 at 13:19, on Zulip):

it's really just like two different crates in the crate graph, that happen to share files

Florian Diebold (Aug 10 2020 at 13:19, on Zulip):

all the HIR is completely separate and duplicated

Paul Faria (Aug 10 2020 at 13:51, on Zulip):

Assuming we get this working, what if, as a way of mitigating performance issues, this is a feature that's defaulted off and only turned on through settings?

Florian Diebold (Aug 10 2020 at 13:54, on Zulip):

as a first step at least, probably

Thiébaud Weksteen (Aug 11 2020 at 07:35, on Zulip):

Thanks, I'll add this info to an issue on github so this can be tracked.

woody77 (Aug 12 2020 at 23:12, on Zulip):

Florian Diebold said:

it's really just like two different crates in the crate graph, that happen to share files

This is actually how we're currently generating our rust-project.json, because I haven't worked on deduplication, yet. And for now "first one wins" seems to be the behavior.

woody77 (Aug 12 2020 at 23:15, on Zulip):

Paul Faria said:

As I understand it, this isn't currently possible. It would break goto def functionality (which item do you route to?). It would also not be clear what functionality is available. What happens when the types, functions or fields conflict with each other?

I had proposed configuring cfg items with some kind of UI, and matklad had brought up an example of a checkbox UI that could appear on Cargo.toml files. Basically having the ability to toggle the features on and off.

I've definitely run into your issue before in my own crates and at work, so I understand the frustration. I could also be wrong, and the above might be possible, but I don't have enough experience with all the details to say for sure.

I wonder how difficult it would be to wire up a "config-switcher" that just reloaded rust-analyzer after switching cfgs? (so you could switch between cfgs, and then just kick off a re-start of the language server for the workspace). Seems like it could be implemented in the VSCode extension itself, maybe?

Paul Faria (Aug 13 2020 at 13:17, on Zulip):

I was actually wondering if we could tie it into the lookup logic Florian shared above. Rather than just passing in a FileId, we could pass in a FileId and a filtering callback of some kind. Like, if there are multiple options, then use the callback to filter down and then if there are still options, fallback to a single value. I'm not sure how complicated it would be to thread that callback through to that code though. And Florian did mention this would cause perf issues. I think I'll experiment with something on the side there.

Florian Diebold (Aug 13 2020 at 13:20, on Zulip):

I think it's more that the lookup logic needs to be able to deal with the possibility that the container exists in a certain crate, but the child that we actually want doesn't, and then we need to try the next crate

Florian Diebold (Aug 13 2020 at 13:20, on Zulip):

but the config switcher thing has also been talked about and is probably the easier short-term solution

Paul Faria (Aug 13 2020 at 13:21, on Zulip):

ok, I'll focus on that then

Last update: Sep 27 2020 at 14:00UTC