Parser: modular refactor complete; orchestrator mod.rs, stable behavior, docs + TODOs

Docs/TODOs/parser-modularization.md

@@ -0,0 +1,29 @@
+# TODOs - Parser Modularization Follow-ups
+
+These items were noted during/after the modular refactor of the WebFirst Language (WFL) parser. Capturing here for future handling.
+
+- Verify and tighten handling of stray "end X" tokens within nested constructs:
+  - We added defensive skipping of unexpected `end <keyword>` pairs inside action bodies to preserve old behavior and avoid desynchronization.
+  - Revisit this approach for a more principled synchronization strategy in `util::synchronize()` that can be shared across contexts (not just actions).
+
+- Re-validate control-flow edge cases inside action definitions:
+  - Specifically, `count ... end count` within `define action ... end action`, and early returns (`give back` / `return`) inside nested loops.
+  - Ensure token consumption invariants hold and that inner constructs fully consume their own `end X` tokens.
+
+- Complete doc polish for parser module responsibilities:
+  - Confirm Docs/technical/wfl-parser.md references the final modules and that examples reflect the orchestrator-only `mod.rs`.
+  - Add a brief section describing the lookahead pattern used to avoid borrow checker issues and unintended consumption.
+
+- Consolidate shared helpers:
+  - Audit helpers across `statements.rs`, `expressions.rs`, and `container_parser.rs` to ensure anything reusable is in `util.rs`.
+  - Consider general utilities like identifier sequence parsing and argument-list parsing to reduce duplication.
+
+- Testing additions:
+  - Add parser unit tests specifically targeting nested structures and stray `end` recovery cases.
+  - Add token-consumption progress tests to prevent infinite-loop regressions.
+
+- Performance/memory:
+  - Optionally run `scripts/run_heaptrack.sh` on larger programs and document peak usage targets in Docs/technical if not already present.
+
+- Known Actions:
+  - Double-check all action-call resolution paths remain confined to `expressions.rs` and that no new call sites were added outside expressions.

Docs/technical/wfl-parser.md

@@ -2,26 +2,58 @@
 
 ## Overview
 
-The WFL parser is a recursive descent parser that transforms a stream of tokens from the lexer into an Abstract Syntax Tree (AST). It implements sophisticated error recovery mechanisms and supports WFL's natural language syntax while maintaining backward compatibility with existing code.
+The WFL parser is a recursive descent parser that transforms a stream of tokens from the lexer into an Abstract Syntax Tree (AST). It implements robust error recovery and supports WFL's natural-language-friendly syntax while maintaining backward compatibility. The parser has been refactored into a modular architecture with `mod.rs` serving as a thin orchestrator under 300 lines. All existing tests pass with no behavior changes.
 
 ## Architecture
 
-### Core Components
+### Core Components (Modular Structure)
 
-1. **Parser Structure** (`src/parser/mod.rs`)
-   - Main parser implementation using recursive descent
-   - Error collection and reporting
-   - Action tracking for function resolution
+1. **Parser Orchestrator** (`src/parser/mod.rs`)
+   - Holds the `Parser` struct, `new()`, `parse()` loop, and `is_statement_starter()`
+   - Delegates to submodules for statements, expressions, patterns, and containers
+   - Under 300 lines by design
 
 2. **AST Definitions** (`src/parser/ast.rs`)
-   - Complete AST node types
-   - Container and OOP support structures
+   - AST node types only; no parsing logic
+   - Includes container and OOP support structures
    - Position information for error reporting
 
