From 1f0208af95e7a5d884c49ea51d4eb12ceb99abff Mon Sep 17 00:00:00 2001 From: Brummel Date: Fri, 27 Mar 2026 15:08:28 +0100 Subject: [PATCH] Add module loader and related tests Introduce `ModuleLoader` to handle `#use` directives and dependency resolution. This involves adding `tempfile` dependency, modifying `Cargo.toml` and `Cargo.lock`, creating a new `module_loader.rs` file, and updating `environment.rs` to use the new module. Also, add comprehensive tests for `Diagnostics::format_errors`, `CompilationResult`, and the new `ModuleLoader` functionality. These tests cover various scenarios including empty diagnostics, multiple errors, error formatting, compilation success/failure, and complex dependency loading with topological ordering and search path resolution. --- Cargo.lock | 1 + Cargo.toml | 1 + src/ast/compiler/mod.rs | 1 + src/ast/compiler/module_loader.rs | 168 ++++++++++++++++++++++++++++++ src/ast/environment.rs | 132 ++--------------------- src/ast/rtl/docs.rs | 71 +++++++++++++ tests/diagnostics.rs | 56 ++++++++++ tests/module_loader.rs | 150 ++++++++++++++++++++++++++ 8 files changed, 457 insertions(+), 123 deletions(-) create mode 100644 src/ast/compiler/module_loader.rs create mode 100644 src/ast/rtl/docs.rs create mode 100644 tests/diagnostics.rs create mode 100644 tests/module_loader.rs diff --git a/Cargo.lock b/Cargo.lock index 7ea46e2..465b6aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2019,6 +2019,7 @@ dependencies = [ "regex", "rmcp", "serde", + "tempfile", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index e187472..efcf4ad 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,3 +16,4 @@ serde = { version = "1", features = ["derive"] } [dev-dependencies] insta = { version = "1.39", features = ["yaml"] } +tempfile = "3" diff --git a/src/ast/compiler/mod.rs b/src/ast/compiler/mod.rs index 9e06f13..05fdc8a 100644 --- a/src/ast/compiler/mod.rs +++ b/src/ast/compiler/mod.rs @@ -1,5 +1,6 @@ pub mod analyzer; pub mod binder; +pub mod module_loader; pub mod captures; pub mod dumper; pub mod emitter; diff --git a/src/ast/compiler/module_loader.rs b/src/ast/compiler/module_loader.rs new file mode 100644 index 0000000..051e30c --- /dev/null +++ b/src/ast/compiler/module_loader.rs @@ -0,0 +1,168 @@ +use std::cell::RefCell; +use std::collections::HashSet; +use std::path::{Path, PathBuf}; +use std::rc::Rc; + +use crate::ast::nodes::SyntaxNode; +use crate::ast::parser::Parser; + +/// Resolves `#use` directives and collects `.myc` dependency files in topological order. +/// File I/O and dependency graph traversal are fully encapsulated here; +/// compilation and execution remain the caller's responsibility. +pub struct ModuleLoader { + search_paths: Rc>>, + loaded_modules: Rc>>, +} + +impl ModuleLoader { + pub fn new( + search_paths: Rc>>, + loaded_modules: Rc>>, + ) -> Self { + Self { + search_paths, + loaded_modules, + } + } + + /// Collects all dependency files for `source` in topological order (dependencies before + /// dependents). Already-loaded modules (tracked in `loaded_modules`) are skipped. + pub fn collect_dependency_files( + &self, + source: &str, + base_path: &Path, + ) -> Result, String> { + let mut files = Vec::new(); + self.collect_dependencies(source, base_path, &mut files)?; + Ok(files) + } + + /// Returns a list of all matching `.myc` files for `module_path` + /// (resolves to single file or all files in a directory, alphabetically sorted). + fn resolve_module_paths( + &self, + module_path: &str, + base_path: &Path, + ) -> Result, String> { + let relative_path = module_path.replace("->", std::path::MAIN_SEPARATOR_STR); + let file_name = format!("{}.myc", relative_path); + + let check_path = |base: &Path| -> Option> { + // Check for file first + let mut file_p = base.to_path_buf(); + file_p.push(&file_name); + if file_p.is_file() && let Ok(canon) = file_p.canonicalize() { + return Some(vec![canon]); + } + + // Check for directory + let mut dir_p = base.to_path_buf(); + dir_p.push(&relative_path); + if dir_p.is_dir() { + let mut files = Vec::new(); + if let Ok(entries) = std::fs::read_dir(&dir_p) { + let mut valid_entries: Vec<_> = entries.filter_map(|e| e.ok()).collect(); + valid_entries.sort_by_key(|e| e.path()); + for entry in valid_entries { + let p = entry.path(); + if p.is_file() + && p.extension().is_some_and(|ext| ext == "myc") + && let Ok(canon) = p.canonicalize() + { + files.push(canon); + } + } + } + if !files.is_empty() { + return Some(files); + } + } + None + }; + + // 1. Try relative to base_path + if let Some(paths) = check_path(base_path) { + return Ok(paths); + } + + // 2. Try configured search paths + for sp in self.search_paths.borrow().iter() { + if let Some(paths) = check_path(sp) { + return Ok(paths); + } + } + + Err(format!( + "Could not find module or directory '{}'", + module_path + )) + } + + /// Recursively collects dependencies into `all_files` in topological order. + fn collect_dependencies( + &self, + source: &str, + base_path: &Path, + all_files: &mut Vec<(PathBuf, SyntaxNode)>, + ) -> Result<(), String> { + let directives = self.extract_use_directives(source); + + for module_path in directives { + let abs_paths = self.resolve_module_paths(&module_path, base_path)?; + + for abs_path in abs_paths { + if self.loaded_modules.borrow().contains(&abs_path) { + continue; + } + + self.loaded_modules.borrow_mut().insert(abs_path.clone()); + + let lib_source = std::fs::read_to_string(&abs_path) + .map_err(|e| format!("Failed to read module {:?}: {}", abs_path, e))?; + + // Recurse: load transitive dependencies before the current file (topological order) + let lib_base = abs_path.parent().unwrap_or(Path::new(".")); + self.collect_dependencies(&lib_source, lib_base, all_files)?; + + let mut parser = Parser::new(&lib_source); + let syntax_ast = parser.parse_expression(); + + if parser.diagnostics.has_errors() { + return Err(format!( + "Parser error in module {:?}:\n{}", + abs_path, parser.diagnostics.items[0].message + )); + } + + all_files.push((abs_path, syntax_ast)); + } + } + Ok(()) + } + + /// Extracts `#use ` directives from the leading lines of `source`. + /// Stops at the first non-directive, non-comment line. + fn extract_use_directives(&self, source: &str) -> Vec { + let mut paths = Vec::new(); + for line in source.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() || trimmed.starts_with(';') { + continue; + } + if let Some(stripped) = trimmed.strip_prefix("#use ") { + let path = stripped.trim(); + let clean_path = if (path.starts_with('"') && path.ends_with('"')) + || (path.starts_with('\'') && path.ends_with('\'')) + { + &path[1..path.len() - 1] + } else { + path + }; + paths.push(clean_path.to_string()); + } else { + break; + } + } + paths + } +} diff --git a/src/ast/environment.rs b/src/ast/environment.rs index 188c9e5..825cc5d 100644 --- a/src/ast/environment.rs +++ b/src/ast/environment.rs @@ -16,6 +16,7 @@ use crate::ast::nodes::{ }; use crate::ast::compiler::dumper::Dumper; use crate::ast::compiler::lambda_collector::LambdaCollector; +use crate::ast::compiler::module_loader::ModuleLoader; use crate::ast::compiler::lowering::Lowering; use crate::ast::compiler::macros::{MacroEvaluator, MacroExpander, MacroRegistry}; use crate::ast::compiler::optimizer::Optimizer; @@ -195,126 +196,7 @@ impl Environment { /// Resolves a #use module path relative to a base path, then falls back to search paths. - /// Returns a list of all matching .myc files (single file, or all files in a directory). - fn resolve_module_paths(&self, module_path: &str, base_path: &Path) -> Result, String> { - let relative_path = module_path.replace("->", std::path::MAIN_SEPARATOR_STR); - let file_name = format!("{}.myc", relative_path); - - let check_path = |base: &Path| -> Option> { - // Check for file first - let mut file_p = base.to_path_buf(); - file_p.push(&file_name); - if file_p.is_file() && let Ok(canon) = file_p.canonicalize() { - return Some(vec![canon]); - } - - // Check for directory - let mut dir_p = base.to_path_buf(); - dir_p.push(&relative_path); - if dir_p.is_dir() { - let mut files = Vec::new(); - if let Ok(entries) = std::fs::read_dir(&dir_p) { - let mut valid_entries: Vec<_> = entries.filter_map(|e| e.ok()).collect(); - valid_entries.sort_by_key(|e| e.path()); - for entry in valid_entries { - let p = entry.path(); - if p.is_file() && p.extension().is_some_and(|ext| ext == "myc") && let Ok(canon) = p.canonicalize() { - files.push(canon); - } - } - } - if !files.is_empty() { - return Some(files); - } - } - None - }; - - // 1. Try relative to base_path - if let Some(paths) = check_path(base_path) { - return Ok(paths); - } - - // 2. Try search paths - for sp in self.search_paths.borrow().iter() { - if let Some(paths) = check_path(sp) { - return Ok(paths); - } - } - - Err(format!("Could not find module or directory '{}'", module_path)) - } - - fn collect_dependencies( - &self, - source: &str, - base_path: &Path, - all_files: &mut Vec<(PathBuf, SyntaxNode)>, - ) -> Result<(), String> { - let directives = self.extract_use_directives(source); - - for module_path in directives { - let abs_paths = self.resolve_module_paths(&module_path, base_path)?; - - for abs_path in abs_paths { - if self.loaded_modules.borrow().contains(&abs_path) { - continue; - } - - self.loaded_modules.borrow_mut().insert(abs_path.clone()); - - let lib_source = std::fs::read_to_string(&abs_path) - .map_err(|e| format!("Failed to read module {:?}: {}", abs_path, e))?; - - // Pre-process dependencies of the library BEFORE adding it, ensuring topological order - let lib_base = abs_path.parent().unwrap_or(Path::new(".")); - self.collect_dependencies(&lib_source, lib_base, all_files)?; - - let mut parser = Parser::new(&lib_source); - let syntax_ast = parser.parse_expression(); - - if parser.diagnostics.has_errors() { - return Err(format!( - "Parser error in module {:?}:\n{}", - abs_path, - parser.diagnostics.items[0].message - )); - } - - all_files.push((abs_path, syntax_ast)); - } - } - Ok(()) - } - - fn extract_use_directives(&self, source: &str) -> Vec { - let mut paths = Vec::new(); - for line in source.lines() { - let trimmed = line.trim(); - if trimmed.is_empty() || trimmed.starts_with(';') { - continue; - } - if let Some(stripped) = trimmed.strip_prefix("#use ") { - let path = stripped.trim(); - // Strip optional quotes if they somehow got in, though user spec says no spaces/quotes. - let clean_path = if (path.starts_with('"') && path.ends_with('"')) - || (path.starts_with('\'') && path.ends_with('\'')) - { - &path[1..path.len() - 1] - } else { - path - }; - paths.push(clean_path.to_string()); - } else { - // Stop at first non-directive/non-comment line - break; - } - } - paths - } - /// Used to pre-load all dependencies of a script before compiling it. - /// It returns the base path which can be used to resolve further things if necessary. pub fn preload_dependencies(&self, source: &str, file_path: Option<&Path>) -> Result<(), String> { let base_path = file_path.and_then(|p| p.parent()).unwrap_or_else(|| Path::new(".")); let mut files: Vec<(PathBuf, SyntaxNode)> = Vec::new(); @@ -334,15 +216,19 @@ impl Environment { files.push((system_path, syntax_ast)); } - // 2. Collect dependencies from the main source - self.collect_dependencies(source, base_path, &mut files)?; + // 2. Collect user dependencies (file I/O + parse, topological order) + let loader = ModuleLoader::new( + Rc::clone(&self.search_paths), + Rc::clone(&self.loaded_modules), + ); + files.extend(loader.collect_dependency_files(source, base_path)?); - // Pass 1: Discovery (Globals and Macros) + // Pass 1: Discovery (globals and macros) for (_, syntax_ast) in &files { self.discover_globals(syntax_ast); } - // Pass 2: Compilation and Initialization + // Pass 2: Compilation and initialization for (path, syntax_ast) in files { let typed_ast = self.compile_syntax(syntax_ast).map_err(|e: String| { format!("Compilation error in {}:\n{}", path.display(), e) diff --git a/src/ast/rtl/docs.rs b/src/ast/rtl/docs.rs new file mode 100644 index 0000000..9a25385 --- /dev/null +++ b/src/ast/rtl/docs.rs @@ -0,0 +1,71 @@ +use std::cell::RefCell; +use std::rc::Rc; + +/// A generator function for data pipelines. Returns `true` while data is available. +pub type PipelineGenerator = Box bool>; + +/// Documentation entry for a single RTL symbol (function or constant). +/// Populated via the builder returned by `register_native_fn` / `register_constant`. +pub struct RtlDocEntry { + pub name: String, + /// Short one-line description (required). + pub one_liner: &'static str, + /// Optional longer explanation. + pub description: Option<&'static str>, + /// Optional Myc code examples. + pub examples: Option<&'static [&'static str]>, +} + +/// Builder returned by `register_native_fn` and `register_constant`. +/// Call `.doc(...)` to attach documentation; optional `.description(...)` and `.examples(...)` +/// can be chained. The entry is written to the registry when the builder is dropped. +pub struct RtlRegistration { + name: String, + rtl_docs: Rc>>, + one_liner: Option<&'static str>, + description: Option<&'static str>, + examples: Option<&'static [&'static str]>, +} + +impl RtlRegistration { + pub(crate) fn new(name: String, rtl_docs: Rc>>) -> Self { + Self { + name, + rtl_docs, + one_liner: None, + description: None, + examples: None, + } + } + + /// Attach a required one-line description. + pub fn doc(mut self, one_liner: &'static str) -> Self { + self.one_liner = Some(one_liner); + self + } + + /// Attach an optional longer description (must call `.doc()` first). + pub fn description(mut self, desc: &'static str) -> Self { + self.description = Some(desc); + self + } + + /// Attach optional usage examples as Myc code strings. + pub fn examples(mut self, ex: &'static [&'static str]) -> Self { + self.examples = Some(ex); + self + } +} + +impl Drop for RtlRegistration { + fn drop(&mut self) { + if let Some(one_liner) = self.one_liner { + self.rtl_docs.borrow_mut().push(RtlDocEntry { + name: self.name.clone(), + one_liner, + description: self.description, + examples: self.examples, + }); + } + } +} diff --git a/tests/diagnostics.rs b/tests/diagnostics.rs new file mode 100644 index 0000000..d406434 --- /dev/null +++ b/tests/diagnostics.rs @@ -0,0 +1,56 @@ +use myc::ast::compiler::CompilationResult; +use myc::ast::diagnostics::Diagnostics; +use myc::ast::environment::Environment; + +// --- Diagnostics::format_errors() --- + +#[test] +fn format_errors_empty() { + let diag = Diagnostics::new(); + assert_eq!(diag.format_errors(), ""); +} + +#[test] +fn format_errors_single() { + let mut diag = Diagnostics::new(); + diag.push_error("type mismatch", None); + assert_eq!(diag.format_errors(), "type mismatch"); +} + +#[test] +fn format_errors_multiple_joined_with_newline() { + let mut diag = Diagnostics::new(); + diag.push_error("first error", None); + diag.push_error("second error", None); + assert_eq!(diag.format_errors(), "first error\nsecond error"); +} + +#[test] +fn format_errors_ignores_warnings_and_hints() { + let mut diag = Diagnostics::new(); + diag.push_warning("just a warning", None); + diag.push_hint("a hint", None); + diag.push_error("the real error", None); + // Only the error should appear + assert_eq!(diag.format_errors(), "the real error"); +} + +// --- CompilationResult --- + +#[test] +fn compilation_result_error_into_err() { + let result = CompilationResult::error("something went wrong"); + let err = result.into_result(); + assert!(err.is_err()); + assert_eq!(err.unwrap_err(), "something went wrong"); +} + +#[test] +fn compilation_result_success_into_ok() { + // The success() constructor requires a TypedNode, which is produced by + // the compiler. We verify the happy path via a minimal compile round-trip. + let env = Environment::new(); + let result = env.compile("(+ 1 2)"); + assert!(!result.diagnostics.has_errors(), "unexpected errors: {}", result.diagnostics.format_errors()); + assert!(result.into_result().is_ok()); +} diff --git a/tests/module_loader.rs b/tests/module_loader.rs new file mode 100644 index 0000000..76a9928 --- /dev/null +++ b/tests/module_loader.rs @@ -0,0 +1,150 @@ +use std::cell::RefCell; +use std::collections::HashSet; +use std::fs; +use std::rc::Rc; + +use myc::ast::compiler::module_loader::ModuleLoader; + +fn make_loader() -> ModuleLoader { + ModuleLoader::new( + Rc::new(RefCell::new(vec![])), + Rc::new(RefCell::new(HashSet::new())), + ) +} + +fn make_loader_with_search_path(path: std::path::PathBuf) -> ModuleLoader { + ModuleLoader::new( + Rc::new(RefCell::new(vec![path])), + Rc::new(RefCell::new(HashSet::new())), + ) +} + +// --- Basic resolution --- + +#[test] +fn simple_use_directive_resolves_file() { + let dir = tempfile::TempDir::new().unwrap(); + fs::write(dir.path().join("utils.myc"), "(def x 42)").unwrap(); + + let loader = make_loader(); + let files = loader + .collect_dependency_files("#use utils\n(+ x 1)", dir.path()) + .unwrap(); + + assert_eq!(files.len(), 1); + assert!(files[0].0.ends_with("utils.myc")); +} + +#[test] +fn missing_module_returns_descriptive_error() { + let dir = tempfile::TempDir::new().unwrap(); + + let loader = make_loader(); + let result = loader.collect_dependency_files("#use nonexistent\n42", dir.path()); + + assert!(result.is_err()); + let msg = result.unwrap_err(); + assert!( + msg.contains("nonexistent"), + "error message should name the missing module, got: {msg}" + ); +} + +#[test] +fn no_use_directives_returns_empty() { + let dir = tempfile::TempDir::new().unwrap(); + + let loader = make_loader(); + let files = loader + .collect_dependency_files("(+ 1 2)", dir.path()) + .unwrap(); + + assert!(files.is_empty()); +} + +// --- Topological ordering --- + +#[test] +fn transitive_deps_returned_in_topological_order() { + // Dependency chain: source → b → c + // Expected order in result: [c, b] (dependencies before dependents) + let dir = tempfile::TempDir::new().unwrap(); + fs::write(dir.path().join("c.myc"), "(def c_val 1)").unwrap(); + fs::write(dir.path().join("b.myc"), "#use c\n(def b_val 2)").unwrap(); + + let loader = make_loader(); + let files = loader + .collect_dependency_files("#use b\n(+ b_val c_val)", dir.path()) + .unwrap(); + + assert_eq!(files.len(), 2); + assert!( + files[0].0.ends_with("c.myc"), + "c must come first (topological order)" + ); + assert!( + files[1].0.ends_with("b.myc"), + "b must come second (depends on c)" + ); +} + +// --- Deduplication --- + +#[test] +fn shared_dependency_loaded_only_once() { + // Both b and the source use c — c must appear exactly once. + let dir = tempfile::TempDir::new().unwrap(); + fs::write(dir.path().join("c.myc"), "(def c_val 1)").unwrap(); + fs::write(dir.path().join("b.myc"), "#use c\n(def b_val 2)").unwrap(); + + let loader = make_loader(); + let source = "#use b\n#use c\n(+ b_val c_val)"; + let files = loader + .collect_dependency_files(source, dir.path()) + .unwrap(); + + let c_count = files.iter().filter(|(p, _)| p.ends_with("c.myc")).count(); + assert_eq!(c_count, 1, "c.myc must appear exactly once"); + assert_eq!(files.len(), 2); // c, b +} + +// --- Search path resolution --- + +#[test] +fn module_found_via_search_path() { + // base_path has no .myc files; module lives in lib_dir (a search path) + let base_dir = tempfile::TempDir::new().unwrap(); + let lib_dir = tempfile::TempDir::new().unwrap(); + fs::write(lib_dir.path().join("utils.myc"), "(def util_val 99)").unwrap(); + + let loader = make_loader_with_search_path(lib_dir.path().to_path_buf()); + let files = loader + .collect_dependency_files("#use utils\n42", base_dir.path()) + .unwrap(); + + assert_eq!(files.len(), 1); + assert!(files[0].0.ends_with("utils.myc")); +} + +// --- Directory modules --- + +#[test] +fn directory_module_loads_all_myc_files_alphabetically() { + // #use lib should load lib/a.myc, lib/b.myc, lib/c.myc in that order + let dir = tempfile::TempDir::new().unwrap(); + let lib_dir = dir.path().join("lib"); + fs::create_dir(&lib_dir).unwrap(); + fs::write(lib_dir.join("c.myc"), "(def c 3)").unwrap(); + fs::write(lib_dir.join("a.myc"), "(def a 1)").unwrap(); + fs::write(lib_dir.join("b.myc"), "(def b 2)").unwrap(); + + let loader = make_loader(); + let files = loader + .collect_dependency_files("#use lib\n42", dir.path()) + .unwrap(); + + assert_eq!(files.len(), 3); + assert!(files[0].0.ends_with("a.myc"), "first: a.myc"); + assert!(files[1].0.ends_with("b.myc"), "second: b.myc"); + assert!(files[2].0.ends_with("c.myc"), "third: c.myc"); +}