fix: filter the DOS device path prefix #16677
Conversation
rustbot
added
the
S-waiting-on-review
|
Looking at the code it feels a bit jank to join these two paths together. The rust compiler itself probably just opens the file since it is executed inside the src directory I presume. We however must resolve the path ourselves which leads to many edge cases. I would suggest calling fs::canonicalize in a subprocess where the working directory is set to the source directory. This would resolve basically all the issues. For example I have found this error which wouldn't be resolved as well: |
|
We intentionally do not use path canonicalization as that gives us a bunch of problems, see #14430 (comment) |
|
Is there a reason why we replace "\" with "/"? From my experience on windows a mix of "/" and "\" works fine. |
|
gotcha! |
I do wonder myself, CC @matklad. |
There was a problem hiding this comment.
Is there a reason why we replace "" with "/"? From my experience on windows a mix of "/" and "" works fine.
Alternatively canonicalize could be used as a fallback.
Perhaps a better question to ask is why this code doesn’t just use Path? The reason for that is that Path is platform specific: std::os::Path behaves differently on windows and Linux.
But we explicitly don’t want that in rust-analyzer: analysis is a pure function of the safe of the code, running rust-analyzer on a codebase should give exactly the same, determinist result irrespective of the host operating system.
So, we need to model our own Path in cross-platform way, and we do that by using String and normalizing path delimiters.
I think the code here predates introduction of
anchored paths is probably the right abstraction we should be using here.
| @@ -154,6 +154,14 @@ impl DirPath { | |||
| fn join_attr(&self, mut attr: &str, relative_to_parent: bool) -> String { | |||
| let base = if relative_to_parent { self.parent().unwrap() } else { &self.0 }; | |||
|
|
|||
| if cfg!(windows) { | |||
There was a problem hiding this comment.
There shouldn’t be any cfgs in the analysis parts of rust-analyzer.
A rust-analyzer server running on a Unix host should successfully analyzer code of a windows-only project.
Host operating system shouldn’t be an input to analysis.
If there’s some need to vary semantics of paths, it should be triggered by an explicit property of a Crate data structure stored in salsa database.
|
Note also the docs at the top of |
|
I will review the relevant code again, or maybe we should do nothing at all... |
fix #16666