diff --git a/Cargo.toml b/Cargo.toml index afbe6d01..00971354 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -137,5 +137,8 @@ insta.workspace = true utils = { workspace = true, features = ["test-support"] } [features] +default = ["server-file-watcher"] +# Server OS file watching (off for wasm). +server-file-watcher = ["vfs/notify-backend"] profile-trace = ["dep:tracing-chrome"] user-config-schema = ["dep:schemars"] diff --git a/crates/hir/src/base_db/change.rs b/crates/hir/src/base_db/change.rs index 57250cb7..9401a68b 100644 --- a/crates/hir/src/base_db/change.rs +++ b/crates/hir/src/base_db/change.rs @@ -1,6 +1,6 @@ use salsa::Durability; use triomphe::Arc; -use vfs::ChangedFile; +use vfs::{Change as VfsChange, ChangedFile}; use crate::base_db::{ project::{PreprocessConfig, SharedProjectConfig}, @@ -66,15 +66,15 @@ impl Change { .path_for_file(&file_id) .and_then(|path| path.as_abs_path().map(ToOwned::to_owned)); - match changed_file.change_kind { - vfs::ChangeKind::Create(_, _) => { + match &changed_file.change { + VfsChange::Create(_, _) => { files_changed |= files.insert(file_id); } - vfs::ChangeKind::Delete => { + VfsChange::Delete => { files.remove(&file_id); files_changed = true; } - vfs::ChangeKind::Modify(_, _) => {} + VfsChange::Modify(_, _) => {} } let text = changed_file.text().unwrap_or_else(|| Arc::from("")); diff --git a/crates/hir/src/base_db/source_db.rs b/crates/hir/src/base_db/source_db.rs index c813020c..a0e27c63 100644 --- a/crates/hir/src/base_db/source_db.rs +++ b/crates/hir/src/base_db/source_db.rs @@ -5,7 +5,7 @@ use syntax::{ }; use triomphe::Arc; use utils::{line_index::TextSize, path_identity::PathIdentityIndex}; -use vfs::{FileId, anchored_path::AnchoredPath}; +use vfs::{AnchoredPath, FileId}; use crate::base_db::{ compilation_plan::{self, CompilationPlan}, @@ -643,8 +643,8 @@ mod tests { salsa::{self, Durability}, }; - const TOP: FileId = FileId(0); - const MANIFEST: FileId = FileId(1); + const TOP: FileId = FileId::from_raw(0); + const MANIFEST: FileId = FileId::from_raw(1); const ROOT: SourceRootId = SourceRootId(0); #[salsa::database(SourceDbStorage, SourceRootDbStorage)] @@ -663,7 +663,7 @@ mod tests { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/hir/src/file.rs b/crates/hir/src/file.rs index b397c755..06bb5502 100644 --- a/crates/hir/src/file.rs +++ b/crates/hir/src/file.rs @@ -37,7 +37,7 @@ mod tests { #[test] fn as_file_returns_source_file_when_hir_file_is_real_file() { - let file_id = FileId(7); + let file_id = FileId::from_raw(7); let hir_file_id = HirFileId::from(file_id); diff --git a/crates/hir/src/hir_def/macro_file/tests.rs b/crates/hir/src/hir_def/macro_file/tests.rs index 88c59fd2..70334578 100644 --- a/crates/hir/src/hir_def/macro_file/tests.rs +++ b/crates/hir/src/hir_def/macro_file/tests.rs @@ -12,7 +12,7 @@ use utils::{ line_index::{TextRange, TextSize}, paths::{AbsPathBuf, Utf8PathBuf}, }; -use vfs::{FileId, FileSet, VfsPath, anchored_path::AnchoredPath}; +use vfs::{AnchoredPath, FileId, FileSet, VfsPath}; use super::*; use crate::{ @@ -30,7 +30,7 @@ use crate::{ file::HirFileId, }; -const TOP: FileId = FileId(0); +const TOP: FileId = FileId::from_raw(0); const ROOT: SourceRootId = SourceRootId(0); const PROFILE: CompilationProfileId = CompilationProfileId(0); @@ -50,7 +50,7 @@ impl fmt::Debug for TestDb { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/hir/src/preproc/tests.rs b/crates/hir/src/preproc/tests.rs index ecd16b1e..c95ca70d 100644 --- a/crates/hir/src/preproc/tests.rs +++ b/crates/hir/src/preproc/tests.rs @@ -7,7 +7,7 @@ use utils::{ line_index::{TextRange, TextSize}, paths::{AbsPathBuf, Utf8PathBuf}, }; -use vfs::{FileId, FileSet, VfsPath, anchored_path::AnchoredPath}; +use vfs::{AnchoredPath, FileId, FileSet, VfsPath}; use super::*; use crate::{ @@ -33,10 +33,10 @@ use crate::{ source_map::IsSrc, }; -const TOP: FileId = FileId(0); -const HEADER: FileId = FileId(1); -const LEAF: FileId = FileId(2); -const MANIFEST: FileId = FileId(3); +const TOP: FileId = FileId::from_raw(0); +const HEADER: FileId = FileId::from_raw(1); +const LEAF: FileId = FileId::from_raw(2); +const MANIFEST: FileId = FileId::from_raw(3); const ROOT: SourceRootId = SourceRootId(0); const PROFILE: CompilationProfileId = CompilationProfileId(0); @@ -56,7 +56,7 @@ impl fmt::Debug for TestDb { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/hir/src/scope.rs b/crates/hir/src/scope.rs index 7e039e07..868252f3 100644 --- a/crates/hir/src/scope.rs +++ b/crates/hir/src/scope.rs @@ -690,7 +690,7 @@ mod tests { get::GetRef, paths::{AbsPathBuf, Utf8PathBuf}, }; - use vfs::{FileId, FileSet, VfsPath, anchored_path::AnchoredPath}; + use vfs::{AnchoredPath, FileId, FileSet, VfsPath}; use crate::{ base_db::{ @@ -710,7 +710,7 @@ mod tests { symbol::{DefKind, DefLoc, NameContext}, }; - const TOP: FileId = FileId(0); + const TOP: FileId = FileId::from_raw(0); const ROOT: SourceRootId = SourceRootId(0); const PROFILE: CompilationProfileId = CompilationProfileId(0); @@ -730,7 +730,7 @@ mod tests { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/hir/src/semantics/pathres.rs b/crates/hir/src/semantics/pathres.rs index 8ea81897..5b0294d2 100644 --- a/crates/hir/src/semantics/pathres.rs +++ b/crates/hir/src/semantics/pathres.rs @@ -302,7 +302,7 @@ mod tests { use smol_str::SmolStr; use triomphe::Arc; use utils::paths::{AbsPathBuf, Utf8PathBuf}; - use vfs::{FileId, FileSet, VfsPath, anchored_path::AnchoredPath}; + use vfs::{AnchoredPath, FileId, FileSet, VfsPath}; use super::*; use crate::{ @@ -323,7 +323,7 @@ mod tests { symbol::{DefKind, NameContext}, }; - const TOP: FileId = FileId(0); + const TOP: FileId = FileId::from_raw(0); const ROOT: SourceRootId = SourceRootId(0); const PROFILE: CompilationProfileId = CompilationProfileId(0); @@ -343,7 +343,7 @@ mod tests { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/hir/src/type_infer.rs b/crates/hir/src/type_infer.rs index 956ac1d2..584c002e 100644 --- a/crates/hir/src/type_infer.rs +++ b/crates/hir/src/type_infer.rs @@ -858,7 +858,7 @@ mod tests { use smol_str::SmolStr; use triomphe::Arc; use utils::paths::{AbsPathBuf, Utf8PathBuf}; - use vfs::{FileId, FileSet, VfsPath, anchored_path::AnchoredPath}; + use vfs::{AnchoredPath, FileId, FileSet, VfsPath}; use super::*; use crate::{ @@ -878,7 +878,7 @@ mod tests { symbol::{DefLoc, NameContext}, }; - const TOP: FileId = FileId(0); + const TOP: FileId = FileId::from_raw(0); const ROOT: SourceRootId = SourceRootId(0); const PROFILE: CompilationProfileId = CompilationProfileId(0); @@ -898,7 +898,7 @@ mod tests { impl FileLoader for TestDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); SourceRootDb::source_root(self, source_root_id).resolve_path(path) } } diff --git a/crates/ide/src/analysis.rs b/crates/ide/src/analysis.rs index b3789e41..cdc5912c 100644 --- a/crates/ide/src/analysis.rs +++ b/crates/ide/src/analysis.rs @@ -142,7 +142,7 @@ impl Analysis { let plan = db.compilation_plan_for_profile(Some(profile_id)); let mut file_ids = plan.roots.clone(); file_ids.extend(plan.include_only.iter().copied()); - file_ids.sort_unstable_by_key(|file_id| file_id.0); + file_ids.sort_unstable_by_key(|file_id| file_id.index()); file_ids.dedup(); file_ids }) diff --git a/crates/ide/src/code_action/tests.rs b/crates/ide/src/code_action/tests.rs index fa7b9bd8..2a1dcd3c 100644 --- a/crates/ide/src/code_action/tests.rs +++ b/crates/ide/src/code_action/tests.rs @@ -1,12 +1,8 @@ use std::{fmt::Write, fs, path::Path}; use hir::base_db::{change::Change, source_root::SourceRoot}; -use triomphe::Arc; -use utils::{ - lines::LineEnding, - text_edit::{TextRange, TextSize}, -}; -use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; +use utils::text_edit::{TextRange, TextSize}; +use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::db::root_db::RootDb; @@ -113,16 +109,13 @@ fn db_with_file(text: &str) -> (RootDb, FileId, TextSize) { } fn db_with_text(text: &str) -> (RootDb, FileId) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let mut file_set = FileSet::default(); file_set.insert(file_id, VfsPath::new_virtual_path("/test.sv".to_owned())); let mut change = Change::new(); change.set_roots(vec![SourceRoot::new_local(file_set)]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut db = RootDb::new(None); db.apply_change(change); diff --git a/crates/ide/src/completion/engine/tests.rs b/crates/ide/src/completion/engine/tests.rs index 8931edcd..5d6bf55c 100644 --- a/crates/ide/src/completion/engine/tests.rs +++ b/crates/ide/src/completion/engine/tests.rs @@ -1,9 +1,8 @@ use std::path::Path; use hir::base_db::{change::Change, source_root::SourceRoot}; -use triomphe::Arc; -use utils::{lines::LineEnding, text_edit::TextSize}; -use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; +use utils::text_edit::TextSize; +use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::{ @@ -18,7 +17,7 @@ fn setup_with_path(text: &str, path: &str) -> (AnalysisHost, FilePosition) { let mut owned = text; owned = owned.replace(marker, ""); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path(path.to_string()); let mut file_set = FileSet::default(); @@ -27,10 +26,7 @@ fn setup_with_path(text: &str, path: &str) -> (AnalysisHost, FilePosition) { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(owned.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, owned.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/db/root_db.rs b/crates/ide/src/db/root_db.rs index 49d9586a..2b9af162 100644 --- a/crates/ide/src/db/root_db.rs +++ b/crates/ide/src/db/root_db.rs @@ -17,7 +17,7 @@ use hir::{ }, }; use triomphe::Arc; -use vfs::{FileId, anchored_path::AnchoredPath}; +use vfs::{AnchoredPath, FileId}; use crate::db::{ line_index_db::LineIndexDbStorage, workspace_symbol_index_db::WorkspaceSymbolIndexDbStorage, @@ -53,7 +53,7 @@ impl fmt::Debug for RootDb { impl FileLoader for RootDb { fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let source_root_id = SourceRootDb::source_root_id(self, path.anchor_id); + let source_root_id = SourceRootDb::source_root_id(self, path.anchor); let source_root = SourceRootDb::source_root(self, source_root_id); source_root.resolve_path(path) } diff --git a/crates/ide/src/definitions.rs b/crates/ide/src/definitions.rs index 82bbe533..10139205 100644 --- a/crates/ide/src/definitions.rs +++ b/crates/ide/src/definitions.rs @@ -366,15 +366,14 @@ mod tests { symbol::DefKind, }; use syntax::SyntaxNodeExt; - use triomphe::Arc; - use utils::{lines::LineEnding, text_edit::TextSize}; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use utils::text_edit::TextSize; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::{analysis_host::AnalysisHost, db::root_db::RootDb}; fn host_with_file(text: &str) -> (AnalysisHost, FileId) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.v".to_string()); let mut file_set = FileSet::default(); @@ -383,10 +382,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 6a403e1f..06b4d31e 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -185,7 +185,7 @@ fn compilation_profile_file_ids(db: &RootDb, profile_id: CompilationProfileId) - let plan = db.compilation_plan_for_profile(Some(profile_id)); let mut file_ids = plan.roots.clone(); file_ids.extend(plan.include_only.iter().copied()); - file_ids.sort_unstable_by_key(|file_id| file_id.0); + file_ids.sort_unstable_by_key(|file_id| file_id.index()); file_ids.dedup(); file_ids } @@ -429,11 +429,10 @@ mod tests { use triomphe::Arc; use utils::{ line_index::{TextRange, TextSize}, - lines::LineEnding, paths::AbsPathBuf, test_support::TestDir, }; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::{ AMBIGUOUS_MODULE_INSTANTIATION, DIAGNOSTIC_INACTIVE_PREPROCESSOR_BRANCH, DiagnosticSource, @@ -486,13 +485,10 @@ mod tests { let mut change = Change::new(); for (idx, (path, text)) in files.iter().enumerate() { - let file_id = FileId(idx as u32); + let file_id = FileId::from_raw(idx as u32); let path = VfsPath::new_virtual_path((*path).to_owned()); file_set.insert(file_id, path); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(*text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, *text)); } change.set_roots(vec![SourceRoot::new(role, file_set)]); @@ -527,7 +523,7 @@ mod tests { false, ); - let diagnostics = diagnostics(&db, FileId(2)); + let diagnostics = diagnostics(&db, FileId::from_raw(2)); assert!( diagnostics.iter().any(|diag| { @@ -552,7 +548,7 @@ mod tests { false, ); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = diagnostics(&db, FileId::from_raw(1)); assert!( diagnostics.iter().all(|diag| diag.source != DiagnosticSource::Vide), @@ -571,7 +567,7 @@ mod tests { false, ); - let diagnostics = diagnostics(&db, FileId(2)); + let diagnostics = diagnostics(&db, FileId::from_raw(2)); assert!( diagnostics.iter().any(|diag| { @@ -596,7 +592,7 @@ mod tests { false, ); - let diagnostics = diagnostics(&db, FileId(2)); + let diagnostics = diagnostics(&db, FileId::from_raw(2)); let diagnostic = diagnostics .iter() .find(|diag| { @@ -607,7 +603,7 @@ mod tests { panic!("expected generated instantiation diagnostic: {diagnostics:?}") }); - assert_eq!(diagnostic.file_id, FileId(2)); + assert_eq!(diagnostic.file_id, FileId::from_raw(2)); assert_eq!(diagnostic.range, range_of(top, "child")); assert_ne!(diagnostic.range, range_of(top, "`MAKE")); } @@ -625,7 +621,7 @@ mod tests { ); disable_semantic_diagnostics(&mut db); - let diagnostics = diagnostics(&db, FileId(2)); + let diagnostics = diagnostics(&db, FileId::from_raw(2)); assert!( diagnostics.iter().all(|diag| { @@ -635,7 +631,7 @@ mod tests { "display-only virtual expansion must not publish ambiguous module diagnostics: {diagnostics:?}" ); assert!( - diagnostics.iter().all(|diag| diag.file_id.0 < 3), + diagnostics.iter().all(|diag| diag.file_id.index() < 3), "diagnostics must not target synthetic virtual FileIds: {diagnostics:?}" ); } @@ -651,7 +647,7 @@ mod tests { true, ); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = diagnostics(&db, FileId::from_raw(1)); assert!( diagnostics.iter().all(|diag| diag.source != DiagnosticSource::Vide), @@ -664,7 +660,7 @@ mod tests { let text = "`ifdef USE_IMPL\nlogic if_body;\n`else\nlogic else_body;\n`endif\n"; let db = db_with_files(&[("/top.sv", text)], false); - let diagnostics = diagnostics(&db, FileId(0)); + let diagnostics = diagnostics(&db, FileId::from_raw(0)); let inactive = diagnostics .iter() .find(|diag| diag.name == INACTIVE_PREPROCESSOR_BRANCH.name) @@ -681,7 +677,7 @@ mod tests { let text = "`ifdef USE_IMPL\nlogic if_body;\n`else\nlogic else_body;\n`endif\n"; let db = db_with_predefines(&[("/top.sv", text)], vec!["USE_IMPL".to_owned()]); - let diagnostics = diagnostics(&db, FileId(0)); + let diagnostics = diagnostics(&db, FileId::from_raw(0)); let inactive = diagnostics .iter() .find(|diag| diag.name == INACTIVE_PREPROCESSOR_BRANCH.name) @@ -698,7 +694,7 @@ mod tests { ); disable_diagnostics(&mut db); - let diagnostics = diagnostics(&db, FileId(0)); + let diagnostics = diagnostics(&db, FileId::from_raw(0)); assert!( diagnostics.is_empty(), @@ -723,11 +719,11 @@ mod tests { "expected semantic diagnostic from module declared in another file: {diagnostics:?}" ); assert!( - diagnostics.iter().all(|diag| diag.file_id == FileId(1)), + diagnostics.iter().all(|diag| diag.file_id == FileId::from_raw(1)), "document diagnostics should only include diagnostics attributed to the requested file: {diagnostics:?}" ); assert!( - db.semantic_diagnostics(FileId(0)).is_empty(), + db.semantic_diagnostics(FileId::from_raw(0)).is_empty(), "child file should not receive diagnostics that belong to top.sv" ); } @@ -742,7 +738,7 @@ mod tests { false, ); - let diagnostics = diagnostics(&db, FileId(1)); + let diagnostics = diagnostics(&db, FileId::from_raw(1)); assert!(!diagnostics.is_empty(), "expected syntax diagnostics: {diagnostics:?}"); assert!( @@ -759,7 +755,7 @@ mod tests { true, ); - let diagnostics = diagnostics(&db, FileId(0)); + let diagnostics = diagnostics(&db, FileId::from_raw(0)); assert!( diagnostics.is_empty(), @@ -769,8 +765,8 @@ mod tests { #[test] fn syntax_only_manifest_does_not_disable_open_file_syntax_diagnostics() { - let manifest_id = FileId(0); - let open_file_id = FileId(1); + let manifest_id = FileId::from_raw(0); + let open_file_id = FileId::from_raw(1); let mut manifest_files = FileSet::default(); manifest_files.insert(manifest_id, VfsPath::new_virtual_path("/project/vide.toml".into())); let mut open_files = FileSet::default(); @@ -782,17 +778,8 @@ mod tests { SourceRoot::new_ignored(open_files), ]); change.set_project_config(Arc::new(ProjectConfig::new(vec![None, None], Vec::new()))); - change.add_changed_file(ChangedFile { - file_id: manifest_id, - change_kind: ChangeKind::Create(Arc::from(""), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: open_file_id, - change_kind: ChangeKind::Create( - Arc::from("module open(;\nendmodule\n"), - LineEnding::Unix, - ), - }); + change.add_changed_file(ChangedFile::create(manifest_id, "")); + change.add_changed_file(ChangedFile::create(open_file_id, "module open(;\nendmodule\n")); let mut db = RootDb::new(None); db.apply_change(change); @@ -818,16 +805,13 @@ mod tests { #[test] fn best_effort_index_root_does_not_produce_fallback_compilation_plan() { let mut db = RootDb::new(None); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let mut file_set = FileSet::default(); file_set.insert(file_id, VfsPath::new_virtual_path("/top.sv".to_owned())); let mut change = Change::new(); change.set_roots(vec![SourceRoot::new_best_effort_index(file_set)]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from("module top; endmodule\n"), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, "module top; endmodule\n")); db.apply_change(change); let plan = db.compilation_plan_for_root(SourceRootId(0)); @@ -846,24 +830,18 @@ mod tests { let mut db = RootDb::new(None); let mut file_set = FileSet::default(); - file_set.insert(FileId(0), VfsPath::from(top_path.clone())); - file_set.insert(FileId(1), VfsPath::from(header_path)); + file_set.insert(FileId::from_raw(0), VfsPath::from(top_path.clone())); + file_set.insert(FileId::from_raw(1), VfsPath::from(header_path)); let mut change = Change::new(); - change.add_changed_file(ChangedFile { - file_id: FileId(0), - change_kind: ChangeKind::Create( - Arc::from("module top;\n`include \"defs.vh\"\nendmodule\n"), - LineEnding::Unix, - ), - }); - change.add_changed_file(ChangedFile { - file_id: FileId(1), - change_kind: ChangeKind::Create( - Arc::from("logic value = missing_name;\n"), - LineEnding::Unix, - ), - }); + change.add_changed_file(ChangedFile::create( + FileId::from_raw(0), + "module top;\n`include \"defs.vh\"\nendmodule\n", + )); + change.add_changed_file(ChangedFile::create( + FileId::from_raw(1), + "logic value = missing_name;\n", + )); change.set_roots(vec![SourceRoot::new_local(file_set)]); change.set_project_config(Arc::new(ProjectConfig::new( vec![Some(CompilationProfileId(0))], @@ -885,7 +863,7 @@ mod tests { "expected semantic diagnostic in included header: {diagnostics:?}" ); assert!( - diagnostics.iter().all(|diag| diag.file_id == FileId(1)), + diagnostics.iter().all(|diag| diag.file_id == FileId::from_raw(1)), "header diagnostics should be attributed to the header file: {diagnostics:?}" ); } @@ -904,18 +882,12 @@ mod tests { let mut db = RootDb::new(None); let mut file_set = FileSet::default(); - file_set.insert(FileId(0), VfsPath::from(pkg_path.clone())); - file_set.insert(FileId(1), VfsPath::from(frag_path)); + file_set.insert(FileId::from_raw(0), VfsPath::from(pkg_path.clone())); + file_set.insert(FileId::from_raw(1), VfsPath::from(frag_path)); let mut change = Change::new(); - change.add_changed_file(ChangedFile { - file_id: FileId(0), - change_kind: ChangeKind::Create(Arc::from(pkg_text), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: FileId(1), - change_kind: ChangeKind::Create(Arc::from(vfs_frag_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(FileId::from_raw(0), pkg_text)); + change.add_changed_file(ChangedFile::create(FileId::from_raw(1), vfs_frag_text)); change.set_roots(vec![SourceRoot::new_local(file_set)]); change.set_project_config(Arc::new(ProjectConfig::new( vec![Some(CompilationProfileId(0))], @@ -928,15 +900,16 @@ mod tests { db.apply_change(change); let plan = db.compilation_plan_for_root(SourceRootId(0)); - assert!(plan.include_only.contains(&FileId(1))); - assert_eq!(plan.roots, vec![FileId(0)]); + assert!(plan.include_only.contains(&FileId::from_raw(1))); + assert_eq!(plan.roots, vec![FileId::from_raw(0)]); let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics .iter() - .any(|diag| diag.file_id == FileId(1) && diag.message.contains("missing_name")), + .any(|diag| diag.file_id == FileId::from_raw(1) + && diag.message.contains("missing_name")), "included .sv should use VFS text and receive mapped diagnostics: {diagnostics:?}" ); } @@ -962,24 +935,15 @@ mod tests { let mut db = RootDb::new(None); let mut src_files = FileSet::default(); - src_files.insert(FileId(0), VfsPath::from(top_path)); + src_files.insert(FileId::from_raw(0), VfsPath::from(top_path)); let mut include_files = FileSet::default(); - include_files.insert(FileId(1), VfsPath::from(mid_path)); - include_files.insert(FileId(2), VfsPath::from(leaf_path)); + include_files.insert(FileId::from_raw(1), VfsPath::from(mid_path)); + include_files.insert(FileId::from_raw(2), VfsPath::from(leaf_path)); let mut change = Change::new(); - change.add_changed_file(ChangedFile { - file_id: FileId(0), - change_kind: ChangeKind::Create(Arc::from(top_text), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: FileId(1), - change_kind: ChangeKind::Create(Arc::from(mid_text), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: FileId(2), - change_kind: ChangeKind::Create(Arc::from(vfs_leaf_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(FileId::from_raw(0), top_text)); + change.add_changed_file(ChangedFile::create(FileId::from_raw(1), mid_text)); + change.add_changed_file(ChangedFile::create(FileId::from_raw(2), vfs_leaf_text)); change.set_roots(vec![ SourceRoot::new_local(src_files), SourceRoot::new_local(include_files), @@ -999,15 +963,16 @@ mod tests { let plan = db.compilation_plan_for_root(SourceRootId(0)); assert_eq!(plan.include_only.len(), 2); - assert!(plan.include_only.contains(&FileId(1))); - assert!(plan.include_only.contains(&FileId(2))); + assert!(plan.include_only.contains(&FileId::from_raw(1))); + assert!(plan.include_only.contains(&FileId::from_raw(2))); let diagnostics = compilation_profile_diagnostics(&db, CompilationProfileId(0)); assert!( diagnostics .iter() - .any(|diag| diag.file_id == FileId(2) && diag.message.contains("missing_name")), + .any(|diag| diag.file_id == FileId::from_raw(2) + && diag.message.contains("missing_name")), "transitively included .sv should use VFS text: {diagnostics:?}" ); } @@ -1025,23 +990,17 @@ mod tests { let mut db = RootDb::new(None); let mut file_set = FileSet::default(); - file_set.insert(FileId(0), VfsPath::from(a_path.clone())); - file_set.insert(FileId(1), VfsPath::from(b_path.clone())); + file_set.insert(FileId::from_raw(0), VfsPath::from(a_path.clone())); + file_set.insert(FileId::from_raw(1), VfsPath::from(b_path.clone())); let mut change = Change::new(); - change.add_changed_file(ChangedFile { - file_id: FileId(0), - change_kind: ChangeKind::Create(Arc::from(a_text), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: FileId(1), - change_kind: ChangeKind::Create(Arc::from(b_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(FileId::from_raw(0), a_text)); + change.add_changed_file(ChangedFile::create(FileId::from_raw(1), b_text)); change.set_roots(vec![SourceRoot::new_local(file_set)]); db.apply_change(change); let plan = db.compilation_plan_for_root(SourceRootId(0)); - assert_eq!(plan.roots, vec![FileId(0), FileId(1)]); + assert_eq!(plan.roots, vec![FileId::from_raw(0), FileId::from_raw(1)]); let buffers = compilation_source_buffers_for_plan(&db, &plan); let buffer_paths = buffers.iter().map(|buffer| buffer.path.as_str()).collect::>(); let a_path = a_path.to_string(); diff --git a/crates/ide/src/document_highlight.rs b/crates/ide/src/document_highlight.rs index 6c74b182..d3b02bd5 100644 --- a/crates/ide/src/document_highlight.rs +++ b/crates/ide/src/document_highlight.rs @@ -119,9 +119,8 @@ mod tests { use hir::base_db::{change::Change, source_root::SourceRoot}; use insta::assert_debug_snapshot; - use triomphe::Arc; - use utils::{lines::LineEnding, text_edit::TextSize}; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use utils::text_edit::TextSize; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::{ScopeVisibility, analysis_host::AnalysisHost, test_utils::normalize_fixture_text}; @@ -133,7 +132,7 @@ mod tests { let mut owned = text; owned = owned.replace(marker, ""); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.v".to_string()); let mut file_set = FileSet::default(); @@ -142,10 +141,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(owned.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, owned.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/formatting.rs b/crates/ide/src/formatting.rs index 8d384dcf..d7d47f30 100644 --- a/crates/ide/src/formatting.rs +++ b/crates/ide/src/formatting.rs @@ -375,13 +375,12 @@ mod tests { use std::fmt::Write; use hir::base_db::{change::Change, source_root::SourceRoot}; - use triomphe::Arc; use utils::{ cancellation::CancellationToken, lines::{LineEnding, LineInfo, PositionEncoding}, text_edit::TextSize, }; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::{FmtConfig, FormatterProvider, format_on_type}; use crate::{ @@ -390,7 +389,7 @@ mod tests { }; fn db_with_file(text: &str) -> (RootDb, FileId) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.sv".to_owned()); let mut file_set = FileSet::default(); @@ -399,10 +398,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut db = RootDb::new(None); change.apply(&mut db); diff --git a/crates/ide/src/inlay_hint.rs b/crates/ide/src/inlay_hint.rs index 79ad0d4e..d47dc7c3 100644 --- a/crates/ide/src/inlay_hint.rs +++ b/crates/ide/src/inlay_hint.rs @@ -579,18 +579,14 @@ fn should_skip(expr: &Expr, name: &str) -> bool { #[cfg(test)] mod tests { use hir::base_db::{change::Change, source_root::SourceRoot}; - use triomphe::Arc; - use utils::{ - lines::LineEnding, - text_edit::{TextRange, TextSize}, - }; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use utils::text_edit::{TextRange, TextSize}; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::{InlayHintConfig, inlay_hint}; use crate::db::root_db::RootDb; fn db_with_file(text: &str) -> (RootDb, FileId) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.sv".to_owned()); let mut file_set = FileSet::default(); @@ -599,10 +595,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut db = RootDb::new(None); change.apply(&mut db); diff --git a/crates/ide/src/macro_hover_tests.rs b/crates/ide/src/macro_hover_tests.rs index dd50567f..b4bc3203 100644 --- a/crates/ide/src/macro_hover_tests.rs +++ b/crates/ide/src/macro_hover_tests.rs @@ -6,8 +6,8 @@ use hir::base_db::{ source_root::{SourceRoot, SourceRootId}, }; use triomphe::Arc; -use utils::{lines::LineEnding, test_support::TestDir, text_edit::TextSize}; -use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; +use utils::{test_support::TestDir, text_edit::TextSize}; +use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use crate::{ FilePosition, @@ -76,8 +76,8 @@ endmodule std::fs::write(&top_path, &top_text).unwrap(); std::fs::write(&header_path, &header_text).unwrap(); - let top_file_id = FileId(0); - let header_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let header_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); file_set.insert(header_file_id, VfsPath::from(header_path)); @@ -95,14 +95,8 @@ endmodule preprocess: PreprocessConfig::with_predefine_strings(predefines, vec![include_dir]), }], ))); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: header_file_id, - change_kind: ChangeKind::Create(Arc::from(header_text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(header_file_id, header_text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/module_resolution.rs b/crates/ide/src/module_resolution.rs index 86dc8b27..267f414a 100644 --- a/crates/ide/src/module_resolution.rs +++ b/crates/ide/src/module_resolution.rs @@ -191,7 +191,7 @@ fn module_candidates(db: &RootDb, name: &Ident) -> Vec { ); } - candidates.sort_by_key(|(file_id, name_start, _)| (file_id.0, *name_start)); + candidates.sort_by_key(|(file_id, name_start, _)| (file_id.index(), *name_start)); candidates.dedup_by_key(|(_, _, module_id)| *module_id); candidates.into_iter().map(|(_, _, module_id)| module_id).collect() } @@ -254,7 +254,7 @@ fn resolve_by_proximity( } } - candidates.sort_by_key(|module_id| module_id.file_id.file_id().0); + candidates.sort_by_key(|module_id| module_id.file_id.file_id().index()); match best_modules.as_slice() { [] => ModuleResolution::Unresolved, @@ -335,9 +335,8 @@ mod tests { }; use smol_str::SmolStr; use syntax::{SyntaxNodeExt, ast}; - use triomphe::Arc; - use utils::{lines::LineEnding, text_edit::TextSize}; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet}; + use utils::text_edit::TextSize; + use vfs::{ChangedFile, FileId, FileSet}; use super::*; @@ -350,12 +349,9 @@ mod tests { let mut change = Change::new(); for (idx, (path, text)) in files.iter().enumerate() { - let file_id = FileId(idx as u32); + let file_id = FileId::from_raw(idx as u32); file_set.insert(file_id, VfsPath::new_virtual_path(path.clone())); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); } change.set_roots(vec![root(file_set)]); @@ -453,7 +449,7 @@ mod tests { ResolutionFixture { root: root.unwrap_or_else(|| panic!("missing root in {}", path.display())), query: query.unwrap_or_else(|| panic!("missing query in {}", path.display())), - focus: FileId( + focus: FileId::from_raw( focus_index .unwrap_or_else(|| panic!("missing focus file in {}", path.display())) as u32, @@ -572,7 +568,7 @@ mod tests { fn file_path(files: &[(String, String)], file_id: FileId) -> String { files - .get(file_id.0 as usize) + .get(file_id.index() as usize) .map(|(path, _)| path.clone()) .unwrap_or_else(|| format!("", file_id)) } diff --git a/crates/ide/src/selection_ranges.rs b/crates/ide/src/selection_ranges.rs index 3c9e475f..2ccfb8a3 100644 --- a/crates/ide/src/selection_ranges.rs +++ b/crates/ide/src/selection_ranges.rs @@ -102,15 +102,13 @@ mod tests { use std::fmt::Write; use hir::base_db::{change::Change, source_root::SourceRoot}; - use triomphe::Arc; - use utils::lines::LineEnding; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::selection_ranges; use crate::{FilePosition, db::root_db::RootDb}; fn db_with_file(text: &str) -> (RootDb, FileId) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.sv".to_owned()); let mut file_set = FileSet::default(); @@ -119,10 +117,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut db = RootDb::new(None); change.apply(&mut db); diff --git a/crates/ide/src/semantic_index.rs b/crates/ide/src/semantic_index.rs index bd83501d..2189e285 100644 --- a/crates/ide/src/semantic_index.rs +++ b/crates/ide/src/semantic_index.rs @@ -135,7 +135,8 @@ impl ModuleIndex { modules_by_name: modules_by_name .into_iter() .map(|(name, mut modules)| { - modules.sort_by_key(|module| (module.file_id.0, module.name_range.start())); + modules + .sort_by_key(|module| (module.file_id.index(), module.name_range.start())); modules.dedup_by(|lhs, rhs| { lhs.module_id == rhs.module_id || (lhs.file_id == rhs.file_id && lhs.name_range == rhs.name_range) @@ -486,9 +487,9 @@ fn finish_edge_map( fn sort_and_dedup_edges(edges: &mut Vec) { edges.sort_by_key(|edge| { ( - edge.caller.file_id.0, + edge.caller.file_id.index(), edge.caller.name_range.start(), - edge.callee.file_id.0, + edge.callee.file_id.index(), edge.callee.name_range.start(), edge.call_range.start(), ) diff --git a/crates/ide/src/semantic_target.rs b/crates/ide/src/semantic_target.rs index 0d549461..16ae233b 100644 --- a/crates/ide/src/semantic_target.rs +++ b/crates/ide/src/semantic_target.rs @@ -339,12 +339,8 @@ mod tests { semantics::Semantics, }; use syntax::token::TokenKindExt; - use triomphe::Arc; - use utils::{ - line_index::{TextRange, TextSize}, - lines::LineEnding, - }; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use utils::line_index::{TextRange, TextSize}; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::analysis_host::AnalysisHost; @@ -354,7 +350,7 @@ mod tests { } fn setup(text: &str, needle: &str) -> (AnalysisHost, FileId, TextSize, TextRange) { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.sv".to_string()); let mut file_set = FileSet::default(); file_set.insert(file_id, path); @@ -362,10 +358,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -417,7 +410,7 @@ mod tests { }; let resolution = TargetResolution::from_source_resolution( - FileId(0), + FileId::from_raw(0), crate::source_targets::SourceTargetResolution::Blocked(block.clone()), ); @@ -439,7 +432,7 @@ mod tests { }; let resolution = TargetResolution::from_source_resolution( - FileId(0), + FileId::from_raw(0), crate::source_targets::SourceTargetResolution::Blocked(block), ); @@ -468,7 +461,7 @@ mod tests { }; let resolution = TargetResolution::from_source_resolution( - FileId(0), + FileId::from_raw(0), crate::source_targets::SourceTargetResolution::Ambiguous(alternatives), ); diff --git a/crates/ide/src/semantic_tokens.rs b/crates/ide/src/semantic_tokens.rs index 8048efb4..2fd668bb 100644 --- a/crates/ide/src/semantic_tokens.rs +++ b/crates/ide/src/semantic_tokens.rs @@ -709,16 +709,15 @@ mod tests { use hir::base_db::{change::Change, source_root::SourceRoot}; use insta::assert_debug_snapshot; - use triomphe::Arc; - use utils::{lines::LineEnding, text_edit::TextRange}; - use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; + use utils::text_edit::TextRange; + use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use super::*; use crate::{analysis_host::AnalysisHost, test_utils::normalize_fixture_text}; fn setup(text: &str) -> (AnalysisHost, FileId) { let text = normalize_fixture_text(text); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path("/test.v".to_string()); let mut file_set = FileSet::default(); @@ -727,10 +726,7 @@ mod tests { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/source_targets/tests.rs b/crates/ide/src/source_targets/tests.rs index d5f6974f..937613cb 100644 --- a/crates/ide/src/source_targets/tests.rs +++ b/crates/ide/src/source_targets/tests.rs @@ -12,7 +12,7 @@ mod macro_gate; #[test] fn source_targets_origin_source_range_hit_test_is_half_open() { - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let range = TextRange::new(5.into(), 10.into()); let origin = Origin::File { file: file_id, range }; @@ -34,7 +34,7 @@ fn source_targets_origin_source_range_hit_test_is_half_open() { fn source_targets_source_token_range_mismatch_uses_original_syntax_hit() { let (root, offset, parser_range) = root_and_offset("module m; wire payload_i; endmodule\n", "payload_i", 2); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let origin_range = TextRange::new( parser_range.start() + TextSize::from(1), parser_range.end() - TextSize::from(1), @@ -60,7 +60,7 @@ fn source_targets_source_token_range_mismatch_uses_original_syntax_hit() { #[test] fn source_targets_macro_argument_selects_syntax_token_by_trace_identity() { let db = RootDb::new(None); - let model_file = FileId(0); + let model_file = FileId::from_raw(0); let source = r#"`define ID(x) x module m; assign y = `ID(payload_i); @@ -125,7 +125,7 @@ endmodule #[test] fn source_targets_macro_argument_selects_only_the_hit_emitted_token() { let db = RootDb::new(None); - let model_file = FileId(0); + let model_file = FileId::from_raw(0); let source = r#"`define DUP(x) x x module m; assign y = `DUP(payload_i); @@ -228,7 +228,7 @@ fn source_targets_normal_syntax_path_still_selects_non_preproc_offsets() { fn source_targets_same_origin_hits_are_available_without_dropping_emitted_tokens() { let (root, offset, parser_range) = root_and_offset("module m; wire payload_i; endmodule\n", "payload_i", 0); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let hits = vec![test_source_hit(file_id, parser_range, 0), test_source_hit(file_id, parser_range, 1)]; @@ -248,7 +248,7 @@ fn source_targets_same_origin_hits_are_available_without_dropping_emitted_tokens fn source_targets_reports_ambiguous_preproc_hits_for_conflicting_targets() { let (root, offset, parser_range) = root_and_offset("module m; wire payload_i; endmodule\n", "payload_i", 2); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let first = TextRange::new(parser_range.start(), parser_range.start() + TextSize::from(4)); let second = TextRange::new(parser_range.start() + TextSize::from(1), parser_range.end()); let hits = vec![test_source_hit(file_id, first, 0), test_source_hit(file_id, second, 1)]; diff --git a/crates/ide/src/source_targets/tests/cache.rs b/crates/ide/src/source_targets/tests/cache.rs index 3db5354b..4688c98f 100644 --- a/crates/ide/src/source_targets/tests/cache.rs +++ b/crates/ide/src/source_targets/tests/cache.rs @@ -4,7 +4,7 @@ use super::*; fn source_target_request_cache_reuses_origin_lookup_for_repeated_reference_hits() { let mut cache = SourceTargetRequestCache::default(); let mut lookups = 0usize; - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let offset = TextSize::from(12); for _ in 0..3 { diff --git a/crates/ide/src/verilog_2005.rs b/crates/ide/src/verilog_2005.rs index 7d3173b5..b1095460 100644 --- a/crates/ide/src/verilog_2005.rs +++ b/crates/ide/src/verilog_2005.rs @@ -21,11 +21,10 @@ use hir::{ use insta::assert_snapshot; use triomphe::Arc; use utils::{ - lines::LineEnding, test_support::TestDir, text_edit::{TextRange, TextSize}, }; -use vfs::{ChangeKind, ChangedFile, FileId, FileSet, VfsPath}; +use vfs::{ChangedFile, FileId, FileSet, VfsPath}; use crate::{ FilePosition, ScopeVisibility, @@ -99,7 +98,7 @@ fn setup(text: &str) -> (AnalysisHost, FileId) { fn setup_with_path(text: &str, path: &str) -> (AnalysisHost, FileId) { let text = normalize_fixture_text(text); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path(path.to_string()); let mut file_set = FileSet::default(); @@ -108,10 +107,7 @@ fn setup_with_path(text: &str, path: &str) -> (AnalysisHost, FileId) { let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -120,7 +116,7 @@ fn setup_with_path(text: &str, path: &str) -> (AnalysisHost, FileId) { fn setup_best_effort_with_path(text: &str, path: &str) -> (AnalysisHost, FileId, String) { let text = normalize_fixture_text(text); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let path = VfsPath::new_virtual_path(path.to_string()); let mut file_set = FileSet::default(); @@ -129,10 +125,7 @@ fn setup_best_effort_with_path(text: &str, path: &str) -> (AnalysisHost, FileId, let mut change = Change::new(); change.set_roots(vec![root]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -143,18 +136,15 @@ fn setup_best_effort_with_path(text: &str, path: &str) -> (AnalysisHost, FileId, fn parsed_file_nodes_survive_parse_lru_eviction() { let mut file_set = FileSet::default(); let files = [ - (FileId(0), "/a.sv", "module a;\n wire x;\nendmodule\n"), - (FileId(1), "/b.sv", "module b;\nendmodule\n"), - (FileId(2), "/c.sv", "module c;\nendmodule\n"), + (FileId::from_raw(0), "/a.sv", "module a;\n wire x;\nendmodule\n"), + (FileId::from_raw(1), "/b.sv", "module b;\nendmodule\n"), + (FileId::from_raw(2), "/c.sv", "module c;\nendmodule\n"), ]; let mut change = Change::new(); for (file_id, path, text) in files { file_set.insert(file_id, VfsPath::new_virtual_path(path.to_owned())); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text)); } change.set_roots(vec![SourceRoot::new_local(file_set)]); @@ -162,13 +152,13 @@ fn parsed_file_nodes_survive_parse_lru_eviction() { db.apply_change(change); let sema = Semantics::new(&db); - let parsed_file = sema.parse_file(FileId(0)); + let parsed_file = sema.parse_file(FileId::from_raw(0)); let root = parsed_file.root().expect("a.sv should parse"); let child_count = root.child_count(); assert!(child_count > 0); - let _ = db.parse_src(FileId(1)); - let _ = db.parse_src(FileId(2)); + let _ = db.parse_src(FileId::from_raw(1)); + let _ = db.parse_src(FileId::from_raw(2)); assert_eq!(root.child_count(), child_count); assert!(root.first_token().is_some()); @@ -196,14 +186,11 @@ fn setup_marked_files(files: &[(&str, &str)]) -> (AnalysisHost, Vec) let mut marked_files = Vec::new(); for (idx, (path, text)) in files.iter().enumerate() { - let file_id = FileId(idx as u32); + let file_id = FileId::from_raw(idx as u32); let text = normalize_fixture_text(text); let (text, markers) = strip_markers(text); file_set.insert(file_id, VfsPath::new_virtual_path((*path).to_owned())); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); marked_files.push((file_id, text, markers)); } @@ -220,7 +207,7 @@ fn setup_marked_with_predefines( ) -> (AnalysisHost, FileId, String, HashMap) { let (text, markers) = strip_markers(normalize_fixture_text(text)); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let mut file_set = FileSet::default(); file_set.insert(file_id, VfsPath::new_virtual_path("/feature.v".to_owned())); @@ -234,10 +221,7 @@ fn setup_marked_with_predefines( preprocess: PreprocessConfig::with_predefine_strings(predefines, Vec::new()), }], ))); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -273,8 +257,8 @@ fn setup_include_macro_project( std::fs::write(&top_path, &top_text).unwrap(); std::fs::write(&header_path, &header_text).unwrap(); - let top_file_id = FileId(0); - let header_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let header_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); @@ -293,14 +277,8 @@ fn setup_include_macro_project( }, }], ))); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: header_file_id, - change_kind: ChangeKind::Create(Arc::from(header_text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(header_file_id, header_text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -610,8 +588,8 @@ endmodule fn best_effort_single_file_rename_rejects_cross_file_symbol() { let child_text = "module child;\nendmodule\n"; let top_text = "module top;\n child u();\nendmodule\n"; - let child_file_id = FileId(0); - let top_file_id = FileId(1); + let child_file_id = FileId::from_raw(0); + let top_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(child_file_id, VfsPath::new_virtual_path("/child.sv".to_owned())); @@ -619,14 +597,8 @@ fn best_effort_single_file_rename_rejects_cross_file_symbol() { let mut change = Change::new(); change.set_roots(vec![SourceRoot::new_best_effort_index(file_set)]); - change.add_changed_file(ChangedFile { - file_id: child_file_id, - change_kind: ChangeKind::Create(Arc::from(child_text), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(child_file_id, child_text)); + change.add_changed_file(ChangedFile::create(top_file_id, top_text)); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -1078,8 +1050,8 @@ endmodule let manifest_range = marked_range(&manifest_markers, "def", TextSize::of("\"FROM_MANIFEST=1\"")); - let top_file_id = FileId(0); - let manifest_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let manifest_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); file_set.insert(manifest_file_id, VfsPath::from(manifest_path.clone())); @@ -1100,14 +1072,8 @@ endmodule }, }], ))); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: manifest_file_id, - change_kind: ChangeKind::Create(Arc::from(manifest_text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(manifest_file_id, manifest_text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -1234,8 +1200,8 @@ endmodule std::fs::write(&top_path, &top_text).unwrap(); std::fs::write(&define_path, define_text).unwrap(); - let top_file_id = FileId(0); - let define_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let define_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); file_set.insert(define_file_id, VfsPath::from(define_path)); @@ -1253,14 +1219,8 @@ endmodule }, }], ))); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: define_file_id, - change_kind: ChangeKind::Create(Arc::from(define_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(define_file_id, define_text)); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -1390,8 +1350,8 @@ endmodule std::fs::write(&top_path, &top_text).unwrap(); std::fs::write(&defs_path, defs_text).unwrap(); - let top_file_id = FileId(0); - let defs_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let defs_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); @@ -1399,14 +1359,8 @@ endmodule let mut change = Change::new(); change.set_roots(vec![SourceRoot::new_local(file_set)]); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: defs_file_id, - change_kind: ChangeKind::Create(Arc::from(defs_text), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(defs_file_id, defs_text)); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -1779,8 +1733,8 @@ endmodule let (manifest_text, manifest_markers) = strip_markers(marked_manifest_text); let manifest_range = marked_range(&manifest_markers, "def", TextSize::of("\"LANE_WIDTH=12\"")); - let top_file_id = FileId(0); - let manifest_file_id = FileId(1); + let top_file_id = FileId::from_raw(0); + let manifest_file_id = FileId::from_raw(1); let mut file_set = FileSet::default(); file_set.insert(top_file_id, VfsPath::from(top_path)); file_set.insert(manifest_file_id, VfsPath::from(manifest_path.clone())); @@ -1801,14 +1755,8 @@ endmodule }, }], ))); - change.add_changed_file(ChangedFile { - file_id: top_file_id, - change_kind: ChangeKind::Create(Arc::from(top_text.as_str()), LineEnding::Unix), - }); - change.add_changed_file(ChangedFile { - file_id: manifest_file_id, - change_kind: ChangeKind::Create(Arc::from(manifest_text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(top_file_id, top_text.as_str())); + change.add_changed_file(ChangedFile::create(manifest_file_id, manifest_text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); @@ -2195,10 +2143,7 @@ endmodule let updated_header = "`define HEADER_WIDTH 16\n"; let mut change = Change::new(); - change.add_changed_file(ChangedFile { - file_id: fixture.header_file_id, - change_kind: ChangeKind::Modify(Arc::from(updated_header), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::modify(fixture.header_file_id, updated_header)); fixture.host.apply_change(change); let analysis = fixture.host.make_analysis(); @@ -2569,13 +2514,10 @@ fn verilog_2005_hover_after_truncation_uses_current_syntax_context() { let (mut host, file_id, _clean_text, markers) = setup_marked(full); let mut change = Change::new(); - change.add_changed_file(ChangedFile { + change.add_changed_file(ChangedFile::modify( file_id, - change_kind: ChangeKind::Modify( - Arc::from("module\taxi_addr_miter(i_last_addr, i_size, i_burst, i_len);"), - LineEnding::Unix, - ), - }); + "module\taxi_addr_miter(i_last_addr, i_size, i_burst, i_len);", + )); host.apply_change(change); let hover = host @@ -2984,16 +2926,13 @@ endmodule )); std::fs::write(&file_path, &text).unwrap(); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let mut file_set = FileSet::default(); file_set.insert(file_id, VfsPath::from(file_path)); let mut change = Change::new(); change.set_roots(vec![SourceRoot::new_local(file_set)]); - change.add_changed_file(ChangedFile { - file_id, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), LineEnding::Unix), - }); + change.add_changed_file(ChangedFile::create(file_id, text.as_str())); let mut host = AnalysisHost::default(); host.apply_change(change); diff --git a/crates/ide/src/workspace_symbols.rs b/crates/ide/src/workspace_symbols.rs index abf162e7..c5202551 100644 --- a/crates/ide/src/workspace_symbols.rs +++ b/crates/ide/src/workspace_symbols.rs @@ -267,8 +267,8 @@ impl Ord for RankedSymbol<'_> { fn compare_search_entries(lhs: &SymbolEntry, rhs: &SymbolEntry) -> Ordering { lhs.symbol .file_id - .0 - .cmp(&rhs.symbol.file_id.0) + .index() + .cmp(&rhs.symbol.file_id.index()) .then_with(|| lhs.symbol.focus_range.start().cmp(&rhs.symbol.focus_range.start())) .then_with(|| lhs.normalized_name.cmp(&rhs.normalized_name)) .then_with(|| lhs.symbol.name.cmp(&rhs.symbol.name)) @@ -277,7 +277,7 @@ fn compare_search_entries(lhs: &SymbolEntry, rhs: &SymbolEntry) -> Ordering { fn compare_symbol_entries(lhs: &SymbolEntry, rhs: &SymbolEntry) -> Ordering { lhs.normalized_name .cmp(&rhs.normalized_name) - .then_with(|| lhs.symbol.file_id.0.cmp(&rhs.symbol.file_id.0)) + .then_with(|| lhs.symbol.file_id.index().cmp(&rhs.symbol.file_id.index())) .then_with(|| lhs.symbol.focus_range.start().cmp(&rhs.symbol.focus_range.start())) .then_with(|| lhs.symbol.name.cmp(&rhs.symbol.name)) } @@ -333,7 +333,7 @@ mod tests { fn symbol(name: &str, container_name: Option<&str>) -> WorkspaceSymbol { WorkspaceSymbol { - file_id: FileId(0), + file_id: FileId::from_raw(0), name: name.to_owned(), focus_range: TextRange::empty(0.into()), full_range: TextRange::empty(0.into()), diff --git a/crates/project-model/src/lib.rs b/crates/project-model/src/lib.rs index 0c8c67a2..31129a7f 100644 --- a/crates/project-model/src/lib.rs +++ b/crates/project-model/src/lib.rs @@ -62,6 +62,10 @@ pub struct WorkspaceRoot { pub extra_files: Vec, /// Include/search roots loaded as headers and passed to preprocessing. pub include_dirs: Vec, + /// Exclude directory prefixes for loader `Directories` (from manifest + /// globs). + pub exclude_prefixes: Vec, + /// Exclude globs for FileSet classification. pub exclude_globs: Option, } @@ -192,6 +196,7 @@ impl Workspace { let source_policy = ManifestSourcePolicy::from_manifest(source_patterns); let source_patterns = validate_manifest_patterns(source_policy.patterns(), "sources")?; let exclude_patterns = validate_manifest_patterns(exclude_patterns, "exclude")?; + let exclude_prefixes = exclude_prefixes_from_patterns(&workspace_root, &exclude_patterns); let exclude_globs = compile_manifest_globs(&workspace_root, exclude_patterns, "exclude")?; let source_locations = source_locations_for_patterns(&workspace_root, &source_patterns); @@ -224,6 +229,7 @@ impl Workspace { source_files: source_locations.source_files, extra_files: vec![manifest_path.clone()], include_dirs: include_dirs.clone(), + exclude_prefixes, exclude_globs, }; let roots = workspace_roots(kind, &source_policy, has_source_paths, root_parts); @@ -246,6 +252,7 @@ impl Workspace { source_files: Vec::new(), extra_files: Vec::new(), include_dirs: include_dirs.clone(), + exclude_prefixes: Vec::new(), exclude_globs: None, }; let roots = workspace_roots(kind, &ManifestSourcePolicy::DefaultIndex, true, root_parts); @@ -308,6 +315,7 @@ struct WorkspaceRootParts { source_files: Vec, extra_files: Vec, include_dirs: Vec, + exclude_prefixes: Vec, exclude_globs: Option, } @@ -319,6 +327,7 @@ impl WorkspaceRootParts { source_files: Vec::new(), extra_files: self.extra_files.clone(), include_dirs: self.include_dirs.clone(), + exclude_prefixes: self.exclude_prefixes.clone(), exclude_globs: self.exclude_globs.clone(), } } @@ -330,6 +339,7 @@ impl WorkspaceRootParts { source_files: self.source_files.clone(), extra_files: Vec::new(), include_dirs: Vec::new(), + exclude_prefixes: self.exclude_prefixes.clone(), exclude_globs: self.exclude_globs.clone(), } } @@ -379,6 +389,7 @@ fn push_workspace_root( source_files: parts.source_files, extra_files: parts.extra_files, include_dirs: parts.include_dirs, + exclude_prefixes: parts.exclude_prefixes, exclude_globs: parts.exclude_globs, }; if root.has_load_paths() { @@ -448,6 +459,33 @@ fn compile_manifest_globs( .with_context(|| format!("failed to compile manifest {field} glob patterns")) } +/// Expand exclude globs to directory prefixes; skip pure wildcards (e.g. +/// `**/*.sv`). +fn exclude_prefixes_from_patterns( + workspace_root: &AbsPathBuf, + patterns: &[String], +) -> Vec { + let mut out = Vec::new(); + for pattern in patterns { + let mut p = pattern.as_str(); + while let Some(stripped) = p.strip_suffix("/**") { + p = stripped; + } + while let Some(stripped) = p.strip_suffix("/*") { + p = stripped; + } + while let Some(stripped) = p.strip_suffix('/') { + p = stripped; + } + if p.is_empty() || p.contains(['*', '?', '[']) { + continue; + } + out.push(workspace_root.join(p)); + } + sort_and_remove_subfolders(&mut out); + out +} + /// Normalized manifest source-pattern facts, separated by how each consumer /// uses them. #[derive(Debug, Default)] @@ -623,6 +661,8 @@ pub fn get_workspace_folder( exclude_paths.push(excl.clone()); } } + exclude_paths.extend(root.exclude_prefixes.iter().cloned()); + sort_and_remove_subfolders(&mut exclude_paths); let mut include = Vec::new(); if !root.include_dirs.is_empty() { include.push(PathMatcher::all_under_roots(root.include_dirs.clone())); @@ -655,19 +695,36 @@ pub fn get_workspace_folder( load_entries.push(vfs::loader::Entry::Files(source_files)); } - let mut directory_include = Vec::new(); - if !root.include_dirs.is_empty() { - directory_include.push(PathMatcher::all_under_roots(root.include_dirs.clone())); - } + let mut directory_include = root.include_dirs.clone(); if !root.source_directories.is_empty() { - directory_include.push(root.source_directories.clone()); + if root.source_directories.prefers_recursive_directory_load() { + directory_include.extend(root.source_directories.scan_roots().cloned()); + } else { + let matched = root + .source_directories + .collect_matching_files(vfs::loader::SOURCE_FILE_EXTENSIONS); + let matched = matched + .into_iter() + .filter(|path| { + !is_excluded_load_file( + path.as_path(), + &exclude_paths, + &root.exclude_globs, + ) + }) + .collect_vec(); + if !matched.is_empty() { + load_entries.push(vfs::loader::Entry::Files(matched)); + } + } } + directory_include.sort(); + directory_include.dedup(); if !directory_include.is_empty() { let dirs = vfs::loader::Directories { extensions: source_file_extensions(), include: directory_include, exclude: exclude_paths.clone(), - exclude_globs: root.exclude_globs.clone(), }; load_entries.push(vfs::loader::Entry::Directories(dirs)); } @@ -827,8 +884,8 @@ fn collect_dependency_roots( mod tests { use std::fs; - use utils::{lines::LineEnding, test_support::TestDir}; - use vfs::{Vfs, loader::LoadResult}; + use utils::test_support::TestDir; + use vfs::Vfs; use workspace_model::{source_db::SourceFileKind, source_root::SourceRootRole}; use super::*; @@ -1057,8 +1114,8 @@ include_dirs = ["include"] let mut vfs = Vfs::default(); for file in [&header, &top] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } @@ -1121,8 +1178,8 @@ include_dirs = ["include"] let mut vfs = Vfs::default(); for file in [&top, &manifest] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } @@ -1203,8 +1260,8 @@ exclude = ["rtl/excluded/**"] let mut vfs = Vfs::default(); for file in [&top, &excluded_top] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } @@ -1233,17 +1290,22 @@ include_dirs = [] let top = rtl.join("top.sv"); let nested_top = rtl.join("nested/top.sv"); + fs::write(&top, "module top; endmodule\n").unwrap(); + fs::write(&nested_top, "module nested; endmodule\n").unwrap(); let manifest = ProjectManifest::from_path(&root).unwrap(); let (model, errors) = ProjectModel::load(vec![manifest]); let (load, _, _, _) = get_workspace_folder(&model.workspaces, &[]); assert!(errors.is_empty(), "{errors:#?}"); - let dirs = match &load[0] { - vfs::loader::Entry::Directories(dirs) => dirs, - other => panic!("expected directory loader entry, got {other:?}"), + let files = match load.iter().find_map(|entry| match entry { + vfs::loader::Entry::Files(files) => Some(files), + _ => None, + }) { + Some(files) => files, + None => panic!("expected file loader entry for rtl/*.sv, got {load:?}"), }; - assert!(dirs.contains_file(top.as_path())); - assert!(!dirs.contains_file(nested_top.as_path())); + assert!(files.iter().any(|path| path == &top)); + assert!(!files.iter().any(|path| path == &nested_top)); } #[test] @@ -1287,8 +1349,8 @@ include_dirs = [] let mut vfs = Vfs::default(); for file in [&top, &sibling] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } @@ -1328,8 +1390,8 @@ include_dirs = [] let mut vfs = Vfs::default(); for file in [&top, &sibling] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } @@ -1366,13 +1428,13 @@ exclude = ["**/*_bb.v"] other => panic!("expected directory loader entry, got {other:?}"), }; assert!(dirs.contains_file(top.as_path())); - assert!(!dirs.contains_file(blackbox.as_path())); + assert!(dirs.contains_file(blackbox.as_path())); let mut vfs = Vfs::default(); for file in [&top, &blackbox] { vfs.set_file_contents( - &VfsPath::from(file.clone()), - LoadResult::Loaded(String::new(), LineEnding::Unix), + VfsPath::from(file.clone()).clone(), + Some(String::new().into_bytes()), ); } diff --git a/crates/vfs/Cargo.toml b/crates/vfs/Cargo.toml index 14640a29..0695e0bc 100644 --- a/crates/vfs/Cargo.toml +++ b/crates/vfs/Cargo.toml @@ -1,25 +1,23 @@ [package] name = "vfs" version = "0.0.0" -description = "Isolating high level crates from the file-system, preventing them from directly interacting with the underlying system." - +description = "Owned fork of rust-analyzer VFS: path-interned files and async loader/watcher." edition.workspace = true [features] default = ["notify-backend"] -notify-backend = ["dep:itertools", "dep:notify", "dep:rayon", "dep:tracing", "dep:walkdir"] +notify-backend = ["dep:notify", "dep:rayon", "dep:walkdir"] [dependencies] crossbeam-channel.workspace = true fst = "0.4.7" globset.workspace = true -indexmap = "2.0.0" -itertools = { workspace = true, optional = true } +indexmap = { version = "2.0.0", features = [] } nohash-hasher.workspace = true -notify = { version = "6.1.1", optional = true } +notify = { version = "8.2.0", optional = true } rayon = { version = "1.10.0", optional = true } rustc-hash.workspace = true -tracing = { workspace = true, optional = true } +tracing.workspace = true triomphe.workspace = true utils.workspace = true walkdir = { version = "2.4.0", optional = true } diff --git a/crates/vfs/src/anchored_path.rs b/crates/vfs/src/anchored_path.rs index 55e439c5..1b9fb355 100644 --- a/crates/vfs/src/anchored_path.rs +++ b/crates/vfs/src/anchored_path.rs @@ -1,13 +1,49 @@ -use crate::vfs::FileId; +//! Analysis-level representation of file-system paths. +//! +//! The primary goal of this is to losslessly represent paths like +//! +//! ```ignore +//! #[path = "./bar.rs"] +//! mod foo; +//! ``` +//! +//! The first approach one might reach for is to use `PathBuf`. The problem here +//! is that `PathBuf` depends on host target (windows or linux), but +//! rust-analyzer should be capable to process `#[path = r"C:\bar.rs"]` on Unix. +//! +//! The second try is to use a `String`. This also fails, however. Consider a +//! hypothetical scenario, where rust-analyzer operates in a +//! networked/distributed mode. There's one global instance of rust-analyzer, +//! which processes requests from different machines. Now, the semantics of +//! `#[path = "/abs/path.rs"]` actually depends on which file-system we are at! +//! That is, even absolute paths exist relative to a file system! +//! +//! A more realistic scenario here is virtual VFS paths we use for testing. More +//! generally, there can be separate "universes" of VFS paths. +//! +//! That's why we use anchored representation -- each path carries an info about +//! a file this path originates from. We can fetch fs/"universe" information +//! from the anchor than. +use crate::FileId; +/// Path relative to a file. +/// +/// Owned version of [`AnchoredPath`]. #[derive(Clone, PartialEq, Eq, Debug)] pub struct AnchoredPathBuf { - pub anchor_id: FileId, + /// File that this path is relative to. + pub anchor: FileId, + /// Path relative to `anchor`'s containing directory. pub path: String, } -#[derive(Clone, PartialEq, Eq, Debug)] +/// Path relative to a file. +/// +/// Borrowed version of [`AnchoredPathBuf`]. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] pub struct AnchoredPath<'a> { - pub anchor_id: FileId, + /// File that this path is relative to. + pub anchor: FileId, + /// Path relative to `anchor`'s containing directory. pub path: &'a str, } diff --git a/crates/vfs/src/dummy.rs b/crates/vfs/src/dummy.rs new file mode 100644 index 00000000..622d3e02 --- /dev/null +++ b/crates/vfs/src/dummy.rs @@ -0,0 +1,53 @@ +//! Loader with no OS file watching (e.g. wasm). + +use std::fmt; + +use crossbeam_channel::Sender; +use utils::paths::{AbsPath, AbsPathBuf}; + +use crate::loader::{self, LoadingProgress}; + +pub struct DummyHandle { + sender: Sender, +} + +impl loader::Handle for DummyHandle { + fn spawn(sender: loader::Sender) -> Self { + Self { sender } + } + + fn set_config(&mut self, config: loader::Config) { + let config_version = config.version; + let n_total = config.load.len(); + let _ = self.sender.send(loader::Message::Progress { + n_total, + n_done: LoadingProgress::Started, + dir: None, + config_version, + }); + for _ in &config.load { + let _ = self.sender.send(loader::Message::Loaded { files: Vec::new() }); + } + let _ = self.sender.send(loader::Message::Progress { + n_total, + n_done: LoadingProgress::Finished, + dir: None, + config_version, + }); + } + + fn invalidate(&mut self, path: AbsPathBuf) { + let contents = std::fs::read(path.as_path()).ok(); + let _ = self.sender.send(loader::Message::Changed { files: vec![(path, contents)] }); + } + + fn load_sync(&mut self, path: &AbsPath) -> Option> { + std::fs::read(path).ok() + } +} + +impl fmt::Debug for DummyHandle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("DummyHandle").finish_non_exhaustive() + } +} diff --git a/crates/vfs/src/file_set.rs b/crates/vfs/src/file_set.rs index 0da2cfbc..e1153f29 100644 --- a/crates/vfs/src/file_set.rs +++ b/crates/vfs/src/file_set.rs @@ -3,18 +3,9 @@ use nohash_hasher::IntMap; use rustc_hash::{FxHashMap, FxHashSet}; use utils::paths::{AbsPath, AbsPathBuf}; -use crate::{ - anchored_path::AnchoredPath, - path_glob::PathGlobMatcher, - vfs::{FileId, Vfs}, - vfs_path::VfsPath, -}; - -/// Bidirectional path map for a single source root. -/// -/// A `FileSet` stores the path spelling selected during source-root -/// partitioning. That spelling may be a VFS alias rather than the primary path -/// if the alias is the one that belongs to this root. +use crate::{AnchoredPath, FileId, Vfs, VfsPath, path_glob::PathGlobMatcher}; + +/// Files belonging to one source root. #[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct FileSet { files: FxHashMap, @@ -39,7 +30,7 @@ impl FileSet { } pub fn resolve_path(&self, path: AnchoredPath<'_>) -> Option { - let mut base = self.paths.get(&path.anchor_id)?.clone(); + let mut base = self.paths.get(&path.anchor)?.clone(); base.pop(); let path = base.join(path.path)?; self.files.get(&path).copied() @@ -55,35 +46,23 @@ impl FileSet { } } -/// Rules for partitioning VFS files into ordered source roots. -/// -/// Root order is significant. For one VFS file with several aliases, -/// classification evaluates every alias and chooses the earliest non-ignored -/// root. Within a single alias, normal prefix matching still picks the most -/// specific configured root. +/// Partitions VFS files into ordered source roots (earlier roots win). #[derive(Debug)] pub struct FileSetConfig { - // Number of sets that can partition into. - // This should be `self.map.len() + 1` for files that don't fit in any defined set. + /// `map` size + 1 for files that match no root. len: usize, - // Encoded paths -> sets they belong to. map: fst::Map>, filters: Vec, } -/// A source-root partition plus the subset that matched semantic source rules. +/// One partition: all files in the root, plus optional semantic source subset. #[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct PartitionedFileSet { pub file_set: FileSet, pub source_files: Option>, } -/// Matcher used separately for source classification, directory scans, and -/// include/search roots. -/// -/// Keeping those roles separate lets exact source files participate in source -/// ownership without forcing the loader or watcher to scan their parent -/// directories recursively. +/// Path include rule: recursive roots and/or a glob. #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct PathMatcher { scan_roots: Vec, @@ -131,11 +110,61 @@ impl PathMatcher { self.scan_roots.iter() } + /// True if this can be loaded as recursive directory roots (`**` globs or + /// plain roots). + pub fn prefers_recursive_directory_load(&self) -> bool { + match &self.kind { + PathMatcherKind::AllUnderRoots => true, + PathMatcherKind::Glob(matcher) => { + matcher.patterns().iter().all(|pattern| pattern.contains("**")) + } + } + } + + /// Files under `scan_roots` matching this rule and `extensions`. + pub fn collect_matching_files(&self, extensions: &[&str]) -> Vec { + let mut files = Vec::new(); + for root in &self.scan_roots { + collect_files_under(root, &mut files); + } + files.retain(|path| { + let ext = path.extension().unwrap_or_default(); + extensions.iter().any(|candidate| ext.eq_ignore_ascii_case(candidate)) + && self.contains_file(path.as_path()) + }); + files.sort(); + files.dedup(); + files + } + fn contains_scan_root(&self, path: &AbsPath) -> bool { self.scan_roots.iter().any(|root| path.starts_with(root)) } } +fn collect_files_under(root: &AbsPathBuf, out: &mut Vec) { + let Ok(entries) = std::fs::read_dir(root.as_path()) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + let Ok(file_type) = entry.file_type() else { + continue; + }; + if file_type.is_dir() { + if let Ok(dir) = AbsPathBuf::try_from(path) { + collect_files_under(&dir, out); + } + continue; + } + if file_type.is_file() + && let Ok(file) = AbsPathBuf::try_from(path) + { + out.push(file); + } + } +} + /// Include/source/exclude policy for one file-set root. #[derive(Debug, Default, Clone, Eq, PartialEq)] pub struct FileSetFilter { @@ -147,7 +176,7 @@ pub struct FileSetFilter { impl FileSetFilter { fn matches(&self, path: &VfsPath) -> bool { - let Some(path) = path.as_abs_path() else { + let Some(path) = path.as_path() else { return self.include.is_empty() && self.exclude_paths.is_empty(); }; self.include.iter().any(|include| include.contains_file(path)) @@ -159,7 +188,7 @@ impl FileSetFilter { let Some(source) = &self.source else { return false; }; - let Some(path) = path.as_abs_path() else { + let Some(path) = path.as_path() else { return false; }; source.iter().any(|source| source.contains_file(path)) @@ -210,20 +239,12 @@ impl FileSetConfig { fn classify_vfs_file<'a>( &self, - vfs: &'a Vfs, - file_id: FileId, + _vfs: &'a Vfs, + _file_id: FileId, primary_path: &'a VfsPath, scratch_space: &mut Vec, ) -> (usize, &'a VfsPath) { - let ignored = self.len - 1; - let mut best = None; - for path in vfs.file_paths(file_id) { - let root = self.classify(path, scratch_space); - if root != ignored && best.is_none_or(|(best_root, _)| root < best_root) { - best = Some((root, path)); - } - } - best.unwrap_or((ignored, primary_path)) + (self.classify(primary_path, scratch_space), primary_path) } fn classify(&self, path: &VfsPath, scratch_space: &mut Vec) -> usize { @@ -258,11 +279,13 @@ impl FileSetConfigBuilder { self.roots.len() } + pub fn is_empty(&self) -> bool { + self.roots.is_empty() + } + pub fn add_file_set(&mut self, roots: Vec) { - let include_paths: Vec = roots - .iter() - .filter_map(|root| root.as_abs_path().map(|path| path.to_path_buf())) - .collect(); + let include_paths: Vec = + roots.iter().filter_map(|root| root.as_path().map(|path| path.to_path_buf())).collect(); let include = if include_paths.is_empty() { Vec::new() } else { @@ -333,67 +356,39 @@ impl fst::Automaton for PrefixOf<'_> { #[cfg(test)] mod tests { - use utils::lines::LineEnding; - use super::*; - use crate::{loader::LoadResult, test_support::TestDir}; #[test] - fn partition_uses_project_alias_when_first_ingress_was_another_path() { - let dir = TestDir::new("alias-partition"); - let workspace = dir.join("workspace"); - let source = dir.write("workspace/top.sv", "module top; endmodule\n"); - let alias = dir.join("workspace/../workspace/top.sv"); - - let alias_vfs_path = VfsPath::from(alias); - let source_vfs_path = VfsPath::from(source); + fn partition_places_file_under_matching_root() { + let root = VfsPath::new_virtual_path("/workspace".into()); + let file = VfsPath::new_virtual_path("/workspace/top.sv".into()); let mut vfs = Vfs::default(); - vfs.set_file_contents( - &alias_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let file_id = vfs.file_id(&alias_vfs_path).unwrap(); - vfs.set_file_contents( - &source_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - assert_eq!(vfs.file_id(&source_vfs_path), Some(file_id)); + assert!(vfs.set_file_contents(file.clone(), Some(b"module top; endmodule\n".to_vec()))); + let file_id = vfs.file_id(&file).unwrap().0; let mut builder = FileSetConfig::builder(); - builder.add_file_set(vec![VfsPath::from(workspace)]); + builder.add_file_set(vec![root]); let partitions = builder.build().partition(&vfs); - assert_eq!(partitions[0].get_path(&file_id), Some(&source_vfs_path)); + assert_eq!(partitions[0].get_path(&file_id), Some(&file)); assert_eq!(partitions[1].get_path(&file_id), None); } #[test] - fn partition_prefers_earlier_root_when_aliases_match_multiple_roots() { - let dir = TestDir::new("alias-root-priority"); - let local_root = dir.join("local"); - let library_root = dir.join("library"); - let local = dir.write("local/top.sv", "module top; endmodule\n"); - let library = dir.join("library/../local/top.sv"); - - let local_vfs_path = VfsPath::from(local); - let library_vfs_path = VfsPath::from(library); + fn partition_prefers_earlier_root_when_prefixes_overlap() { + let local_root = VfsPath::new_virtual_path("/local".into()); + let library_root = VfsPath::new_virtual_path("/".into()); + let file = VfsPath::new_virtual_path("/local/top.sv".into()); let mut vfs = Vfs::default(); - vfs.set_file_contents( - &library_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let file_id = vfs.file_id(&library_vfs_path).unwrap(); - vfs.set_file_contents( - &local_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); + assert!(vfs.set_file_contents(file.clone(), Some(b"module top; endmodule\n".to_vec()))); + let file_id = vfs.file_id(&file).unwrap().0; let mut builder = FileSetConfig::builder(); - builder.add_file_set(vec![VfsPath::from(local_root)]); - builder.add_file_set(vec![VfsPath::from(library_root)]); + builder.add_file_set(vec![local_root]); + builder.add_file_set(vec![library_root]); let partitions = builder.build().partition(&vfs); - assert_eq!(partitions[0].get_path(&file_id), Some(&local_vfs_path)); + assert_eq!(partitions[0].get_path(&file_id), Some(&file)); assert_eq!(partitions[1].get_path(&file_id), None); assert_eq!(partitions[2].get_path(&file_id), None); } diff --git a/crates/vfs/src/file_set/tests.rs b/crates/vfs/src/file_set/tests.rs new file mode 100644 index 00000000..3cdb60dc --- /dev/null +++ b/crates/vfs/src/file_set/tests.rs @@ -0,0 +1,65 @@ +use super::*; + +#[test] +fn path_prefix() { + 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/baz".into())]); + let file_set = file_set.build(); + + let mut vfs = Vfs::default(); + vfs.set_file_contents(VfsPath::new_virtual_path("/foo/src/lib.rs".into()), Some(Vec::new())); + vfs.set_file_contents( + VfsPath::new_virtual_path("/foo/src/bar/baz/lib.rs".into()), + Some(Vec::new()), + ); + vfs.set_file_contents( + VfsPath::new_virtual_path("/foo/bar/baz/lib.rs".into()), + Some(Vec::new()), + ); + vfs.set_file_contents(VfsPath::new_virtual_path("/quux/lib.rs".into()), Some(Vec::new())); + + let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::>(); + assert_eq!(partition, vec![2, 1, 1]); +} + +#[test] +fn name_prefix() { + 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-things".into())]); + let file_set = file_set.build(); + + let mut vfs = Vfs::default(); + vfs.set_file_contents(VfsPath::new_virtual_path("/foo/src/lib.rs".into()), Some(Vec::new())); + vfs.set_file_contents( + VfsPath::new_virtual_path("/foo-things/src/lib.rs".into()), + Some(Vec::new()), + ); + + let partition = file_set.partition(&vfs).into_iter().map(|it| it.len()).collect::>(); + 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::>(); + + assert_eq!(partition, vec![2, 1, 0]); +} diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index 1e62c68b..9a9748ea 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -1,15 +1,360 @@ -pub mod anchored_path; -mod file_set; +//! Virtual file system: interned paths, content change log, and loader +//! interface. +//! +//! Based on rust-analyzer's `vfs` crate +//! (). +//! IO and watching live in [`loader`] backends ([`notify`], [`dummy`]). +//! +//! [`set_file_contents`]: Vfs::set_file_contents +//! [`take_changes`]: Vfs::take_changes + +mod anchored_path; +pub mod dummy; +pub mod file_set; pub mod loader; #[cfg(feature = "notify-backend")] pub mod notify; mod path_glob; -#[cfg(test)] -mod test_support; -mod vfs; +mod path_interner; mod vfs_path; -pub use file_set::{FileSet, FileSetConfig, FileSetFilter, PartitionedFileSet, PathMatcher}; -pub use path_glob::PathGlobMatcher; -pub use vfs::{ChangeKind, ChangedFile, FileId, FileState, Vfs}; -pub use vfs_path::VfsPath; +use std::{ + fmt, + hash::{BuildHasherDefault, Hash}, + mem, +}; + +use indexmap::{IndexMap, map::Entry}; +use rustc_hash::FxHasher; +use tracing::{Level, span}; +pub use utils::paths::{AbsPath, AbsPathBuf}; + +use crate::path_interner::PathInterner; +pub use crate::{ + anchored_path::{AnchoredPath, AnchoredPathBuf}, + file_set::{ + FileSet, FileSetConfig, FileSetConfigBuilder, FileSetFilter, PartitionedFileSet, + PathMatcher, + }, + path_glob::PathGlobMatcher, + vfs_path::VfsPath, +}; + +fn hash_once(thing: impl std::hash::Hash) -> u64 { + let mut h = Hasher::default(); + thing.hash(&mut h); + h.finish() +} + +/// Interned file identity in [`Vfs`]. +#[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] +pub struct FileId(u32); + +impl FileId { + const MAX: u32 = 0x7fff_ffff; + + #[inline] + pub const fn from_raw(raw: u32) -> FileId { + assert!(raw <= Self::MAX); + FileId(raw) + } + + #[inline] + pub const fn index(self) -> u32 { + self.0 + } +} + +/// safe because `FileId` is a newtype of `u32` +impl nohash_hasher::IsEnabled for FileId {} + +/// Storage for all file changes and the file id to path mapping. +/// +/// For more information see the [crate-level](crate) documentation. +#[derive(Default)] +pub struct Vfs { + interner: PathInterner, + data: Vec, + changes: IndexMap>, +} + +#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] +pub enum FileState { + /// The file exists with the given content hash. + Exists(u64), + /// The file is deleted. + Deleted, + /// The file was specifically excluded by the user. We still include + /// excluded files when they're opened (without their contents). + Excluded, +} + +/// Changed file in the [`Vfs`]. +#[derive(Debug)] +pub struct ChangedFile { + /// Id of the changed file + pub file_id: FileId, + /// Kind of change + pub change: Change, +} + +impl ChangedFile { + /// Returns `true` if the change is not [`Delete`](ChangeKind::Delete). + pub fn exists(&self) -> bool { + !matches!(self.change, Change::Delete) + } + + /// Returns `true` if the change is [`Create`](ChangeKind::Create) or + /// [`Delete`](Change::Delete). + pub fn is_created_or_deleted(&self) -> bool { + matches!(self.change, Change::Create(_, _) | Change::Delete) + } + + /// Returns `true` if the change is [`Create`](ChangeKind::Create). + pub fn is_created(&self) -> bool { + matches!(self.change, Change::Create(_, _)) + } + + /// Returns `true` if the change is [`Modify`](ChangeKind::Modify). + pub fn is_modified(&self) -> bool { + matches!(self.change, Change::Modify(_, _)) + } + + pub fn kind(&self) -> ChangeKind { + match self.change { + Change::Create(_, _) => ChangeKind::Create, + Change::Modify(_, _) => ChangeKind::Modify, + Change::Delete => ChangeKind::Delete, + } + } + + /// UTF-8 text; invalid UTF-8 becomes empty. + pub fn text(&self) -> Option> { + let bytes = match &self.change { + Change::Create(bytes, _) | Change::Modify(bytes, _) => bytes.as_slice(), + Change::Delete => return None, + }; + let text = std::str::from_utf8(bytes).unwrap_or(""); + Some(triomphe::Arc::::from(text)) + } + + pub fn contents(&self) -> Option<&[u8]> { + match &self.change { + Change::Create(bytes, _) | Change::Modify(bytes, _) => Some(bytes.as_slice()), + Change::Delete => None, + } + } + + pub fn create(file_id: FileId, text: impl AsRef) -> Self { + let bytes = text.as_ref().as_bytes().to_vec(); + let hash = hash_once::(&*bytes); + Self { file_id, change: Change::Create(bytes, hash) } + } + + pub fn modify(file_id: FileId, text: impl AsRef) -> Self { + let bytes = text.as_ref().as_bytes().to_vec(); + let hash = hash_once::(&*bytes); + Self { file_id, change: Change::Modify(bytes, hash) } + } + + pub fn delete(file_id: FileId) -> Self { + Self { file_id, change: Change::Delete } + } +} + +impl Change { + pub fn from_bytes(bytes: Option>) -> Self { + match bytes { + None => Change::Delete, + Some(v) => { + let hash = hash_once::(&*v); + Change::Create(v, hash) + } + } + } +} + +/// Kind of [file change](ChangedFile). +#[derive(Eq, PartialEq, Debug)] +pub enum Change { + /// The file was (re-)created + Create(Vec, u64), + /// The file was modified + Modify(Vec, u64), + /// The file was deleted + Delete, +} + +/// Kind of [file change](ChangedFile). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum ChangeKind { + /// The file was (re-)created + Create, + /// The file was modified + Modify, + /// The file was deleted + Delete, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FileExcluded { + Yes, + No, +} + +impl Vfs { + /// Id of the given path if it exists in the `Vfs` and is not deleted. + pub fn file_id(&self, path: &VfsPath) -> Option<(FileId, FileExcluded)> { + let file_id = self.interner.get(path)?; + let file_state = self.get(file_id); + match file_state { + FileState::Exists(_) => Some((file_id, FileExcluded::No)), + FileState::Deleted => None, + FileState::Excluded => Some((file_id, FileExcluded::Yes)), + } + } + + /// File path corresponding to the given `file_id`. + /// + /// # Panics + /// + /// Panics if the id is not present in the `Vfs`. + pub fn file_path(&self, file_id: FileId) -> &VfsPath { + self.interner.lookup(file_id) + } + + /// Returns an iterator over the stored ids and their corresponding paths. + /// + /// This will skip deleted files. + pub fn iter(&self) -> impl Iterator + '_ { + (0..self.data.len()) + .map(|it| FileId(it as u32)) + .filter(move |&file_id| matches!(self.get(file_id), FileState::Exists(_))) + .map(move |file_id| { + let path = self.interner.lookup(file_id); + (file_id, path) + }) + } + + /// Update the `path` with the given `contents`. `None` means the file was + /// deleted. + /// + /// Returns `true` if the file was modified, and saves the + /// [change](ChangedFile). + /// + /// If the path does not currently exists in the `Vfs`, allocates a new + /// [`FileId`] for it. + pub fn set_file_contents(&mut self, path: VfsPath, contents: Option>) -> bool { + let _p = span!(Level::INFO, "Vfs::set_file_contents").entered(); + let file_id = self.alloc_file_id(path); + let state: FileState = self.get(file_id); + let change = match (state, contents) { + (FileState::Deleted, None) => return false, + (FileState::Deleted, Some(v)) => { + let hash = hash_once::(&*v); + Change::Create(v, hash) + } + (FileState::Exists(_), None) => Change::Delete, + (FileState::Exists(hash), Some(v)) => { + let new_hash = hash_once::(&*v); + if new_hash == hash { + return false; + } + Change::Modify(v, new_hash) + } + (FileState::Excluded, _) => return false, + }; + + let mut set_data = |change_kind| { + self.data[file_id.0 as usize] = match change_kind { + &Change::Create(_, hash) | &Change::Modify(_, hash) => FileState::Exists(hash), + Change::Delete => FileState::Deleted, + }; + }; + + let changed_file = ChangedFile { file_id, change }; + match self.changes.entry(file_id) { + // two changes to the same file in one cycle, merge them appropriately + Entry::Occupied(mut o) => { + use Change::*; + + match (&mut o.get_mut().change, changed_file.change) { + // newer `Delete` wins + (change, Delete) => *change = Delete, + // merge `Create` with `Create` or `Modify` + (Create(prev, old_hash), Create(new, new_hash) | Modify(new, new_hash)) => { + *prev = new; + *old_hash = new_hash; + } + // collapse identical `Modify`es + (Modify(prev, old_hash), Modify(new, new_hash)) => { + *prev = new; + *old_hash = new_hash; + } + // equivalent to `Modify` + (change @ Delete, Create(new, new_hash)) => { + *change = Modify(new, new_hash); + } + // shouldn't occur, but collapse into `Create` + (change @ Delete, Modify(new, new_hash)) => { + *change = Create(new, new_hash); + } + // shouldn't occur, but keep the Create + (prev @ Modify(_, _), new @ Create(_, _)) => *prev = new, + } + set_data(&o.get().change); + } + Entry::Vacant(v) => set_data(&v.insert(changed_file).change), + }; + + true + } + + /// Drain and returns all the changes in the `Vfs`. + pub fn take_changes(&mut self) -> IndexMap> { + mem::take(&mut self.changes) + } + + /// Provides a panic-less way to verify file_id validity. + pub fn exists(&self, file_id: FileId) -> bool { + matches!(self.get(file_id), FileState::Exists(_)) + } + + /// Returns the id associated with `path` + /// + /// - If `path` does not exists in the `Vfs`, allocate a new id for it, + /// associated with a deleted file; + /// - Else, returns `path`'s id. + /// + /// Does not record a change. + fn alloc_file_id(&mut self, path: VfsPath) -> FileId { + let file_id = self.interner.intern(path); + let idx = file_id.0 as usize; + let len = self.data.len().max(idx + 1); + self.data.resize(len, FileState::Deleted); + file_id + } + + /// Returns the status of the file associated with the given `file_id`. + /// + /// # Panics + /// + /// Panics if no file is associated to that id. + fn get(&self, file_id: FileId) -> FileState { + self.data[file_id.0 as usize] + } + + /// We cannot ignore excluded files, because this will lead to errors when + /// the client requests semantic information for them, so we instead + /// mark them specially. + pub fn insert_excluded_file(&mut self, path: VfsPath) { + let file_id = self.alloc_file_id(path); + self.data[file_id.0 as usize] = FileState::Excluded; + } +} + +impl fmt::Debug for Vfs { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Vfs").field("n_files", &self.data.len()).finish() + } +} diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs index 00628d8c..e93519cf 100644 --- a/crates/vfs/src/loader.rs +++ b/crates/vfs/src/loader.rs @@ -1,75 +1,141 @@ -// Object safe interface for file watching and reading. +//! File loading and watching interface used by the VFS. use std::fmt; -use utils::{ - lines::LineEnding, - paths::{AbsPath, AbsPathBuf}, -}; +use utils::paths::{AbsPath, AbsPathBuf}; -use crate::{PathGlobMatcher, PathMatcher}; - -/// File extensions loaded from recursive directory entries. -/// -/// Exact file entries are represented by [`Entry::Files`] and do not rely on -/// extension expansion. +/// Extensions for recursive directory loads (SystemVerilog sources). pub const SOURCE_FILE_EXTENSIONS: &[&str] = &["v", "sv", "vh", "svh", "svi", "map"]; -/// A loader input boundary. -/// -/// Exact files are loaded as listed. Directory entries are expanded using -/// extension and matcher rules and are also eligible for recursive watching. +/// A set of files on the file system. #[derive(Debug, Clone)] pub enum Entry { + /// The `Entry` is represented by a raw set of files. Files(Vec), + /// The `Entry` is represented by `Directories`. Directories(Directories), } -/// Recursive directory load policy. +/// Specifies a set of files on the file system. +/// +/// A file is included if: +/// * it has included extension +/// * it is under an `include` path +/// * it is not under `exclude` path +/// +/// If many include/exclude paths match, the longest one wins. +/// +/// If a path is in both `include` and `exclude`, the `exclude` one wins. #[derive(Debug, Clone, Default)] pub struct Directories { pub extensions: Vec, - pub include: Vec, + pub include: Vec, pub exclude: Vec, - pub exclude_globs: Option, } -/// Complete loader configuration for one generation. +/// [`Handle`]'s configuration. #[derive(Debug)] pub struct Config { + /// Version number to associate progress updates to the right config + /// version. pub version: u32, - pub to_load: Vec, - pub to_watch: Vec, + /// Set of initially loaded files. + pub load: Vec, + /// Index of watched entries in `load`. + /// + /// If a path in a watched entry is modified,the [`Handle`] should notify + /// it. + pub watch: Vec, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum LoadingProgress { + Started, + Progress(usize), + Finished, } -/// Messages sent by a loader generation back to the main loop. +/// Message about an action taken by a [`Handle`]. pub enum Message { - Progress { n_total: usize, n_done: usize, config_version: u32 }, - Loaded { files: Vec<(AbsPathBuf, LoadResult)>, config_version: u32 }, + /// Indicate a gradual progress. + /// + /// This is supposed to be the number of loaded files. + Progress { + /// The total files to be loaded. + n_total: usize, + /// The files that have been loaded successfully. + n_done: LoadingProgress, + /// The dir being loaded, `None` if its for a file. + dir: Option, + /// The [`Config`] version. + config_version: u32, + }, + /// The handle loaded the following files' content for the first time. + Loaded { files: Vec<(AbsPathBuf, Option>)> }, + /// The handle loaded the following files' content. + Changed { files: Vec<(AbsPathBuf, Option>)> }, } +/// Type that will receive [`Messages`](Message) from a [`Handle`]. pub type Sender = crossbeam_channel::Sender; -/// Result of reading one file from the loader. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub enum LoadResult { - Loaded(String, LineEnding), - LoadError, - DecodeError, -} - +/// Interface for reading and watching files. pub trait Handle: fmt::Debug { + /// Spawn a new handle with the given `sender`. fn spawn(sender: Sender) -> Self where Self: Sized; + /// Set this handle's configuration. fn set_config(&mut self, config: Config); + /// The file's content at `path` has been modified, and should be reloaded. fn invalidate(&mut self, path: AbsPathBuf); - fn load_sync(&mut self, path: &AbsPath) -> LoadResult; + /// Load the content of the given file, returning [`None`] if it does not + /// exists. + fn load_sync(&mut self, path: &AbsPath) -> Option>; } impl Entry { + /// Returns: + /// ```text + /// Entry::Directories(Directories { + /// extensions: ["rs"], + /// include: [base], + /// exclude: [base/.git], + /// }) + /// ``` + pub fn rs_files_recursively(base: AbsPathBuf) -> Entry { + Entry::Directories(dirs(base, &[".git"])) + } + + /// Returns: + /// ```text + /// Entry::Directories(Directories { + /// extensions: ["rs"], + /// include: [base], + /// exclude: [base/.git, base/target], + /// }) + /// ``` + pub fn local_cargo_package(base: AbsPathBuf) -> Entry { + Entry::Directories(dirs(base, &[".git", "target"])) + } + + /// Returns: + /// ```text + /// Entry::Directories(Directories { + /// extensions: ["rs"], + /// include: [base], + /// exclude: [base/.git, /tests, /examples, /benches], + /// }) + /// ``` + pub fn cargo_package_dependency(base: AbsPathBuf) -> Entry { + Entry::Directories(dirs(base, &[".git", "/tests", "/examples", "/benches"])) + } + + /// Returns `true` if `path` is included in `self`. + /// + /// See [`Directories::contains_file`]. pub fn contains_file(&self, path: &AbsPath) -> bool { match self { Entry::Files(files) => files.iter().any(|it| it == path), @@ -77,6 +143,10 @@ impl Entry { } } + /// Returns `true` if `path` is included in `self`. + /// + /// - If `self` is `Entry::Files`, returns `false` + /// - Else, see [`Directories::contains_dir`]. pub fn contains_dir(&self, path: &AbsPath) -> bool { match self { Entry::Files(_) => false, @@ -86,57 +156,86 @@ impl Entry { } impl Directories { + /// Returns `true` if `path` is included in `self`. pub fn contains_file(&self, path: &AbsPath) -> bool { + // First, check the file extension... let ext = path.extension().unwrap_or_default(); if self.extensions.iter().all(|it| it.as_str() != ext) { return false; } - self.includes_file(path) + // Then, check for path inclusion... + self.includes_path(path) } + /// Returns `true` if `path` is included in `self`. + /// + /// Since `path` is supposed to be a directory, this will not take extension + /// into account. pub fn contains_dir(&self, path: &AbsPath) -> bool { - self.includes_dir(path) - } - - pub fn include_roots(&self) -> impl Iterator { - self.include.iter().flat_map(PathMatcher::scan_roots) + self.includes_path(path) } /// Returns `true` if `path` is included in `self`. /// /// It is included if - /// - An include root is a prefix of `path`. - /// - No literal exclude prefix matches `path`. - fn includes_file(&self, path: &AbsPath) -> bool { - self.include.iter().any(|include| include.contains_file(path)) && !self.is_excluded(path) - } + /// - An element in `self.include` is a prefix of `path`. + /// - This path is longer than any element in `self.exclude` that is a + /// prefix of `path`. In case of equality, exclusion wins. + fn includes_path(&self, path: &AbsPath) -> bool { + let mut include: Option<&AbsPathBuf> = None; + for incl in &self.include { + if path.starts_with(incl) { + include = Some(match include { + Some(prev) if prev.starts_with(incl) => prev, + _ => incl, + }); + } + } - fn includes_dir(&self, path: &AbsPath) -> bool { - self.include.iter().any(|include| include.contains_dir(path)) - && !self.exclude.iter().any(|excl| path.starts_with(excl)) - } + let include = match include { + Some(it) => it, + None => return false, + }; - fn is_excluded(&self, path: &AbsPath) -> bool { - self.exclude.iter().any(|excl| path.starts_with(excl)) - || self.exclude_globs.as_ref().is_some_and(|exclude| exclude.is_match(path)) + !self.exclude.iter().any(|excl| path.starts_with(excl) && excl.starts_with(include)) } } +/// Returns : +/// ```text +/// Directories { +/// extensions: ["rs"], +/// include: [base], +/// exclude: [base/], +/// } +/// ``` +fn dirs(base: AbsPathBuf, exclude: &[&str]) -> Directories { + let exclude = exclude.iter().map(|it| base.join(it)).collect::>(); + Directories { extensions: vec!["rs".to_owned()], include: vec![base], exclude } +} + impl fmt::Debug for Message { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Message::Loaded { files, config_version } => f - .debug_struct("Loaded") - .field("n_files", &files.len()) - .field("config_version", config_version) - .finish(), - Message::Progress { n_total, n_done, config_version } => f + Message::Loaded { files } => { + f.debug_struct("Loaded").field("n_files", &files.len()).finish() + } + Message::Changed { files } => { + f.debug_struct("Changed").field("n_files", &files.len()).finish() + } + Message::Progress { n_total, n_done, dir, config_version } => f .debug_struct("Progress") .field("n_total", n_total) .field("n_done", n_done) + .field("dir", dir) .field("config_version", config_version) .finish(), } } } + +#[test] +fn handle_is_dyn_compatible() { + fn _assert(_: &dyn Handle) {} +} diff --git a/crates/vfs/src/notify.rs b/crates/vfs/src/notify.rs index 99fc38b4..300075a5 100644 --- a/crates/vfs/src/notify.rs +++ b/crates/vfs/src/notify.rs @@ -1,181 +1,71 @@ -use std::{fs, mem, sync::atomic::AtomicUsize}; +//! `loader::Handle` backed by `walkdir` and OS `notify` (best-effort). + +use std::{ + fs, + path::{Component, Path}, + sync::atomic::AtomicUsize, +}; -use ::notify::{Config, RecommendedWatcher, RecursiveMode, Watcher}; use crossbeam_channel::{Receiver, Sender, select, unbounded}; -use itertools::Itertools; -use rayon::iter::{IndexedParallelIterator, IntoParallelIterator, ParallelIterator}; +use notify::{Config, EventKind, RecommendedWatcher, RecursiveMode, Watcher, event::AccessKind}; +use rayon::iter::{IndexedParallelIterator as _, IntoParallelIterator as _, ParallelIterator}; use rustc_hash::FxHashSet; -use utils::{ - lines::LineEnding, - paths::{AbsPath, AbsPathBuf}, - thread, -}; +use utils::paths::{AbsPath, AbsPathBuf, Utf8PathBuf}; use walkdir::WalkDir; -use crate::loader::{self, LoadResult}; +use crate::loader::{self, LoadingProgress}; #[derive(Debug)] pub struct NotifyHandle { // Relative order of fields below is significant. - sender: Sender, - _handler: Option, - browser_loader: Option, + sender: Sender, + _thread: utils::thread::JoinHandle, } #[derive(Debug)] -enum ServerMsg { +enum Message { Config(loader::Config), Invalidate(AbsPathBuf), } impl loader::Handle for NotifyHandle { fn spawn(sender: loader::Sender) -> NotifyHandle { - if cfg!(target_os = "emscripten") { - let (server_sender, _) = unbounded::(); - return NotifyHandle { - sender: server_sender, - _handler: None, - browser_loader: Some(BrowserLoader::new(sender)), - }; - } - let actor = NotifyActor::new(sender); - let (sender, receiver) = unbounded::(); - let thread = match thread::Builder::new(thread::ThreadIntent::Worker) + let (sender, receiver) = unbounded::(); + let thread = utils::thread::Builder::new(utils::thread::ThreadIntent::Worker) .name("VfsLoader".to_owned()) .spawn(move || actor.run(receiver)) - { - Ok(thread) => Some(thread), - Err(err) => { - tracing::error!(%err, "failed to spawn VFS loader thread"); - None - } - }; - NotifyHandle { sender, _handler: thread, browser_loader: None } + .expect("failed to spawn thread"); + NotifyHandle { sender, _thread: thread } } fn set_config(&mut self, config: loader::Config) { - if let Some(loader) = &mut self.browser_loader { - loader.set_config(config); - return; - } - - if self.sender.send(ServerMsg::Config(config)).is_err() { - tracing::error!("failed to send VFS config to loader thread"); - } + self.sender.send(Message::Config(config)).unwrap(); } fn invalidate(&mut self, path: AbsPathBuf) { - if let Some(loader) = &mut self.browser_loader { - loader.invalidate(path); - return; - } - - if self.sender.send(ServerMsg::Invalidate(path)).is_err() { - tracing::error!("failed to send VFS invalidation to loader thread"); - } + self.sender.send(Message::Invalidate(path)).unwrap(); } - fn load_sync(&mut self, path: &AbsPath) -> LoadResult { + fn load_sync(&mut self, path: &AbsPath) -> Option> { read(path) } } -#[derive(Debug)] -struct BrowserLoader { - sender: loader::Sender, - config_version: u32, - loaded_paths: FxHashSet, -} - -impl BrowserLoader { - fn new(sender: loader::Sender) -> Self { - Self { sender, config_version: 0, loaded_paths: FxHashSet::default() } - } - - fn set_config(&mut self, config: loader::Config) { - let config_version = config.version; - self.config_version = config_version; - let has_reconcile_step = !self.loaded_paths.is_empty(); - let n_entries = config.to_load.len(); - let n_total = n_entries + usize::from(has_reconcile_step); - if n_total > 0 { - self.send(loader::Message::Progress { n_total, n_done: 0, config_version }); - } - - let previous_loaded_paths = mem::take(&mut self.loaded_paths); - let mut reported_paths = FxHashSet::default(); - let mut loaded_paths = FxHashSet::default(); - - for (index, entry) in config.to_load.into_iter().enumerate() { - let (watch_tx, _) = unbounded(); - let files = NotifyActor::load_entry(&watch_tx, entry, false); - reported_paths.extend(files.iter().map(|(path, _)| path.clone())); - loaded_paths.extend( - files - .iter() - .filter(|(_, result)| !matches!(result, LoadResult::LoadError)) - .map(|(path, _)| path.clone()), - ); - self.send(loader::Message::Loaded { files, config_version }); - self.send(loader::Message::Progress { n_total, n_done: index + 1, config_version }); - } - - let unloaded = previous_loaded_paths - .difference(&reported_paths) - .cloned() - .map(|path| (path, LoadResult::LoadError)) - .collect_vec(); - self.loaded_paths = loaded_paths; - if !unloaded.is_empty() { - self.send(loader::Message::Loaded { files: unloaded, config_version }); - } - if has_reconcile_step { - self.send(loader::Message::Progress { n_total, n_done: n_total, config_version }); - } else if n_total == 0 { - self.send(loader::Message::Progress { n_total, n_done: 0, config_version }); - } - } - - fn invalidate(&mut self, path: AbsPathBuf) { - let contents = read(path.as_path()); - let files = vec![(path, contents)]; - self.record_loaded_files(&files); - self.send(loader::Message::Loaded { files, config_version: self.config_version }); - } - - fn record_loaded_files(&mut self, files: &[(AbsPathBuf, LoadResult)]) { - for (path, result) in files { - if matches!(result, LoadResult::LoadError) { - self.loaded_paths.remove(path); - } else { - self.loaded_paths.insert(path.clone()); - } - } - } - - fn send(&self, msg: loader::Message) { - if self.sender.send(msg).is_err() { - tracing::error!("failed to send browser VFS loader message to main loop"); - } - } -} - -type NotifyEvent = ::notify::Result<::notify::Event>; +type NotifyEvent = notify::Result; struct NotifyActor { sender: loader::Sender, - config_version: u32, - watched_files: FxHashSet, - watched_dirs: Vec, - loaded_paths: FxHashSet, + watched_file_entries: FxHashSet, + watched_dir_entries: Vec, + seen_paths: FxHashSet, // Drop order is significant. watcher: Option<(RecommendedWatcher, Receiver)>, } #[derive(Debug)] enum Event { - ServerMsg(ServerMsg), + Message(Message), NotifyEvent(NotifyEvent), } @@ -183,41 +73,39 @@ impl NotifyActor { fn new(sender: loader::Sender) -> NotifyActor { NotifyActor { sender, - config_version: 0, - watched_files: FxHashSet::default(), - watched_dirs: Vec::new(), - loaded_paths: FxHashSet::default(), + watched_dir_entries: Vec::new(), + watched_file_entries: FxHashSet::default(), + seen_paths: FxHashSet::default(), watcher: None, } } - fn next_event(&self, receiver: &Receiver) -> Option { + fn next_event(&self, receiver: &Receiver) -> Option { let Some((_, watcher_receiver)) = &self.watcher else { - return receiver.recv().ok().map(Event::ServerMsg); + return receiver.recv().ok().map(Event::Message); }; select! { - recv(receiver) -> it => it.ok().map(Event::ServerMsg), - recv(watcher_receiver) -> it => it.ok().map(Event::NotifyEvent), + recv(receiver) -> it => it.ok().map(Event::Message), + recv(watcher_receiver) -> it => Some(Event::NotifyEvent(it.unwrap())), } } - fn run(mut self, server_inbox: Receiver) { - while let Some(event) = self.next_event(&server_inbox) { - tracing::debug!(?event, "vfs-loader event"); + fn run(mut self, inbox: Receiver) { + while let Some(event) = self.next_event(&inbox) { + tracing::debug!(?event, "vfs-notify event"); match event { - Event::ServerMsg(msg) => match msg { - ServerMsg::Config(config) => { + Event::Message(msg) => match msg { + Message::Config(config) => { self.watcher = None; - if !config.to_watch.is_empty() { + if !config.watch.is_empty() { let (watcher_sender, watcher_receiver) = unbounded(); let watcher = log_notify_error(RecommendedWatcher::new( move |event| { - if watcher_sender.send(event).is_err() { - tracing::debug!( - "notify event dropped because watcher receiver is closed" - ); - } + // we don't care about the error. If sending fails that usually + // means we were dropped, so unwrapping will just add to the + // panic noise. + _ = watcher_sender.send(event); }, Config::default(), )); @@ -225,62 +113,54 @@ impl NotifyActor { } let config_version = config.version; - self.config_version = config_version; - let has_reconcile_step = !self.loaded_paths.is_empty(); - let n_entries = config.to_load.len(); - let n_total = n_entries + usize::from(has_reconcile_step); - if n_total > 0 { - self.send(loader::Message::Progress { - n_total, - n_done: 0, - config_version, - }); - } - self.watched_files.clear(); - self.watched_dirs.clear(); - let previous_loaded_paths = mem::take(&mut self.loaded_paths); + let n_total = config.load.len(); + self.watched_dir_entries.clear(); + self.watched_file_entries.clear(); + self.seen_paths.clear(); + + self.send(loader::Message::Progress { + n_total, + n_done: LoadingProgress::Started, + config_version, + dir: None, + }); let (entry_tx, entry_rx) = unbounded(); let (watch_tx, watch_rx) = unbounded(); - let (loaded_tx, loaded_rx) = unbounded(); let processed = AtomicUsize::new(0); - config.to_load.into_par_iter().enumerate().for_each(|(i, entry)| { - let do_watch = config.to_watch.contains(&i); - if do_watch && entry_tx.send(entry.clone()).is_err() { - tracing::debug!("watched entry dropped because receiver is closed"); + config.load.into_par_iter().enumerate().for_each(|(i, entry)| { + let do_watch = config.watch.contains(&i); + if do_watch { + _ = entry_tx.send(entry.clone()); } - let files = Self::load_entry(&watch_tx, entry, do_watch); - let reported_paths = - files.iter().map(|(path, _)| path.clone()).collect_vec(); - let loaded_paths = files - .iter() - .filter(|(_, result)| !matches!(result, LoadResult::LoadError)) - .map(|(path, _)| path.clone()) - .collect_vec(); - if loaded_tx.send((reported_paths, loaded_paths)).is_err() { - tracing::debug!( - "loaded path batch dropped because receiver is closed" - ); - } - self.send(loader::Message::Loaded { files, config_version }); + let files = Self::load_entry( + |f| _ = watch_tx.send(f.to_owned()), + entry, + do_watch, + |file| { + self.send(loader::Message::Progress { + n_total, + n_done: LoadingProgress::Progress( + processed.load(std::sync::atomic::Ordering::Relaxed), + ), + dir: Some(file), + config_version, + }); + }, + ); + self.send(loader::Message::Loaded { files }); self.send(loader::Message::Progress { n_total, - n_done: 1 + processed - .fetch_add(1, std::sync::atomic::Ordering::AcqRel), + n_done: LoadingProgress::Progress( + processed.fetch_add(1, std::sync::atomic::Ordering::AcqRel) + 1, + ), config_version, + dir: None, }); }); - drop(loaded_tx); - let mut reported_paths = FxHashSet::default(); - let mut loaded_paths = FxHashSet::default(); - for (reported, loaded) in loaded_rx { - reported_paths.extend(reported); - loaded_paths.extend(loaded); - } - drop(watch_tx); for path in watch_rx { self.watch(&path); @@ -289,529 +169,222 @@ impl NotifyActor { drop(entry_tx); for entry in entry_rx { match entry { - loader::Entry::Files(files) => self.watched_files.extend(files), - loader::Entry::Directories(dir) => self.watched_dirs.push(dir), + loader::Entry::Files(files) => { + self.watched_file_entries.extend(files) + } + loader::Entry::Directories(dir) => { + self.watched_dir_entries.push(dir) + } } } - let unloaded = previous_loaded_paths - .difference(&reported_paths) - .cloned() - .map(|path| (path, LoadResult::LoadError)) - .collect_vec(); - self.loaded_paths = loaded_paths; - if !unloaded.is_empty() { - self.send(loader::Message::Loaded { files: unloaded, config_version }); - } - if has_reconcile_step { - self.send(loader::Message::Progress { - n_total, - n_done: n_total, - config_version, - }); - } else if n_total == 0 { - self.send(loader::Message::Progress { - n_total, - n_done: 0, - config_version, - }); - } + self.send(loader::Message::Progress { + n_total, + n_done: LoadingProgress::Finished, + config_version, + dir: None, + }); } - ServerMsg::Invalidate(path) => { + Message::Invalidate(path) => { let contents = read(path.as_path()); let files = vec![(path, contents)]; - self.record_loaded_files(&files); - self.send(loader::Message::Loaded { - files, - config_version: self.config_version, - }); + self.send(loader::Message::Changed { files }); } }, Event::NotifyEvent(event) => { - let Some(event) = log_notify_error(event) else { - continue; - }; - - let files = self.process_notify_event(event); - self.record_loaded_files(&files); - self.send(loader::Message::Loaded { - files, - config_version: self.config_version, - }); - } - } - } - } + if let Some(event) = log_notify_error(event) + && let EventKind::Create(_) + | EventKind::Modify(_) + | EventKind::Remove(_) + | EventKind::Access(AccessKind::Open(_)) = event.kind + { + let abs_paths: Vec = event + .paths + .into_iter() + .filter_map(|path| { + Some( + AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?) + .expect("path is absolute"), + ) + }) + .collect(); + + let mut saw_new_file = false; + for abs_path in &abs_paths { + if self.seen_paths.insert(abs_path.clone()) { + saw_new_file = true; + } + } - fn process_notify_event(&mut self, event: ::notify::Event) -> Vec<(AbsPathBuf, LoadResult)> { - if !(event.kind.is_create() || event.kind.is_modify() || event.kind.is_remove()) { - return Vec::new(); - } + // Only consider access events for files that we haven't seen + // before. + // + // This is important on FUSE filesystems, where we may not get a + // Create event. In other cases we're about to access the file, so + // we don't want an infinite loop where processing an Access event + // creates another Access event. + if matches!(event.kind, EventKind::Access(_)) && !saw_new_file { + continue; + } - let mut files = Vec::new(); - for path in event.paths.into_iter().filter_map(|path| AbsPathBuf::try_from(path).ok()) { - if event.kind.is_remove() { - let unloaded = self.unload_removed_path(&path); - if !unloaded.is_empty() { - files.extend(unloaded); - continue; + let files = abs_paths + .into_iter() + .filter_map(|path| -> Option<(AbsPathBuf, Option>)> { + // Ignore events for files/directories that we're not watching. + if !(self.watched_file_entries.contains(&path) + || self + .watched_dir_entries + .iter() + .any(|dir| dir.contains_file(&path))) + { + return None; + } + + // For removed files, fs::metadata() will return Err, but + // we still want to update the VFS. + if matches!(event.kind, EventKind::Remove(_)) { + return Some((path, None)); + } + + let meta = fs::metadata(&path).ok()?; + if meta.file_type().is_dir() + && self + .watched_dir_entries + .iter() + .any(|dir| dir.contains_dir(&path)) + { + self.watch(path.as_ref()); + return None; + } + + if !meta.file_type().is_file() { + return None; + } + + let contents = read(&path); + Some((path, contents)) + }) + .collect(); + self.send(loader::Message::Changed { files }); + } } } - - let metadata = fs::metadata(&path).ok(); - let file_type = metadata.as_ref().map(|meta| meta.file_type()); - let is_file = file_type.as_ref().is_some_and(|it| it.is_file()); - let is_dir = file_type.as_ref().is_some_and(|it| it.is_dir()); - - if is_dir && self.is_watched_dir(&path) { - files.extend(self.load_created_directory(&path)); - continue; - } - - if metadata.is_some() && !is_file { - continue; - } - - if !self.is_watched_file(&path) { - continue; - } - - files.push((path.clone(), read(&path))); - } - - files - } - - fn is_watched_dir(&self, path: &AbsPathBuf) -> bool { - self.watched_dirs.iter().any(|dir| dir.contains_dir(path)) - } - - fn is_watched_file(&self, path: &AbsPathBuf) -> bool { - self.watched_files.contains(path) - || self.watched_dirs.iter().any(|dir| dir.contains_file(path)) - } - - fn load_created_directory(&mut self, path: &AbsPathBuf) -> Vec<(AbsPathBuf, LoadResult)> { - let dirs = - self.watched_dirs.iter().filter(|dir| dir.contains_dir(path)).cloned().collect_vec(); - let mut files = Vec::new(); - - for dir in dirs { - let (watch_tx, watch_rx) = unbounded(); - files.extend(Self::load_directory_subtree(&watch_tx, &dir, path, true)); - drop(watch_tx); - for path in watch_rx { - self.watch(&path); - } } - - files - } - - fn unload_removed_path(&self, path: &AbsPathBuf) -> Vec<(AbsPathBuf, LoadResult)> { - self.loaded_paths - .iter() - .filter(|loaded_path| loaded_path.starts_with(path)) - .cloned() - .map(|path| (path, LoadResult::LoadError)) - .collect_vec() } fn load_entry( - watch_tx: &Sender, + mut watch: impl FnMut(&Path), entry: loader::Entry, - watch: bool, - ) -> Vec<(AbsPathBuf, LoadResult)> { + do_watch: bool, + send_message: impl Fn(AbsPathBuf), + ) -> Vec<(AbsPathBuf, Option>)> { match entry { loader::Entry::Files(files) => files .into_iter() .map(|file| { - if watch && watch_tx.send(file.to_owned()).is_err() { - tracing::debug!("watched file path dropped because receiver is closed"); + if do_watch { + watch(file.as_ref()); } let contents = read(file.as_path()); (file, contents) }) - .collect_vec(), + .collect::>(), loader::Entry::Directories(dirs) => { let mut res = Vec::new(); - for root in dirs.include_roots() { - res.extend(Self::load_directory_subtree(watch_tx, &dirs, root, watch)); - } - res - } - } - } - - fn load_directory_subtree( - watch_tx: &Sender, - dirs: &loader::Directories, - root: &AbsPathBuf, - watch: bool, - ) -> Vec<(AbsPathBuf, LoadResult)> { - let walkdir = WalkDir::new(root).follow_links(true).into_iter().filter_entry(|entry| { - if !entry.file_type().is_dir() { - return true; - } - let Ok(path) = AbsPathBuf::try_from(entry.path().to_path_buf()) else { - return false; - }; - root == &path || dirs.contains_dir(&path) - }); - - let files = walkdir.filter_map(|it| it.ok()).filter_map(|entry| { - let is_dir = entry.file_type().is_dir(); - let is_file = entry.file_type().is_file(); - let abs_path = AbsPathBuf::try_from(entry.into_path()).ok()?; - - if is_dir && watch && watch_tx.send(abs_path.to_owned()).is_err() { - tracing::debug!("watched directory path dropped because receiver is closed"); - } - - if !is_file { - return None; - } + for root in &dirs.include { + send_message(root.clone()); + let walkdir = + WalkDir::new(root).follow_links(true).into_iter().filter_entry(|entry| { + if !entry.file_type().is_dir() { + return true; + } + let path = entry.path(); - if !dirs.contains_file(&abs_path) { - return None; - } + if path_might_be_cyclic(path) { + return false; + } - Some(abs_path) - }); + // We want to filter out subdirectories that are roots themselves, + // because they will be visited separately. + let path: &Path = path; + dirs.exclude + .iter() + .all(|it| >::as_ref(it) != path) + && (>::as_ref(root) == path + || dirs + .include + .iter() + .all(|it| >::as_ref(it) != path)) + }); - files - .map(|file| { - let contents = read(file.as_path()); - (file, contents) - }) - .collect_vec() - } + let files = walkdir.filter_map(|it| it.ok()).filter_map(|entry| { + let depth = entry.depth(); + let is_dir = entry.file_type().is_dir(); + let is_file = entry.file_type().is_file(); + let abs_path = AbsPathBuf::try_from( + Utf8PathBuf::from_path_buf(entry.into_path()).ok()?, + ) + .ok()?; + if depth < 2 && is_dir { + send_message(abs_path.clone()); + } + if is_dir && do_watch { + watch(abs_path.as_ref()); + } + if !is_file { + return None; + } + if !dirs.contains_file(abs_path.as_path()) { + return None; + } + Some(abs_path) + }); - fn watch(&mut self, path: &AbsPathBuf) { - if let Some((watcher, _)) = &mut self.watcher { - log_notify_error(watcher.watch(path.as_ref(), RecursiveMode::NonRecursive)); + res.extend(files.map(|file| { + let contents = read(file.as_path()); + (file, contents) + })); + } + res + } } } - fn record_loaded_files(&mut self, files: &[(AbsPathBuf, LoadResult)]) { - for (path, result) in files { - if matches!(result, LoadResult::LoadError) { - self.loaded_paths.remove(path); - } else { - self.loaded_paths.insert(path.clone()); - } + fn watch(&mut self, path: &Path) { + if let Some((watcher, _)) = &mut self.watcher { + log_notify_error(watcher.watch(path, RecursiveMode::Recursive)); } } + #[track_caller] fn send(&self, msg: loader::Message) { - // Call self.sender with msg - if self.sender.send(msg).is_err() { - tracing::error!("failed to send VFS loader message to main loop"); - } + self.sender.send(msg).unwrap(); } } -fn read(path: &AbsPath) -> LoadResult { - let Ok(bytes) = std::fs::read(path) else { - return LoadResult::LoadError; - }; - let Ok(text) = String::from_utf8(bytes) else { - return LoadResult::DecodeError; - }; - let (text, ending) = LineEnding::normalize(text); - LoadResult::Loaded(text, ending) +fn read(path: &AbsPath) -> Option> { + std::fs::read(path).ok() } -fn log_notify_error(res: ::notify::Result) -> Option { +fn log_notify_error(res: notify::Result) -> Option { res.map_err(|err| tracing::warn!("notify error: {}", err)).ok() } -#[cfg(test)] -mod tests { - use std::time::Duration; - - use ::notify::{ - Event as NotifyEvent, EventKind, - event::{CreateKind, RemoveKind}, +/// Is `path` a symlink to a parent directory? +/// +/// Including this path is guaranteed to cause an infinite loop. This +/// heuristic is not sufficient to catch all symlink cycles (it's +/// possible to construct cycle using two or more symlinks), but it +/// catches common cases. +fn path_might_be_cyclic(path: &Path) -> bool { + let Ok(destination) = std::fs::read_link(path) else { + return false; }; - use utils::paths::AbsPathBuf; - use super::*; - use crate::{ - PathMatcher, - loader::{self, Handle as _}, - }; + // If the symlink is of the form "../..", it's a parent symlink. + let is_relative_parent = + destination.components().all(|c| matches!(c, Component::CurDir | Component::ParentDir)); - struct TestDir { - _dir: tempfile::TempDir, - path: AbsPathBuf, - } - - impl TestDir { - fn new(name: &str) -> Self { - let dir = tempfile::Builder::new().prefix(&format!("vide-{name}-")).tempdir().unwrap(); - let path = AbsPathBuf::assert_utf8(dir.path().to_path_buf()); - Self { _dir: dir, path } - } - - fn join(&self, path: &str) -> AbsPathBuf { - self.path.join(path) - } - } - - fn collect_until_progress_done( - receiver: &Receiver, - version: u32, - ) -> Vec> { - let mut loaded_batches = Vec::new(); - loop { - match receiver.recv_timeout(Duration::from_secs(1)).unwrap() { - loader::Message::Loaded { files, config_version } if config_version == version => { - loaded_batches.push(files); - } - loader::Message::Progress { n_total, n_done, config_version } - if config_version == version && n_done == n_total => - { - return loaded_batches; - } - _ => {} - } - } - } - - fn recv_version_message(receiver: &Receiver, version: u32) -> loader::Message { - loop { - let message = receiver.recv_timeout(Duration::from_secs(1)).unwrap(); - match &message { - loader::Message::Loaded { config_version, .. } - | loader::Message::Progress { config_version, .. } - if *config_version == version => - { - return message; - } - _ => {} - } - } - } - - fn spawn_loader() -> (NotifyHandle, Receiver) { - let (sender, receiver) = unbounded(); - (::spawn(sender), receiver) - } - - fn assert_loaded(batches: &[Vec<(AbsPathBuf, LoadResult)>], expected_path: &AbsPathBuf) { - assert!( - batches.iter().flatten().any(|(path, result)| { - path == expected_path && matches!(result, LoadResult::Loaded(_, _)) - }), - "expected loaded path {expected_path}, got {batches:?}" - ); - } - - fn path_buf(path: &AbsPathBuf) -> std::path::PathBuf { - let path: &std::path::Path = path.as_ref(); - path.to_path_buf() - } - - fn watched_sv_dir(root: AbsPathBuf) -> loader::Directories { - loader::Directories { - extensions: vec!["sv".to_owned()], - include: vec![PathMatcher::all_under_roots(vec![root])], - exclude: Vec::new(), - exclude_globs: None, - } - } - - fn actor() -> NotifyActor { - let (sender, _receiver) = unbounded(); - NotifyActor::new(sender) - } - - #[test] - fn empty_config_emits_ready_ack_progress() { - let (mut handle, receiver) = spawn_loader(); - - handle.set_config(loader::Config { version: 1, to_load: Vec::new(), to_watch: Vec::new() }); - - assert!(matches!( - recv_version_message(&receiver, 1), - loader::Message::Progress { n_done: 0, n_total: 0, .. } - )); - } - - #[test] - fn removed_config_file_is_unloaded() { - let dir = TestDir::new("vfs-loader-unload-file"); - let file = dir.join("top.sv"); - std::fs::write(&file, "module top; endmodule\n").unwrap(); - let (mut handle, receiver) = spawn_loader(); - - handle.set_config(loader::Config { - version: 1, - to_load: vec![loader::Entry::Files(vec![file.clone()])], - to_watch: Vec::new(), - }); - let loaded = collect_until_progress_done(&receiver, 1); - assert_loaded(&loaded, &file); - - handle.set_config(loader::Config { version: 2, to_load: Vec::new(), to_watch: Vec::new() }); - - assert!(matches!( - recv_version_message(&receiver, 2), - loader::Message::Progress { n_done: 0, n_total: 1, .. } - )); - let loader::Message::Loaded { files: unloaded, .. } = recv_version_message(&receiver, 2) - else { - panic!("expected unload batch before final progress"); - }; - assert_eq!(unloaded, vec![(file, LoadResult::LoadError)]); - assert!(matches!( - recv_version_message(&receiver, 2), - loader::Message::Progress { n_done: 1, n_total: 1, .. } - )); - } - - #[test] - fn configured_missing_file_is_not_reconciled_twice() { - let dir = TestDir::new("vfs-loader-missing-config-file"); - let file = dir.join("top.sv"); - std::fs::write(&file, "module top; endmodule\n").unwrap(); - let (mut handle, receiver) = spawn_loader(); - - handle.set_config(loader::Config { - version: 1, - to_load: vec![loader::Entry::Files(vec![file.clone()])], - to_watch: Vec::new(), - }); - let loaded = collect_until_progress_done(&receiver, 1); - assert_loaded(&loaded, &file); - - std::fs::remove_file(&file).unwrap(); - handle.set_config(loader::Config { - version: 2, - to_load: vec![loader::Entry::Files(vec![file.clone()])], - to_watch: Vec::new(), - }); - - let mut unload_count = 0; - loop { - match recv_version_message(&receiver, 2) { - loader::Message::Loaded { files, .. } => { - unload_count += files - .iter() - .filter(|(path, result)| { - path == &file && matches!(result, LoadResult::LoadError) - }) - .count(); - } - loader::Message::Progress { n_done, n_total, .. } if n_done == n_total => break, - _ => {} - } - } - - assert_eq!(unload_count, 1); - } - - #[test] - fn removed_config_directory_is_unloaded() { - let dir = TestDir::new("vfs-loader-unload-directory"); - let source_dir = dir.join("rtl"); - std::fs::create_dir_all(&source_dir).unwrap(); - let file = source_dir.join("top.sv"); - std::fs::write(&file, "module top; endmodule\n").unwrap(); - let (mut handle, receiver) = spawn_loader(); - - handle.set_config(loader::Config { - version: 1, - to_load: vec![loader::Entry::Directories(loader::Directories { - extensions: vec!["sv".to_owned()], - include: vec![PathMatcher::all_under_roots(vec![source_dir])], - exclude: Vec::new(), - exclude_globs: None, - })], - to_watch: Vec::new(), - }); - let loaded = collect_until_progress_done(&receiver, 1); - assert_loaded(&loaded, &file); - - handle.set_config(loader::Config { version: 2, to_load: Vec::new(), to_watch: Vec::new() }); - - assert!(matches!( - recv_version_message(&receiver, 2), - loader::Message::Progress { n_done: 0, n_total: 1, .. } - )); - let loader::Message::Loaded { files: unloaded, .. } = recv_version_message(&receiver, 2) - else { - panic!("expected unload batch before final progress"); - }; - assert_eq!(unloaded, vec![(file, LoadResult::LoadError)]); - assert!(matches!( - recv_version_message(&receiver, 2), - loader::Message::Progress { n_done: 1, n_total: 1, .. } - )); - } - - #[test] - fn created_watched_directory_is_loaded_immediately() { - let dir = TestDir::new("vfs-loader-created-directory-load"); - let root = dir.join("workspace"); - let created_dir = root.join("generated"); - let nested_dir = created_dir.join("nested"); - std::fs::create_dir_all(&nested_dir).unwrap(); - let top = created_dir.join("top.sv"); - let child = nested_dir.join("child.sv"); - let ignored = created_dir.join("notes.txt"); - std::fs::write(&top, "module top; endmodule\n").unwrap(); - std::fs::write(&child, "module child; endmodule\n").unwrap(); - std::fs::write(&ignored, "not systemverilog").unwrap(); - let mut actor = actor(); - actor.watched_dirs.push(watched_sv_dir(root)); - - let files = actor.process_notify_event( - NotifyEvent::new(EventKind::Create(CreateKind::Folder)) - .add_path(path_buf(&created_dir)), - ); - actor.record_loaded_files(&files); - - assert!( - files.iter().any(|(path, result)| { - path == &top && matches!(result, LoadResult::Loaded(_, _)) - }) - ); - assert!(files.iter().any(|(path, result)| { - path == &child && matches!(result, LoadResult::Loaded(_, _)) - })); - assert!(!files.iter().any(|(path, _)| path == &ignored)); - assert!(actor.loaded_paths.contains(&top)); - assert!(actor.loaded_paths.contains(&child)); - } - - #[test] - fn removed_watched_directory_unloads_loaded_descendants() { - let dir = TestDir::new("vfs-loader-removed-directory-unload"); - let root = dir.join("workspace"); - let removed_dir = root.join("removed"); - let top = removed_dir.join("top.sv"); - let child = removed_dir.join("nested/child.sv"); - let sibling = root.join("sibling.sv"); - let mut actor = actor(); - actor.watched_dirs.push(watched_sv_dir(root)); - actor.loaded_paths.extend([top.clone(), child.clone(), sibling.clone()]); - - let mut files = actor.process_notify_event( - NotifyEvent::new(EventKind::Remove(RemoveKind::Folder)) - .add_path(path_buf(&removed_dir)), - ); - files.sort_by(|lhs, rhs| lhs.0.cmp(&rhs.0)); - actor.record_loaded_files(&files); - - let mut expected = - vec![(child.clone(), LoadResult::LoadError), (top.clone(), LoadResult::LoadError)]; - expected.sort_by(|lhs, rhs| lhs.0.cmp(&rhs.0)); - assert_eq!(files, expected); - assert!(!actor.loaded_paths.contains(&top)); - assert!(!actor.loaded_paths.contains(&child)); - assert!(actor.loaded_paths.contains(&sibling)); - } + is_relative_parent || path.starts_with(destination) } diff --git a/crates/vfs/src/path_interner.rs b/crates/vfs/src/path_interner.rs new file mode 100644 index 00000000..225bfc72 --- /dev/null +++ b/crates/vfs/src/path_interner.rs @@ -0,0 +1,43 @@ +//! Maps paths to compact integer ids. We don't care about clearings paths which +//! no longer exist -- the assumption is total size of paths we ever look at is +//! not too big. +use std::hash::BuildHasherDefault; + +use indexmap::IndexSet; +use rustc_hash::FxHasher; + +use crate::{FileId, VfsPath}; + +/// Structure to map between [`VfsPath`] and [`FileId`]. +#[derive(Default)] +pub(crate) struct PathInterner { + map: IndexSet>, +} + +impl PathInterner { + /// Get the id corresponding to `path`. + /// + /// If `path` does not exists in `self`, returns [`None`]. + pub(crate) fn get(&self, path: &VfsPath) -> Option { + self.map.get_index_of(path).map(|i| FileId(i as u32)) + } + + /// Insert `path` in `self`. + /// + /// - If `path` already exists in `self`, returns its associated id; + /// - Else, returns a newly allocated id. + pub(crate) fn intern(&mut self, path: VfsPath) -> FileId { + let (id, _added) = self.map.insert_full(path); + assert!(id < FileId::MAX as usize); + FileId(id as u32) + } + + /// Returns the path corresponding to `id`. + /// + /// # Panics + /// + /// Panics if `id` does not exists in `self`. + pub(crate) fn lookup(&self, id: FileId) -> &VfsPath { + self.map.get_index(id.0 as usize).unwrap() + } +} diff --git a/crates/vfs/src/test_support.rs b/crates/vfs/src/test_support.rs deleted file mode 100644 index c3810688..00000000 --- a/crates/vfs/src/test_support.rs +++ /dev/null @@ -1,53 +0,0 @@ -use std::{ - fs, - time::{SystemTime, UNIX_EPOCH}, -}; - -use utils::paths::{AbsPathBuf, Utf8Path}; - -pub(crate) struct TestDir { - path: AbsPathBuf, -} - -impl TestDir { - pub(crate) fn new(name: &str) -> Self { - let suffix = SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("system time before unix epoch") - .as_nanos(); - let path = - std::env::temp_dir().join(format!("vide-vfs-{name}-{}-{suffix}", std::process::id())); - fs::create_dir_all(&path).unwrap_or_else(|err| { - panic!("failed to create test directory {}: {err}", path.display()); - }); - let path = AbsPathBuf::assert_utf8(path); - Self { path } - } - - pub(crate) fn join(&self, path: impl AsRef) -> AbsPathBuf { - self.path.absolutize(path) - } - - pub(crate) fn write( - &self, - path: impl AsRef, - contents: impl AsRef<[u8]>, - ) -> AbsPathBuf { - let path = self.join(path); - if let Some(parent) = path.parent() { - fs::create_dir_all(parent).unwrap_or_else(|err| { - panic!("failed to create test directory {parent}: {err}"); - }); - } - fs::write(&path, contents).unwrap_or_else(|err| { - panic!("failed to write test file {path}: {err}"); - }); - path - } -} - -impl Drop for TestDir { - fn drop(&mut self) { - let _ = fs::remove_dir_all(&self.path); - } -} diff --git a/crates/vfs/src/vfs.rs b/crates/vfs/src/vfs.rs deleted file mode 100644 index 7afc288f..00000000 --- a/crates/vfs/src/vfs.rs +++ /dev/null @@ -1,595 +0,0 @@ -use std::{fmt, mem}; - -use rustc_hash::FxHashMap; -use triomphe::Arc; -use utils::{ - lines::LineEnding, - path_identity::{PathKey, path_alias_paths}, -}; - -use crate::{loader::LoadResult, vfs_path::VfsPath}; - -#[derive(Copy, Clone, Debug, Ord, PartialOrd, Eq, PartialEq, Hash)] -pub struct FileId(pub u32); - -impl nohash_hasher::IsEnabled for FileId {} - -#[derive(Default)] -pub struct Vfs { - identities: VfsIdentityIndex, - file_states: Vec, - changes: Vec, -} - -#[derive(PartialEq, PartialOrd, Clone)] -pub enum FileState { - Exists(String, LineEnding), - Deleted, -} - -#[derive(Debug)] -pub struct ChangedFile { - pub file_id: FileId, - pub change_kind: ChangeKind, -} - -impl ChangedFile { - pub fn is_created_or_deleted(&self) -> bool { - matches!(self.change_kind, ChangeKind::Create(_, _) | ChangeKind::Delete) - } - - pub fn text(&self) -> Option> { - match &self.change_kind { - ChangeKind::Create(text, _) | ChangeKind::Modify(text, _) => Some(text.clone()), - ChangeKind::Delete => None, - } - } - - pub fn ending(&self) -> Option { - match &self.change_kind { - ChangeKind::Create(_, ending) | ChangeKind::Modify(_, ending) => Some(*ending), - ChangeKind::Delete => None, - } - } -} - -#[derive(Clone, Debug)] -pub enum ChangeKind { - Create(Arc, LineEnding), - Modify(Arc, LineEnding), - Delete, -} - -/// Identity service behind [`Vfs`]. -/// -/// A single [`FileId`] can be reached through several path spellings. The -/// primary path preserves the first spelling for legacy callers, while aliases -/// record every proven spelling for identity-aware consumers such as file-set -/// partitioning. File identity is path-based: distinct live paths such as -/// hard links remain distinct source files. -#[derive(Default)] -struct VfsIdentityIndex { - primary_paths: Vec, - aliases: Vec>, - exact_paths: FxHashMap, - real_path_keys: FxHashMap, - redirects: Vec, -} - -/// Result of registering a path ingress with the VFS identity service. -/// -/// `file_id` is the canonical owner after all alias evidence has been applied. -/// `merged` contains duplicate ids that were redirected to that owner and whose -/// file state still needs to be reconciled by [`Vfs`]. -struct IdentityRegistration { - file_id: FileId, - merged: Vec, -} - -impl VfsIdentityIndex { - fn file_id(&self, path: &VfsPath) -> Option { - if let Some(file_id) = self.exact_paths.get(path).copied() { - return Some(self.canonical_file_id(file_id)); - } - - let path = path.as_abs_path()?; - if let Some(file_id) = self.real_path_keys.get(&PathKey::from_abs_path(path)).copied() { - return Some(self.canonical_file_id(file_id)); - } - for alias in path_alias_paths(path).into_iter().map(VfsPath::from) { - if let Some(file_id) = self.exact_paths.get(&alias).copied() { - return Some(self.canonical_file_id(file_id)); - } - } - - None - } - - fn file_path(&self, file_id: FileId) -> Option<&VfsPath> { - let file_id = self.canonical_file_id(file_id); - self.primary_paths.get(file_id.0 as usize) - } - - fn original_file_path(&self, file_id: FileId) -> Option<&VfsPath> { - self.primary_paths.get(file_id.0 as usize) - } - - fn file_paths(&self, file_id: FileId) -> &[VfsPath] { - let file_id = self.canonical_file_id(file_id); - self.aliases.get(file_id.0 as usize).map_or(&[], Vec::as_slice) - } - - /// Resolves or allocates a file id and records the current path as alias - /// evidence for that id. - fn file_id_or_alloc(&mut self, path: &VfsPath) -> IdentityRegistration { - let file_id = self.file_id(path).unwrap_or_else(|| self.alloc_file_id(path)); - self.register_path(path, file_id) - } - - fn len(&self) -> usize { - self.primary_paths.len() - } - - fn canonical_file_id(&self, file_id: FileId) -> FileId { - let mut current = file_id; - while let Some(next) = self.redirects.get(current.0 as usize).copied() - && next != current - { - current = next; - } - current - } - - fn alloc_file_id(&mut self, path: &VfsPath) -> FileId { - let id = self.primary_paths.len(); - assert!(id < u32::MAX as usize); - let file_id = FileId(id as u32); - self.primary_paths.push(path.clone()); - self.aliases.push(Vec::new()); - self.redirects.push(file_id); - file_id - } - - fn register_path(&mut self, path: &VfsPath, file_id: FileId) -> IdentityRegistration { - let mut owner = self.canonical_file_id(file_id); - let mut merged = Vec::new(); - - if let Some(path) = path.as_abs_path() { - let aliases = path_alias_paths(path).into_iter().map(VfsPath::from).collect::>(); - for alias in &aliases { - if let Some(existing) = self.exact_paths.get(alias).copied() { - owner = self.merge_file_ids(owner, existing, &mut merged); - } - if let Some(path) = alias.as_abs_path() - && let Some(existing) = - self.real_path_keys.get(&PathKey::from_abs_path(path)).copied() - { - owner = self.merge_file_ids(owner, existing, &mut merged); - } - } - for alias in aliases { - self.register_exact_path(alias, owner); - } - } else { - if let Some(existing) = self.exact_paths.get(path).copied() { - owner = self.merge_file_ids(owner, existing, &mut merged); - } - self.register_exact_path(path.clone(), owner); - } - - IdentityRegistration { file_id: owner, merged } - } - - fn register_exact_path(&mut self, path: VfsPath, file_id: FileId) { - let file_id = self.canonical_file_id(file_id); - if let Some(path) = path.as_abs_path() { - self.real_path_keys.insert(PathKey::from_abs_path(path), file_id); - } - self.exact_paths.insert(path.clone(), file_id); - - let Some(aliases) = self.aliases.get_mut(file_id.0 as usize) else { - return; - }; - if !aliases.contains(&path) { - aliases.push(path); - } - } - - fn merge_file_ids(&mut self, lhs: FileId, rhs: FileId, merged: &mut Vec) -> FileId { - let lhs = self.canonical_file_id(lhs); - let rhs = self.canonical_file_id(rhs); - if lhs == rhs { - return lhs; - } - - let (owner, duplicate) = if lhs <= rhs { (lhs, rhs) } else { (rhs, lhs) }; - self.redirects[duplicate.0 as usize] = owner; - merged.push(duplicate); - - let duplicate_aliases = std::mem::take(&mut self.aliases[duplicate.0 as usize]); - for alias in duplicate_aliases { - self.register_exact_path(alias, owner); - } - for file_id in self.real_path_keys.values_mut() { - if *file_id == duplicate { - *file_id = owner; - } - } - - owner - } -} - -impl Vfs { - pub(crate) fn file_paths(&self, file_id: FileId) -> &[VfsPath] { - self.identities.file_paths(file_id) - } -} - -impl Vfs { - pub fn file_id(&self, path: &VfsPath) -> Option { - self.identities.file_id(path).filter(|id| self.exists(*id)) - } - - pub fn file_path(&self, file_id: FileId) -> Option<&VfsPath> { - self.identities.file_path(file_id) - } - - /// Registers a path at the VFS identity boundary without changing file - /// contents. - /// - /// LSP open notifications use this to learn whether a URI aliases an - /// already-open analysis buffer before deciding whether the incoming text - /// should become the canonical VFS text. - pub fn register_file_ingress(&mut self, path: &VfsPath) -> FileId { - self.file_id_or_alloc(path) - } - - /// Returns the first path spelling recorded for this exact [`FileId`]. - /// - /// Unlike [`Self::file_path`], this does not follow identity redirects. - /// It is intended for cleanup of historical changes, for example clearing - /// diagnostics that were published before a duplicate id was merged. - pub fn original_file_path(&self, file_id: FileId) -> Option<&VfsPath> { - self.identities.original_file_path(file_id) - } - - pub fn canonical_file_id(&self, file_id: FileId) -> FileId { - self.identities.canonical_file_id(file_id) - } - - pub fn line_ending(&self, file_id: FileId) -> Option { - match self.file_state(file_id)? { - FileState::Exists(_, ending) => Some(*ending), - FileState::Deleted => None, - } - } - - pub fn iter(&self) -> impl Iterator + '_ { - (0..self.file_states.len()) - .map(|it| FileId(it as u32)) - .filter(move |&file_id| self.exists(file_id)) - .filter_map(move |file_id| self.file_path(file_id).map(|path| (file_id, path))) - } - - pub fn set_file_contents(&mut self, path: &VfsPath, contents: LoadResult) { - let file_id = self.file_id_or_alloc(path); - use FileState::*; - use LoadResult::*; - let Some(state) = self.file_states.get_mut(file_id.0 as usize) else { - return; - }; - let change_kind = match contents { - Loaded(new, new_ending) => match state { - Exists(old, _) if *old == new => return, - Exists(_, _) => { - let change_kind = ChangeKind::Modify(Arc::from(new.as_str()), new_ending); - *state = Exists(new, new_ending); - change_kind - } - Deleted => { - let change_kind = ChangeKind::Create(Arc::from(new.as_str()), new_ending); - *state = Exists(new, new_ending); - change_kind - } - }, - LoadError => match state { - Exists(_, _) => { - *state = Deleted; - ChangeKind::Delete - } - Deleted => return, - }, - DecodeError => return, - }; - - let changed_file = ChangedFile { file_id, change_kind }; - self.changes.push(changed_file); - } - - pub fn has_changes(&self) -> bool { - !self.changes.is_empty() - } - - pub fn take_changes(&mut self) -> Vec { - mem::take(&mut self.changes) - } - - pub fn exists(&self, file_id: FileId) -> bool { - matches!(self.file_state(file_id), Some(FileState::Exists(_, _))) - } - - fn file_id_or_alloc(&mut self, path: &VfsPath) -> FileId { - let registration = self.identities.file_id_or_alloc(path); - let file_id = registration.file_id; - let id = file_id.0 as usize; - let len = self.file_states.len().max(id + 1); - self.file_states.resize(len, FileState::Deleted); - for duplicate in registration.merged { - self.merge_file_state(file_id, duplicate); - } - file_id - } - - fn merge_file_state(&mut self, owner: FileId, duplicate: FileId) { - if owner == duplicate { - return; - } - - let owner_idx = owner.0 as usize; - let duplicate_idx = duplicate.0 as usize; - let Some(duplicate_state) = self.file_states.get(duplicate_idx).cloned() else { - return; - }; - - if matches!(self.file_states.get(owner_idx), Some(FileState::Deleted)) - && let FileState::Exists(text, ending) = duplicate_state.clone() - { - self.file_states[owner_idx] = FileState::Exists(text.clone(), ending); - self.changes.push(ChangedFile { - file_id: owner, - change_kind: ChangeKind::Create(Arc::from(text.as_str()), ending), - }); - } - - if matches!(duplicate_state, FileState::Exists(_, _)) { - self.file_states[duplicate_idx] = FileState::Deleted; - self.changes.push(ChangedFile { file_id: duplicate, change_kind: ChangeKind::Delete }); - } - } - - fn file_state(&self, file_id: FileId) -> Option<&FileState> { - self.file_states.get(file_id.0 as usize) - } -} - -impl fmt::Debug for Vfs { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Vfs").field("n_files", &self.identities.len()).finish() - } -} - -#[cfg(test)] -mod tests { - use std::fs; - - use utils::lines::LineEnding; - #[cfg(windows)] - use utils::paths::AbsPathBuf; - - use super::*; - use crate::test_support::TestDir; - - #[test] - fn load_error_marks_existing_file_deleted() { - let mut vfs = Vfs::default(); - let path = VfsPath::new_virtual_path("/workspace/vide.toml".to_owned()); - - vfs.set_file_contents( - &path, - LoadResult::Loaded("sources = []\n".to_owned(), LineEnding::Unix), - ); - let file_id = vfs.file_id(&path).unwrap(); - vfs.take_changes(); - - vfs.set_file_contents(&path, LoadResult::LoadError); - - assert!(!vfs.exists(file_id)); - assert_eq!(vfs.file_id(&path), None); - let changes = vfs.take_changes(); - assert_eq!(changes.len(), 1); - assert!(matches!(changes[0].change_kind, ChangeKind::Delete)); - } - - #[test] - fn renamed_file_uses_new_path_identity_after_delete_then_create() { - let dir = TestDir::new("rename-delete-create"); - let old_path = dir.write("old_child.sv", "module child; endmodule\n"); - let new_path = dir.join("child.sv"); - let old_vfs_path = VfsPath::from(old_path.clone()); - let new_vfs_path = VfsPath::from(new_path.clone()); - let mut vfs = Vfs::default(); - - vfs.set_file_contents( - &old_vfs_path, - LoadResult::Loaded("module child; endmodule\n".to_owned(), LineEnding::Unix), - ); - let old_file_id = vfs.file_id(&old_vfs_path).unwrap(); - vfs.take_changes(); - - fs::rename(&old_path, &new_path).unwrap(); - vfs.set_file_contents(&old_vfs_path, LoadResult::LoadError); - vfs.set_file_contents( - &new_vfs_path, - LoadResult::Loaded("module child; endmodule\n".to_owned(), LineEnding::Unix), - ); - - let new_file_id = vfs.file_id(&new_vfs_path).unwrap(); - assert_ne!(old_file_id, new_file_id); - assert_eq!(vfs.file_id(&old_vfs_path), None); - assert_eq!(vfs.file_path(new_file_id), Some(&new_vfs_path)); - } - - #[test] - fn renamed_file_uses_new_path_identity_after_create_then_delete() { - let dir = TestDir::new("rename-create-delete"); - let old_path = dir.write("old_child.sv", "module child; endmodule\n"); - let new_path = dir.join("child.sv"); - let old_vfs_path = VfsPath::from(old_path.clone()); - let new_vfs_path = VfsPath::from(new_path.clone()); - let mut vfs = Vfs::default(); - - vfs.set_file_contents( - &old_vfs_path, - LoadResult::Loaded("module child; endmodule\n".to_owned(), LineEnding::Unix), - ); - let old_file_id = vfs.file_id(&old_vfs_path).unwrap(); - vfs.take_changes(); - - fs::rename(&old_path, &new_path).unwrap(); - vfs.set_file_contents( - &new_vfs_path, - LoadResult::Loaded("module child; endmodule\n".to_owned(), LineEnding::Unix), - ); - vfs.set_file_contents(&old_vfs_path, LoadResult::LoadError); - - let new_file_id = vfs.file_id(&new_vfs_path).unwrap(); - assert_ne!(old_file_id, new_file_id); - assert_eq!(vfs.file_id(&old_vfs_path), None); - assert_eq!(vfs.file_path(new_file_id), Some(&new_vfs_path)); - } - - #[cfg(windows)] - #[test] - fn path_spellings_with_different_drive_letter_case_share_file_id() { - let path = AbsPathBuf::assert_utf8(std::env::current_dir().unwrap()).join("top.sv"); - let mut lower_drive_path = path.to_string(); - assert_eq!(lower_drive_path.as_bytes().get(1), Some(&b':')); - lower_drive_path.replace_range(0..1, &lower_drive_path[0..1].to_ascii_lowercase()); - let lower_drive_path = AbsPathBuf::try_from(lower_drive_path.as_str()).unwrap(); - - let mut vfs = Vfs::default(); - let upper_vfs_path = VfsPath::from(path); - let lower_vfs_path = VfsPath::from(lower_drive_path); - - vfs.set_file_contents( - &upper_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let first_file_id = vfs.file_id(&upper_vfs_path).unwrap(); - - vfs.set_file_contents( - &lower_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let second_file_id = vfs.file_id(&lower_vfs_path).unwrap(); - - assert_eq!(first_file_id, second_file_id); - assert_eq!(vfs.iter().count(), 1); - } - - #[cfg(windows)] - #[test] - fn path_spellings_with_verbatim_and_normal_spelling_share_file_id() { - let path = AbsPathBuf::assert_utf8(std::env::current_dir().unwrap()).join("top.sv"); - let verbatim_path = AbsPathBuf::try_from(format!(r"\\?\{path}").as_str()).unwrap(); - - let mut vfs = Vfs::default(); - let normal_vfs_path = VfsPath::from(path); - let verbatim_vfs_path = VfsPath::from(verbatim_path); - - vfs.set_file_contents( - &normal_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let first_file_id = vfs.file_id(&normal_vfs_path).unwrap(); - - vfs.set_file_contents( - &verbatim_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let second_file_id = vfs.file_id(&verbatim_vfs_path).unwrap(); - - assert_eq!(first_file_id, second_file_id); - assert_eq!(vfs.iter().count(), 1); - } - - #[test] - fn hard_links_remain_distinct_source_files() { - let dir = TestDir::new("hard-link-source-identity"); - let source = dir.join("workspace/top.sv"); - let alias = dir.join("alias/top.sv"); - let source_vfs_path = VfsPath::from(source.clone()); - let alias_vfs_path = VfsPath::from(alias.clone()); - let mut vfs = Vfs::default(); - - vfs.set_file_contents( - &source_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let source_file_id = vfs.file_id(&source_vfs_path).unwrap(); - - if let Some(parent) = source.parent() { - fs::create_dir_all(parent).unwrap(); - } - if let Some(parent) = alias.parent() { - fs::create_dir_all(parent).unwrap(); - } - fs::write(&source, "module top; endmodule\n").unwrap(); - fs::hard_link(&source, &alias).unwrap(); - - vfs.set_file_contents( - &alias_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let alias_file_id = vfs.file_id(&alias_vfs_path).unwrap(); - - assert_ne!(source_file_id, alias_file_id); - assert_eq!(vfs.iter().count(), 2); - } - - #[test] - fn hard_links_stay_distinct_after_existing_identity_evidence() { - let dir = TestDir::new("hard-link-existing-identity"); - let source = dir.join("workspace/top.sv"); - let alias = dir.join("alias/top.sv"); - let source_vfs_path = VfsPath::from(source.clone()); - let alias_vfs_path = VfsPath::from(alias.clone()); - let mut vfs = Vfs::default(); - - vfs.set_file_contents( - &source_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let source_file_id = vfs.file_id(&source_vfs_path).unwrap(); - vfs.set_file_contents( - &alias_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - let alias_file_id = vfs.file_id(&alias_vfs_path).unwrap(); - assert_ne!(source_file_id, alias_file_id); - vfs.take_changes(); - - if let Some(parent) = source.parent() { - fs::create_dir_all(parent).unwrap(); - } - if let Some(parent) = alias.parent() { - fs::create_dir_all(parent).unwrap(); - } - fs::write(&source, "module top; endmodule\n").unwrap(); - fs::hard_link(&source, &alias).unwrap(); - - vfs.set_file_contents( - &source_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - vfs.set_file_contents( - &alias_vfs_path, - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); - - assert_eq!(vfs.file_id(&source_vfs_path), Some(source_file_id)); - assert_eq!(vfs.file_id(&alias_vfs_path), Some(alias_file_id)); - assert_eq!(vfs.iter().count(), 2); - } -} diff --git a/crates/vfs/src/vfs_path.rs b/crates/vfs/src/vfs_path.rs index 0caac3f0..e122d0c4 100644 --- a/crates/vfs/src/vfs_path.rs +++ b/crates/vfs/src/vfs_path.rs @@ -1,38 +1,66 @@ -// Abstract-ish representation of paths for VFS. +//! Abstract-ish representation of paths for VFS. use std::fmt; -use utils::paths::{AbsPath, AbsPathBuf, RelPath, Utf8Path}; +use utils::paths::{AbsPath, AbsPathBuf, RelPath}; +/// Path in [`Vfs`]. +/// +/// Long-term, we want to support files which do not reside in the file-system, +/// so we treat `VfsPath`s as opaque identifiers. +/// +/// [`Vfs`]: crate::Vfs #[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] -pub struct VfsPath(VfsPathKinds); +pub struct VfsPath(VfsPathRepr); impl VfsPath { + /// Creates an "in-memory" path from `/`-separated string. + /// + /// This is most useful for testing, to avoid windows/linux differences + /// + /// # Panics + /// + /// Panics if `path` does not start with `'/'`. pub fn new_virtual_path(path: String) -> VfsPath { assert!(path.starts_with('/')); - VfsPath(VfsPathKinds::VirtualPath(VirtualPath(path))) + VfsPath(VfsPathRepr::VirtualPath(VirtualPath(path))) } + /// Create a path from string. Input should be a string representation of + /// an absolute path inside filesystem pub fn new_real_path(path: String) -> VfsPath { VfsPath::from(AbsPathBuf::assert(path.into())) } - pub fn as_abs_path(&self) -> Option<&AbsPath> { + /// Returns the `AbsPath` representation of `self` if `self` is on the file + /// system. + pub fn as_path(&self) -> Option<&AbsPath> { match &self.0 { - VfsPathKinds::RealPath(it) => Some(it.as_path()), - VfsPathKinds::VirtualPath(_) => None, + VfsPathRepr::PathBuf(it) => Some(it.as_path()), + VfsPathRepr::VirtualPath(_) => None, + } + } + + pub fn as_abs_path(&self) -> Option<&AbsPath> { + self.as_path() + } + + pub fn into_abs_path(self) -> Option { + match self.0 { + VfsPathRepr::PathBuf(it) => Some(it), + VfsPathRepr::VirtualPath(_) => None, } } - /// Creates a new [`VfsPath`] with `path` adjoined to `self`. + /// Creates a new `VfsPath` with `path` adjoined to `self`. pub fn join(&self, path: &str) -> Option { match &self.0 { - VfsPathKinds::RealPath(it) => { + VfsPathRepr::PathBuf(it) => { let res = it.join(path).normalize(); - Some(VfsPath(VfsPathKinds::RealPath(res))) + Some(VfsPath(VfsPathRepr::PathBuf(res))) } - VfsPathKinds::VirtualPath(it) => { + VfsPathRepr::VirtualPath(it) => { let res = it.join(path)?; - Some(VfsPath(VfsPathKinds::VirtualPath(res))) + Some(VfsPath(VfsPathRepr::VirtualPath(res))) } } } @@ -40,31 +68,39 @@ impl VfsPath { /// Remove the last component of `self` if there is one. /// /// If `self` has no component, returns `false`; else returns `true`. + /// + /// # Example + /// + /// ```ignore + /// # use vfs::{AbsPathBuf, VfsPath}; + /// let mut path = VfsPath::from(AbsPathBuf::assert("/foo/bar".into())); + /// assert!(path.pop()); + /// assert_eq!(path, VfsPath::from(AbsPathBuf::assert("/foo".into()))); + /// assert!(path.pop()); + /// assert_eq!(path, VfsPath::from(AbsPathBuf::assert("/".into()))); + /// assert!(!path.pop()); + /// ``` pub fn pop(&mut self) -> bool { match &mut self.0 { - VfsPathKinds::RealPath(it) => it.pop(), - VfsPathKinds::VirtualPath(it) => it.pop(), + VfsPathRepr::PathBuf(it) => it.pop(), + VfsPathRepr::VirtualPath(it) => it.pop(), } } /// Returns `true` if `other` is a prefix of `self`. pub fn starts_with(&self, other: &VfsPath) -> bool { match (&self.0, &other.0) { - (VfsPathKinds::RealPath(lhs), VfsPathKinds::RealPath(rhs)) => lhs.starts_with(rhs), - (VfsPathKinds::VirtualPath(lhs), VfsPathKinds::VirtualPath(rhs)) => { - lhs.starts_with(rhs) - } - (VfsPathKinds::RealPath(_) | VfsPathKinds::VirtualPath(_), _) => false, + (VfsPathRepr::PathBuf(lhs), VfsPathRepr::PathBuf(rhs)) => lhs.starts_with(rhs), + (VfsPathRepr::VirtualPath(lhs), VfsPathRepr::VirtualPath(rhs)) => lhs.starts_with(rhs), + (VfsPathRepr::PathBuf(_) | VfsPathRepr::VirtualPath(_), _) => false, } } pub fn strip_prefix(&self, other: &VfsPath) -> Option<&RelPath> { match (&self.0, &other.0) { - (VfsPathKinds::RealPath(lhs), VfsPathKinds::RealPath(rhs)) => lhs.strip_prefix(rhs), - (VfsPathKinds::VirtualPath(lhs), VfsPathKinds::VirtualPath(rhs)) => { - lhs.strip_prefix(rhs) - } - (VfsPathKinds::RealPath(_) | VfsPathKinds::VirtualPath(_), _) => None, + (VfsPathRepr::PathBuf(lhs), VfsPathRepr::PathBuf(rhs)) => lhs.strip_prefix(rhs), + (VfsPathRepr::VirtualPath(lhs), VfsPathRepr::VirtualPath(rhs)) => lhs.strip_prefix(rhs), + (VfsPathRepr::PathBuf(_) | VfsPathRepr::VirtualPath(_), _) => None, } } @@ -73,17 +109,19 @@ impl VfsPath { /// Returns [`None`] if the path is a root or prefix. pub fn parent(&self) -> Option { let mut parent = self.clone(); - parent.pop().then_some(parent) + if parent.pop() { Some(parent) } else { None } } /// Returns `self`'s base name and file extension. pub fn name_and_extension(&self) -> Option<(&str, Option<&str>)> { match &self.0 { - VfsPathKinds::RealPath(p) => p.name_and_extension(), - VfsPathKinds::VirtualPath(p) => p.name_and_extension(), + VfsPathRepr::PathBuf(p) => p.name_and_extension(), + VfsPathRepr::VirtualPath(p) => p.name_and_extension(), } } + /// **Don't make this `pub`** + /// /// Encode the path in the given buffer. /// /// The encoding will be `0` if [`AbsPathBuf`], `1` if [`VirtualPath`], @@ -92,12 +130,12 @@ impl VfsPath { /// Note that this encoding is dependent on the operating system. pub(crate) fn encode(&self, buf: &mut Vec) { let tag = match &self.0 { - VfsPathKinds::RealPath(_) => 0, - VfsPathKinds::VirtualPath(_) => 1, + VfsPathRepr::PathBuf(_) => 0, + VfsPathRepr::VirtualPath(_) => 1, }; buf.push(tag); match &self.0 { - VfsPathKinds::RealPath(path) => { + VfsPathRepr::PathBuf(path) => { #[cfg(windows)] { use windows_paths::Encode; @@ -136,7 +174,7 @@ impl VfsPath { buf.extend(path.as_os_str().to_string_lossy().as_bytes()); } } - VfsPathKinds::VirtualPath(VirtualPath(s)) => buf.extend(s.as_bytes()), + VfsPathRepr::VirtualPath(VirtualPath(s)) => buf.extend(s.as_bytes()), } } } @@ -216,26 +254,54 @@ mod windows_paths { } } } + #[test] + fn paths_encoding() { + // drive letter casing agnostic + test_eq("C:/x.rs", "c:/x.rs"); + // separator agnostic + test_eq("C:/x/y.rs", "C:\\x\\y.rs"); + + fn test_eq(a: &str, b: &str) { + let mut b1 = Vec::new(); + let mut b2 = Vec::new(); + vfs(a).encode(&mut b1); + vfs(b).encode(&mut b2); + assert_eq!(b1, b2); + } + } + + #[test] + fn test_sep_root_dir_encoding() { + let mut buf = Vec::new(); + vfs("C:/x/y").encode(&mut buf); + assert_eq!(&buf, &[0, 67, 0, 58, 0, 92, 0, 120, 0, 92, 0, 121, 0]) + } + + #[cfg(test)] + fn vfs(str: &str) -> super::VfsPath { + use super::{AbsPathBuf, VfsPath}; + VfsPath::from(AbsPathBuf::try_from(str).unwrap()) + } } /// Internal, private representation of [`VfsPath`]. #[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] -enum VfsPathKinds { - RealPath(AbsPathBuf), +enum VfsPathRepr { + PathBuf(AbsPathBuf), VirtualPath(VirtualPath), } impl From for VfsPath { fn from(v: AbsPathBuf) -> Self { - VfsPath(VfsPathKinds::RealPath(v.normalize())) + VfsPath(VfsPathRepr::PathBuf(v.normalize())) } } impl fmt::Display for VfsPath { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match &self.0 { - VfsPathKinds::RealPath(it) => it.fmt(f), - VfsPathKinds::VirtualPath(VirtualPath(it)) => it.fmt(f), + VfsPathRepr::PathBuf(it) => it.fmt(f), + VfsPathRepr::VirtualPath(VirtualPath(it)) => it.fmt(f), } } } @@ -246,14 +312,28 @@ impl fmt::Debug for VfsPath { } } -impl fmt::Debug for VfsPathKinds { +impl fmt::Debug for VfsPathRepr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match &self { - VfsPathKinds::RealPath(it) => it.fmt(f), - VfsPathKinds::VirtualPath(VirtualPath(it)) => it.fmt(f), + VfsPathRepr::PathBuf(it) => it.fmt(f), + VfsPathRepr::VirtualPath(VirtualPath(it)) => it.fmt(f), + } + } +} + +impl PartialEq for VfsPath { + fn eq(&self, other: &AbsPath) -> bool { + match &self.0 { + VfsPathRepr::PathBuf(lhs) => lhs == other, + VfsPathRepr::VirtualPath(_) => false, } } } +impl PartialEq for AbsPath { + fn eq(&self, other: &VfsPath) -> bool { + other == self + } +} /// `/`-separated virtual path. /// @@ -262,26 +342,53 @@ impl fmt::Debug for VfsPathKinds { struct VirtualPath(String); impl VirtualPath { + /// Returns `true` if `other` is a prefix of `self` (as strings). fn starts_with(&self, other: &VirtualPath) -> bool { self.0.starts_with(&other.0) } fn strip_prefix(&self, base: &VirtualPath) -> Option<&RelPath> { - <_ as AsRef>::as_ref(&self.0) + <_ as AsRef>::as_ref(&self.0) .strip_prefix(&base.0) .ok() .map(RelPath::new_unchecked) } + /// Remove the last component of `self`. + /// + /// This will find the last `'/'` in `self`, and remove everything after it, + /// including the `'/'`. + /// + /// If `self` contains no `'/'`, returns `false`; else returns `true`. + /// + /// # Example + /// + /// ```rust,ignore + /// let mut path = VirtualPath("/foo/bar".to_string()); + /// path.pop(); + /// assert_eq!(path.0, "/foo"); + /// path.pop(); + /// assert_eq!(path.0, ""); + /// ``` fn pop(&mut self) -> bool { - let Some(pos) = self.0.rfind('/') else { - return false; + let pos = match self.0.rfind('/') { + Some(pos) => pos, + None => return false, }; - self.0 = self.0[..pos].to_string(); true } + /// Append the given *relative* path `path` to `self`. + /// + /// This will resolve any leading `"../"` in `path` before appending it. + /// + /// Returns [`None`] if `path` has more leading `"../"` than the number of + /// components in `self`. + /// + /// # Notes + /// + /// In practice, appending here means `self/path` as strings. fn join(&self, mut path: &str) -> Option { let mut res = self.clone(); while path.starts_with("../") { @@ -328,3 +435,6 @@ impl VirtualPath { } } } + +#[cfg(test)] +mod tests; diff --git a/crates/vfs/src/vfs_path/tests.rs b/crates/vfs/src/vfs_path/tests.rs new file mode 100644 index 00000000..2d89362e --- /dev/null +++ b/crates/vfs/src/vfs_path/tests.rs @@ -0,0 +1,30 @@ +use super::*; + +#[test] +fn virtual_path_extensions() { + assert_eq!(VirtualPath("/".to_owned()).name_and_extension(), None); + assert_eq!( + VirtualPath("/directory".to_owned()).name_and_extension(), + Some(("directory", None)) + ); + assert_eq!( + VirtualPath("/directory/".to_owned()).name_and_extension(), + Some(("directory", None)) + ); + assert_eq!( + VirtualPath("/directory/file".to_owned()).name_and_extension(), + Some(("file", None)) + ); + assert_eq!( + VirtualPath("/directory/.file".to_owned()).name_and_extension(), + Some((".file", None)) + ); + assert_eq!( + VirtualPath("/directory/.file.rs".to_owned()).name_and_extension(), + Some((".file", Some("rs"))) + ); + assert_eq!( + VirtualPath("/directory/file.rs".to_owned()).name_and_extension(), + Some(("file", Some("rs"))) + ); +} diff --git a/crates/vide-lsp-wasm/Cargo.toml b/crates/vide-lsp-wasm/Cargo.toml index 1e8cac19..3b4fcdae 100644 --- a/crates/vide-lsp-wasm/Cargo.toml +++ b/crates/vide-lsp-wasm/Cargo.toml @@ -9,5 +9,6 @@ name = "vide-lsp-wasm" path = "src/main.rs" [dependencies] -vide = { path = "../.." } +# No server-side notify watcher under wasm/emscripten; client supplies files. +vide = { path = "../..", default-features = false } serde_json.workspace = true diff --git a/crates/workspace-model/src/source_root.rs b/crates/workspace-model/src/source_root.rs index f0abff31..6b313109 100644 --- a/crates/workspace-model/src/source_root.rs +++ b/crates/workspace-model/src/source_root.rs @@ -1,4 +1,4 @@ -use vfs::{FileId, FileSet, FileSetConfig, Vfs, VfsPath, anchored_path::AnchoredPath}; +use vfs::{AnchoredPath, FileId, FileSet, FileSetConfig, Vfs, VfsPath}; use crate::source_db::SourceFileKind; @@ -238,7 +238,7 @@ mod tests { #[test] fn ignored_root_preserves_file_kind() { let mut file_set = FileSet::default(); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); file_set.insert(file_id, VfsPath::new_virtual_path("/ignored/file.sv".into())); let root = SourceRoot::new_ignored(file_set); @@ -249,7 +249,7 @@ mod tests { #[test] fn best_effort_index_root_preserves_file_kind() { let mut file_set = FileSet::default(); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); file_set.insert(file_id, VfsPath::new_virtual_path("/indexed/file.sv".into())); let root = SourceRoot::new_best_effort_index(file_set); @@ -282,7 +282,7 @@ mod tests { #[test] fn source_filtered_root_preserves_project_manifest_kind() { let mut file_set = FileSet::default(); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); file_set.insert(file_id, VfsPath::new_virtual_path("/root/vide.toml".into())); let root = SourceRoot::new_local_with_source_files(file_set, Vec::new()); diff --git a/src/global_state.rs b/src/global_state.rs index f90674a2..208f3d9a 100644 --- a/src/global_state.rs +++ b/src/global_state.rs @@ -35,7 +35,7 @@ use project_model::Workspace; use rustc_hash::{FxHashMap, FxHashSet}; use triomphe::Arc; use utils::{cancellation::CancellationToken, excl_task::ExclTask, lines::LineEnding}; -use vfs::{self, FileId, Vfs, notify::NotifyHandle}; +use vfs::{self, FileId, Vfs}; #[cfg(test)] pub(crate) use self::workspace_state::VfsProgress; @@ -117,14 +117,27 @@ pub(crate) struct WorkspaceState { pub(crate) vfs_loader: Handle, Receiver>, pub(crate) vfs: Arc)>>, pub(crate) workspace_vfs: WorkspaceVfsReadiness, - pub(crate) pending_workspace_readiness_requests: Vec, - pub(crate) workspaces: Arc>, pub(crate) fetch_workspaces_task: ExclTask<(Arc>, Vec), WorkspaceFetchCause>, pub(crate) registered_client_file_watcher_globs: Option>, } +fn spawn_vfs_loader( + sender: crossbeam_channel::Sender, +) -> Box { + #[cfg(feature = "server-file-watcher")] + { + let handle: vfs::notify::NotifyHandle = vfs::loader::Handle::spawn(sender); + Box::new(handle) + } + #[cfg(not(feature = "server-file-watcher"))] + { + let handle: vfs::dummy::DummyHandle = vfs::loader::Handle::spawn(sender); + Box::new(handle) + } +} + impl GlobalState { pub(crate) fn new( sender: Sender, @@ -133,8 +146,7 @@ impl GlobalState { ) -> GlobalState { let vfs_loader = { let (sender, receiver) = unbounded::(); - let handle: NotifyHandle = vfs::loader::Handle::spawn(sender); - let handle = Box::new(handle) as Box; + let handle = spawn_vfs_loader(sender); Handle { handle, receiver } }; @@ -188,7 +200,6 @@ impl GlobalState { vfs_loader, vfs: Arc::new(RwLock::new((Vfs::default(), IntMap::default()))), workspace_vfs: WorkspaceVfsReadiness::default(), - pending_workspace_readiness_requests: Vec::new(), workspaces: Arc::from(vec![]), fetch_workspaces_task: ExclTask::default(), registered_client_file_watcher_globs: None, diff --git a/src/global_state/diagnostics.rs b/src/global_state/diagnostics.rs index 92de4ef1..957cee84 100644 --- a/src/global_state/diagnostics.rs +++ b/src/global_state/diagnostics.rs @@ -94,14 +94,16 @@ pub(crate) enum DiagnosticOwner { impl DiagnosticOwner { fn result_id_fragment(self) -> String { match self { - DiagnosticOwner::File(file_id) => format!("file:{}", file_id.0), + DiagnosticOwner::File(file_id) => format!("file:{}", file_id.index()), DiagnosticOwner::SourceRoot(source_root_id) => { format!("source-root:{}", source_root_id.0) } DiagnosticOwner::CompilationProfile(profile_id) => { format!("compilation-profile:{}", profile_id.0) } - DiagnosticOwner::External { source, file } => format!("external-{source}:{}", file.0), + DiagnosticOwner::External { source, file } => { + format!("external-{source}:{}", file.index()) + } } } } @@ -197,7 +199,9 @@ impl DiagnosticSnapshotKey { let dependency_revisions = self .dependency_revisions .iter() - .map(|(file_id, revision)| format!("{}:{}", file_id.0, revision.result_id_fragment())) + .map(|(file_id, revision)| { + format!("{}:{}", file_id.index(), revision.result_id_fragment()) + }) .collect::>() .join(","); let external_revisions = self diff --git a/src/global_state/dispatch/request.rs b/src/global_state/dispatch/request.rs index 341c5191..90207e21 100644 --- a/src/global_state/dispatch/request.rs +++ b/src/global_state/dispatch/request.rs @@ -1,6 +1,8 @@ use lsp_server::Request; -use lsp_types::request::{ - DocumentDiagnosticRequest, Request as _, WorkspaceDiagnosticRequest, WorkspaceSymbolRequest, +use lsp_types::{ + DocumentDiagnosticReport, DocumentDiagnosticReportResult, FullDocumentDiagnosticReport, + RelatedFullDocumentDiagnosticReport, WorkspaceDiagnosticReport, + WorkspaceDiagnosticReportResult, WorkspaceSymbolResponse, }; use crate::{ @@ -9,14 +11,31 @@ use crate::{ }; impl GlobalState { - pub(in crate::global_state) fn handle_request(&mut self, req: Request) { - if !self.is_workspace_ready() && Self::is_workspace_readiness_request(&req) { - if Self::is_pull_diagnostic_request(&req) { - self.workspace.workspace_vfs.defer_diagnostics_until_ready(); - } + pub(in crate::global_state) fn handle_request(&mut self, mut req: Request) { + if !self.is_workspace_ready() { + tracing::debug!( + method = %req.method, + id = ?req.id, + readiness = ?self.workspace.workspace_vfs, + "workspace is not ready; checking for a terminal fallback response" + ); + + let mut readiness_dispatcher = ReqDispatcher { req: Some(req), global_state: self }; + readiness_dispatcher + .on_sync_mut::( + handle_document_diagnostic_before_workspace_ready, + ) + .on_sync_mut::( + handle_workspace_diagnostic_before_workspace_ready, + ) + .on_sync_mut::( + handle_workspace_symbol_before_workspace_ready, + ); - self.workspace.pending_workspace_readiness_requests.push(req); - return; + let Some(pending_req) = readiness_dispatcher.req.take() else { + return; + }; + req = pending_req; } let mut dispatcher = ReqDispatcher { req: Some(req), global_state: self }; @@ -84,20 +103,34 @@ impl GlobalState { .on::(handle_selection_range) .finish(); } +} - fn is_pull_diagnostic_request(req: &Request) -> bool { - matches!( - req.method.as_str(), - DocumentDiagnosticRequest::METHOD | WorkspaceDiagnosticRequest::METHOD - ) - } +fn handle_document_diagnostic_before_workspace_ready( + state: &mut GlobalState, + _: lsp_types::DocumentDiagnosticParams, +) -> anyhow::Result { + state.workspace.workspace_vfs.defer_diagnostics_until_ready(); + Ok(DocumentDiagnosticReport::Full(RelatedFullDocumentDiagnosticReport { + related_documents: None, + full_document_diagnostic_report: FullDocumentDiagnosticReport { + result_id: None, + items: Vec::new(), + }, + }) + .into()) +} - fn is_workspace_readiness_request(req: &Request) -> bool { - matches!( - req.method.as_str(), - DocumentDiagnosticRequest::METHOD - | WorkspaceDiagnosticRequest::METHOD - | WorkspaceSymbolRequest::METHOD - ) - } +fn handle_workspace_diagnostic_before_workspace_ready( + state: &mut GlobalState, + _: lsp_types::WorkspaceDiagnosticParams, +) -> anyhow::Result { + state.workspace.workspace_vfs.defer_diagnostics_until_ready(); + Ok(WorkspaceDiagnosticReportResult::Report(WorkspaceDiagnosticReport { items: Vec::new() })) +} + +fn handle_workspace_symbol_before_workspace_ready( + _: &mut GlobalState, + _: lsp_types::WorkspaceSymbolParams, +) -> anyhow::Result> { + Ok(Some(WorkspaceSymbolResponse::Flat(Vec::new()))) } diff --git a/src/global_state/event_loop.rs b/src/global_state/event_loop.rs index 0c05fa9f..50395263 100644 --- a/src/global_state/event_loop.rs +++ b/src/global_state/event_loop.rs @@ -10,7 +10,7 @@ use vfs::{VfsPath, loader as vfs_loader}; use super::{ GlobalState, WorkspaceFetchCompletion, process_changes::DiagnosticInvalidation, - reload::FetchWorkspaceProgress, + reload::{self, FetchWorkspaceProgress}, respond::Progress, task::{ResponseTask, Task}, }; @@ -23,6 +23,12 @@ pub(in crate::global_state) enum Event { Vfs(vfs_loader::Message), } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum VfsFileSource { + ConfigScan, + ExternalChange, +} + impl Event { fn kind(&self) -> &'static str { match self { @@ -32,6 +38,7 @@ impl Event { Event::Task(task) => task.kind(), Event::Vfs(vfs_loader::Message::Progress { .. }) => "vfs.progress", Event::Vfs(vfs_loader::Message::Loaded { .. }) => "vfs.loaded", + Event::Vfs(vfs_loader::Message::Changed { .. }) => "vfs.changed", } } @@ -48,11 +55,14 @@ impl Event { } Event::Task(task) => task.summary(), Event::Vfs(vfs_loader::Message::Progress { n_done, n_total, .. }) => { - format!("vfs progress {n_done}/{n_total}") + format!("vfs progress {n_done:?}/{n_total}") } - Event::Vfs(vfs_loader::Message::Loaded { files, .. }) => { + Event::Vfs(vfs_loader::Message::Loaded { files }) => { format!("vfs loaded files={}", files.len()) } + Event::Vfs(vfs_loader::Message::Changed { files }) => { + format!("vfs changed files={}", files.len()) + } } } } @@ -176,8 +186,6 @@ impl GlobalState { } if self.is_workspace_ready() { - self.drain_pending_workspace_readiness_requests(); - let client_refresh = !was_workspace_ready || state_changed; if client_refresh && self.config_state.config.cli_code_lens_refresh_support() { @@ -336,15 +344,23 @@ impl GlobalState { pub(in crate::global_state) fn process_vfs_msg(&mut self, msg: vfs_loader::Message) { match msg { - vfs_loader::Message::Progress { n_total, n_done, config_version } => { + vfs_loader::Message::Progress { n_total, n_done, dir: _, config_version } => { always!( config_version <= self.workspace.workspace_vfs.current_vfs_config_version() ); + use vfs_loader::LoadingProgress; + let (n_done_count, state) = match n_done { + LoadingProgress::Started => (0, Progress::Begin), + LoadingProgress::Progress(n) => (n.min(n_total), Progress::Report), + LoadingProgress::Finished => (n_total, Progress::End), + }; + let Some(progress) = self.workspace.workspace_vfs.accept_vfs_progress( config_version, - n_done, + n_done_count, n_total, + matches!(n_done, LoadingProgress::Finished), ) else { tracing::debug!( config_version, @@ -355,18 +371,11 @@ impl GlobalState { return; }; + // Ready already updated; skip empty 0/0 client progress. if progress.n_total == 0 { return; } - let state = if progress.n_done == 0 { - Progress::Begin - } else if progress.n_done < progress.n_total { - Progress::Report - } else { - assert_eq!(progress.n_done, progress.n_total); - Progress::End - }; let starts_client_progress = state == Progress::Begin; let finishes_client_progress = state == Progress::End; @@ -386,33 +395,56 @@ impl GlobalState { } } } - vfs_loader::Message::Loaded { files, config_version } => { - always!( - config_version <= self.workspace.workspace_vfs.current_vfs_config_version() - ); - if !self.workspace.workspace_vfs.accepts_vfs_loaded(config_version) { - tracing::debug!( - config_version, - current_config_version = - self.workspace.workspace_vfs.current_vfs_config_version(), - files = files.len(), - "stale VFS loaded batch ignored" - ); - return; - } + vfs_loader::Message::Loaded { files } => { + self.process_vfs_files(files, VfsFileSource::ConfigScan); + } + vfs_loader::Message::Changed { files } => { + self.process_vfs_files(files, VfsFileSource::ExternalChange); + } + } + } - let vfs = &mut self.workspace.vfs.write().0; + fn process_vfs_files( + &mut self, + files: Vec<(utils::paths::AbsPathBuf, Option>)>, + source: VfsFileSource, + ) { + let workspace_manifest_change = { + let vfs = &mut self.workspace.vfs.write().0; + let mut manifest_change = None; + + for (abs_path, content) in files { + let path = VfsPath::from(abs_path.clone()); + let existing = vfs.file_id(&path); + let existed = existing.is_some_and(|(file_id, _)| vfs.exists(file_id)); + let exists_after = content.is_some(); + let has_structure_change = existed != exists_after; + let open_file_id = existing + .is_some_and(|(file_id, _)| self.analysis.mem_docs.contains_file_id(file_id)); + if self.analysis.mem_docs.contains_path(&path) || open_file_id { + continue; + } - for (path, content) in files { - let path = VfsPath::from(path); - let open_file_id = vfs - .file_id(&path) - .is_some_and(|file_id| self.analysis.mem_docs.contains_file_id(file_id)); - if !self.analysis.mem_docs.contains_path(&path) && !open_file_id { - vfs.set_file_contents(&path, content); - } + let changed = vfs.set_file_contents(path, content); + if changed + && source == VfsFileSource::ExternalChange + && reload::should_refresh_for_change(&abs_path, has_structure_change) + { + manifest_change.get_or_insert(abs_path); } } + + manifest_change + }; + + if let Some(path) = workspace_manifest_change { + tracing::info!( + %path, + "server file watcher reported a workspace manifest change" + ); + let config = Arc::make_mut(&mut self.config_state.config); + config.refresh_project_manifests(); + self.request_workspace_auto_reload(format!("server file watcher change: {path}")); } } } diff --git a/src/global_state/handlers/notification/text_changes.rs b/src/global_state/handlers/notification/text_changes.rs index 2e59d7bc..ce92c9cd 100644 --- a/src/global_state/handlers/notification/text_changes.rs +++ b/src/global_state/handlers/notification/text_changes.rs @@ -5,7 +5,7 @@ use utils::{ line_index::LineIndex, lines::{LineEnding, LineInfo, PositionEncoding}, }; -use vfs::{FileId, VfsPath, loader::LoadResult}; +use vfs::{FileId, VfsPath}; use crate::{global_state::GlobalState, lsp_ext::from_proto}; @@ -16,8 +16,15 @@ pub(super) fn set_vfs_file_contents( ) -> anyhow::Result { let (text, endings) = LineEnding::normalize(text); let mut vfs = state.workspace.vfs.write(); - vfs.0.set_file_contents(path, LoadResult::Loaded(text, endings)); - vfs.0.file_id(path).ok_or_else(|| anyhow::format_err!("loaded file has no FileId: {path}")) + let path = path.clone(); + vfs.0.set_file_contents(path.clone(), Some(text.into_bytes())); + let file_id = vfs + .0 + .file_id(&path) + .map(|(id, _)| id) + .ok_or_else(|| anyhow::format_err!("loaded file has no FileId: {path}"))?; + vfs.1.insert(file_id, endings); + Ok(file_id) } pub(super) fn open_vfs_file_contents( @@ -26,25 +33,29 @@ pub(super) fn open_vfs_file_contents( text: &str, ) -> anyhow::Result { let mut vfs = state.workspace.vfs.write(); - let file_id = vfs.0.register_file_ingress(path); - if state.analysis.mem_docs.contains_file_id(file_id) { + if let Some((file_id, _)) = vfs.0.file_id(path) + && state.analysis.mem_docs.contains_file_id(file_id) + { return Ok(file_id); } let (text, endings) = LineEnding::normalize(text.to_owned()); - vfs.0.set_file_contents(path, LoadResult::Loaded(text, endings)); - vfs.0.file_id(path).ok_or_else(|| anyhow::format_err!("loaded file has no FileId: {path}")) + let path = path.clone(); + vfs.0.set_file_contents(path.clone(), Some(text.into_bytes())); + let file_id = vfs + .0 + .file_id(&path) + .map(|(id, _)| id) + .ok_or_else(|| anyhow::format_err!("loaded file has no FileId: {path}"))?; + vfs.1.insert(file_id, endings); + Ok(file_id) } pub(super) fn open_mem_doc_file_id(state: &GlobalState, path: &VfsPath) -> Option { state.analysis.mem_docs.file_id(path).or_else(|| { - state - .workspace - .vfs - .read() - .0 - .file_id(path) - .filter(|file_id| state.analysis.mem_docs.contains_file_id(*file_id)) + state.workspace.vfs.read().0.file_id(path).and_then(|(file_id, _)| { + state.analysis.mem_docs.contains_file_id(file_id).then_some(file_id) + }) }) } diff --git a/src/global_state/handlers/request/code_action.rs b/src/global_state/handlers/request/code_action.rs index 40ebe450..081eae0d 100644 --- a/src/global_state/handlers/request/code_action.rs +++ b/src/global_state/handlers/request/code_action.rs @@ -286,7 +286,7 @@ mod tests { option_name: Option<&str>, ) -> IdeDiagnostic { IdeDiagnostic { - file_id: FileId(0), + file_id: FileId::from_raw(0), code, subsystem, name: name.to_owned(), @@ -368,7 +368,7 @@ mod tests { data: None, }; let ide_diag = IdeDiagnostic { - file_id: FileId(0), + file_id: FileId::from_raw(0), code: 129, subsystem: 6, name: "MixingOrderedAndNamedPorts".to_owned(), @@ -393,7 +393,7 @@ mod tests { #[test] fn code_action_diagnostics_are_built_from_server_diagnostics() { let diag = IdeDiagnostic { - file_id: FileId(0), + file_id: FileId::from_raw(0), code: 129, subsystem: 6, name: "MixingOrderedAndNamedPorts".to_owned(), diff --git a/src/global_state/handlers/request/symbols.rs b/src/global_state/handlers/request/symbols.rs index 990389d6..7b9cb8e8 100644 --- a/src/global_state/handlers/request/symbols.rs +++ b/src/global_state/handlers/request/symbols.rs @@ -38,7 +38,7 @@ pub(crate) fn handle_workspace_symbol( params: lsp_types::WorkspaceSymbolParams, ) -> anyhow::Result> { let mut file_ids = snap.file_ids(); - file_ids.sort_unstable_by_key(|file_id| file_id.0); + file_ids.sort_unstable_by_key(|file_id| file_id.index()); file_ids.dedup(); let symbols = snap.analysis.workspace_symbol(¶ms.query, file_ids)?; diff --git a/src/global_state/main_loop.rs b/src/global_state/main_loop.rs index 03c20286..f6a64cbd 100644 --- a/src/global_state/main_loop.rs +++ b/src/global_state/main_loop.rs @@ -51,16 +51,6 @@ impl GlobalState { }; handler(self, res) } - - pub(in crate::global_state) fn drain_pending_workspace_readiness_requests(&mut self) { - let pending_requests = - std::mem::take(&mut self.workspace.pending_workspace_readiness_requests); - for req in pending_requests { - if !self.is_completed(&req) { - self.handle_request(req); - } - } - } } #[cfg(test)] @@ -78,8 +68,8 @@ mod tests { use project_model::{ProjectModel, project_manifest::ProjectManifest}; use rustc_hash::FxHashSet; use triomphe::Arc; - use utils::{lines::LineEnding, paths::AbsPathBuf, test_support::TestDir}; - use vfs::{FileId, VfsPath, loader as vfs_loader, loader::LoadResult}; + use utils::{paths::AbsPathBuf, test_support::TestDir}; + use vfs::{FileId, VfsPath, loader as vfs_loader}; use super::*; use crate::{ @@ -182,7 +172,7 @@ mod tests { ); let (server, client) = Connection::memory(); let mut state = GlobalState::new(server.sender, config, lsp_types::TraceValue::Off); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let primary_uri = to_proto::url_from_abs_path(root.write("workspace/top.sv", "").as_path()).unwrap(); let alias_uri = @@ -247,7 +237,7 @@ mod tests { ); let (server, client) = Connection::memory(); let mut state = GlobalState::new(server.sender, config, lsp_types::TraceValue::Off); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let alias_uri = to_proto::url_from_abs_path(root.write("alias/top.sv", "").as_path()).unwrap(); let diagnostic = Diagnostic { @@ -306,7 +296,7 @@ mod tests { let (server, client) = Connection::memory(); let mut state = GlobalState::new(server.sender, config, lsp_types::TraceValue::Off); state.diagnostics.diagnostics_revision = 2; - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let uri = to_proto::url_from_abs_path(root.write("workspace/top.sv", "").as_path()).unwrap(); let diagnostic = Diagnostic { @@ -327,28 +317,22 @@ mod tests { } #[test] - fn stale_loaded_batches_do_not_update_vfs() { - let root = TestDir::new("stale-loaded-batches"); + fn content_batches_apply_without_generation_token() { + let root = TestDir::new("content-batches-apply"); let root_path = root.path().to_path_buf(); - let file_path = root_path.join("stale.sv"); + let file_path = root_path.join("file.sv"); let mut state = test_state(root_path); - let stale_load = state.workspace.workspace_vfs.begin_vfs_load(1); - let current_load = state.workspace.workspace_vfs.begin_vfs_load(1); - assert!(!stale_load.superseded_client_progress_active); - assert!(!current_load.superseded_client_progress_active); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); state.process_vfs_msg(vfs_loader::Message::Loaded { - files: vec![( - file_path.clone(), - LoadResult::Loaded("module stale; endmodule\n".to_string(), LineEnding::Unix), - )], - config_version: 1, + files: vec![(file_path.clone(), Some(b"module top; endmodule\n".to_vec()))], }); let vfs_path = VfsPath::from(file_path); let mut vfs = state.workspace.vfs.write(); - assert!(vfs.0.file_id(&vfs_path).is_none()); - assert!(vfs.0.take_changes().is_empty()); + assert!(vfs.0.file_id(&vfs_path).is_some()); + assert!(!vfs.0.take_changes().is_empty()); } #[test] @@ -371,7 +355,8 @@ mod tests { state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 0, - n_done: 0, + n_done: vfs_loader::LoadingProgress::Finished, + dir: None, config_version, }); @@ -380,10 +365,25 @@ mod tests { } #[test] - fn diagnostic_requests_are_parked_until_workspace_ready() { - let root = TestDir::new("diagnostic-request-readiness-queue"); + fn diagnostic_request_gets_terminal_fallback_before_workspace_ready() { + let root = TestDir::new("diagnostic-request-readiness-fallback"); let root_path = root.path().to_path_buf(); - let mut state = test_state(root_path); + let (mut state, client) = test_state_with_caps( + root_path, + ClientCapabilities { + text_document: Some(lsp_types::TextDocumentClientCapabilities { + diagnostic: Some(Default::default()), + ..Default::default() + }), + workspace: Some(lsp_types::WorkspaceClientCapabilities { + diagnostic: Some(lsp_types::DiagnosticWorkspaceClientCapabilities { + refresh_support: Some(true), + }), + ..Default::default() + }), + ..Default::default() + }, + ); let config_version = state.workspace.workspace_vfs.begin_vfs_load(1).config_version; let request_id = lsp_server::RequestId::from(7); let req = Request::new( @@ -400,31 +400,85 @@ mod tests { state.register_request(Instant::now(), &req); state.handle_request(req); - assert_eq!(state.workspace.pending_workspace_readiness_requests.len(), 1); - assert!(state.tasks.task_pool.receiver.recv_timeout(Duration::from_millis(50)).is_err()); + let response = client.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); + let Message::Response(response) = response else { + panic!("expected immediate workspace diagnostic fallback response"); + }; + assert_eq!(response.id, request_id); + assert!(response.error.is_none(), "{:?}", response.error); + let report: lsp_types::WorkspaceDiagnosticReportResult = + serde_json::from_value(response.result.unwrap()).unwrap(); + let lsp_types::WorkspaceDiagnosticReportResult::Report(report) = report else { + panic!("expected a full workspace diagnostic fallback report"); + }; + assert!(report.items.is_empty()); + assert!(state.tasks.task_pool.receiver.try_recv().is_err()); state .handle_event(Event::Vfs(vfs_loader::Message::Progress { n_total: 1, - n_done: 1, + n_done: vfs_loader::LoadingProgress::Finished, + dir: None, config_version, })) .unwrap(); - assert!(state.workspace.pending_workspace_readiness_requests.is_empty()); - let task = state.tasks.task_pool.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); - let Task::Response(response) = task else { - panic!("expected parked diagnostic request to resume as response task, got {task:?}"); + let refresh = client.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); + let Message::Request(refresh) = refresh else { + panic!("expected diagnostic refresh after workspace readiness"); }; - assert_eq!(response.response.id, request_id); + assert_eq!(refresh.method, lsp_types::request::WorkspaceDiagnosticRefresh::METHOD); } #[test] - fn workspace_symbol_requests_are_parked_until_workspace_ready() { - let root = TestDir::new("workspace-symbol-readiness-queue"); + fn document_diagnostic_request_gets_empty_report_before_workspace_ready() { + let root = TestDir::new("document-diagnostic-readiness-fallback"); let root_path = root.path().to_path_buf(); - let mut state = test_state(root_path); - let config_version = state.workspace.workspace_vfs.begin_vfs_load(1).config_version; + let (mut state, client) = test_state_with_caps(root_path, ClientCapabilities::default()); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); + let request_id = lsp_server::RequestId::from(9); + let req = Request::new( + request_id.clone(), + lsp_types::request::DocumentDiagnosticRequest::METHOD.to_owned(), + lsp_types::DocumentDiagnosticParams { + text_document: lsp_types::TextDocumentIdentifier { + uri: lsp_types::Url::parse("file:///not-loaded-yet.sv").unwrap(), + }, + identifier: None, + previous_result_id: None, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }, + ); + + state.register_request(Instant::now(), &req); + state.handle_request(req); + + let response = client.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); + let Message::Response(response) = response else { + panic!("expected immediate document diagnostic fallback response"); + }; + assert_eq!(response.id, request_id); + assert!(response.error.is_none(), "{:?}", response.error); + let report: lsp_types::DocumentDiagnosticReportResult = + serde_json::from_value(response.result.unwrap()).unwrap(); + let lsp_types::DocumentDiagnosticReportResult::Report( + lsp_types::DocumentDiagnosticReport::Full(report), + ) = report + else { + panic!("expected a full document diagnostic fallback report"); + }; + assert!(report.full_document_diagnostic_report.result_id.is_none()); + assert!(report.full_document_diagnostic_report.items.is_empty()); + assert!(state.tasks.task_pool.receiver.try_recv().is_err()); + } + + #[test] + fn workspace_symbol_request_gets_empty_result_before_workspace_ready() { + let root = TestDir::new("workspace-symbol-readiness-fallback"); + let root_path = root.path().to_path_buf(); + let (mut state, client) = test_state_with_caps(root_path, ClientCapabilities::default()); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); let request_id = lsp_server::RequestId::from(8); let req = Request::new( request_id.clone(), @@ -439,25 +493,16 @@ mod tests { state.register_request(Instant::now(), &req); state.handle_request(req); - assert_eq!(state.workspace.pending_workspace_readiness_requests.len(), 1); - assert!(state.tasks.task_pool.receiver.try_recv().is_err()); - - state - .handle_event(Event::Vfs(vfs_loader::Message::Progress { - n_total: 1, - n_done: 1, - config_version, - })) - .unwrap(); - - assert!(state.workspace.pending_workspace_readiness_requests.is_empty()); - let task = state.tasks.task_pool.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); - let Task::Response(response) = task else { - panic!( - "expected parked workspace symbol request to resume as response task, got {task:?}" - ); + let response = client.receiver.recv_timeout(Duration::from_secs(1)).unwrap(); + let Message::Response(response) = response else { + panic!("expected immediate workspace symbol fallback response"); }; - assert_eq!(response.response.id, request_id); + assert_eq!(response.id, request_id); + assert!(response.error.is_none(), "{:?}", response.error); + let symbols: Option = + serde_json::from_value(response.result.unwrap()).unwrap(); + assert_eq!(symbols, Some(lsp_types::WorkspaceSymbolResponse::Flat(Vec::new()))); + assert!(state.tasks.task_pool.receiver.try_recv().is_err()); } #[test] @@ -535,7 +580,8 @@ mod tests { state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 4, - n_done: 4, + n_done: vfs_loader::LoadingProgress::Finished, + dir: None, config_version: stale_config, }); @@ -552,7 +598,8 @@ mod tests { state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 4, - n_done: 2, + n_done: vfs_loader::LoadingProgress::Progress(2), + dir: None, config_version: current_config, }); @@ -595,7 +642,8 @@ mod tests { let stale_config = state.workspace.workspace_vfs.begin_vfs_load(4).config_version; state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 4, - n_done: 0, + n_done: vfs_loader::LoadingProgress::Started, + dir: None, config_version: stale_config, }); assert!(matches!(recv_work_done_progress(&client), WorkDoneProgress::Begin(_))); @@ -630,7 +678,8 @@ mod tests { state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 2, - n_done: 2, + n_done: vfs_loader::LoadingProgress::Finished, + dir: None, config_version, }); @@ -643,7 +692,8 @@ mod tests { state.process_vfs_msg(vfs_loader::Message::Progress { n_total: 2, - n_done: 1, + n_done: vfs_loader::LoadingProgress::Progress(1), + dir: None, config_version, }); diff --git a/src/global_state/mem_docs.rs b/src/global_state/mem_docs.rs index 9b054416..d79925e4 100644 --- a/src/global_state/mem_docs.rs +++ b/src/global_state/mem_docs.rs @@ -94,6 +94,7 @@ impl MemDocs { true } + #[allow(dead_code)] pub(crate) fn remap_file_id(&mut self, from: FileId, to: FileId) { if from == to { return; @@ -219,8 +220,8 @@ mod tests { fn mem_docs_alias_matrix() { let canonical_path = VfsPath::new_virtual_path("/workspace/top.sv".to_owned()); let alias_path = VfsPath::new_virtual_path("/alias/top.sv".to_owned()); - let duplicate = FileId(1); - let canonical = FileId(0); + let duplicate = FileId::from_raw(1); + let canonical = FileId::from_raw(0); let mut report = String::new(); let path = canonical_path.clone(); diff --git a/src/global_state/process_changes.rs b/src/global_state/process_changes.rs index 5e97e91c..4d12ad0f 100644 --- a/src/global_state/process_changes.rs +++ b/src/global_state/process_changes.rs @@ -12,7 +12,6 @@ use vfs::{ChangedFile, FileId, Vfs, VfsPath}; use super::{ DEFAULT_REQ_HANDLER, GlobalState, diagnostics::publisher::{PublishDiagnosticsBatch, PublishDiagnosticsTask}, - reload::should_refresh_for_change, task::Task, }; use crate::{config::user_config::DiagnosticsUpdateUserConfig, lsp_ext::to_proto}; @@ -28,23 +27,12 @@ impl GlobalState { pub(crate) fn process_changes(&mut self) -> bool { let pending_diagnostic_targets = std::mem::take(&mut self.diagnostics.pending_document_diagnostic_targets); - let mut diagnostic_targets_changed = !pending_diagnostic_targets.is_empty(); + let diagnostic_targets_changed = !pending_diagnostic_targets.is_empty(); let mut write_guard = self.workspace.vfs.write(); - let changed_files = write_guard.0.take_changes(); + let changed_files: Vec = write_guard.0.take_changes().into_values().collect(); // downgrade earlier to allow more reader let read_guard = RwLockWriteGuard::downgrade_to_upgradable(write_guard); let vfs = &read_guard.0; - let file_id_redirects = changed_files - .iter() - .filter_map(|changed_file| { - let canonical = vfs.canonical_file_id(changed_file.file_id); - (canonical != changed_file.file_id).then_some((changed_file.file_id, canonical)) - }) - .collect_vec(); - diagnostic_targets_changed |= !file_id_redirects.is_empty(); - for (from, to) in file_id_redirects { - self.analysis.mem_docs.remap_file_id(from, to); - } // collect changes let Some(changed_files) = Self::colease_modifications(changed_files) else { @@ -56,7 +44,6 @@ impl GlobalState { return false; }; - let mut workspace_structure_change = None; let mut has_structure_changes = false; // Any file was added or deleted let mut bytes = vec![]; let mut changed_file_ids = FxHashSet::default(); @@ -64,24 +51,12 @@ impl GlobalState { let mut deleted_file_ids = FxHashSet::default(); let mut deleted_push_diagnostics = Vec::new(); for changed_file in changed_files { - let is_identity_redirect = - vfs.canonical_file_id(changed_file.file_id) != changed_file.file_id; - let path = if is_identity_redirect { - vfs.original_file_path(changed_file.file_id) - } else { - vfs.file_path(changed_file.file_id) - }; - if let Some(path) = - path.and_then(|path| path.as_abs_path()).map(|apath| apath.to_path_buf()) - { - let created_or_deleted = changed_file.is_created_or_deleted(); - has_structure_changes |= created_or_deleted; - if !is_identity_redirect && should_refresh_for_change(&path, created_or_deleted) { - workspace_structure_change = Some(path.clone()); - } + let path = Some(vfs.file_path(changed_file.file_id)); + if path.and_then(|path| path.as_abs_path()).is_some() { + has_structure_changes |= changed_file.is_created_or_deleted(); } - if matches!(&changed_file.change_kind, vfs::ChangeKind::Delete) { + if matches!(changed_file.kind(), vfs::ChangeKind::Delete) { deleted_file_ids.insert(changed_file.file_id); if let Some(path) = path.cloned() { deleted_push_diagnostics.push((changed_file.file_id, path)); @@ -146,12 +121,6 @@ impl GlobalState { self.request_diagnostics(pending_diagnostic_targets.into_iter().collect()); } - if let Some(path) = workspace_structure_change { - let config = triomphe::Arc::make_mut(&mut self.config_state.config); - config.refresh_project_manifests(); - self.request_workspace_auto_reload(format!("workspace vfs change: {:?}", path)); - } - true } @@ -276,8 +245,13 @@ impl GlobalState { let mut change = Change::new(); for changed_file in bytes { let file_id = changed_file.file_id; - if let Some(line_ending) = changed_file.ending() { - line_ending_map.insert(file_id, line_ending); + if let Some(raw) = changed_file.contents() { + if let Ok(text) = std::str::from_utf8(raw) { + let (_, ending) = LineEnding::normalize(text.to_owned()); + line_ending_map.insert(file_id, ending); + } + } else { + line_ending_map.remove(&file_id); } change.add_changed_file(changed_file) } @@ -294,8 +268,8 @@ impl GlobalState { return None; } - // collapse modifications - use vfs::ChangeKind::*; + // collapse modifications (r-a Change carries raw bytes + content hash) + use vfs::Change::*; let mut file_changes = FxHashMap::default(); for changed_file in vfs_changes { @@ -303,54 +277,52 @@ impl GlobalState { Occupied(mut entry) => { let (change, just_created) = entry.get_mut(); - match (change, *just_created, changed_file.change_kind) { + match (change, *just_created, changed_file.change) { (change, _, Delete) => *change = Delete, ( - Create(prev, prev_ending), + Create(prev, prev_hash), _, - Create(new, new_ending) | Modify(new, new_ending), + Create(new, new_hash) | Modify(new, new_hash), ) => { *prev = new; - *prev_ending = new_ending; + *prev_hash = new_hash; } - (Modify(prev, prev_ending), _, Modify(new, new_ending)) => { + (Modify(prev, prev_hash), _, Modify(new, new_hash)) => { *prev = new; - *prev_ending = new_ending; + *prev_hash = new_hash; } - (change @ Delete, _, Create(new, new_ending)) => { - *change = Modify(new, new_ending); + (change @ Delete, _, Create(new, new_hash)) => { + *change = Modify(new, new_hash); *just_created = true; } - (change @ Delete, _, Modify(new, new_ending)) => { + (change @ Delete, _, Modify(new, new_hash)) => { tracing::debug!( ?changed_file.file_id, "received modify after delete while coalescing VFS changes" ); - *change = Modify(new, new_ending); + *change = Modify(new, new_hash); } - (Modify(prev, prev_ending), _, Create(new, new_ending)) => { + (Modify(prev, prev_hash), _, Create(new, new_hash)) => { tracing::debug!( ?changed_file.file_id, "received create after modify while coalescing VFS changes" ); *prev = new; - *prev_ending = new_ending; + *prev_hash = new_hash; } } } Vacant(v) => { - let just_created = matches!(&changed_file.change_kind, Create(_, _)); - v.insert((changed_file.change_kind, just_created)); + let just_created = matches!(&changed_file.change, Create(_, _)); + v.insert((changed_file.change, just_created)); } } } let changed_file = file_changes .into_iter() - .filter(|(_, (change_kind, just_created))| { - !(*just_created && matches!(change_kind, Delete)) - }) - .map(|(file_id, (change_kind, _))| ChangedFile { file_id, change_kind }) + .filter(|(_, (change, just_created))| !(*just_created && matches!(change, Delete))) + .map(|(file_id, (change, _))| ChangedFile { file_id, change }) .collect_vec(); Some(changed_file) @@ -421,8 +393,8 @@ impl GlobalState { mod tests { use lsp_server::Connection; use lsp_types::{ClientCapabilities, TraceValue}; - use utils::{lines::LineEnding, test_support::TestDir}; - use vfs::{VfsPath, loader::LoadResult}; + use utils::{paths::AbsPathBuf, test_support::TestDir}; + use vfs::{VfsPath, loader::Message as VfsMessage}; use crate::{ Opt, @@ -431,10 +403,7 @@ mod tests { i18n::I18n, }; - #[test] - fn ordinary_file_creation_does_not_request_workspace_reload() { - let root = TestDir::new("ordinary-file-no-workspace-reload"); - let root_path = root.path().to_path_buf(); + fn test_state(root_path: AbsPathBuf) -> GlobalState { let config = config::Config::new( Opt { process_name: "vide-test".to_string(), @@ -450,13 +419,27 @@ mod tests { Vec::new(), ); let (server, _client) = Connection::memory(); - let mut state = GlobalState::new(server.sender, config, TraceValue::Off); + GlobalState::new(server.sender, config, TraceValue::Off) + } + + fn vfs_manifest_message(manifest_path: AbsPathBuf, changed: bool) -> VfsMessage { + let files = vec![(manifest_path, Some(b"[project]\n".to_vec()))]; + if changed { VfsMessage::Changed { files } } else { VfsMessage::Loaded { files } } + } + + #[test] + fn ordinary_file_creation_does_not_request_workspace_reload() { + let root = TestDir::new("ordinary-file-no-workspace-reload"); + let root_path = root.path().to_path_buf(); + let mut state = test_state(root_path); let file_path = root.join("top.sv"); - state.workspace.vfs.write().0.set_file_contents( - &VfsPath::from(file_path), - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), - ); + state + .workspace + .vfs + .write() + .0 + .set_file_contents(VfsPath::from(file_path), Some(b"module top; endmodule\n".to_vec())); assert!(state.process_changes()); assert!( @@ -464,4 +447,50 @@ mod tests { "loading an ordinary source file should not queue a project configuration reload" ); } + + #[test] + fn config_scan_manifest_does_not_request_workspace_reload() { + let root = TestDir::new("config-scan-manifest-no-reload"); + let mut state = test_state(root.path().to_path_buf()); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); + + state.process_vfs_msg(vfs_manifest_message(root.join("vide.toml"), false)); + assert!(state.process_changes()); + + assert!( + !state.workspace.fetch_workspaces_task.has_op_requested(), + "initial manifest discovery must not recursively reload the workspace" + ); + } + + #[test] + fn external_manifest_change_requests_workspace_reload() { + let root = TestDir::new("external-manifest-reload"); + let mut state = test_state(root.path().to_path_buf()); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); + + state.process_vfs_msg(vfs_manifest_message(root.join("vide.toml"), true)); + + assert!( + state.workspace.fetch_workspaces_task.has_op_requested(), + "an external manifest change must reload the workspace" + ); + } + + #[test] + fn unchanged_external_manifest_does_not_request_workspace_reload() { + let root = TestDir::new("unchanged-external-manifest-no-reload"); + let mut state = test_state(root.path().to_path_buf()); + let manifest_path = root.join("vide.toml"); + let _ = state.workspace.workspace_vfs.begin_vfs_load(1); + + state.process_vfs_msg(vfs_manifest_message(manifest_path.clone(), false)); + state.process_vfs_msg(vfs_manifest_message(manifest_path, true)); + assert!(state.process_changes()); + + assert!( + !state.workspace.fetch_workspaces_task.has_op_requested(), + "an unchanged watcher event must not reclassify an initial scan change" + ); + } } diff --git a/src/global_state/qihe/tests.rs b/src/global_state/qihe/tests.rs index cbab0585..f72beb75 100644 --- a/src/global_state/qihe/tests.rs +++ b/src/global_state/qihe/tests.rs @@ -147,7 +147,7 @@ fn stale_qihe_result_does_not_replace_current_diagnostics() { let (server, _client) = lsp_server::Connection::memory(); let mut state = GlobalState::new(server.sender, config, TraceValue::Off); state.qihe.run_generation = QiheRunId::new(2); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let current = Diagnostic { range: Range::new(Position::new(0, 0), Position::new(0, 1)), severity: Some(DiagnosticSeverity::WARNING), @@ -214,7 +214,7 @@ fn current_qihe_result_closes_active_progress() { #[test] fn cancelled_qihe_finished_result_does_not_commit_diagnostics() { let (_root, mut state) = new_test_state("cancelled-qihe-finished-result"); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let diagnostic = Diagnostic { range: Range::new(Position::new(0, 0), Position::new(0, 1)), severity: Some(DiagnosticSeverity::WARNING), @@ -295,7 +295,7 @@ fn qihe_diagnostics_are_scoped_to_diagnostic_commit_freshness() { ); let (server, _client) = lsp_server::Connection::memory(); let mut state = GlobalState::new(server.sender, config, TraceValue::Off); - let file_id = FileId(0); + let file_id = FileId::from_raw(0); let diagnostic = Diagnostic { range: Range::new(Position::new(0, 0), Position::new(0, 1)), severity: Some(DiagnosticSeverity::WARNING), @@ -359,7 +359,7 @@ fn qihe_result_with_stale_diagnostic_freshness_does_not_commit() { run_id: QiheRunId::new(1), update: QiheUpdate { by_file: rustc_hash::FxHashMap::from_iter([( - FileId(0), + FileId::from_raw(0), vec![Diagnostic { range: Range::new(Position::new(0, 0), Position::new(0, 1)), severity: Some(DiagnosticSeverity::WARNING), @@ -409,7 +409,7 @@ fn qihe_diagnostics_use_pull_refresh_for_pull_capable_clients() { let (server, client) = lsp_server::Connection::memory(); let mut state = GlobalState::new(server.sender, config, TraceValue::Off); - state.publish_qihe_diagnostics(rustc_hash::FxHashSet::from_iter([FileId(0)])); + state.publish_qihe_diagnostics(rustc_hash::FxHashSet::from_iter([FileId::from_raw(0)])); let message = client .receiver diff --git a/src/global_state/reload.rs b/src/global_state/reload.rs index de069f19..bf15abba 100644 --- a/src/global_state/reload.rs +++ b/src/global_state/reload.rs @@ -160,7 +160,7 @@ impl GlobalState { } let files_config = self.config_state.config.files(); - let (to_load, to_watch, source_root_config, project_config) = + let (load, watch, source_root_config, project_config) = get_workspace_folder(&self.workspace.workspaces, &files_config.exclude); let mut change = Change::new(); { @@ -171,13 +171,13 @@ impl GlobalState { change.set_project_config(project_config); self.analysis.analysis_host.apply_change(change); - let to_watch = match files_config.watcher { + let watch = match files_config.watcher { FilesWatcher::Client => vec![], - FilesWatcher::Server => to_watch, + FilesWatcher::Server => watch, }; - let to_load_len = to_load.len(); - let vfs_load = self.workspace.workspace_vfs.begin_vfs_load(to_load_len); + let load_len = load.len(); + let vfs_load = self.workspace.workspace_vfs.begin_vfs_load(load_len); if vfs_load.superseded_client_progress_active { self.report_progress( self.config_state.config.i18n.text(keys::PROGRESS_ROOTS_SCANNING), @@ -189,8 +189,8 @@ impl GlobalState { } self.workspace.vfs_loader.handle.set_config(vfs::loader::Config { - to_load, - to_watch, + load, + watch, version: vfs_load.config_version, }); diff --git a/src/global_state/snapshot.rs b/src/global_state/snapshot.rs index 020d4492..27de92a6 100644 --- a/src/global_state/snapshot.rs +++ b/src/global_state/snapshot.rs @@ -1,6 +1,5 @@ use std::{path::Path, sync::Arc as StdArc}; -use anyhow::Context; use hir::base_db::source_root::{SourceRootDiagnosticScope, SourceRootRole}; use ide::{Cancellable, analysis::Analysis}; use lsp_types::Url; @@ -89,7 +88,7 @@ impl GlobalStateSnapshot { pub(crate) fn file_id(&self, url: &lsp_types::Url) -> anyhow::Result { let path = from_proto::vfs_path(url)?; let vfs = self.vfs_read(); - let file_id = + let (file_id, _) = vfs.file_id(&path).ok_or_else(|| anyhow::format_err!("file not found: {path}"))?; Ok(file_id) } @@ -104,21 +103,17 @@ impl GlobalStateSnapshot { pub(crate) fn file_id_for_path(&self, path: &Path) -> Option { let path = VfsPath::from(AbsPathBuf::try_from(path.to_path_buf()).ok()?); - self.vfs_read().file_id(&path) + self.vfs_read().file_id(&path).map(|(file_id, _)| file_id) } pub(crate) fn file_path(&self, file_id: FileId) -> Option { - self.vfs_read().file_path(file_id)?.as_abs_path().map(ToOwned::to_owned) + self.vfs_read().file_path(file_id).as_abs_path().map(ToOwned::to_owned) } pub(crate) fn line_info(&self, file_id: FileId) -> anyhow::Result { let ending = { let vfs = self.vfs.read(); - vfs.1 - .get(&file_id) - .copied() - .or_else(|| vfs.0.line_ending(file_id)) - .with_context(|| format!("missing line ending metadata for {file_id:?}"))? + vfs.1.get(&file_id).copied().unwrap_or(utils::lines::LineEnding::Unix) }; let index = self.analysis.line_index(file_id)?; let encoding = self.config.position_encoding(); @@ -458,8 +453,7 @@ impl GlobalStateSnapshot { /// URI and document version come from the same open document spelling. pub(crate) fn url(&self, id: FileId) -> anyhow::Result { let vfs = &self.vfs_read(); - let path = - vfs.file_path(id).ok_or_else(|| anyhow::format_err!("unknown file id: {id:?}"))?; + let path = vfs.file_path(id); let path = path .as_abs_path() .ok_or_else(|| anyhow::format_err!("file {id:?} has no file URI: {path}"))?; @@ -517,8 +511,8 @@ impl GlobalStateSnapshot { mod tests { use lsp_server::Connection; use lsp_types::{ClientCapabilities, TraceValue}; - use utils::{lines::LineEnding, test_support::TestDir}; - use vfs::{FileId, VfsPath, loader::LoadResult}; + use utils::test_support::TestDir; + use vfs::{FileId, VfsPath /* removed */}; use crate::{ Opt, @@ -549,22 +543,22 @@ mod tests { let mut state = GlobalState::new(server.sender, config, TraceValue::Off); state.workspace.vfs.write().0.set_file_contents( - &VfsPath::from(root.join("top.sv")), - LoadResult::Loaded("module top; endmodule\n".to_owned(), LineEnding::Unix), + VfsPath::from(root.join("top.sv")), + Some(b"module top; endmodule\n".to_vec()), ); assert!(state.process_changes()); let snapshot = state.make_snapshot(); state.workspace.vfs.write().0.set_file_contents( - &VfsPath::from(root.join("child.sv")), - LoadResult::Loaded("module child; endmodule\n".to_owned(), LineEnding::Unix), + VfsPath::from(root.join("child.sv")), + Some(b"module child; endmodule\n".to_vec()), ); let mut live_file_ids = state.workspace.vfs.read().0.iter().map(|(file_id, _)| file_id).collect::>(); - live_file_ids.sort_unstable_by_key(|file_id| file_id.0); - assert_eq!(live_file_ids, vec![FileId(0), FileId(1)]); + live_file_ids.sort_unstable_by_key(|file_id| file_id.index()); + assert_eq!(live_file_ids, vec![FileId::from_raw(0), FileId::from_raw(1)]); - assert_eq!(snapshot.file_ids(), vec![FileId(0)]); + assert_eq!(snapshot.file_ids(), vec![FileId::from_raw(0)]); } } diff --git a/src/global_state/workspace_state.rs b/src/global_state/workspace_state.rs index da04b785..2534d64a 100644 --- a/src/global_state/workspace_state.rs +++ b/src/global_state/workspace_state.rs @@ -28,6 +28,7 @@ pub(crate) struct VfsProgress { } impl VfsProgress { + #[allow(dead_code)] pub(crate) fn in_progress(&self) -> bool { self.n_done < self.n_total } @@ -142,16 +143,20 @@ impl WorkspaceVfsReadiness { config_version: u32, n_done: usize, n_total: usize, + finished: bool, ) -> Option { if config_version != self.vfs_config_version { return None; } - if n_done < self.vfs_progress.n_done { + if !finished && n_done < self.vfs_progress.n_done { return None; } self.vfs_progress = VfsProgress { config_version, n_done, n_total }; - self.vfs_ready = !self.vfs_progress.in_progress(); + self.vfs_ready = finished || (n_total == 0 && n_done == 0); + if finished { + self.vfs_progress.n_done = n_total; + } Some(self.vfs_progress) } @@ -163,10 +168,6 @@ impl WorkspaceVfsReadiness { self.vfs_client_progress_active = false; } - pub(crate) fn accepts_vfs_loaded(&self, config_version: u32) -> bool { - config_version == self.vfs_config_version - } - pub(crate) fn current_vfs_config_version(&self) -> u32 { self.vfs_config_version } diff --git a/src/lsp_ext/to_proto.rs b/src/lsp_ext/to_proto.rs index d13b6fb6..56af331d 100644 --- a/src/lsp_ext/to_proto.rs +++ b/src/lsp_ext/to_proto.rs @@ -1032,7 +1032,7 @@ mod tests { encoding: PositionEncoding::Utf8, }; let diag = IdeDiagnostic { - file_id: FileId(0), + file_id: FileId::from_raw(0), code: 2, subsystem: 0, name: "inactive-preprocessor-branch".to_owned(), diff --git a/src/tests.rs b/src/tests.rs index 744f0b33..7fa779ed 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -370,7 +370,42 @@ fn request_document_diagnostics_with_previous_result_id( request_id: i32, previous_result_id: Option, ) -> (Option, Vec) { - let request_id = lsp_server::RequestId::from(request_id); + for attempt in 0..50 { + let current_request_id = if attempt == 0 { + lsp_server::RequestId::from(request_id) + } else { + lsp_server::RequestId::from(format!( + "document-diagnostic-{request_id}-readiness-{attempt}" + )) + }; + let result = request_document_diagnostics_once( + client, + uri.clone(), + current_request_id, + previous_result_id.clone(), + ); + // A diagnostically enabled document gets a result id once the workspace + // is ready, even when its diagnostic list is legitimately empty. The + // cold-start fallback deliberately has no result id. Do not use the + // item count as a readiness signal: an empty list is valid data. + if result.0.is_some() { + return result; + } + wait_for_workspace_diagnostic_refresh_or_tick( + client, + "document diagnostics workspace readiness", + ); + } + + panic!("document diagnostics remained in the cold-start fallback state"); +} + +fn request_document_diagnostics_once( + client: &Connection, + uri: Url, + request_id: lsp_server::RequestId, + previous_result_id: Option, +) -> (Option, Vec) { client .sender .send(Message::Request(Request::new( @@ -732,7 +767,18 @@ fn request_workspace_diagnostic_report( request_id: i32, previous_result_ids: Vec, ) -> lsp_types::WorkspaceDiagnosticReport { - let request_id = lsp_server::RequestId::from(request_id); + request_workspace_diagnostic_report_once( + client, + lsp_server::RequestId::from(request_id), + previous_result_ids, + ) +} + +fn request_workspace_diagnostic_report_once( + client: &Connection, + request_id: lsp_server::RequestId, + previous_result_ids: Vec, +) -> lsp_types::WorkspaceDiagnosticReport { client .sender .send(Message::Request(Request::new( diff --git a/src/tests/code_actions.rs b/src/tests/code_actions.rs index 3322df41..2486f96f 100644 --- a/src/tests/code_actions.rs +++ b/src/tests/code_actions.rs @@ -180,22 +180,7 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_diagnostics_test(code_action_client_caps(), UserConfig::default(), text); - let diagnostics_id = lsp_server::RequestId::from(212); - client - .sender - .send(Message::Request(Request::new( - diagnostics_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri: uri.clone() }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let (_result_id, diagnostics) = recv_document_diagnostics(&client, diagnostics_id); + let (_result_id, diagnostics) = request_document_diagnostics(&client, uri.clone(), 212); assert!( diagnostics.iter().any(|diag| diag.message == "expected ';'"), "expected parse diagnostic for missing semicolon, got {diagnostics:?}" diff --git a/src/tests/diagnostics.rs b/src/tests/diagnostics.rs index 6e8045fe..064cc902 100644 --- a/src/tests/diagnostics.rs +++ b/src/tests/diagnostics.rs @@ -76,8 +76,22 @@ fn pull_capable_client_does_not_receive_duplicate_publish_diagnostics() { }), ..Default::default() }; - let (_temp_dir, client, server_thread, uri) = - setup_diagnostics_test(pull_caps, UserConfig::default(), "module broken(;\nendmodule\n"); + let file_text = "module broken(;\nendmodule\n"; + let temp_dir = TempDir::new("pull-diagnostics-no-publish"); + let file_path = temp_dir.path().join("broken.sv"); + let readiness_path = temp_dir.path().join("readiness.sv"); + fs::write(&readiness_path, "module readiness;\nendmodule\n").unwrap(); + let (client, server_thread) = + spawn_test_workspace(temp_dir.path().to_path_buf(), pull_caps, UserConfig::default()); + let uri = to_proto::url_from_abs_path(file_path.as_path()).unwrap(); + let readiness_uri = to_proto::url_from_abs_path(readiness_path.as_path()).unwrap(); + + // Establish workspace readiness before opening the document. This keeps + // the publishDiagnostics observation below intact: no notification for the + // target document can be consumed as part of readiness synchronization. + let _ = request_document_diagnostics(&client, readiness_uri, 100); + open_test_document(&client, uri.clone(), file_text); + let request_id = lsp_server::RequestId::from(1); let request = Request::new( request_id.clone(), @@ -309,23 +323,7 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_diagnostics_test(pull_caps, UserConfig::default(), file_text); - let request_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - request_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - - let (_result_id, diagnostics) = recv_document_diagnostics(&client, request_id); + let (_result_id, diagnostics) = request_document_diagnostics(&client, uri, 1); assert!( diagnostics.iter().all(|diag| !diag.message.contains("port 'b' has no connection")), "unconfigured workspaces should suppress semantic diagnostics: {diagnostics:?}" @@ -393,23 +391,7 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_syntax_only_config_diagnostics_test(pull_caps, UserConfig::default(), file_text); - let request_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - request_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - - let (_result_id, diagnostics) = recv_document_diagnostics(&client, request_id); + let (_result_id, diagnostics) = request_document_diagnostics(&client, uri, 1); assert!( diagnostics.iter().all(|diag| !diag.message.contains("port 'b' has no connection")), "syntax-only configs should suppress semantic diagnostics: {diagnostics:?}" @@ -439,23 +421,7 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_empty_config_diagnostics_test(pull_caps, UserConfig::default(), file_text); - let request_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - request_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - - let (_result_id, diagnostics) = recv_document_diagnostics(&client, request_id); + let (_result_id, diagnostics) = request_document_diagnostics(&client, uri, 1); assert!( diagnostics.iter().all(|diag| !diag.message.contains("port 'b' has no connection")), "empty configs should suppress semantic diagnostics: {diagnostics:?}" @@ -480,23 +446,7 @@ endmodule let (_temp_dir, client, server_thread, uri) = setup_syntax_only_config_diagnostics_test(pull_caps, UserConfig::default(), file_text); - let request_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - request_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - - let (_result_id, diagnostics) = recv_document_diagnostics(&client, request_id); + let (_result_id, diagnostics) = request_document_diagnostics(&client, uri, 1); assert!( diagnostics.iter().any(|diag| diag.message.contains("expected")), "syntax-only configs should still report parse diagnostics: {diagnostics:?}" @@ -529,14 +479,25 @@ fn document_diagnostics_respect_disabled_source_root_policy() { let ignored_path = ignored_dir.join("ignored.sv"); let ignored_text = "module ignored(;\nendmodule\n"; fs::write(&ignored_path, ignored_text).unwrap(); - fs::write(rtl_dir.join("top.sv"), "module top;\nendmodule\n").unwrap(); + let top_path = rtl_dir.join("top.sv"); + fs::write(&top_path, "module top;\nendmodule\n").unwrap(); let (client, server_thread) = spawn_test_workspace(temp_dir.path().to_path_buf(), pull_caps, user_config); let ignored_uri = to_proto::url_from_abs_path(ignored_path.as_path()).unwrap(); + let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); open_test_document(&client, ignored_uri.clone(), ignored_text); - let (result_id, diagnostics) = request_document_diagnostics(&client, ignored_uri, 1); + // The ignored document legitimately has no result id, just like the cold + // fallback. Synchronize through an enabled source first, then make one raw + // request so the two empty responses are never conflated. + let _ = request_document_diagnostics(&client, top_uri, 1); + let (result_id, diagnostics) = request_document_diagnostics_once( + &client, + ignored_uri, + lsp_server::RequestId::from(3), + None, + ); assert!(result_id.is_none(), "disabled source roots should not receive result ids"); assert!( diagnostics.is_empty(), @@ -861,22 +822,7 @@ fn unsaved_library_include_header_changes_are_used_for_dependent_diagnostics() { let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); let header_uri = to_proto::url_from_abs_path(header_path.as_path()).unwrap(); - let first_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - first_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri: top_uri.clone() }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let (_, initial_diagnostics) = recv_document_diagnostics(&client, first_id); + let (_, initial_diagnostics) = request_document_diagnostics(&client, top_uri.clone(), 1); assert!( initial_diagnostics.is_empty(), "saved library include header should define ENABLE_COUNTER: {initial_diagnostics:?}" @@ -975,22 +921,7 @@ fn unsaved_include_header_changes_are_used_for_dependent_diagnostics() { let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); let header_uri = to_proto::url_from_abs_path(header_path.as_path()).unwrap(); - let first_id = lsp_server::RequestId::from(1); - client - .sender - .send(Message::Request(Request::new( - first_id.clone(), - DocumentDiagnosticRequest::METHOD.to_string(), - DocumentDiagnosticParams { - text_document: TextDocumentIdentifier { uri: top_uri.clone() }, - identifier: None, - previous_result_id: None, - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let (_, initial_diagnostics) = recv_document_diagnostics(&client, first_id); + let (_, initial_diagnostics) = request_document_diagnostics(&client, top_uri.clone(), 1); assert!( initial_diagnostics.is_empty(), "saved include header should define ENABLE_COUNTER: {initial_diagnostics:?}" @@ -1053,7 +984,8 @@ fn restored_project_manifest_clears_diagnostics_for_excluded_files() { fs::create_dir_all(&rtl_dir).unwrap(); fs::write(&manifest_path, DEFAULT_TEST_CONFIG).unwrap(); fs::write(ignored_dir.join("ignored.sv"), "module ignored(;\nendmodule\n").unwrap(); - fs::write(rtl_dir.join("top.sv"), "module top;\nendmodule\n").unwrap(); + let top_path = rtl_dir.join("top.sv"); + fs::write(&top_path, "module top;\nendmodule\n").unwrap(); let root_path = temp_dir.path().to_path_buf(); let opt = Opt { @@ -1076,8 +1008,10 @@ fn restored_project_manifest_clears_diagnostics_for_excluded_files() { let server_thread = spawn_default_test_server(config, server); let ignored_uri = to_proto::url_from_abs_path(ignored_dir.join("ignored.sv").as_path()).unwrap(); + let top_uri = to_proto::url_from_abs_path(top_path.as_path()).unwrap(); let manifest_uri = to_proto::url_from_abs_path(manifest_path.as_path()).unwrap(); + let _ = request_document_diagnostics(&client, ignored_uri.clone(), 100); let first_report = request_workspace_diagnostic_report(&client, 1, Vec::new()); let mut saw_ignored_diagnostic = false; let mut ignored_result_id = None; @@ -1113,29 +1047,15 @@ fn restored_project_manifest_clears_diagnostics_for_excluded_files() { ))) .unwrap(); - let second_id = lsp_server::RequestId::from(2); - client - .sender - .send(Message::Request(Request::new( - second_id.clone(), - WorkspaceDiagnosticRequest::METHOD.to_string(), - WorkspaceDiagnosticParams { - identifier: None, - previous_result_ids: vec![lsp_types::PreviousResultId { - uri: ignored_uri.clone(), - value: ignored_result_id, - }], - work_done_progress_params: WorkDoneProgressParams::default(), - partial_result_params: Default::default(), - }, - ))) - .unwrap(); - let second: WorkspaceDiagnosticReportResult = - recv_response(&client, second_id, "workspaceDiagnostic"); - let second_report = match second { - WorkspaceDiagnosticReportResult::Report(report) => report, - other => panic!("unexpected workspace diagnostic response: {other:?}"), - }; + // The second workspace report is expected to contain a legitimate empty + // cleanup entry. Synchronize through the still-enabled top-level source so + // an empty cold fallback cannot satisfy that assertion by accident. + let _ = request_document_diagnostics(&client, top_uri, 101); + let second_report = request_workspace_diagnostic_report( + &client, + 2, + vec![lsp_types::PreviousResultId { uri: ignored_uri.clone(), value: ignored_result_id }], + ); for item in second_report.items { if let lsp_types::WorkspaceDocumentDiagnosticReport::Full(full) = item && full.uri == ignored_uri @@ -1296,6 +1216,7 @@ fn deleted_workspace_file_requests_diagnostic_refresh() { let server_thread = spawn_default_test_server(config, server); let broken_uri = to_proto::url_from_abs_path(broken_path.as_path()).unwrap(); + let _ = request_document_diagnostics(&client, broken_uri.clone(), 100); let first_report = request_workspace_diagnostic_report(&client, 1, Vec::new()); let mut saw_broken_diagnostic = false; let mut broken_result_id = None; diff --git a/src/tests/workspace_symbols.rs b/src/tests/workspace_symbols.rs index 3fc351c1..26750f54 100644 --- a/src/tests/workspace_symbols.rs +++ b/src/tests/workspace_symbols.rs @@ -26,6 +26,11 @@ endmodule &files, ); + // Workspace symbols use the same workspace/VFS readiness boundary. A + // document diagnostic result id gives the test an explicit synchronization + // point before asserting on symbol contents. + let _ = request_document_diagnostics(&client, uris[0].clone(), 180); + let symbols_id = lsp_server::RequestId::from(181); client .sender