Skip to content

unresolved-module in rapier repository #7715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
emilk opened this issue Feb 18, 2021 · 17 comments · Fixed by #9353
Closed

unresolved-module in rapier repository #7715

emilk opened this issue Feb 18, 2021 · 17 comments · Fixed by #9353

Comments

@emilk
Copy link

emilk commented Feb 18, 2021

Hi, and thanks for an amazing tool!

I'm using rust-analyzer in VS code and am currently doing some work on this project: https://github.com/dimforge/rapier/

The problem is that rust-analyzer doesn't seem to work at all for that repository. I get no type annotations, and things like rename doesn't work.

rust-analyzer reports the problem unresolved-module for these lines: https://github.com/dimforge/rapier/blob/12c5dd4caa6af5750b682ba17229601e3a418155/src/lib.rs#L131:L136 despite the modules seemingly being in the normal place (e.g. https://github.com/dimforge/rapier/blob/12c5dd4caa6af5750b682ba17229601e3a418155/src/data/mod.rs).

Hopefully reproducing this problem will be as easy as checking out the above repository (not exactly a minimal reproduce though).

@Veykril
Copy link
Member

Veykril commented Feb 18, 2021

Looks like we are having trouble resolving paths correctly due to rapier's "interesting" workspace structure, https://github.com/dimforge/rapier/tree/master/build. All of these crates direct their lib path to ../../lib.rs. Everything resolves fine if one replaces the top level workspace toml with one of the ones in the build directory and removing the explicit lib path.

@flodiebold
Copy link
Member

That's a really weird structure... but in theory we should be fine with it. Are we getting the current path from lib.rs mixed up somehow?

@Veykril
Copy link
Member

Veykril commented Feb 18, 2021

Commenting out all but one of the members in the workspace.members entry in the top level Cargo.toml also fixes unresolved module errors so I guess there is some mixup/collision happening

@bjorn3
Copy link
Member

bjorn3 commented Feb 18, 2021

Is it possible that the problem is the fact that the same src/lib.rs file is used by two crates?

@flodiebold
Copy link
Member

It shouldn't be, but maybe it is. Although I'd certainly expect it to lead to errors being reported multiple times etc.

@rezural
Copy link
Contributor

rezural commented Jun 19, 2021

I just noticed that if you navigate to each of the modules that cannot be found using the file explorer, then rust-analyzer is now aware of that module.

@rezural
Copy link
Contributor

rezural commented Jun 19, 2021

Symlinking to /src/ in each of the build directories, and removing the path works:

[lib]
name = "rapier2d_f64"
#path = "../../src/lib.rs"
required-features = [ "dim2", "f64" ]

@rezural
Copy link
Contributor

rezural commented Jun 19, 2021

Any pointers on where the code is that should pick up path would be appreciated.

I assume it is in the rust-analyzer server that this is not being picked up properly.

@lnicola
Copy link
Member

lnicola commented Jun 19, 2021

See also #9173, but I think the path isn't outside of the workspace in this issue.

Actually, I'm not sure we support this at all.

@lnicola
Copy link
Member

lnicola commented Jun 19, 2021

I think you can look into the project_model (we have two, look for the cargo one) and vfs crates. It works once you open the file because we load it into the VFS, but we miss it during the initial project load.

This is probably a bit different from the other issue because the path is available in Cargo.toml, and we don't need to parse an attribute to find it.

@rezural
Copy link
Contributor

rezural commented Jun 19, 2021

Ive found the code in project_model, and I'm pretty sure that at least doesnt support path. Not sure where the code currently generates the path from though, for to add support for it.

Thanks

@flodiebold
Copy link
Member

@rezural
Copy link
Contributor

rezural commented Jun 19, 2021

@rezural
Copy link
Contributor

rezural commented Jun 21, 2021

It looks like when vfs-notify code registers a workspace, it uses the project root (which in the repro is build/biccies), which contains no entries. We need to pass the parent directory of the lib.rs file, (probably via root.include) to this code here:

https://github.com/rust-analyzer/rust-analyzer/blob/9fdb8f90376c02ec2a267cf9eb3bdb7b6027e1e6/crates/rust-analyzer/src/reload.rs#L458-L473

probably in here:

https://github.com/rust-analyzer/rust-analyzer/blob/9fdb8f90376c02ec2a267cf9eb3bdb7b6027e1e6/crates/project_model/src/workspace.rs#L216-L228

Hopefully I will have a PR soon.

@rezural
Copy link
Contributor

rezural commented Jun 21, 2021

@emilk, FYI

Just put up this PR. 0 errors now in rapier master. looks beautiful 😄

@emilk
Copy link
Author

emilk commented Jun 21, 2021

Thank you @rezural !

@bors bors bot closed this as completed in 85a59de Jun 23, 2021
@rezural
Copy link
Contributor

rezural commented Jun 24, 2021

Please test this in nightly.

Works for me, probably can close this.

@emilk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants