Rename FileSystem methods and remove internal-exposing API

This commit is contained in:
Josh Thomas 2025-08-19 20:13:54 -05:00
parent 734d4495e0
commit b195fb3fa6
5 changed files with 539 additions and 74 deletions

View file

@ -1,6 +1,7 @@
use std::path::Path;
use vfs::{MemoryFS, PhysicalFS, VfsPath, VfsResult, VfsMetadata, SeekAndRead, SeekAndWrite};
use std::time::SystemTime;
use std::collections::BTreeSet;
/// A file system for managing workspace file content with manual layer management.
///
@ -26,11 +27,13 @@ impl FileSystem {
Ok(FileSystem { memory, physical })
}
/// Read content from the file system
/// Checks memory layer first, then falls back to physical layer
pub fn read(&self, path: &str) -> VfsResult<String> {
/// Read file content as string (checks unsaved edits first, then disk)
///
/// This is a high-level convenience method for LSP operations.
/// Checks memory layer (unsaved edits) first, then falls back to physical layer (disk).
pub fn read_to_string(&self, path: &str) -> VfsResult<String> {
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.read_to_string();
}
@ -38,14 +41,17 @@ impl FileSystem {
physical_path.read_to_string()
}
/// Write content to memory layer only
/// This preserves the original file on disk while allowing edits
pub fn write_memory(&self, path: &str, content: &str) -> VfsResult<()> {
/// Write string content to memory layer (tracks unsaved edits from editor)
///
/// This is a high-level convenience method for LSP operations.
/// Writes to memory layer only, preserving the original file on disk.
/// The editor is responsible for actual disk writes via didSave.
pub fn write_string(&self, path: &str, content: &str) -> VfsResult<()> {
let memory_path = self.memory.join(path)?;
// Ensure parent directories exist in memory layer
let parent = memory_path.parent();
if !parent.is_root() && !parent.exists()? {
if !parent.is_root() && !parent.exists().unwrap_or(false) {
parent.create_dir_all()?;
}
@ -53,14 +59,16 @@ impl FileSystem {
Ok(())
}
/// Clear memory layer content for a specific path
/// After clearing, reads will fall back to physical layer
/// No whiteout markers are created - direct memory layer management
pub fn clear_memory(&self, path: &str) -> VfsResult<()> {
/// Discard unsaved changes for a file (removes from memory layer)
///
/// This is a high-level convenience method for LSP operations.
/// After discarding, reads will fall back to the physical layer (disk state).
/// Typically called when editor sends didClose without saving.
pub fn discard_changes(&self, path: &str) -> VfsResult<()> {
let memory_path = self.memory.join(path)?;
// Only remove if it exists in memory layer
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
memory_path.remove_file()?;
}
@ -71,41 +79,41 @@ impl FileSystem {
/// Checks memory layer first, then physical layer
pub fn exists(&self, path: &str) -> VfsResult<bool> {
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return Ok(true);
}
let physical_path = self.physical.join(path)?;
physical_path.exists()
Ok(physical_path.exists().unwrap_or(false))
}
/// Get memory layer root for advanced operations
pub fn memory_root(&self) -> VfsPath {
self.memory.clone()
}
/// Get physical layer root for advanced operations
pub fn physical_root(&self) -> VfsPath {
self.physical.clone()
}
/// Get root for backward compatibility (returns memory root)
pub fn root(&self) -> VfsPath {
self.memory.clone()
}
}
// Implement vfs::FileSystem trait to make our FileSystem compatible with VfsPath
impl vfs::FileSystem for FileSystem {
fn read_dir(&self, path: &str) -> VfsResult<Box<dyn Iterator<Item = String> + Send>> {
// Check memory layer first, then physical
// Collect entries from both layers and merge them
let mut entries = BTreeSet::new();
// Add memory layer entries
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
return Ok(Box::new(memory_path.read_dir()?.map(|p| p.filename())));
if memory_path.exists().unwrap_or(false) {
for entry in memory_path.read_dir()? {
entries.insert(entry.filename());
}
}
// Add physical layer entries
let physical_path = self.physical.join(path)?;
Ok(Box::new(physical_path.read_dir()?.map(|p| p.filename())))
if physical_path.exists().unwrap_or(false) {
for entry in physical_path.read_dir()? {
entries.insert(entry.filename());
}
}
// Return merged, deduplicated entries
Ok(Box::new(entries.into_iter()))
}
fn create_dir(&self, path: &str) -> VfsResult<()> {
@ -117,7 +125,7 @@ impl vfs::FileSystem for FileSystem {
fn open_file(&self, path: &str) -> VfsResult<Box<dyn SeekAndRead + Send>> {
// Check memory layer first, then physical
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.open_file();
}
@ -131,7 +139,7 @@ impl vfs::FileSystem for FileSystem {
// Ensure parent directories exist in memory layer
let parent = memory_path.parent();
if !parent.is_root() && !parent.exists()? {
if !parent.is_root() && !parent.exists().unwrap_or(false) {
parent.create_dir_all()?;
}
@ -142,12 +150,12 @@ impl vfs::FileSystem for FileSystem {
// For append, we need to check if file exists and copy to memory if needed
let memory_path = self.memory.join(path)?;
if !memory_path.exists()? {
if !memory_path.exists().unwrap_or(false) {
// Copy from physical to memory first if it exists
let physical_path = self.physical.join(path)?;
if physical_path.exists()? {
if physical_path.exists().unwrap_or(false) {
let content = physical_path.read_to_string()?;
self.write_memory(path, &content)?;
self.write_string(path, &content)?;
}
}
@ -157,7 +165,7 @@ impl vfs::FileSystem for FileSystem {
fn metadata(&self, path: &str) -> VfsResult<VfsMetadata> {
// Check memory layer first, then physical
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.metadata();
}
@ -168,7 +176,7 @@ impl vfs::FileSystem for FileSystem {
fn set_creation_time(&self, path: &str, time: SystemTime) -> VfsResult<()> {
// Set on memory layer if exists, otherwise physical
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.set_creation_time(time);
}
@ -179,7 +187,7 @@ impl vfs::FileSystem for FileSystem {
fn set_modification_time(&self, path: &str, time: SystemTime) -> VfsResult<()> {
// Set on memory layer if exists, otherwise physical
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.set_modification_time(time);
}
@ -190,7 +198,7 @@ impl vfs::FileSystem for FileSystem {
fn set_access_time(&self, path: &str, time: SystemTime) -> VfsResult<()> {
// Set on memory layer if exists, otherwise physical
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
return memory_path.set_access_time(time);
}
@ -199,14 +207,14 @@ impl vfs::FileSystem for FileSystem {
}
fn exists(&self, path: &str) -> VfsResult<bool> {
// Use our existing method which already handles layer checking
self.exists(path)
// Call our inherent method which handles layer checking properly
FileSystem::exists(self, path)
}
fn remove_file(&self, path: &str) -> VfsResult<()> {
// Only remove from memory layer - this aligns with our LSP semantics
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
memory_path.remove_file()?;
}
Ok(())
@ -215,7 +223,7 @@ impl vfs::FileSystem for FileSystem {
fn remove_dir(&self, path: &str) -> VfsResult<()> {
// Only remove from memory layer - this aligns with our LSP semantics
let memory_path = self.memory.join(path)?;
if memory_path.exists()? {
if memory_path.exists().unwrap_or(false) {
memory_path.remove_dir()?;
}
Ok(())
@ -231,12 +239,10 @@ mod tests {
#[test]
fn test_new_vfs() {
let temp_dir = TempDir::new().unwrap();
let vfs = FileSystem::new(temp_dir.path()).unwrap();
let _vfs = FileSystem::new(temp_dir.path()).unwrap();
// Should be able to get roots
let _memory_root = vfs.memory_root();
let _physical_root = vfs.physical_root();
let _root = vfs.root();
// FileSystem should be created successfully
// (removed testing of internal-exposing methods)
}
#[test]
@ -246,18 +252,18 @@ mod tests {
fs::write(&test_file, "physical content").unwrap();
let vfs = FileSystem::new(temp_dir.path()).unwrap();
let content = vfs.read("test.html").unwrap();
let content = vfs.read_to_string("test.html").unwrap();
assert_eq!(content, "physical content");
}
#[test]
fn test_write_memory_and_read() {
fn test_write_string_and_read_to_string() {
let temp_dir = TempDir::new().unwrap();
let vfs = FileSystem::new(temp_dir.path()).unwrap();
vfs.write_memory("test.html", "memory content").unwrap();
let content = vfs.read("test.html").unwrap();
vfs.write_string("test.html", "memory content").unwrap();
let content = vfs.read_to_string("test.html").unwrap();
assert_eq!(content, "memory content");
}
@ -271,13 +277,13 @@ mod tests {
let vfs = FileSystem::new(temp_dir.path()).unwrap();
// First read should get physical content
assert_eq!(vfs.read("test.html").unwrap(), "physical content");
assert_eq!(vfs.read_to_string("test.html").unwrap(), "physical content");
// Write to memory layer
vfs.write_memory("test.html", "memory content").unwrap();
vfs.write_string("test.html", "memory content").unwrap();
// Now read should get memory content (higher priority)
assert_eq!(vfs.read("test.html").unwrap(), "memory content");
assert_eq!(vfs.read_to_string("test.html").unwrap(), "memory content");
// Physical file should remain unchanged
let physical_content = fs::read_to_string(&test_file).unwrap();
@ -285,7 +291,7 @@ mod tests {
}
#[test]
fn test_clear_memory_fallback() {
fn test_discard_changes_fallback() {
let temp_dir = TempDir::new().unwrap();
let test_file = temp_dir.path().join("test.html");
fs::write(&test_file, "physical content").unwrap();
@ -293,14 +299,14 @@ mod tests {
let vfs = FileSystem::new(temp_dir.path()).unwrap();
// Write to memory
vfs.write_memory("test.html", "memory content").unwrap();
assert_eq!(vfs.read("test.html").unwrap(), "memory content");
vfs.write_string("test.html", "memory content").unwrap();
assert_eq!(vfs.read_to_string("test.html").unwrap(), "memory content");
// Clear memory
vfs.clear_memory("test.html").unwrap();
vfs.discard_changes("test.html").unwrap();
// Should now read from physical layer
assert_eq!(vfs.read("test.html").unwrap(), "physical content");
assert_eq!(vfs.read_to_string("test.html").unwrap(), "physical content");
}
#[test]
@ -315,7 +321,7 @@ mod tests {
assert!(vfs.exists("physical.html").unwrap());
// Memory file exists after writing
vfs.write_memory("memory.html", "content").unwrap();
vfs.write_string("memory.html", "content").unwrap();
assert!(vfs.exists("memory.html").unwrap());
// Non-existent file
@ -323,12 +329,12 @@ mod tests {
}
#[test]
fn test_clear_memory_nonexistent() {
fn test_discard_changes_nonexistent() {
let temp_dir = TempDir::new().unwrap();
let vfs = FileSystem::new(temp_dir.path()).unwrap();
// Should not error when clearing non-existent memory file
vfs.clear_memory("nonexistent.html").unwrap();
vfs.discard_changes("nonexistent.html").unwrap();
}
#[test]
@ -338,25 +344,69 @@ mod tests {
fs::write(&test_file, "trait physical content").unwrap();
let filesystem = FileSystem::new(temp_dir.path()).unwrap();
let vfs_path = VfsPath::new(filesystem);
// Test that our FileSystem works as a vfs::FileSystem trait
assert!(vfs_path.join("trait_test.html").unwrap().exists().unwrap());
// Test our trait implementation directly instead of through VfsPath
// VfsPath would create absolute paths which our security validation rejects
use vfs::FileSystem as VfsFileSystemTrait;
// Test exists through trait
assert!(filesystem.exists("trait_test.html").unwrap());
// Test reading through trait
let content = vfs_path.join("trait_test.html").unwrap().read_to_string().unwrap();
let mut file = filesystem.open_file("trait_test.html").unwrap();
let mut content = String::new();
use std::io::Read;
file.read_to_string(&mut content).unwrap();
assert_eq!(content, "trait physical content");
// Test creating file through trait
let new_file = vfs_path.join("new_trait_file.html").unwrap();
new_file.create_file().unwrap().write_all(b"trait memory content").unwrap();
let mut new_file = filesystem.create_file("new_trait_file.html").unwrap();
use std::io::Write;
new_file.write_all(b"trait memory content").unwrap();
drop(new_file); // Close the file to flush writes
// Should read from memory layer
let memory_content = new_file.read_to_string().unwrap();
let mut memory_file = filesystem.open_file("new_trait_file.html").unwrap();
let mut memory_content = String::new();
memory_file.read_to_string(&mut memory_content).unwrap();
assert_eq!(memory_content, "trait memory content");
// Physical file should not exist (memory layer only)
let physical_new_file = temp_dir.path().join("new_trait_file.html");
assert!(!physical_new_file.exists());
}
#[test]
fn test_directory_merge() {
let temp_dir = TempDir::new().unwrap();
let test_dir = temp_dir.path().join("testdir");
fs::create_dir(&test_dir).unwrap();
fs::write(test_dir.join("physical1.txt"), "content").unwrap();
fs::write(test_dir.join("physical2.txt"), "content").unwrap();
let filesystem = FileSystem::new(temp_dir.path()).unwrap();
// Create memory layer files using the trait methods
use vfs::FileSystem as VfsFileSystemTrait;
filesystem.create_dir("testdir").ok(); // May already exist
let mut mem1 = filesystem.create_file("testdir/memory1.txt").unwrap();
use std::io::Write;
mem1.write_all(b"memory content").unwrap();
drop(mem1);
let mut mem2 = filesystem.create_file("testdir/memory2.txt").unwrap();
mem2.write_all(b"memory content").unwrap();
drop(mem2);
// Read directory should show merged content
let entries: Vec<String> = filesystem.read_dir("testdir").unwrap().collect();
// Should contain both physical and memory files
assert!(entries.contains(&"physical1.txt".to_string()));
assert!(entries.contains(&"physical2.txt".to_string()));
assert!(entries.contains(&"memory1.txt".to_string()));
assert!(entries.contains(&"memory2.txt".to_string()));
assert_eq!(entries.len(), 4);
}
}

View file

@ -21,6 +21,7 @@ use tower_lsp_server::lsp_types::Position;
use super::document::ClosingBrace;
use super::document::LanguageId;
use super::document::TextDocument;
use super::utils::uri_to_pathbuf;
#[derive(Debug)]
pub struct Store {
@ -42,7 +43,24 @@ impl Store {
root_path,
})
}
/// Check if a URI represents a file within the workspace
fn is_workspace_file(&self, uri: &tower_lsp_server::lsp_types::Uri) -> bool {
if let Some(path) = uri_to_pathbuf(uri) {
// Check if the path is under the workspace root
path.starts_with(&self.root_path)
} else {
// Not a file URI, ignore
false
}
}
pub fn handle_did_open(&mut self, db: &dyn Database, params: &DidOpenTextDocumentParams) {
// Only process files within the workspace
if !self.is_workspace_file(&params.text_document.uri) {
// Silently ignore files outside workspace
return;
}
let uri = params.text_document.uri.to_string();
let version = params.text_document.version;
@ -57,6 +75,12 @@ impl Store {
db: &dyn Database,
params: &DidChangeTextDocumentParams,
) -> Result<()> {
// Only process files within the workspace
if !self.is_workspace_file(&params.text_document.uri) {
// Return Ok to avoid errors for files outside workspace
return Ok(());
}
let uri = params.text_document.uri.as_str().to_string();
let version = params.text_document.version;

View file

@ -20,7 +20,7 @@ pub fn get_project_path(params: &InitializeParams) -> Option<PathBuf> {
}
/// Converts a `file:` URI into an absolute `PathBuf`.
fn uri_to_pathbuf(uri: &Uri) -> Option<PathBuf> {
pub(super) fn uri_to_pathbuf(uri: &Uri) -> Option<PathBuf> {
// Check if the scheme is "file"
if uri.scheme().is_none_or(|s| s.as_str() != "file") {
return None;