Merge #312: Fix of external SDK crate override bug

f2af871 Added DependencyMapBuilder for safe public API usage (Sdoba16)

Pull request description:

  Closes #310
  Previously, users could interact directly with the DependencyMap API in SimplicityHL, they could manually set the crate keyword to any path they want. This happened because the insert method allowed adding the crate alias without throwing any validation errors.

  Fix
  Was introduced `DependencyMapBuilder ` which collects dependencies and then could with build command it can be built.

  While building are made following validations:
  - entry source must be canonical and non-anonymous
  - every dependency target must be a canonical directory
  - every context prefix must be a canonical directory
  - alias must be a valid dependency identifier
  - alias must not be keyword
  - duplicate context+alias remappings rejected
  - overlapping crate roots are explicitly modeled
  - resolved files must remain inside their package root

  Remapping was made pub(crate) to hide from external API.

  `resolution.rs` was separated into 2 files `source.rs` and `resolution.rs` for readability.

ACKs for top commit:
  KyrylR:
    ACK f2af871; successfully ran local tests, review

Tree-SHA512: e68727b24fea181162386a8ea6e02eed263aa548762715501eafb9bdf604427c636adc608c3e248852d4dee5d998dffc664d24d543e1b1273fca7e621000338b

Author: KyrylR

src/driver/mod.rs

@@ -38,66 +38,20 @@ use chumsky::container::Container;
 
 use crate::error::{Error, ErrorCollector, RichError, Span};
 use crate::parse::{self, ParseFromStrWithErrors};
-use crate::resolution::{CanonPath, DependencyMap, SourceFile};
+use crate::resolution::DependencyMap;
+use crate::source::{CanonPath, CanonSourceFile, SourceFile};
 
 pub use crate::driver::resolve_order::{FileScoped, Program, SymbolTable};
 
 /// The reserved identifier for the program's entry point.
 pub(crate) const MAIN_STR: &str = "main";
 
 /// The reserved identifier for the local workspace root.
-pub const CRATE_STR: &str = "crate";
+pub(crate) const CRATE_STR: &str = "crate";
 
 /// The root node index in the [`DependencyGraph`] representing the entry file.
 pub(crate) const MAIN_MODULE: usize = 0;
 
-/// Caches the canonicalized path of a source file to prevent redundant,
-/// expensive, and potentially failing filesystem operations.
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
-pub struct CanonSourceFile {
-    /// The path of the source file (e.g., "./src/main.simf").
-    name: CanonPath,
-    /// The actual text content of the source file.
-    content: Arc<str>,
-}
-
-impl TryFrom<SourceFile> for CanonSourceFile {
-    type Error = String;
-
-    fn try_from(source: SourceFile) -> Result<Self, Self::Error> {
-        let name = if let Some(root_name) = source.name() {
-            CanonPath::canonicalize(root_name)?
-        } else {
-            return Err(
-                "Cannot canonicalize the SourceFile because it is missing a file name.".to_string(),
-            );
-        };
-
-        Ok(CanonSourceFile {
-            name,
-            content: source.content(),
-        })
-    }
-}
-
-impl CanonSourceFile {
-    pub fn new(name: CanonPath, content: Arc<str>) -> Self {
-        Self { name, content }
-    }
-
-    pub fn name(&self) -> &CanonPath {
-        &self.name
-    }
-
-    pub fn str_name(&self) -> String {
-        self.name.as_path().display().to_string()
-    }
-
-    pub fn content(&self) -> Arc<str> {
-        self.content.clone()
-    }
-}
-
 /// Represents a single, isolated file in the SimplicityHL project.
 /// In this architecture, a file and a module are the exact same thing.
 #[derive(Debug, Clone)]
