From 1379ab366a1ddc6fbcdfc6f0772e2e3e653c81f6 Mon Sep 17 00:00:00 2001
From: Brummel <mould@mycelium.de>
Date: Fri, 27 Mar 2026 16:16:45 +0100
Subject: [PATCH] Refactor module loading to improve clarity

The module loading logic has been refactored to enhance readability and
maintainability. Key changes include:

*   **Path Handling**: Simplified path construction using `join` for
    better clarity and consistency.
*   **Error Reporting**: Improved error messages during parsing by
    utilizing `parser.diagnostics.format_errors()`.
*   **Module Loading Logic**:
    *   The `loaded_modules` set now uses `insert()` to both add new
        modules and efficiently check for duplicates, returning `false`
        if the module was already loaded.
    *   The recursive call to `collect_dependencies` is now placed
        before processing the current file's AST, ensuring a topological
        order of dependency loading.
*   **Helper Function Scope**: `extract_use_directives` is now a static
    method as it doesn't depend on the `ModuleLoader` instance.
---
 src/ast/compiler/module_loader.rs | 37 +++++++++++++------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/ast/compiler/module_loader.rs b/src/ast/compiler/module_loader.rs
index 051e30c..3cf871a 100644
--- a/src/ast/compiler/module_loader.rs
+++ b/src/ast/compiler/module_loader.rs
@@ -48,28 +48,26 @@ impl ModuleLoader {
         let file_name = format!("{}.myc", relative_path);
 
         let check_path = |base: &Path| -> Option<Vec<PathBuf>> {
-            // Check for file first
-            let mut file_p = base.to_path_buf();
-            file_p.push(&file_name);
+            let file_p = base.join(&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);
+            let dir_p = base.join(&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);
+                        let Ok(file_type) = entry.file_type() else { continue };
+                        if file_type.is_file() {
+                            let p = entry.path();
+                            if p.extension().is_some_and(|ext| ext == "myc")
+                                && let Ok(canon) = p.canonicalize()
+                            {
+                                files.push(canon);
+                            }
                         }
                     }
                 }
@@ -80,12 +78,10 @@ impl ModuleLoader {
             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);
@@ -105,22 +101,20 @@ impl ModuleLoader {
         base_path: &Path,
         all_files: &mut Vec<(PathBuf, SyntaxNode)>,
     ) -> Result<(), String> {
-        let directives = self.extract_use_directives(source);
+        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) {
+                // insert() returns false if the path was already present — skip in that case.
+                if !self.loaded_modules.borrow_mut().insert(abs_path.clone()) {
                     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)?;
 
@@ -130,7 +124,8 @@ impl ModuleLoader {
                 if parser.diagnostics.has_errors() {
                     return Err(format!(
                         "Parser error in module {:?}:\n{}",
-                        abs_path, parser.diagnostics.items[0].message
+                        abs_path,
+                        parser.diagnostics.format_errors()
                     ));
                 }
 
@@ -142,7 +137,7 @@ impl ModuleLoader {
 
     /// Extracts `#use <path>` directives from the leading lines of `source`.
     /// Stops at the first non-directive, non-comment line.
-    fn extract_use_directives(&self, source: &str) -> Vec<String> {
+    fn extract_use_directives(source: &str) -> Vec<String> {
         let mut paths = Vec::new();
         for line in source.lines() {
             let trimmed = line.trim();