-3. **Container Parser** (`src/parser/container_parser.rs`)
-   - Specialized parsing for container definitions
-   - Interface parsing
-   - Property and method parsing
+3. **Error Handling** (`src/parser/error.rs`)
+   - `ParseError` type, constructors, Display/Debug
+   - Centralized utilities for consistent error creation
+
+4. **Parser Utilities** (`src/parser/util.rs`)
+   - Shared helpers such as `expect_token()` and `synchronize()`
+   - Comma-separated and line-boundary list parsing helpers
+
+5. **Statement Parsing** (`src/parser/statements.rs`)
+   - All statement-level parsing:
+     - Variable declarations, assignments
+     - Control flow (if/single-line-if, loops: for-each, count, repeat/main)
+     - I/O (open/close/read/write, file/directory operations)
+     - Action definitions, return/exit/push, try blocks
+   - Delegates to expressions and container/pattern modules as needed
+
+6. **Expression Parsing** (`src/parser/expressions.rs`)
+   - Expression precedence, binary/unary operations, member/index access
+   - Natural language operators and concatenation
+   - Action call resolution and argument parsing
+   - Contains and isolates `known_actions` logic
+
+7. **Pattern Parsing** (`src/parser/pattern_parser.rs`)
+   - Pattern statements and grammar:
+     - Sequences, alternatives, quantifiers, character classes
+     - Token-based grouping and error reporting
+
+8. **Container/Interface/Event Parsing** (`src/parser/container_parser.rs`)
+   - Container/interface definitions, inheritance/implements
+   - Properties, methods (including static), and events
+   - Container instantiation bodies and parameter lists
+
+Notes:
+- Lexer boundary is clean: parser uses only `Token` and `TokenWithPosition`
+- AST remains in `ast.rs` with no parsing logic
+- Behavior preserved; all tests pass
 
 ### Key Design Principles
 
@@ -529,4 +561,4 @@ The new pattern system maintains full backward compatibility:
 
 - Existing code continues to work unchanged
 - Old pattern syntax is still supported (marked as legacy)
-- No breaking changes to existing APIs
+- No breaking changes to existing APIs

src/analyzer/mod.rs

