Unnamed repository; edit this file 'description' to name the repository.
Auto merge of #17019 - Wilfred:source_root_prefixes, r=Veykril
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.
| -rw-r--r-- | crates/vfs/src/file_set.rs | 4 | ||||
| -rw-r--r-- | crates/vfs/src/file_set/tests.rs | 23 |
2 files changed, 27 insertions, 0 deletions
diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 7eeb10d544..d7d283c3eb 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -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()); diff --git a/crates/vfs/src/file_set/tests.rs b/crates/vfs/src/file_set/tests.rs index 2146df185d..3cdb60dcb2 100644 --- a/crates/vfs/src/file_set/tests.rs +++ b/crates/vfs/src/file_set/tests.rs @@ -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]); +} |