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 `/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 6, 2024
2 parents 9cced6d + 8885d30 commit 4d46cc5
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
6 changes: 6 additions & 0 deletions crates/vfs/src/file_set.rs
Expand Up @@ -170,6 +170,12 @@ impl FileSetConfigBuilder {
for p in paths {
let mut buf = Vec::new();
p.encode(&mut buf);

// A root is a directory, so append a trailing separator to the string
// used in prefix checks. This ensures that r-a doesn't think that
// `/foo/bar.rs` lives in the `/foo/b/` root.
buf.extend(std::path::MAIN_SEPARATOR_STR.as_bytes());

entries.push((buf, i as u64));
}
}
Expand Down
23 changes: 23 additions & 0 deletions crates/vfs/src/file_set/tests.rs
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 4d46cc5

Please sign in to comment.