Skip to content

Commit

Permalink
Auto merge of #17019 - Wilfred:source_root_prefixes, r=Veykril
Browse files Browse the repository at this point in the history
fix: VFS should not confuse paths with source roots that have the same prefix

Previously, the VFS would assign paths to the source root that had the longest string prefix match. This would break when we had source roots in subdirectories:

```
/foo
/foo/bar
```

Given a file `/foo/bar_baz.rs`, we would attribute it to the `/foo/bar` source root, which is wrong.

As a result, we would attribute paths to the wrong crate when a crate was in a subdirectory of another one. This is more common in larger monorepos, but could occur in any Rust project.

Fix this in the VFS, and add a test.
  • Loading branch information
bors committed Apr 13, 2024
2 parents 9b19462 + b03844d commit 773b4a5
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
4 changes: 4 additions & 0 deletions crates/vfs/src/file_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ impl FileSetConfig {
///
/// `scratch_space` is used as a buffer and will be entirely replaced.
fn classify(&self, path: &VfsPath, scratch_space: &mut Vec<u8>) -> usize {
// `path` is a file, but r-a only cares about the containing directory. We don't
// want `/foo/bar_baz.rs` to be attributed to source root directory `/foo/bar`.
let path = path.parent().unwrap_or_else(|| path.clone());

scratch_space.clear();
path.encode(scratch_space);
let automaton = PrefixOf::new(scratch_space.as_slice());
Expand Down
23 changes: 23 additions & 0 deletions crates/vfs/src/file_set/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,26 @@ fn name_prefix() {
let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();
assert_eq!(partition, vec![1, 1, 0]);
}

/// Ensure that we don't consider `/foo/bar_baz.rs` to be in the
/// `/foo/bar/` root.
#[test]
fn name_prefix_partially_matches() {
let mut file_set = FileSetConfig::builder();
file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo".into())]);
file_set.add_file_set(vec![VfsPath::new_virtual_path("/foo/bar".into())]);
let file_set = file_set.build();

let mut vfs = Vfs::default();

// These two are both in /foo.
vfs.set_file_contents(VfsPath::new_virtual_path("/foo/lib.rs".into()), Some(Vec::new()));
vfs.set_file_contents(VfsPath::new_virtual_path("/foo/bar_baz.rs".into()), Some(Vec::new()));

// Only this file is in /foo/bar.
vfs.set_file_contents(VfsPath::new_virtual_path("/foo/bar/biz.rs".into()), Some(Vec::new()));

let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::<Vec<_>>();

assert_eq!(partition, vec![2, 1, 0]);
}

0 comments on commit 773b4a5

Please sign in to comment.