diff --git a/crates/common/src/dependencies/graph.rs b/crates/common/src/dependencies/graph.rs index 25ee2ba932..2dcdf3fc6f 100644 --- a/crates/common/src/dependencies/graph.rs +++ b/crates/common/src/dependencies/graph.rs @@ -26,6 +26,11 @@ impl DependencyGraph { self.node_map(db).contains_key(url) } + /// Returns all URLs in the dependency graph. + pub fn all_urls(&self, db: &dyn InputDb) -> Vec { + self.node_map(db).keys().cloned().collect() + } + /// Returns a subgraph containing all cyclic nodes and all nodes that lead to cycles. /// /// This method identifies strongly connected components (SCCs) in the graph and returns diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index ff2eb27188..9566a30d13 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -9,6 +9,7 @@ pub mod urlext; use dependencies::DependencyGraph; use file::Workspace; +pub use tracing; #[salsa::db] // Each database must implement InputDb explicitly with its own storage mechanism diff --git a/crates/driver/src/files.rs b/crates/driver/src/files.rs index 0df4deb94c..dd15a6c510 100644 --- a/crates/driver/src/files.rs +++ b/crates/driver/src/files.rs @@ -1,23 +1 @@ -use camino::Utf8PathBuf; - pub const FE_TOML: &str = "fe.toml"; - -pub fn find_project_root() -> Option { - let mut path = Utf8PathBuf::from_path_buf( - std::env::current_dir().expect("Unable to get current directory"), - ) - .expect("Expected utf8 path"); - - loop { - let fe_toml = path.join(FE_TOML); - if fe_toml.is_file() { - return Some(path); - } - - if !path.pop() { - break; - } - } - - None -} diff --git a/crates/driver/src/lib.rs b/crates/driver/src/lib.rs index 5040dbfb8f..0cf5e1a853 100644 --- a/crates/driver/src/lib.rs +++ b/crates/driver/src/lib.rs @@ -32,15 +32,27 @@ pub fn ingot_graph_resolver<'a>() -> IngotGraphResolver<'a> { .with_required_directory("src") .with_required_file("src/lib.fe") .with_pattern("src/**/*.fe"); + GraphResolverImpl::new(files_resolver) } pub fn init_ingot(db: &mut DriverDataBase, ingot_url: &Url) -> Vec { + // Check if ingot is already initialized + if db.graph().contains_url(db, ingot_url) { + tracing::info!(target: "resolver", "Ingot already initialized: {}", ingot_url); + return Vec::new(); + } + tracing::info!(target: "resolver", "Starting workspace ingot resolution for: {}", ingot_url); + let mut diagnostics: Vec = { let mut handler = InputHandler::from_db(db, ingot_url.clone()); let mut ingot_graph_resolver = ingot_graph_resolver(); + // Always enable search mode for the initial URL + // It will auto-disable after the first resolution so dependencies use exact paths + ingot_graph_resolver.node_resolver.search_mode = true; + // Root ingot resolution should never fail since directory existence is validated earlier. // If it fails, it indicates a bug in the resolver or an unexpected system condition. if let Err(err) = ingot_graph_resolver.graph_resolve(&mut handler, ingot_url) { @@ -304,6 +316,7 @@ impl<'a> GraphResolutionHandler GraphResolutionHandler 0 { + tracing::info!(target: "resolver", "Total ingots in workspace: {}", total_ingots); + } } } diff --git a/crates/fe/src/main.rs b/crates/fe/src/main.rs index 7997121dde..8ab03b4751 100644 --- a/crates/fe/src/main.rs +++ b/crates/fe/src/main.rs @@ -33,7 +33,9 @@ pub enum Command { } fn default_project_path() -> Utf8PathBuf { - driver::files::find_project_root().unwrap_or(Utf8PathBuf::from(".")) + // With search mode enabled in init_ingot, we can just use the current directory + // The FilesResolver will walk up to find the ingot root + Utf8PathBuf::from(".") } fn main() { diff --git a/crates/fe/src/tree.rs b/crates/fe/src/tree.rs index 44092fe709..d6c7607950 100644 --- a/crates/fe/src/tree.rs +++ b/crates/fe/src/tree.rs @@ -73,7 +73,9 @@ pub type IngotGraphResolver = resolver::graph::GraphResolverImpl; pub fn tree_resolver() -> IngotGraphResolver { - let files_resolver = FilesResolver::new().with_required_file("fe.toml"); + let files_resolver = FilesResolver::new() + .with_required_file("fe.toml") + .with_search_mode(); resolver::graph::GraphResolverImpl::new(files_resolver) } diff --git a/crates/language-server/src/backend/db.rs b/crates/language-server/src/backend/db.rs deleted file mode 100644 index a38b2f90b8..0000000000 --- a/crates/language-server/src/backend/db.rs +++ /dev/null @@ -1,34 +0,0 @@ -use common::InputDb; - -use hir::{HirDb, LowerHirDb, SpannedHirDb}; -use hir_analysis::{diagnostics::SpannedHirAnalysisDb, HirAnalysisDb}; - -#[salsa::db] -pub trait LanguageServerDb: - salsa::Database + SpannedHirAnalysisDb + HirAnalysisDb + HirDb + LowerHirDb + SpannedHirDb + InputDb -{ -} - -#[salsa::db] -impl LanguageServerDb for DB where - DB: Sized - + salsa::Database - + SpannedHirAnalysisDb - + HirAnalysisDb - + HirDb - + LowerHirDb - + SpannedHirDb - + InputDb -{ -} - -#[salsa::db] -#[derive(Default, Clone)] -pub struct LanguageServerDatabase { - storage: salsa::Storage, -} - -#[salsa::db] -impl salsa::Database for LanguageServerDatabase { - fn salsa_event(&self, _event: &dyn Fn() -> salsa::Event) {} -} diff --git a/crates/language-server/src/functionality/handlers.rs b/crates/language-server/src/functionality/handlers.rs index b0f9e73215..dd92f3855c 100644 --- a/crates/language-server/src/functionality/handlers.rs +++ b/crates/language-server/src/functionality/handlers.rs @@ -255,6 +255,21 @@ pub async fn handle_file_change( .db .workspace() .update(&mut backend.db, url.clone(), contents); + + // If this is a .fe file, check if its ingot is loaded + if path.extension().and_then(|s| s.to_str()) == Some("fe") { + // Use init_ingot to find and load the ingot root + // It will walk up from the file's directory to find fe.toml + let diagnostics = init_ingot(&mut backend.db, &url); + + // Log any diagnostics + for diagnostic in diagnostics { + warn!( + "Ingot initialization diagnostic for file {:?}: {}", + path, diagnostic + ); + } + } } } ChangeKind::Create => { diff --git a/crates/language-server/src/main.rs b/crates/language-server/src/main.rs index 12d18bdd49..4df967cc55 100644 --- a/crates/language-server/src/main.rs +++ b/crates/language-server/src/main.rs @@ -59,7 +59,9 @@ async fn main() { async fn start_stdio_server() { let (server, client) = async_lsp::MainLoop::new_server(|client| { let tracing_layer = TracingLayer::default(); - let lsp_service = setup(client.clone(), "LSP actor".to_string()); + + let pid = std::process::id(); + let lsp_service = setup(client.clone(), format!("LSP actor {}", pid)); ServiceBuilder::new() .layer(LifecycleLayer::default()) .layer(CatchUnwindLayer::default()) diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_a/fe.toml b/crates/language-server/test_files/dependency_reresolution/ingot_a/fe.toml new file mode 100644 index 0000000000..356e24e180 --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_a/fe.toml @@ -0,0 +1,3 @@ +[ingot] +name = "ingot_a" +version = "0.1.0" diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_a/src/lib.fe b/crates/language-server/test_files/dependency_reresolution/ingot_a/src/lib.fe new file mode 100644 index 0000000000..6644ed1cb8 --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_a/src/lib.fe @@ -0,0 +1 @@ +pub fn a() -> u256 { 1 } diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_b/fe.toml b/crates/language-server/test_files/dependency_reresolution/ingot_b/fe.toml new file mode 100644 index 0000000000..8ef954f286 --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_b/fe.toml @@ -0,0 +1,6 @@ +[ingot] +name = "ingot_b" +version = "0.1.0" + +[dependencies] +ingot_a = { path = "../ingot_a" } diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_b/src/lib.fe b/crates/language-server/test_files/dependency_reresolution/ingot_b/src/lib.fe new file mode 100644 index 0000000000..2f6ba4429a --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_b/src/lib.fe @@ -0,0 +1 @@ +pub fn b() -> u256 { 2 } diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_c/fe.toml b/crates/language-server/test_files/dependency_reresolution/ingot_c/fe.toml new file mode 100644 index 0000000000..09184b143f --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_c/fe.toml @@ -0,0 +1,6 @@ +[ingot] +name = "ingot_c" +version = "0.1.0" + +[dependencies] +ingot_b = { path = "../ingot_b" } diff --git a/crates/language-server/test_files/dependency_reresolution/ingot_c/src/lib.fe b/crates/language-server/test_files/dependency_reresolution/ingot_c/src/lib.fe new file mode 100644 index 0000000000..42c0318f52 --- /dev/null +++ b/crates/language-server/test_files/dependency_reresolution/ingot_c/src/lib.fe @@ -0,0 +1 @@ +pub fn c() -> u256 { 3 } diff --git a/crates/language-server/tests/dependency_reresolution.rs b/crates/language-server/tests/dependency_reresolution.rs new file mode 100644 index 0000000000..060aac8b10 --- /dev/null +++ b/crates/language-server/tests/dependency_reresolution.rs @@ -0,0 +1,155 @@ +use common::InputDb; +use driver::DriverDataBase; +use std::path::PathBuf; + +// Re-implement load_ingot_from_directory here since we can't import from the library crate +fn load_ingot_from_directory(db: &mut DriverDataBase, ingot_dir: &std::path::Path) { + let ingot_url = + url::Url::from_directory_path(ingot_dir).expect("Failed to create URL from directory path"); + + let diagnostics = driver::init_ingot(db, &ingot_url); + + // In tests, we might want to panic on serious errors + for diagnostic in &diagnostics { + match diagnostic { + driver::IngotInitDiagnostics::ConfigParseError { .. } + | driver::IngotInitDiagnostics::ConfigDiagnostics { .. } => { + panic!("Failed to resolve test ingot at {ingot_dir:?}: {diagnostic}"); + } + _ => { + // Log other diagnostics but don't panic + eprintln!("Test ingot diagnostic for {ingot_dir:?}: {diagnostic}"); + } + } + } +} + +/// Test that dependencies are not re-resolved when initializing multiple ingots +/// that share common dependencies. +/// +/// This test creates a scenario where: +/// - Ingot A has no dependencies +/// - Ingot B depends on A +/// - Ingot C depends on B (and transitively on A) +/// +/// When we load B then C, A should only be resolved once (when B is loaded). +#[test] +fn test_dependency_not_reresolved_across_ingots() { + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let test_files_dir = PathBuf::from(&manifest_dir).join("test_files/dependency_reresolution"); + + // Paths to our test ingots + let ingot_a_dir = test_files_dir.join("ingot_a"); + let ingot_b_dir = test_files_dir.join("ingot_b"); + let ingot_c_dir = test_files_dir.join("ingot_c"); + + // Create a single database instance that persists across all loads + let mut db = DriverDataBase::default(); + + // Load ingot B (which depends on A) + // This should resolve both B and A + load_ingot_from_directory(&mut db, &ingot_b_dir); + + // Get the graph state after loading B + let graph_after_b = db.graph(); + let urls_after_b = graph_after_b.all_urls(&db); + + println!("URLs in graph after loading B: {:?}", urls_after_b.len()); + for url in &urls_after_b { + println!(" - {}", url); + } + + // Verify both B and A are in the graph + let ingot_a_url = url::Url::from_directory_path(&ingot_a_dir).unwrap(); + let ingot_b_url = url::Url::from_directory_path(&ingot_b_dir).unwrap(); + + assert!( + graph_after_b.contains_url(&db, &ingot_a_url), + "Ingot A should be in graph after loading B (which depends on A)" + ); + assert!( + graph_after_b.contains_url(&db, &ingot_b_url), + "Ingot B should be in graph after loading B" + ); + + // Now load ingot C (which depends on B) + // B and A are already in the graph, so they should NOT be re-resolved + load_ingot_from_directory(&mut db, &ingot_c_dir); + + // Get the graph state after loading C + let graph_after_c = db.graph(); + let urls_after_c = graph_after_c.all_urls(&db); + + println!("URLs in graph after loading C: {:?}", urls_after_c.len()); + for url in &urls_after_c { + println!(" - {}", url); + } + + // Verify C is now in the graph + let ingot_c_url = url::Url::from_directory_path(&ingot_c_dir).unwrap(); + assert!( + graph_after_c.contains_url(&db, &ingot_c_url), + "Ingot C should be in graph after loading C" + ); + + // Verify A and B are still in the graph (they should be!) + assert!( + graph_after_c.contains_url(&db, &ingot_a_url), + "Ingot A should still be in graph after loading C" + ); + assert!( + graph_after_c.contains_url(&db, &ingot_b_url), + "Ingot B should still be in graph after loading C" + ); + + // Verify the graph has all 3 ingots + assert_eq!( + urls_after_c.len(), + 3, + "Graph should contain exactly 3 ingots (A, B, C)" + ); + + // Verify dependency relationships + let c_deps = graph_after_c.dependency_urls(&db, &ingot_c_url); + assert!(c_deps.contains(&ingot_b_url), "C should depend on B"); + assert!( + c_deps.contains(&ingot_a_url), + "C should transitively depend on A" + ); +} + +/// Test that calling init_ingot multiple times on the same ingot doesn't +/// re-resolve its dependencies. +#[test] +fn test_idempotent_ingot_loading() { + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); + let test_files_dir = PathBuf::from(&manifest_dir).join("test_files/dependency_reresolution"); + + let ingot_b_dir = test_files_dir.join("ingot_b"); + + let mut db = DriverDataBase::default(); + + // Load B the first time + load_ingot_from_directory(&mut db, &ingot_b_dir); + + let graph_after_first = db.graph(); + let urls_after_first = graph_after_first.all_urls(&db); + let count_after_first = urls_after_first.len(); + + println!("URLs after first load: {}", count_after_first); + + // Load B again (should be idempotent) + load_ingot_from_directory(&mut db, &ingot_b_dir); + + let graph_after_second = db.graph(); + let urls_after_second = graph_after_second.all_urls(&db); + let count_after_second = urls_after_second.len(); + + println!("URLs after second load: {}", count_after_second); + + // The graph should have the same number of URLs + assert_eq!( + count_after_first, count_after_second, + "Graph should have same number of URLs after loading the same ingot twice" + ); +} diff --git a/crates/resolver/src/files.rs b/crates/resolver/src/files.rs index 50c3e471c9..2626e49bfc 100644 --- a/crates/resolver/src/files.rs +++ b/crates/resolver/src/files.rs @@ -11,6 +11,7 @@ pub struct FilesResolver { pub file_patterns: Vec, pub required_files: Vec, pub required_directories: Vec, + pub search_mode: bool, diagnostics: Vec, } @@ -123,6 +124,7 @@ impl FilesResolver { file_patterns: vec![], required_files: vec![], required_directories: vec![], + search_mode: false, diagnostics: vec![], } } @@ -132,6 +134,7 @@ impl FilesResolver { file_patterns: patterns.iter().map(|p| p.to_string()).collect(), required_files: vec![], required_directories: vec![], + search_mode: false, diagnostics: vec![], } } @@ -154,6 +157,32 @@ impl FilesResolver { self.file_patterns.push(pattern.to_string()); self } + + pub fn with_search_mode(mut self) -> Self { + self.search_mode = true; + self + } + + /// Check if a directory satisfies all required files and directories + fn check_requirements(&self, dir: &Utf8PathBuf) -> bool { + // Check required files + for required_file in &self.required_files { + let required_path = dir.join(&required_file.path); + if !required_path.exists() { + return false; + } + } + + // Check required directories + for required_dir in &self.required_directories { + let required_dir_path = dir.join(&required_dir.path); + if !required_dir_path.exists() || !required_dir_path.is_dir() { + return false; + } + } + + true + } } impl Resolver for FilesResolver { @@ -162,30 +191,71 @@ impl Resolver for FilesResolver { type Error = FilesResolutionError; type Diagnostic = FilesResolutionDiagnostic; - fn resolve(&mut self, handler: &mut H, ingot_url: &Url) -> Result + fn resolve(&mut self, handler: &mut H, url: &Url) -> Result where H: crate::ResolutionHandler, { - tracing::info!(target: "resolver", "Starting file resolution for URL: {}", ingot_url); + tracing::info!(target: "resolver", "Starting file resolution for URL: {}", url); let mut files = vec![]; - let ingot_path = Utf8PathBuf::from(ingot_url.path()); - tracing::info!(target: "resolver", "Resolving files in path: {}", ingot_path); + let mut path = Utf8PathBuf::from(url.path()); + let mut url = url.clone(); + + // If search mode is enabled, walk up to find the directory satisfying requirements + // Search mode is automatically disabled after first use to ensure only the root + // is searched for, not dependencies + if self.search_mode { + tracing::info!(target: "resolver", "Search mode enabled, looking for root from: {}", path); + + // Disable search mode for subsequent resolutions (dependencies) + self.search_mode = false; + + // If the path is a file, start from its parent directory + if path.is_file() + && let Some(parent) = path.parent() + { + path = parent.to_path_buf(); + } + + // Walk up directory tree to find root matching requirements + loop { + if self.check_requirements(&path) { + // Found the root + url = Url::from_file_path(&path) + .map_err(|_| FilesResolutionError::DirectoryDoesNotExist(url.clone()))?; + tracing::info!(target: "resolver", "Found root at: {}", path); + break; + } + + // Move to parent directory + match path.parent() { + Some(parent) => { + path = parent.to_path_buf(); + } + None => { + // Reached filesystem root without finding matching directory + return Err(FilesResolutionError::DirectoryDoesNotExist(url.clone())); + } + } + } + } + + tracing::info!(target: "resolver", "Resolving files in path: {}", path); // Check if the directory exists - if !ingot_path.exists() || !ingot_path.is_dir() { + if !path.exists() || !path.is_dir() { return Err(FilesResolutionError::DirectoryDoesNotExist( - ingot_url.clone(), + url.clone(), )); } // Check for required directories first for required_dir in &self.required_directories { - let required_dir_path = ingot_path.join(&required_dir.path); + let required_dir_path = path.join(&required_dir.path); if !required_dir_path.exists() || !required_dir_path.is_dir() { self.diagnostics .push(FilesResolutionDiagnostic::RequiredDirectoryMissing( - ingot_url.clone(), + url.clone(), required_dir.path.clone(), )); } @@ -193,11 +263,11 @@ impl Resolver for FilesResolver { // Check for required files for required_file in &self.required_files { - let required_path = ingot_path.join(&required_file.path); + let required_path = path.join(&required_file.path); if !required_path.exists() { self.diagnostics .push(FilesResolutionDiagnostic::RequiredFileMissing( - ingot_url.clone(), + url.clone(), required_file.path.clone(), )); } else { @@ -221,7 +291,7 @@ impl Resolver for FilesResolver { // Process file patterns for pattern in &self.file_patterns { - let pattern_path = ingot_path.join(pattern); + let pattern_path = path.join(pattern); let entries = glob(pattern_path.as_str()).map_err(FilesResolutionError::PatternError)?; @@ -263,7 +333,7 @@ impl Resolver for FilesResolver { tracing::info!(target: "resolver", "File resolution completed successfully, found {} files", files.len()); let resource = FilesResource { files }; - Ok(handler.handle_resolution(ingot_url, resource)) + Ok(handler.handle_resolution(&url, resource)) } fn take_diagnostics(&mut self) -> Vec { diff --git a/crates/resolver/src/graph.rs b/crates/resolver/src/graph.rs index 802aaa23e9..ab8162ceed 100644 --- a/crates/resolver/src/graph.rs +++ b/crates/resolver/src/graph.rs @@ -19,7 +19,7 @@ where H: GraphResolutionHandler> + crate::ResolutionHandler, >::Item: IntoIterator, - NR::Description: Eq + std::hash::Hash + Clone, + NR::Description: Eq + std::hash::Hash + Clone + fmt::Display, E: Clone, { #[allow(clippy::type_complexity)] @@ -41,7 +41,7 @@ where H: GraphResolutionHandler> + crate::ResolutionHandler, >::Item: IntoIterator, - NR::Description: Eq + std::hash::Hash + Clone, + NR::Description: Eq + std::hash::Hash + Clone + fmt::Display, E: Clone, { fn graph_resolve( @@ -52,7 +52,7 @@ where >>::Item, UnresolvableRootNode, > { - tracing::info!(target: "resolver", "Starting graph resolution"); + tracing::info!(target: "resolver", "Starting graph resolution for root node: {}", root_node); let mut graph = DiGraph::default(); let mut nodes: IndexMap = IndexMap::new(); @@ -63,13 +63,13 @@ where unresolved_nodes.entry(root_node.clone()).or_default(); while let Some((unresolved_node_description, back_nodes)) = unresolved_nodes.pop() { - tracing::info!(target: "resolver", "Resolving node"); + tracing::info!(target: "resolver", "Resolving node: {}", unresolved_node_description); match self .node_resolver .resolve(handler, &unresolved_node_description) { Ok(forward_nodes) => { - tracing::info!(target: "resolver", "Successfully resolved node"); + tracing::info!(target: "resolver", "Successfully resolved node: {}", unresolved_node_description); let resolved_node_description = unresolved_node_description; let resolved_node_index = graph.add_node(resolved_node_description.clone()); @@ -96,7 +96,7 @@ where } } Err(error) => { - tracing::warn!(target: "resolver", "Failed to resolve node"); + tracing::warn!(target: "resolver", "Failed to resolve node: {}", unresolved_node_description); self.diagnostics .push(UnresolvableNode(unresolved_node_description.clone(), error)); unresolvable_nodes