@@ -351,6 +305,7 @@ impl DependencyGraph {
 pub(crate) mod tests {
     use super::*;
     use crate::resolution::tests::canon;
+    use crate::resolution::DependencyMapBuilder;
     use crate::test_utils::TempWorkspace;
 
     /// Initializes a raw graph environment for testing, explicitly allowing for and capturing failure states.
@@ -389,15 +344,10 @@ pub(crate) mod tests {
         let lib_dir = canon(&ws.create_dir("workspace/libs/lib"));
 
         // Set up the dependency map for imports (e.g. `use lib::...`)
-        let mut map = DependencyMap::new();
-        map.insert(workspace_dir.clone(), "lib".to_string(), lib_dir.clone())
-            .expect("Failed to insert dependency map");
-
-        // Register the strict crate boundaries so local files are forced to use `crate::`
-        map.insert(workspace_dir.clone(), CRATE_STR.to_string(), workspace_dir)
-            .expect("Failed to insert workspace crate boundary");
-        map.insert(lib_dir.clone(), CRATE_STR.to_string(), lib_dir)
-            .expect("Failed to insert library crate boundary");
+        let map = DependencyMapBuilder::new(workspace_dir.clone())
+            .add_dependency(workspace_dir.clone(), "lib".to_string(), lib_dir.clone())
+            .build()
+            .expect("Failed to create dependency map");
         let map = Arc::new(map);
 
         let mut root_file_path = None;

src/error.rs

@@ -15,7 +15,7 @@ use simplicity::elements;
 
 use crate::lexer::Token;
 use crate::parse::MatchPattern;
-use crate::resolution::SourceFile;
+use crate::source::SourceFile;
 use crate::str::{AliasName, FunctionName, Identifier, JetName, ModuleName, WitnessName};
 use crate::types::{ResolvedType, UIntType};
 
@@ -474,6 +474,11 @@ impl fmt::Display for ErrorCollector {
 /// Records _what_ happened but not where.
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub enum Error {
+    DependencyPathNotFound(String),
+    DependencyNotADirectory(String),
+    ReservedDependencyKeyword(String),
+    DuplicateDependencyAlias(String, String),
+    InvalidDependencyIdentifier(String),
     Internal(String),
     UnknownLibrary(String),
     ArraySizeNonZero(usize),
@@ -533,6 +538,11 @@ pub enum Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
+            Error::DependencyPathNotFound(path) => write!(f, "Path not found: {}", path),
+            Error::DependencyNotADirectory(path) => write!(f, "Path must be a directory: {}", path),
+            Error::ReservedDependencyKeyword(kw) => write!(f, "The '{}' keyword is reserved and cannot be manually mapped. Use the builder's context definitions instead.", kw),
+            Error::DuplicateDependencyAlias(alias, context) => write!(f, "Duplicate dependency mapping: alias '{}' is defined multiple times for context '{}'", alias, context),
+            Error::InvalidDependencyIdentifier(alias) => write!(f, "Invalid dependency alias '{}': must be a valid identifier and not a reserved keyword", alias),
             Error::Internal(err) => write!(
                 f,
                 "INTERNAL ERROR: {err}"

src/lexer.rs

@@ -255,24 +255,14 @@ pub fn lex<'src>(input: &'src str) -> (Option<Tokens<'src>>, Vec<crate::error::R
     )
 }
 
+/// A list of all reserved keywords.
+pub const KEYWORDS: &[&str] = &[
+    "pub", "use", "as", "fn", "let", "type", "mod", "const", "match", CRATE_STR, "true", "false",
+];
+
 /// Checks whether a given string is a keyword.
-#[cfg(feature = "arbitrary")]
 pub fn is_keyword(s: &str) -> bool {
-    matches!(
-        s,
-        "pub"
-            | "use"
-            | "as"
-            | "fn"
-            | "let"
-            | "type"
-            | "mod"
-            | "const"
-            | "match"
-            | CRATE_STR
-            | "true"
-            | "false"
-    )
+    KEYWORDS.contains(&s)
 }
 
 #[cfg(test)]

src/lib.rs

@@ -16,6 +16,8 @@ pub mod num;
 pub mod parse;
 pub mod pattern;
 pub mod resolution;
+pub mod source;
+
 #[cfg(feature = "serde")]
 mod serde;
 pub mod str;
@@ -39,7 +41,8 @@ use crate::debug::DebugSymbols;
 use crate::driver::DependencyGraph;
 use crate::error::{ErrorCollector, WithContent, WithSource as _};
 use crate::parse::ParseFromStrWithErrors;
-use crate::resolution::{DependencyMap, SourceFile};
+use crate::resolution::DependencyMap;
+use crate::source::SourceFile;
 pub use crate::types::ResolvedType;
 pub use crate::value::Value;
 pub use crate::witness::{Arguments, Parameters, WitnessTypes, WitnessValues};
@@ -72,23 +75,18 @@ impl TemplateProgram {
                 .ok_or_else(|| error_handler.to_string())?;
 
         // 2. Create the driver program
-        let driver_program: driver::Program = if dependency_map.is_empty() {
-            driver::Program::from_parse(&parsed_program, source.content(), &mut error_handler)
-                .ok_or_else(|| error_handler.to_string())?
-        } else {
-            let graph = DependencyGraph::new(
-                source.clone(),
-                Arc::from(dependency_map.clone()),
-                &parsed_program,
-                &mut error_handler,
-            )?
+        let graph = DependencyGraph::new(
+            source.clone(),
+            Arc::from(dependency_map.clone()),
+            &parsed_program,
+            &mut error_handler,
+        )?
+        .ok_or_else(|| error_handler.to_string())?;
+
+        let driver_program: driver::Program = graph
+            .linearize_and_build(&mut error_handler)?
             .ok_or_else(|| error_handler.to_string())?;
 
-            graph
-                .linearize_and_build(&mut error_handler)?
-                .ok_or_else(|| error_handler.to_string())?
-        };
-
         // 3. AST Analysis
         let ast_program = ast::Program::analyze(&driver_program).with_source(source.clone())?;
         Ok(Self {
@@ -399,6 +397,10 @@ pub trait ArbitraryOfType: Sized {
 #[cfg(test)]
 pub(crate) mod tests {
     use crate::parse::ParseFromStr;
+    use crate::resolution::tests::canon;
+    use crate::resolution::DependencyMapBuilder;
+    use crate::source::CanonPath;
+    use crate::test_utils::TempWorkspace;
     use base64::display::Base64Display;
     use base64::engine::general_purpose::STANDARD;
     use simplicity::BitMachine;
@@ -494,33 +496,19 @@ pub(crate) mod tests {
             I: IntoIterator<Item = (P, K, P)>,
             K: Into<String>,
         {
-            let mut dependency_map = DependencyMap::new();
-
-            if let Some(parent) = prog_path.as_ref().parent() {
-                let canon_root = crate::resolution::tests::canon(parent);
-                let _ = dependency_map.insert(
-                    canon_root.clone(),
-                    crate::driver::CRATE_STR.to_string(),
-                    canon_root,
-                );
-            }
+            let parent = prog_path.as_ref().parent().unwrap();
+            let canon_root = canon(parent);
+            let mut builder = DependencyMapBuilder::new(canon_root);
 
             for (context, alias, target) in dependencies {
-                let context = crate::resolution::tests::canon(context.as_ref());
-                let target = crate::resolution::tests::canon(target.as_ref());
-
-                dependency_map
-                    .insert(context.clone(), alias.into(), target.clone())
-                    .unwrap();
-
-                // Treat each mapped dependency as an isolated external package to satisfy strict local-file checks
-                let _ = dependency_map.insert(
-                    target.clone(),
-                    crate::driver::CRATE_STR.to_string(),
-                    target,
-                );
+                let context = canon(context.as_ref());
+                let target = canon(target.as_ref());
+
+                builder = builder.add_dependency(context, alias.into(), target);
             }
 
+            let dependency_map = builder.build().unwrap();
+
             TestCase::<TemplateProgram>::template_deps(prog_path.as_ref(), &dependency_map)
                 .with_arguments(Arguments::default())
         }
@@ -727,9 +715,6 @@ pub(crate) mod tests {
 
     #[test]
     fn test_crate_keyword_compilation_success() {
-        use crate::resolution::{CanonPath, DependencyMap};
-        use crate::test_utils::TempWorkspace;
-
         let ws = TempWorkspace::new("crate_success");
         let root = ws.create_dir("workspace");
         ws.create_file(
@@ -742,15 +727,9 @@ pub(crate) mod tests {
         );
 
         let main_path = root.join("main.simf");
-        let mut dependency_map = DependencyMap::new();
         let canon_root = CanonPath::canonicalize(&root).unwrap();
-        dependency_map
-            .insert(
-                canon_root.clone(),
-                crate::driver::CRATE_STR.to_string(),
-                canon_root,
-            )
-            .unwrap();
+
+        let dependency_map = DependencyMapBuilder::new(canon_root).build().unwrap();
 
         TestCase::<TemplateProgram>::template_deps(&main_path, &dependency_map)
             .with_arguments(Arguments::default())
@@ -1164,18 +1143,18 @@ mod error_tests {
     use super::*;
 
     use crate::resolution::tests::canon;
-    use crate::resolution::CanonPath;
+    use crate::resolution::DependencyMapBuilder;
+    use crate::source::CanonPath;
     use crate::test_utils::TempWorkspace;
 
     fn dependency_map(root_dir: &Path, drp: &str, lib_dir: &Path) -> DependencyMap {
-        let mut dependency_map = DependencyMap::new();
-
         let context = CanonPath::canonicalize(root_dir).unwrap();
         let target = CanonPath::canonicalize(lib_dir).unwrap();
 
-        dependency_map.insert(context, drp.into(), target).unwrap();
-
-        dependency_map
+        DependencyMapBuilder::new(context.clone())
+            .add_dependency(context, drp.into(), target)
+            .build()
+            .unwrap()
     }
 
     fn source_file(path: &Path) -> SourceFile {
@@ -1213,6 +1192,7 @@ mod error_tests {
     #[test]
     fn omitted_context_dependency_applies_inside_dependency_files() {
         let ws = TempWorkspace::new("omitted_context_dependency");
+        let root_dir = ws.create_dir("workspace");
         let lib_dir = ws.create_dir("workspace/lib");
         let main_path = ws.create_file(
             "workspace/main.simf",
@@ -1224,7 +1204,7 @@ mod error_tests {
         );
         ws.create_file("workspace/lib/base.simf", "pub fn one() -> u32 { 1 }\n");
 
-        let dependencies = dependency_map(&main_path, "lib", &lib_dir);
+        let dependencies = dependency_map(&root_dir, "lib", &lib_dir);
         let _err = TemplateProgram::new_with_dep(source_file(&main_path), &dependencies)
             .expect_err("omitted-context dependencies");
     }
@@ -1335,7 +1315,7 @@ mod functional_tests {
     }
 
     #[test]
-    #[should_panic(expected = "not found")]
+    #[should_panic(expected = "DependencyPathNotFound")]
     fn file_not_found_error() {
         run_dependency_test(
             format!("{}/file-not-found", ERROR_TESTS_DIR).as_str(),
@@ -1344,7 +1324,7 @@ mod functional_tests {
     }
 
     #[test]
-    #[should_panic(expected = "not found")]
+    #[should_panic(expected = "DependencyPathNotFound")]
     fn lib_not_found_error() {
         run_dependency_test(format!("{}/lib-not-found", ERROR_TESTS_DIR).as_str(), "lib");
     }