@@ -1066,7 +1066,7 @@ impl Analyzer {
                 self.analyze_expression(value);
                 if self.get_symbol(list_name).is_none() {
                     self.errors.push(SemanticError::new(
-                        format!("Variable '{}' is not defined", list_name),
+                        format!("Variable '{list_name}' is not defined"),
                         *line,
                         *column,
                     ));
@@ -1082,7 +1082,7 @@ impl Analyzer {
                 self.analyze_expression(value);
                 if self.get_symbol(list_name).is_none() {
                     self.errors.push(SemanticError::new(
-                        format!("Variable '{}' is not defined", list_name),
+                        format!("Variable '{list_name}' is not defined"),
                         *line,
                         *column,
                     ));
@@ -1096,7 +1096,7 @@ impl Analyzer {
             } => {
                 if self.get_symbol(list_name).is_none() {
                     self.errors.push(SemanticError::new(
-                        format!("Variable '{}' is not defined", list_name),
+                        format!("Variable '{list_name}' is not defined"),
                         *line,
                         *column,
                     ));

src/analyzer/static_analyzer.rs

@@ -936,8 +936,6 @@ impl Analyzer {
                                 Statement::MapCreation { line, .. } => *line,
                                 Statement::CreateDateStatement { line, .. } => *line,
                                 Statement::CreateTimeStatement { line, .. } => *line,
-                            Statement::CreateDateStatement { line, .. } => *line,
-                            Statement::CreateTimeStatement { line, .. } => *line,
                             },
                             column: match stmt {
                                 Statement::VariableDeclaration { column, .. } => *column,
@@ -988,8 +986,6 @@ impl Analyzer {
                                 Statement::MapCreation { column, .. } => *column,
                                 Statement::CreateDateStatement { column, .. } => *column,
                                 Statement::CreateTimeStatement { column, .. } => *column,
-                            Statement::CreateDateStatement { column, .. } => *column,
-                            Statement::CreateTimeStatement { column, .. } => *column,
                             },
                         });
                         then_nodes.push(then_node_idx);
@@ -1056,10 +1052,6 @@ impl Analyzer {
                                     Statement::MapCreation { line, .. } => *line,
                                     Statement::CreateDateStatement { line, .. } => *line,
                                     Statement::CreateTimeStatement { line, .. } => *line,
-                                Statement::CreateDateStatement { line, .. } => *line,
-                                Statement::CreateTimeStatement { line, .. } => *line,
-                            Statement::CreateDateStatement { line, .. } => *line,
-                            Statement::CreateTimeStatement { line, .. } => *line,
                                 },
                                 column: match stmt {
                                     Statement::VariableDeclaration { column, .. } => *column,
@@ -1110,10 +1102,6 @@ impl Analyzer {
                                     Statement::MapCreation { column, .. } => *column,
                                     Statement::CreateDateStatement { column, .. } => *column,
                                     Statement::CreateTimeStatement { column, .. } => *column,
-                                Statement::CreateDateStatement { column, .. } => *column,
-                                Statement::CreateTimeStatement { column, .. } => *column,
-                            Statement::CreateDateStatement { column, .. } => *column,
-                            Statement::CreateTimeStatement { column, .. } => *column,
                                 },
                             });
                             else_nodes.push(else_node_idx);

src/diagnostics/mod.rs

@@ -293,7 +293,7 @@ impl DiagnosticReporter {
     pub fn convert_parse_error(
         &mut self,
         file_id: usize,
-        error: &crate::parser::ast::ParseError,
+        error: &crate::parser::error::ParseError,
     ) -> WflDiagnostic {
         let message = error.message.clone();
 

src/diagnostics/tests.rs

@@ -1,5 +1,5 @@
 use super::*;
-use crate::parser::ast::ParseError;
+use crate::parser::error::ParseError;
 use crate::typechecker::TypeError;
 
 #[test]

src/interpreter/mod.rs

@@ -2127,7 +2127,7 @@ impl Interpreter {
                     let today = chrono::Local::now().date_naive();
                     Value::Date(Rc::new(today))
                 };
-                
+
                 env.borrow_mut().define(name, date_value);
                 Ok((Value::Null, ControlFlow::None))
             }
@@ -2145,7 +2145,7 @@ impl Interpreter {
                     let now = chrono::Local::now().time();
                     Value::Time(Rc::new(now))
                 };
-                
+
                 env.borrow_mut().define(name, time_value);
                 Ok((Value::Null, ControlFlow::None))
             }
@@ -2160,7 +2160,7 @@ impl Interpreter {
 
                 // Get the list from the environment
                 let list_val = env.borrow().get(list_name).ok_or_else(|| {
-                    RuntimeError::new(format!("Undefined variable: {}", list_name), *line, *column)
+                    RuntimeError::new(format!("Undefined variable: {list_name}"), *line, *column)
                 })?;
 
                 match list_val {
@@ -2204,7 +2204,7 @@ impl Interpreter {
 
                 // Get the list from the environment
                 let list_val = env.borrow().get(list_name).ok_or_else(|| {
-                    RuntimeError::new(format!("Undefined variable: {}", list_name), *line, *column)
+                    RuntimeError::new(format!("Undefined variable: {list_name}"), *line, *column)
                 })?;
 
                 match list_val {
@@ -2230,7 +2230,7 @@ impl Interpreter {
             } => {
                 // Get the list from the environment
                 let list_val = env.borrow().get(list_name).ok_or_else(|| {
-                    RuntimeError::new(format!("Undefined variable: {}", list_name), *line, *column)
+                    RuntimeError::new(format!("Undefined variable: {list_name}"), *line, *column)
                 })?;
 
                 match list_val {

src/parser/ast.rs

@@ -1,5 +1,3 @@
-use std::fmt;
-
 #[derive(Debug, Clone, PartialEq, Default)]
 pub struct Program {
     pub statements: Vec<Statement>,
@@ -647,33 +645,6 @@ pub enum Type {
     Interface(String),
 }
 
-#[derive(Debug, Clone)]
-pub struct ParseError {
-    pub message: String,
-    pub line: usize,
-    pub column: usize,
-}
-
-impl ParseError {
-    pub fn new(message: String, line: usize, column: usize) -> Self {
-        ParseError {
-            message,
-            line,
-            column,
-        }
-    }
-}
-
-impl fmt::Display for ParseError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(
-            f,
-            "Parse error at line {}, column {}: {}",
-            self.line, self.column, self.message
-        )
-    }
-}
-
 #[derive(Debug, Clone, PartialEq)]
 pub enum WriteMode {
     Overwrite,
@@ -700,5 +671,3 @@ pub struct WhenClause {
     pub error_name: String,
     pub body: Vec<Statement>,
 }
-
-impl std::error::Error for ParseError {}