Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/common/src/dependencies/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Url> {
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
Expand Down
1 change: 1 addition & 0 deletions crates/common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions crates/driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,14 @@ pub fn ingot_graph_resolver<'a>() -> IngotGraphResolver<'a> {
}

pub fn init_ingot(db: &mut DriverDataBase, ingot_url: &Url) -> Vec<IngotInitDiagnostics> {
// 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<IngotInitDiagnostics> = {
let mut handler = InputHandler::from_db(db, ingot_url.clone());
let mut ingot_graph_resolver = ingot_graph_resolver();
Expand Down Expand Up @@ -304,6 +311,7 @@ impl<'a> GraphResolutionHandler<Url, DiGraph<Url, (SmolStr, DependencyArguments)
let from_url = &graph[edge.source()];
let to_url = &graph[edge.target()];
let (alias, arguments) = edge.weight();

dependency_graph.add_dependency(
self.db,
from_url,
Expand All @@ -312,5 +320,11 @@ impl<'a> GraphResolutionHandler<Url, DiGraph<Url, (SmolStr, DependencyArguments)
arguments.clone(),
);
}

// Log total ingot count after resolution
let total_ingots = dependency_graph.all_urls(self.db).len();
if total_ingots > 0 {
tracing::info!(target: "resolver", "Total ingots in workspace: {}", total_ingots);
}
}
}
34 changes: 0 additions & 34 deletions crates/language-server/src/backend/db.rs

This file was deleted.

21 changes: 21 additions & 0 deletions crates/language-server/src/functionality/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,27 @@ 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") {
// Walk up to find fe.toml
let mut current = path.parent();
while let Some(dir) = current {
let fe_toml = dir.join("fe.toml");
if fe_toml.exists() {
// Found ingot root, check if it's loaded
if let Ok(ingot_url) = Url::from_directory_path(dir) {
let loaded_ingots = backend.db.graph().all_urls(&backend.db);
if !loaded_ingots.contains(&ingot_url) {
info!("Ingot not loaded, initializing: {:?}", dir);
load_ingot_files(backend, dir).await?;
}
}
break;
}
current = dir.parent();
}
}
}
}
ChangeKind::Create => {
Expand Down
4 changes: 3 additions & 1 deletion crates/language-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[ingot]
name = "ingot_a"
version = "0.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn a() -> u256 { 1 }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[ingot]
name = "ingot_b"
version = "0.1.0"

[dependencies]
ingot_a = { path = "../ingot_a" }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn b() -> u256 { 2 }
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[ingot]
name = "ingot_c"
version = "0.1.0"

[dependencies]
ingot_b = { path = "../ingot_b" }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn c() -> u256 { 3 }
155 changes: 155 additions & 0 deletions crates/language-server/tests/dependency_reresolution.rs
Original file line number Diff line number Diff line change
@@ -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"
);
}
12 changes: 6 additions & 6 deletions crates/resolver/src/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ where
H: GraphResolutionHandler<NR::Description, DiGraph<NR::Description, E>>
+ crate::ResolutionHandler<NR>,
<H as crate::ResolutionHandler<NR>>::Item: IntoIterator<Item = (NR::Description, E)>,
NR::Description: Eq + std::hash::Hash + Clone,
NR::Description: Eq + std::hash::Hash + Clone + fmt::Display,
E: Clone,
{
#[allow(clippy::type_complexity)]
Expand All @@ -41,7 +41,7 @@ where
H: GraphResolutionHandler<NR::Description, DiGraph<NR::Description, E>>
+ crate::ResolutionHandler<NR>,
<H as crate::ResolutionHandler<NR>>::Item: IntoIterator<Item = (NR::Description, E)>,
NR::Description: Eq + std::hash::Hash + Clone,
NR::Description: Eq + std::hash::Hash + Clone + fmt::Display,
E: Clone,
{
fn graph_resolve(
Expand All @@ -52,7 +52,7 @@ where
<H as GraphResolutionHandler<NR::Description, DiGraph<NR::Description, E>>>::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<NR::Description, NodeIndex> = IndexMap::new();
Expand All @@ -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());
Expand All @@ -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
Expand Down