Common cleanup

Author: Grant Wuerker

crates/common/src/config.rs

@@ -1,11 +1,11 @@
-use std::{fmt::Display, str::FromStr};
+use std::fmt::Display;
 
 use camino::Utf8PathBuf;
 use smol_str::SmolStr;
 use toml::Value;
 use url::Url;
 
-use crate::{graph::EdgeWeight, ingot::Version, urlext::UrlExt};
+use crate::{dependencies::EdgeWeight, ingot::Version, urlext::UrlExt};
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Config {
@@ -15,12 +15,14 @@ pub struct Config {
 }
 
 impl Config {
-    pub fn parse(content: &str) -> Result<Self, <Value as FromStr>::Err> {
+    pub fn parse(content: &str) -> Result<Self, String> {
         let mut diagnostics = Vec::new();
         let mut metadata = IngotMetadata::default();
         let mut dependencies = Vec::new();
 
-        let parsed: Value = content.parse()?;
+        let parsed: Value = content
+            .parse()
+            .map_err(|e: toml::de::Error| e.to_string())?;
 
         if let Some(table) = parsed.get("ingot").and_then(|value| value.as_table()) {
             if let Some(name) = table.get("name") {
@@ -208,7 +210,6 @@ pub enum ConfigDiagnostic {
     InvalidDependencyAlias(SmolStr),
     InvalidDependencyName(SmolStr),
     InvalidDependencyVersion(SmolStr),
-    InvalidTomlSyntax(String),
     MissingDependencyPath {
         alias: SmolStr,
         description: String,
@@ -237,7 +238,6 @@ impl Display for ConfigDiagnostic {
             Self::InvalidDependencyVersion(version) => {
                 write!(f, "Invalid dependency version \"{version}\"")
             }
-            Self::InvalidTomlSyntax(err) => write!(f, "Invalid TOML syntax: {err}"),
             Self::MissingDependencyPath { alias, description } => write!(
                 f,
                 "The dependency \"{alias}\" is missing a path argument \"{description}\""

crates/common/src/tree.rs renamed to crates/common/src/dependencies/display_tree.rs

@@ -2,7 +2,8 @@ use petgraph::graph::{DiGraph, NodeIndex};
 use std::collections::{HashMap, HashSet};
 use url::Url;
 
-use crate::{config::Config, graph::EdgeWeight};
+use super::graph::Dependency;
+use crate::config::Config;
 
 #[derive(Clone)]
 enum TreePrefix {
@@ -29,8 +30,8 @@ impl TreePrefix {
     }
 }
 
-pub fn display_dependency_tree(
-    graph: &DiGraph<Url, EdgeWeight>,
+pub fn display_tree(
+    graph: &DiGraph<Url, Dependency>,
     root_url: &Url,
     configs: &HashMap<Url, Config>,
 ) -> String {
@@ -61,7 +62,7 @@ pub fn display_dependency_tree(
 }
 
 struct TreeContext<'a> {
-    graph: &'a DiGraph<Url, EdgeWeight>,
+    graph: &'a DiGraph<Url, Dependency>,
     configs: &'a HashMap<Url, Config>,
     cycle_nodes: &'a HashSet<NodeIndex>,
 }

crates/common/src/graph.rs renamed to crates/common/src/dependencies/graph.rs

@@ -9,28 +9,28 @@ use crate::{InputDb, config::DependencyArguments};
 
 #[salsa::input]
 #[derive(Debug)]
-pub struct Graph {
-    graph: DiGraph<Url, EdgeWeight>,
+pub struct DependencyGraph {
+    graph: DiGraph<Url, Dependency>,
     node_map: HashMap<Url, NodeIndex>,
 }
 
 #[derive(Debug, Clone)]
 pub struct JoinEdge {
     pub origin: Url,
     pub destination: Url,
-    pub weight: EdgeWeight,
+    pub weight: Dependency,
 }
 
 #[derive(Clone, Debug)]
-pub struct EdgeWeight {
+pub struct Dependency {
     pub alias: SmolStr,
     pub arguments: DependencyArguments,
 }
 
 #[salsa::tracked]
-impl Graph {
+impl DependencyGraph {
     pub fn default(db: &dyn InputDb) -> Self {
-        Graph::new(db, DiGraph::new(), HashMap::new())
+        DependencyGraph::new(db, DiGraph::new(), HashMap::new())
     }
 
     pub fn contains_url(&self, db: &dyn InputDb, url: &Url) -> bool {
@@ -45,7 +45,7 @@ impl Graph {
     /// - All nodes that have paths leading to cyclic nodes
     ///
     /// Returns an empty graph if no cycles are detected.
-    pub fn cyclic_subgraph(&self, db: &dyn InputDb) -> DiGraph<Url, EdgeWeight> {
+    pub fn cyclic_subgraph(&self, db: &dyn InputDb) -> DiGraph<Url, Dependency> {
         use petgraph::algo::tarjan_scc;
         use std::collections::VecDeque;
 
@@ -119,7 +119,7 @@ impl Graph {
     pub fn join_graph(
         &self,
         db: &mut dyn InputDb,
-        other: DiGraph<Url, EdgeWeight>,
+        other: DiGraph<Url, Dependency>,
         join_edges: Vec<JoinEdge>,
     ) {
         let current_graph = self.graph(db);
@@ -173,7 +173,7 @@ impl Graph {
         new_idx
     }
 
-    pub fn add_edge(&self, db: &mut dyn InputDb, from: &Url, to: &Url, weight: EdgeWeight) -> bool {
+    pub fn add_edge(&self, db: &mut dyn InputDb, from: &Url, to: &Url, weight: Dependency) -> bool {
         let node_map = self.node_map(db);
 
         if let (Some(&from_idx), Some(&to_idx)) = (node_map.get(from), node_map.get(to)) {
@@ -187,11 +187,11 @@ impl Graph {
     }
 }
 
-fn join_graphs(
-    graph1: &DiGraph<Url, EdgeWeight>,
-    graph2: &DiGraph<Url, EdgeWeight>,
+pub fn join_graphs(
+    graph1: &DiGraph<Url, Dependency>,
+    graph2: &DiGraph<Url, Dependency>,
     join_edges: &[JoinEdge],
-) -> DiGraph<Url, EdgeWeight> {
+) -> DiGraph<Url, Dependency> {
     let mut combined = DiGraph::new();
     let mut node_map = HashMap::<Url, NodeIndex>::new();
 

crates/common/src/dependencies/mod.rs

@@ -0,0 +1,10 @@
+pub mod display_tree;
+pub mod graph;
+
+pub use display_tree::display_tree;
+pub use graph::{Dependency, DependencyGraph, JoinEdge, join_graphs};
+
+// Re-export commonly used types for backward compatibility
+pub use display_tree::display_tree as display_dependency_tree;
+pub use graph::Dependency as EdgeWeight;
+pub use graph::DependencyGraph as Graph;

crates/common/src/ingot.rs

@@ -6,7 +6,7 @@ use smol_str::SmolStr;
 use url::Url;
 
 use crate::InputDb;
-use crate::config::{Config, ConfigDiagnostic};
+use crate::config::Config;
 use crate::core::BUILTIN_CORE_BASE_URL;
 use crate::file::{File, Workspace};
 use crate::urlext::UrlExt;
@@ -104,22 +104,25 @@ impl<'db> Ingot<'db> {
         }
     }
 
+    #[salsa::tracked]
+    pub fn config_file(self, db: &'db dyn InputDb) -> Option<File> {
+        db.workspace().containing_ingot_config(db, self.base(db))
+    }
+
+    #[salsa::tracked]
+    fn parse_config(self, db: &'db dyn InputDb) -> Option<Result<Config, String>> {
+        self.config_file(db)
+            .map(|config_file| Config::parse(config_file.text(db)))
+    }
+
     #[salsa::tracked]
     pub fn config(self, db: &'db dyn InputDb) -> Option<Config> {
-        db.workspace()
-            .containing_ingot_config(db, self.base(db))
-            .map(|config_file| match Config::parse(config_file.text(db)) {
-                Ok(config) => config,
-                Err(err) => {
-                    // Create a Config with just the parse error as a diagnostic
-                    let diagnostics = vec![ConfigDiagnostic::InvalidTomlSyntax(err.to_string())];
-                    Config {
-                        metadata: Default::default(),
-                        diagnostics,
-                        dependencies: Vec::new(),
-                    }
-                }
-            })
+        self.parse_config(db).and_then(|result| result.ok())
+    }
+
+    #[salsa::tracked]
+    pub fn config_parse_error(self, db: &'db dyn InputDb) -> Option<String> {
+        self.parse_config(db).and_then(|result| result.err())
     }
 
     #[salsa::tracked]

crates/common/src/lib.rs

@@ -1,15 +1,14 @@
 pub mod config;
 pub mod core;
+pub mod dependencies;
 pub mod diagnostics;
 pub mod file;
-pub mod graph;
 pub mod indexmap;
 pub mod ingot;
-pub mod tree;
 pub mod urlext;
 
+use dependencies::Graph;
 use file::Workspace;
-use graph::Graph;
 
 #[salsa::db]
 // Each database must implement InputDb explicitly with its own storage mechanism
@@ -31,7 +30,7 @@ macro_rules! impl_input_db {
             fn workspace(&self) -> $crate::file::Workspace {
                 self.index.clone().expect("Workspace not initialized")
             }
-            fn graph(&self) -> $crate::graph::Graph {
+            fn graph(&self) -> $crate::dependencies::Graph {
                 self.graph.clone().expect("Graph not initialized")
             }
         }
@@ -57,7 +56,7 @@ macro_rules! impl_db_default {
                 };
                 let index = $crate::file::Workspace::default(&db);
                 db.index = Some(index);
-                let graph = $crate::graph::Graph::default(&db);
+                let graph = $crate::dependencies::Graph::default(&db);
                 db.graph = Some(graph);
                 $crate::core::HasBuiltinCore::initialize_builtin_core(&mut db);
                 db
@@ -75,7 +74,7 @@ macro_rules! define_input_db {
         pub struct $db_name {
             storage: salsa::Storage<Self>,
             index: Option<$crate::file::Workspace>,
-            graph: Option<$crate::graph::Graph>,
+            graph: Option<$crate::dependencies::Graph>,
         }
 
         #[salsa::db]