Parser: modular refactor complete; orchestrator mod.rs, stable behavior, docs + TODOs - #129
Parser: modular refactor complete; orchestrator mod.rs, stable behavior, docs + TODOs#129devin-ai-integration[bot] wants to merge 6 commits into
Conversation
…; move container inheritance parsing to container_parser.rs; wire parser to new module Co-Authored-By: Bradley Byrd <bsbyrd@logbie.com>
…/Time statements; clippy clean Co-Authored-By: Bradley Byrd <bsbyrd@logbie.com>
…nostics; no functional changes Co-Authored-By: Bradley Byrd <bsbyrd@logbie.com>
…action bodies; docs + TODOs Co-Authored-By: Bradley Byrd <bsbyrd@logbie.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
WalkthroughThis update modularizes the parser for the WebFirst Language (WFL), introducing new modules for statements, expressions, patterns, containers, and utilities. The parser's error handling is refactored into a dedicated module. Documentation is extensively updated to reflect the new architecture. Minor stylistic and deduplication changes appear in the analyzer and interpreter, with no functional changes outside the parser. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Parser
participant Statements
participant Expressions
participant Patterns
participant Containers
participant Util
participant Error
User->>Parser: parse(source)
Parser->>Statements: parse_statement()
Statements->>Expressions: parse_expression() (as needed)
Statements->>Patterns: parse_create_pattern_statement() (as needed)
Statements->>Containers: parse_container_definition() (as needed)
Statements->>Util: expect_token(), synchronize() (as needed)
Statements->>Error: return ParseError on failure
Expressions->>Error: return ParseError on failure
Patterns->>Error: return ParseError on failure
Containers->>Error: return ParseError on failure
Parser-->>User: AST or ParseError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…ate lockfile Co-Authored-By: Bradley Byrd <bsbyrd@logbie.com>
|
@claude fix the failed CI checks |
|
Claude finished @logbie's task —— View job CI Check Failures Fixed Successfully ✅Todo List:
Issue Resolution: Technical Details: The fix ensures proper parsing of both:
Final Status: |
Issue: The modular parser refactor broke parsing of ''give back count'' statements, causing the test_return_from_loop_in_action test to fail with the error "Expected 'from' after 'count': expected KeywordFrom, found KeywordEnd". Root Cause: The parse_return_statement method incorrectly handled "give back X" syntax. It would consume "give", then see "back" and set the return value to None, ignoring any expression after "back". This meant "give back count" was parsed as "give back" (return nothing) instead of "give back count" (return count value). Solution: Modified parse_return_statement to properly parse expressions after "back" keyword, while still supporting bare "give back" for returning nothing. Also fixed clippy warnings for format string inlining in analyzer and interpreter. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (2)
Docs/technical/wfl-parser.md (2)
401-403: Mismatch Between Documentation and Parser ImplementationThe documentation for quantifiers in
Docs/technical/wfl-parser.mdlists support forOptional,ZeroOrMore,Exactly(u32), andBetween(u32, u32), but the parser (src/parser/pattern_parser.rs) currently only implementsQuantifier::OneOrMore(lines 178 and 220). Please either:
- Limit the docs’ Phase 1 quantifier list to only “one or more” (
+) or- Extend
pattern_parser.rsto handle?(Optional),*(ZeroOrMore),Exactly, andBetweenas documented.Files to update:
- Docs/technical/wfl-parser.md (lines 401–403)
- src/parser/pattern_parser.rs (add branching for
Token::Question,Token::Star, and parsing logic forExactlyandBetween)
371-396: Update AST documentation to match the code
ThePatternExpressionenum inDocs/technical/wfl-parser.md(lines 371–396) is missing several variants that are defined insrc/parser/ast.rs(lines 553–584). Please update the docs to include these entries or remove the code variants if they’re out of scope.Missing variants to document:
- Backreference(String) – back‐reference to a capture group
- Lookahead(Box) – positive lookahead
- NegativeLookahead(Box) – negative lookahead
- Lookbehind(Box) – positive lookbehind
- NegativeLookbehind(Box) – negative lookbehind
Fix the AST section so it reflects the full set of variants in the Rust code.
🧹 Nitpick comments (19)
src/interpreter/mod.rs (1)
2163-2164: Optional: consider standardizing undefined-variable phrasing across modulesAnalyzer reports "Variable '{name}' is not defined" while interpreter uses "Undefined variable: {name}". Standardizing phrasing (or introducing error codes) would help diagnostics aggregation.
src/diagnostics/mod.rs (1)
601-609: Make diagnostic note matching robust to case and source variantsCurrent checks are case-sensitive (e.g., "division by zero", "index out of bounds"), but interpreter messages use "Division by zero" and "Index ... out of bounds". This prevents helpful notes from appearing.
Quick fix within this block:
- Compare case-insensitively or include common capitalized variants.
Example minimal patch:
- if error.message.contains("division by zero") { + if error.message.contains("division by zero") || error.message.contains("Division by zero") { diag = diag.with_note("Check your divisor to ensure it's never zero"); - } else if error.message.contains("index out of bounds") { + } else if error.message.contains("index out of bounds") || error.message.contains("Index out of bounds") || error.message.contains("Index ") && error.message.contains(" out of bounds") { diag = diag.with_note("Make sure your index is within the valid range of the list");Longer-term, prefer matching on structured error kinds instead of parsing strings.
Docs/TODOs/parser-modularization.md (3)
5-12: Clarify recovery strategy acceptance criteria for strayend XtokensAdd explicit success criteria for util::synchronize():
- Never regress on existing programs (compat suite green).
- After recovery, parser must always advance at least one token.
- Include tests with nested actions/loops where inner/end mismatches occur.
I can help draft targeted tests that exercise nested constructs with mismatched ends.
21-24: Add targeted parser tests to CI to guard token-consumption progressPropose:
- A property test that feeds random extra
end <kw>tokens and asserts the parser never stalls.- Golden tests for nested constructs (if/while/count inside action) with misplaced ends and expected diagnostics.
Consider a “no-infinite-loop” test that aborts after N tokens without advancement.
28-30: Enforce “known actions resolved only in expressions.rs” via a lightweight checkAdd a CI grep/lint to ensure action call resolution is only implemented in expressions.rs.
Example script idea:
#!/bin/bash # Fail if ActionCall parsing logic found outside expressions.rs rg -n "ActionCall" src/parser | grep -v "expressions.rs" && { echo "ActionCall outside expressions.rs"; exit 1; } || trueDocs/technical/wfl-parser.md (1)
82-104: Good: orphaned “end X” handling is aligned with our error-recovery guidance.Cloning the iterator for lookahead and using next() only when consuming matches the guideline “use peek_token() for lookahead; never next_token() unless consuming.” Keep this pattern consistent across modules.
Consider adding a brief note in docs that this approach is the standard and should be reused in all recovery sites.
src/parser/util.rs (1)
7-29: expect_token is correct; minor improvement for EOF positions.Logic is sound. For the EOF case, consider reporting the last seen token’s position (or end-of-input position) instead of 0,0 to aid diagnostics.
src/parser/pattern_parser.rs (3)
167-186: Quantifiers: only “one or more” and “+” are supported (Phase 1).The implementation supports the phrase “one or more” and the plus quantifier, but not optional/zero-or-more/exact/between. That’s fine if intended for Phase 1, but please adjust docs and tests accordingly.
If you want, I can draft implementations for “optional” (Token::Question), “zero or more” (Token::Star), and numeric quantifiers.
Also applies to: 197-205, 209-229
147-158: Grouping returns an extra Sequence wrapper.Returning Sequence(vec![inner]) for parenthesized groups introduces an unnecessary node layer. Prefer returning inner directly unless the AST relies on this normalization.
- *i += 1; - PatternExpression::Sequence(vec![inner]) + *i += 1; + inner
54-61: Error positions for empty patterns should reference the create site.Empty pattern errors currently report 0,0. Consider checking pattern_parts.is_empty() in parse_create_pattern_statement and error with create_token’s position.
- let pattern_expr = Self::parse_pattern_tokens(&pattern_parts)?; + if pattern_parts.is_empty() { + return Err(ParseError::new( + "Empty pattern definition".to_string(), + create_token.line, + create_token.column, + )); + } + let pattern_expr = Self::parse_pattern_tokens(&pattern_parts)?;Also applies to: 75-81
src/parser/expressions.rs (1)
33-53: Confirm minus symbol support: noToken::Minusvariant in the lexer
The lexer definesKeywordMinusbut does not have a plainMinustoken, so the parser’s current match onToken::KeywordMinusis consistent. If you intend to support symbol-based subtraction (-), you’ll need to:
- Add a
Minusvariant insrc/lexer/token.rswith#[token("-")] Minus- Update the parser in
src/parser/expressions.rs(around line 33) to handleToken::Minus, mirroring thePlusbranchesOtherwise, no parser change is needed.
src/parser/container_parser.rs (4)
39-40: Rename parse_inheritance2 → parse_inheritance for clarityThe “2” suffix is confusing and suggests a leftover. Rename for consistency and readability.
- let (extends, implements) = self.parse_inheritance2()?; + let (extends, implements) = self.parse_inheritance()?;
784-788: Align function name with call site: parse_inheritance2 → parse_inheritanceRefactor the function name to remove the “2” suffix.
- pub(crate) fn parse_inheritance2( + pub(crate) fn parse_inheritance( &mut self, ) -> Result<(Option<String>, Vec<String>), ParseError> {
701-709: Instantiation body: fix error message to match accepted tokensOnly ‘property’ and ‘end’ are accepted. Message mentions ‘method’ which is misleading.
- return Err(ParseError::new( - format!( - "Expected 'property', 'method', or 'end' in instantiation body, found {:?}", - token.token - ), - token.line, - token.column, - )); + return Err(ParseError::new( + format!( + "Expected 'property' or 'end' in instantiation body, found {:?}", + token.token + ), + token.line, + token.column, + ));
111-114: Route deprecation warning through diagnostics/logging, not eprintln!Using eprintln! inside the parser can spam STDERR and isn’t test-friendly. Prefer a diagnostics channel or a logger macro.
src/parser/statements.rs (4)
808-809: For-each: preserve starting token span (don’t derive from trailing peek)Capture the ‘for’ token and use its line/column in the resulting statement.
- self.tokens.next(); + let for_tok = self.tokens.next().unwrap(); ... - Ok(Statement::ForEachLoop { + Ok(Statement::ForEachLoop { item_name, collection, reversed, body, - line: token_pos.line, - column: token_pos.column, + line: for_tok.line, + column: for_tok.column, })Also applies to: 872-880
883-888: Count loop: preserve starting token spanSame reasoning as for-each. Use the ‘count’ token for accurate spans.
- self.tokens.next(); + let count_tok = self.tokens.next().unwrap(); ... - Ok(Statement::CountLoop { + Ok(Statement::CountLoop { start, end, step, downward, body, - line: token_pos.line, - column: token_pos.column, + line: count_tok.line, + column: count_tok.column, })Also applies to: 966-975
474-621: Display spans: collapse the exhaustive match into a helperThis giant match for (line, column) is brittle. Prefer a helper to extract the expression’s span to reduce maintenance surface.
Example approach:
- Add a method on Expression (or a small utility) returning (line, column).
- Use that here to set Statement::DisplayStatement spans without a giant match.
77-108: DRY open-file-read lookaheadThe “open file … and read content … as …” lookahead logic is duplicated in parse_statement and parse_wait_for_statement. Factor into a small helper (e.g., looks_like_open_file_read()) to keep logic in one place.
Also applies to: 1352-1374
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
Docs/TODOs/parser-modularization.md(1 hunks)Docs/technical/wfl-parser.md(2 hunks)src/analyzer/mod.rs(3 hunks)src/analyzer/static_analyzer.rs(0 hunks)src/diagnostics/mod.rs(1 hunks)src/diagnostics/tests.rs(1 hunks)src/interpreter/mod.rs(5 hunks)src/parser/ast.rs(0 hunks)src/parser/container_parser.rs(1 hunks)src/parser/error.rs(1 hunks)src/parser/expressions.rs(1 hunks)src/parser/pattern_parser.rs(1 hunks)src/parser/statements.rs(1 hunks)src/parser/util.rs(1 hunks)
💤 Files with no reviewable changes (2)
- src/analyzer/static_analyzer.rs
- src/parser/ast.rs
🧰 Additional context used
🧠 Learnings (27)
📓 Common learnings
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/parser/**/*.rs : Parser logic is implemented in src/parser/ and should support natural language syntax and comprehensive end token handling
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Before making changes, read Docs/wfl-spec.md for language specification
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Never break existing WFL programs; maintain 100% compatibility with all syntax
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to **/*.wfl : All WFL code should be auto-fixed using the built-in fixer (cargo run -- --fix script.wfl --in-place) when possible
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to **/*.wfl : All WFL code should be statically analyzed using the built-in analyzer (cargo run -- --analyze script.wfl)
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to TestPrograms/**/*.wfl : Add new tests for new features in TestPrograms/
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: .cursor/rules/wfl-rules.mdc:0-0
Timestamp: 2025-08-04T12:01:27.889Z
Learning: If we implement something in the parser, also update the bytecode as well
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/diagnostics/**/*.rs : All errors must use the unified diagnostic system in src/diagnostics/ and include source context with precise spans and actionable suggestions
Applied to files:
src/diagnostics/tests.rssrc/analyzer/mod.rssrc/diagnostics/mod.rssrc/interpreter/mod.rssrc/parser/error.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/parser/**/*.rs : Parser logic is implemented in src/parser/ and should support natural language syntax and comprehensive end token handling
Applied to files:
src/diagnostics/tests.rsDocs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.mdsrc/parser/pattern_parser.rssrc/parser/error.rssrc/parser/util.rssrc/parser/expressions.rssrc/parser/container_parser.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter runtime errors must use InterpreterError
Applied to files:
src/diagnostics/tests.rssrc/diagnostics/mod.rssrc/interpreter/mod.rssrc/parser/error.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/parser/**/*.rs : Parser must always consume orphaned tokens during error recovery and use peek_token() for lookahead, never next_token() unless consuming
Applied to files:
src/diagnostics/tests.rsDocs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.mdsrc/parser/pattern_parser.rssrc/parser/error.rssrc/parser/util.rssrc/parser/expressions.rssrc/parser/container_parser.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to {src/typechecker/**/*.rs,src/stdlib/pattern*.rs} : Pattern matching with regex support must be implemented in the type system
Applied to files:
src/diagnostics/tests.rssrc/parser/pattern_parser.rssrc/parser/error.rssrc/parser/expressions.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/typechecker/**/*.rs : Type checking logic is implemented in src/typechecker/ and should perform static type analysis
Applied to files:
src/diagnostics/tests.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/typechecker/**/*.rs : All types in the type system must be statically typed with inference, supporting text, number, boolean, list, null, any, and function types
Applied to files:
src/diagnostics/tests.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/parser/**/*.rs : All Rust code must manage memory carefully, especially in parser (lifetime management), and use Environment HashMap for variable storage with proper scope management
Applied to files:
src/diagnostics/tests.rsDocs/TODOs/parser-modularization.mdsrc/parser/error.rssrc/parser/util.rssrc/parser/expressions.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/analyzer/**/*.rs : Analyzer logic is implemented in src/analyzer/ and should perform semantic analysis and validation
Applied to files:
src/analyzer/mod.rsDocs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.mdsrc/parser/expressions.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/stdlib/**/*.rs : Standard library modules are implemented in src/stdlib/ and should be modular (core, math, text, list, time, pattern)
Applied to files:
src/interpreter/mod.rsDocs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/**/*.rs : Format all Rust code using cargo fmt
Applied to files:
src/interpreter/mod.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to {src/interpreter/**/*.rs,src/stdlib/**/*.rs} : All I/O operations must be async and use the Tokio runtime
Applied to files:
src/interpreter/mod.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to **/*.wfl : All WFL code should be auto-fixed using the built-in fixer (cargo run -- --fix script.wfl --in-place) when possible
Applied to files:
src/interpreter/mod.rsDocs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/interpreter/**/*.rs : Interpreter logic is implemented in src/interpreter/ and should use Tokio async runtime for execution
Applied to files:
src/interpreter/mod.rssrc/parser/util.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Before making changes, read Docs/wfl-spec.md for language specification
Applied to files:
Docs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to {src/config.rs,src/wfl_config/**/*.rs} : Configuration logic is implemented in src/config.rs and src/wfl_config/
Applied to files:
Docs/technical/wfl-parser.md
📚 Learning: 2025-08-04T12:01:27.889Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: .cursor/rules/wfl-rules.mdc:0-0
Timestamp: 2025-08-04T12:01:27.889Z
Learning: All components must be documented (e.g., parser, lexer, bytecode, etc.)
Applied to files:
Docs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to **/*.wfl : All WFL code should be statically analyzed using the built-in analyzer (cargo run -- --analyze script.wfl)
Applied to files:
Docs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Never break existing WFL programs; maintain 100% compatibility with all syntax
Applied to files:
Docs/technical/wfl-parser.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to Dev diary/**/*.md : All significant changes must be documented in Dev diary/
Applied to files:
Docs/technical/wfl-parser.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/lexer/**/*.rs : Lexer logic is implemented in src/lexer/ and should use the Logos library for tokenization
Applied to files:
Docs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.mdsrc/parser/error.rssrc/parser/util.rssrc/parser/expressions.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to src/parser/container_*.rs : Containers (similar to classes) are implemented in src/parser/container_*.rs and should support properties, actions, inheritance, interfaces, and events
Applied to files:
Docs/technical/wfl-parser.mdDocs/TODOs/parser-modularization.mdsrc/parser/container_parser.rssrc/parser/statements.rs
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Before merging any change, run ALL test programs in TestPrograms/ and verify identical behavior for existing syntax
Applied to files:
Docs/technical/wfl-parser.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to TestPrograms/**/*.wfl : Add new tests for new features in TestPrograms/
Applied to files:
Docs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.737Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T12:01:27.737Z
Learning: Applies to Docs/**/*.md : All documentation updates must be made in Docs/ and module-specific docs in Docs/
Applied to files:
Docs/TODOs/parser-modularization.md
📚 Learning: 2025-08-04T12:01:27.889Z
Learnt from: CR
PR: WebFirstLanguage/wfl#0
File: .cursor/rules/wfl-rules.mdc:0-0
Timestamp: 2025-08-04T12:01:27.889Z
Learning: If we implement something in the parser, also update the bytecode as well
Applied to files:
src/parser/statements.rs
🔇 Additional comments (12)
src/diagnostics/tests.rs (1)
2-2: Import path updated to new parser error module — OKThe tests now reference crate::parser::error::ParseError; this aligns with the modularized parser. No behavior change.
src/interpreter/mod.rs (3)
2130-2131: Whitespace-only changeNo functional impact.
2148-2149: Whitespace-only changeNo functional impact.
2163-2164: Use of Rust 1.58+ format string shorthand — LGTMSwitched to format!("... {list_name}") for clarity with no behavioral change.
Also applies to: 2207-2208, 2233-2234
src/diagnostics/mod.rs (2)
293-299: convert_parse_error now consumes parser::error::ParseError — OKSignature updated to the new module path; fits the parser modularization.
293-299: No staleparser::ast::ParseErrorreferences remainA grep across the repository confirms all code now imports and uses
crate::parser::error::ParseErrorexclusively.
No action needed.src/analyzer/mod.rs (1)
1069-1072: Formatting cleanup for undefined-variable messages — OKSwitched to format!("Variable '{list_name}' is not defined"); message content unchanged.
Also applies to: 1085-1088, 1099-1102
Docs/technical/wfl-parser.md (1)
563-565: Legacy pattern syntax support remains unverified
I ran a search through src/parser for any “legacy”, “compat”, or “old pattern” markers but found nothing. Please confirm and cite where this backward-compatibility logic lives (parser flags, compatibility branches, or tests). If it’s not implemented, update the docs to remove or revise that claim.• Verify any CLI or API flags in your parser entrypoint that enable legacy pattern handling
• Point to the module/file containing compatibility shims or test cases covering old syntax
• If no such logic exists, adjust Docs/technical/wfl-parser.md accordinglysrc/parser/error.rs (1)
20-30: Display format stable and clear.Message format is consistent and human-friendly. No changes needed.
src/parser/pattern_parser.rs (1)
159-163: NegativeLookahead already implemented
ThePatternExpressionenum in src/parser/ast.rs (lines 577–583) already includes:/// Negative lookahead - matches if pattern would NOT match ahead NegativeLookahead(Box<PatternExpression>),No changes are needed here—please ignore the original suggestion.
Likely an incorrect or invalid review comment.
src/parser/expressions.rs (1)
28-33: Confirm line-based break behavior in expression parsingI didn’t find any existing tests in src/parser/tests.rs or TestPrograms that exercise how expressions spanning multiple lines are handled by this new sentinel.
If the change to break the precedence loop on a line advance is intentional for NL-friendly syntax, please:
- Add or update compatibility tests (in src/parser/tests.rs or TestPrograms) to assert the expected behavior when an operator appears on the next line.
- Ensure all existing WFL programs still parse identically.
Otherwise, consider reverting the line-based break and rely solely on the grammar’s precedence rules.
src/parser/statements.rs (1)
977-1195: Action definition: robust stray ‘end X’ handling looks goodParses parameters, optional return type, and defensively skips stray end-construct pairs in bodies. Matches PR objective on desynchronization prevention.
| let mut handler_body = Vec::new(); | ||
| while let Some(token) = self.tokens.peek() { | ||
| if matches!(token.token, Token::KeywordEnd) { | ||
| break; | ||
| } | ||
| handler_body.push(self.parse_statement()?); | ||
| } | ||
|
|
||
| self.expect_token(Token::KeywordEnd, "Expected 'end' after event handler body")?; | ||
| self.expect_token(Token::KeywordOn, "Expected 'on' after 'end'")?; | ||
|
|
||
| Ok(Statement::EventHandler { | ||
| event_source: Expression::Variable("self".to_string(), start.line, start.column), | ||
| event_name, | ||
| handler_body, | ||
| line: name_line, | ||
| column: name_column, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Event handler body: handle stray ‘end X’ to prevent desynchronization
Mirroring action bodies, skip unexpected “end ” within handler bodies, not just bare “end”.
- let mut handler_body = Vec::new();
- while let Some(token) = self.tokens.peek() {
- if matches!(token.token, Token::KeywordEnd) {
- break;
- }
- handler_body.push(self.parse_statement()?);
- }
+ let mut handler_body = Vec::new();
+ while let Some(token) = self.tokens.peek() {
+ if matches!(token.token, Token::KeywordEnd) {
+ // Lookahead to decide whether to break (end on) or skip stray end X
+ let mut la = self.tokens.clone();
+ let _ = la.next();
+ if let Some(next_tok) = la.peek() {
+ match next_tok.token {
+ Token::KeywordOn => break, // proper 'end on' follows
+ Token::KeywordCheck
+ | Token::KeywordFor
+ | Token::KeywordCount
+ | Token::KeywordRepeat
+ | Token::KeywordTry
+ | Token::KeywordLoop
+ | Token::KeywordMap
+ | Token::KeywordWhile
+ | Token::KeywordPattern => {
+ // Skip 'end <X>'
+ self.tokens.next();
+ self.tokens.next();
+ continue;
+ }
+ _ => {} // fall through
+ }
+ } else {
+ break;
+ }
+ }
+ handler_body.push(self.parse_statement()?);
+ }🤖 Prompt for AI Agents
In src/parser/container_parser.rs around lines 285 to 303, the current code only
breaks the handler body parsing on a bare 'end' keyword, which can cause
desynchronization if an unexpected 'end <X>' token appears. Modify the loop to
also detect and skip tokens matching 'end' followed by any other token (like
'end X'), ensuring these stray endings are handled gracefully and do not disrupt
parsing. This will keep the parser synchronized by ignoring unexpected 'end <X>'
tokens within the event handler body.
| match &token.token { | ||
| Token::KeywordOf => { | ||
| self.tokens.next(); | ||
| if let Some(t2) = self.tokens.peek() { | ||
| match &t2.token { | ||
| Token::KeywordText => { | ||
| self.tokens.next(); | ||
| Some(Type::Text) | ||
| } | ||
| Token::Identifier(tid) => { | ||
| let tid = tid.clone(); | ||
| self.tokens.next(); | ||
| Some(Type::Custom(tid)) | ||
| } | ||
| _ => None, | ||
| } | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| _ => None, | ||
| } | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| let default_value = if let Some(token) = self.tokens.peek() { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Property types: support number/boolean (consistency with parameter parsing)
Currently only text/custom are recognized. Add number/boolean to avoid treating them as custom types.
let property_type = if let Some(token) = self.tokens.peek() {
match &token.token {
Token::KeywordOf => {
self.tokens.next();
if let Some(t2) = self.tokens.peek() {
match &t2.token {
Token::KeywordText => {
self.tokens.next();
Some(Type::Text)
}
+ Token::Identifier(tid) if tid == "number" => {
+ self.tokens.next();
+ Some(Type::Number)
+ }
+ Token::Identifier(tid) if tid == "boolean" => {
+ self.tokens.next();
+ Some(Type::Boolean)
+ }
Token::Identifier(tid) => {
let tid = tid.clone();
self.tokens.next();
Some(Type::Custom(tid))
}
_ => None,
}
} else {
None
}
}
_ => None,
}
} else {
None
};🤖 Prompt for AI Agents
In src/parser/container_parser.rs between lines 473 and 499, the property_type
parsing only recognizes text and custom types but does not handle number or
boolean types. Update the match on the token after KeywordOf to also check for
Token::KeywordNumber and Token::KeywordBoolean, returning Some(Type::Number) and
Some(Type::Boolean) respectively, to ensure these types are correctly identified
instead of being treated as custom types.
| self.tokens.next(); | ||
|
|
||
| let name = if let Some(token) = self.tokens.peek() { | ||
| if let Token::Identifier(id) = &token.token { | ||
| self.tokens.next(); | ||
| id.clone() | ||
| } else { | ||
| return Err(ParseError::new( | ||
| format!( | ||
| "Expected identifier after 'action', found {:?}", | ||
| token.token | ||
| ), | ||
| token.line, | ||
| token.column, | ||
| )); | ||
| } | ||
| } else { | ||
| return Err(ParseError::new( | ||
| "Expected identifier after 'action'".to_string(), | ||
| 0, | ||
| 0, | ||
| )); | ||
| }; | ||
|
|
||
| let mut parameters = Vec::new(); | ||
|
|
||
| if let Some(token) = self.tokens.peek().cloned() | ||
| && matches!(token.token, Token::KeywordNeeds) | ||
| { | ||
| self.tokens.next(); | ||
| parameters = self.parse_parameter_list()?; | ||
| } | ||
|
|
||
| let return_type = None; | ||
|
|
||
| self.expect_token(Token::Colon, "Expected ':' after action declaration")?; | ||
|
|
||
| let mut body = Vec::new(); | ||
|
|
||
| loop { | ||
| if let Some(token) = self.tokens.peek() { | ||
| if token.token == Token::KeywordEnd { | ||
| self.tokens.next(); | ||
| break; | ||
| } | ||
| body.push(self.parse_statement()?); | ||
| } else { | ||
| return Err(ParseError::new( | ||
| "Unexpected end of input in action body".to_string(), | ||
| 0, | ||
| 0, | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| Ok(Statement::ActionDefinition { | ||
| name, | ||
| parameters, | ||
| body, | ||
| return_type, | ||
| line: 0, | ||
| column: 0, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Container action: add stray ‘end X’ handling and fix line/column spans
Action bodies should defensively skip stray “end ” tokens (parity with statements.rs) and record accurate spans. Current code returns line/column 0 and will error on stray “end check”, etc.
- pub(crate) fn parse_container_action_definition(&mut self) -> Result<Statement, ParseError> {
- self.tokens.next();
+ pub(crate) fn parse_container_action_definition(&mut self) -> Result<Statement, ParseError> {
+ let action_tok = self.tokens.next().unwrap();
let name = if let Some(token) = self.tokens.peek() {
if let Token::Identifier(id) = &token.token {
self.tokens.next();
id.clone()
} else {
return Err(ParseError::new(
format!(
"Expected identifier after 'action', found {:?}",
token.token
),
token.line,
token.column,
));
}
} else {
return Err(ParseError::new(
"Expected identifier after 'action'".to_string(),
0,
0,
));
};
let mut parameters = Vec::new();
if let Some(token) = self.tokens.peek().cloned()
&& matches!(token.token, Token::KeywordNeeds)
{
self.tokens.next();
parameters = self.parse_parameter_list()?;
}
let return_type = None;
self.expect_token(Token::Colon, "Expected ':' after action declaration")?;
- let mut body = Vec::new();
-
- loop {
- if let Some(token) = self.tokens.peek() {
- if token.token == Token::KeywordEnd {
- self.tokens.next();
- break;
- }
- body.push(self.parse_statement()?);
- } else {
- return Err(ParseError::new(
- "Unexpected end of input in action body".to_string(),
- 0,
- 0,
- ));
- }
- }
+ let mut body = Vec::new();
+ loop {
+ if let Some(token) = self.tokens.peek() {
+ if token.token == Token::KeywordEnd {
+ let mut la = self.tokens.clone();
+ let _ = la.next(); // consume 'end' in lookahead
+ if let Some(next_tok) = la.peek() {
+ match next_tok.token {
+ // Proper terminator: end action
+ Token::KeywordAction => {
+ self.tokens.next(); // consume 'end'
+ self.expect_token(Token::KeywordAction, "Expected 'action' after 'end'")?;
+ break;
+ }
+ // Stray terminators inside action body — skip defensively
+ Token::KeywordCheck
+ | Token::KeywordFor
+ | Token::KeywordCount
+ | Token::KeywordRepeat
+ | Token::KeywordTry
+ | Token::KeywordLoop
+ | Token::KeywordMap
+ | Token::KeywordWhile
+ | Token::KeywordPattern => {
+ // Skip 'end <X>'
+ self.tokens.next();
+ self.tokens.next();
+ continue;
+ }
+ _ => { /* fall through to parse_statement */ }
+ }
+ }
+ }
+ body.push(self.parse_statement()?);
+ } else {
+ return Err(ParseError::new(
+ "Unexpected end of input in action body".to_string(),
+ action_tok.line,
+ action_tok.column,
+ ));
+ }
+ }
Ok(Statement::ActionDefinition {
name,
parameters,
body,
return_type,
- line: 0,
- column: 0,
+ line: action_tok.line,
+ column: action_tok.column,
})
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub(crate) fn parse_container_action_definition(&mut self) -> Result<Statement, ParseError> { | |
| self.tokens.next(); | |
| let name = if let Some(token) = self.tokens.peek() { | |
| if let Token::Identifier(id) = &token.token { | |
| self.tokens.next(); | |
| id.clone() | |
| } else { | |
| return Err(ParseError::new( | |
| format!( | |
| "Expected identifier after 'action', found {:?}", | |
| token.token | |
| ), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected identifier after 'action'".to_string(), | |
| 0, | |
| 0, | |
| )); | |
| }; | |
| let mut parameters = Vec::new(); | |
| if let Some(token) = self.tokens.peek().cloned() | |
| && matches!(token.token, Token::KeywordNeeds) | |
| { | |
| self.tokens.next(); | |
| parameters = self.parse_parameter_list()?; | |
| } | |
| let return_type = None; | |
| self.expect_token(Token::Colon, "Expected ':' after action declaration")?; | |
| let mut body = Vec::new(); | |
| loop { | |
| if let Some(token) = self.tokens.peek() { | |
| if token.token == Token::KeywordEnd { | |
| self.tokens.next(); | |
| break; | |
| } | |
| body.push(self.parse_statement()?); | |
| } else { | |
| return Err(ParseError::new( | |
| "Unexpected end of input in action body".to_string(), | |
| 0, | |
| 0, | |
| )); | |
| } | |
| } | |
| Ok(Statement::ActionDefinition { | |
| name, | |
| parameters, | |
| body, | |
| return_type, | |
| line: 0, | |
| column: 0, | |
| }) | |
| } | |
| pub(crate) fn parse_container_action_definition(&mut self) -> Result<Statement, ParseError> { | |
| let action_tok = self.tokens.next().unwrap(); | |
| let name = if let Some(token) = self.tokens.peek() { | |
| if let Token::Identifier(id) = &token.token { | |
| self.tokens.next(); | |
| id.clone() | |
| } else { | |
| return Err(ParseError::new( | |
| format!( | |
| "Expected identifier after 'action', found {:?}", | |
| token.token | |
| ), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected identifier after 'action'".to_string(), | |
| 0, | |
| 0, | |
| )); | |
| }; | |
| let mut parameters = Vec::new(); | |
| if let Some(token) = self.tokens.peek().cloned() | |
| && matches!(token.token, Token::KeywordNeeds) | |
| { | |
| self.tokens.next(); | |
| parameters = self.parse_parameter_list()?; | |
| } | |
| let return_type = None; | |
| self.expect_token(Token::Colon, "Expected ':' after action declaration")?; | |
| let mut body = Vec::new(); | |
| loop { | |
| if let Some(token) = self.tokens.peek() { | |
| if token.token == Token::KeywordEnd { | |
| // Look ahead one more to see if this is a real "end action" or stray "end <X>" | |
| let mut la = self.tokens.clone(); | |
| let _ = la.next(); // consume 'end' in lookahead | |
| if let Some(next_tok) = la.peek() { | |
| match next_tok.token { | |
| // Proper terminator: end action | |
| Token::KeywordAction => { | |
| self.tokens.next(); // consume 'end' | |
| self.expect_token(Token::KeywordAction, "Expected 'action' after 'end'")?; | |
| break; | |
| } | |
| // Stray terminators inside action body — skip defensively | |
| Token::KeywordCheck | |
| | Token::KeywordFor | |
| | Token::KeywordCount | |
| | Token::KeywordRepeat | |
| | Token::KeywordTry | |
| | Token::KeywordLoop | |
| | Token::KeywordMap | |
| | Token::KeywordWhile | |
| | Token::KeywordPattern => { | |
| // Skip 'end <X>' | |
| self.tokens.next(); | |
| self.tokens.next(); | |
| continue; | |
| } | |
| _ => { /* fall through to parse_statement */ } | |
| } | |
| } | |
| } | |
| body.push(self.parse_statement()?); | |
| } else { | |
| return Err(ParseError::new( | |
| "Unexpected end of input in action body".to_string(), | |
| action_tok.line, | |
| action_tok.column, | |
| )); | |
| } | |
| } | |
| Ok(Statement::ActionDefinition { | |
| name, | |
| parameters, | |
| body, | |
| return_type, | |
| line: action_tok.line, | |
| column: action_tok.column, | |
| }) | |
| } |
🤖 Prompt for AI Agents
In src/parser/container_parser.rs around lines 716 to 780, the
parse_container_action_definition function does not handle stray "end <X>"
tokens inside action bodies and sets line and column to 0 in the returned
Statement, which is inaccurate. Update the loop parsing the action body to
detect and skip stray "end <X>" tokens similarly to statements.rs, and capture
the correct line and column information from tokens when creating the
Statement::ActionDefinition to provide accurate span data.
| #[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 { | ||
| Self { | ||
| message, | ||
| line, | ||
| column, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add span/source context to integrate with unified diagnostics.
Current ParseError captures line/column only. Our diagnostics require precise spans (and often source id) + optional help/suggestions. Consider extending ParseError to carry a span/range and optional suggestion, or provide From/Into conversions to the diagnostics type.
If you prefer not to change ParseError’s shape, add a From for the diagnostics error that enriches with spans from TokenWithPosition.
🤖 Prompt for AI Agents
In src/parser/error.rs lines 3 to 18, the ParseError struct currently only
stores line and column, but diagnostics require precise spans and optional
suggestions. To fix this, extend ParseError to include a span or range field and
an optional suggestion field, or alternatively implement a From<ParseError>
conversion to the diagnostics error type that enriches the error with span
information from TokenWithPosition. This will enable better integration with the
unified diagnostics system.
| Token::KeywordDivided => { | ||
| self.tokens.next(); | ||
| self.expect_token(Token::KeywordBy, "Expected 'by' after 'divided'")?; | ||
| self.tokens.next(); | ||
| Some((Operator::Divide, 2)) | ||
| } |
There was a problem hiding this comment.
Critical: extra next() after “divided by” consumes the operand.
In the Token::KeywordDivided arm, expect_token already consumes 'by'. The subsequent self.tokens.next() skips the next token (likely the right-hand operand).
Token::KeywordDivided => {
self.tokens.next();
self.expect_token(Token::KeywordBy, "Expected 'by' after 'divided'")?;
- self.tokens.next();
Some((Operator::Divide, 2))
}🤖 Prompt for AI Agents
In src/parser/expressions.rs around lines 54 to 59, remove the extra
self.tokens.next() call after expect_token(Token::KeywordBy, ...) because
expect_token already consumes the 'by' token, and the additional next() call
incorrectly consumes the following token, which should be the right-hand
operand.
| Token::Identifier(id) if id == "." => { | ||
| self.tokens.next(); | ||
| if let Some(member_token) = self.tokens.peek().cloned() { | ||
| if let Token::Identifier(member) = &member_token.token { | ||
| self.tokens.next(); | ||
| if let Expression::Variable(container_name, _, _) = expr { | ||
| expr = Expression::StaticMemberAccess { | ||
| container: container_name, | ||
| member: member.clone(), | ||
| line: token.line, | ||
| column: token.column, | ||
| }; | ||
| } else { | ||
| return Err(ParseError::new( | ||
| "Static access must start with a container identifier" | ||
| .to_string(), | ||
| token.line, | ||
| token.column, | ||
| )); | ||
| } | ||
| } else { | ||
| return Err(ParseError::new( | ||
| "Expected member name after '.'".to_string(), | ||
| member_token.line, | ||
| member_token.column, | ||
| )); | ||
| } | ||
| } else { | ||
| return Err(ParseError::new( | ||
| "Expected member name after '.'".to_string(), | ||
| token.line, | ||
| token.column, | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
Incorrect dot handling: expecting Identifier(".") instead of Token::Dot.
This path won’t trigger because '.' is tokenized as Token::Dot elsewhere. Use Token::Dot for static access.
- Token::Identifier(id) if id == "." => {
+ Token::Dot => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Token::Identifier(id) if id == "." => { | |
| self.tokens.next(); | |
| if let Some(member_token) = self.tokens.peek().cloned() { | |
| if let Token::Identifier(member) = &member_token.token { | |
| self.tokens.next(); | |
| if let Expression::Variable(container_name, _, _) = expr { | |
| expr = Expression::StaticMemberAccess { | |
| container: container_name, | |
| member: member.clone(), | |
| line: token.line, | |
| column: token.column, | |
| }; | |
| } else { | |
| return Err(ParseError::new( | |
| "Static access must start with a container identifier" | |
| .to_string(), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected member name after '.'".to_string(), | |
| member_token.line, | |
| member_token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected member name after '.'".to_string(), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } | |
| Token::Dot => { | |
| self.tokens.next(); | |
| if let Some(member_token) = self.tokens.peek().cloned() { | |
| if let Token::Identifier(member) = &member_token.token { | |
| self.tokens.next(); | |
| if let Expression::Variable(container_name, _, _) = expr { | |
| expr = Expression::StaticMemberAccess { | |
| container: container_name, | |
| member: member.clone(), | |
| line: token.line, | |
| column: token.column, | |
| }; | |
| } else { | |
| return Err(ParseError::new( | |
| "Static access must start with a container identifier" | |
| .to_string(), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected member name after '.'".to_string(), | |
| member_token.line, | |
| member_token.column, | |
| )); | |
| } | |
| } else { | |
| return Err(ParseError::new( | |
| "Expected member name after '.'".to_string(), | |
| token.line, | |
| token.column, | |
| )); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/parser/expressions.rs around lines 1018 to 1052, the code incorrectly
checks for Token::Identifier with value "." instead of Token::Dot for static
member access. Replace the pattern matching from Token::Identifier(id) if id ==
"." to directly matching Token::Dot to correctly handle the dot token as
intended.
| pub(crate) fn parse_add_operation(&mut self) -> Result<Statement, ParseError> { | ||
| let _saved_position = self.tokens.clone(); | ||
| let add_token = self.tokens.next().unwrap(); | ||
|
|
||
| let value = self.parse_expression()?; | ||
|
|
||
| if let Some(token) = self.tokens.peek() { | ||
| if token.token == Token::KeywordTo { | ||
| self.tokens.next(); | ||
|
|
||
| let target_name = self.parse_variable_name_simple()?; | ||
|
|
||
| match &value { | ||
| Expression::Literal(Literal::Integer(_), _, _) | ||
| | Expression::Literal(Literal::Float(_), _, _) => { | ||
| let operator = Operator::Plus; | ||
| Ok(Statement::Assignment { | ||
| name: target_name.clone(), | ||
| value: Expression::BinaryOperation { | ||
| left: Box::new(Expression::Variable( | ||
| target_name, | ||
| add_token.line, | ||
| add_token.column, | ||
| )), | ||
| operator, | ||
| right: Box::new(value), | ||
| line: add_token.line, | ||
| column: add_token.column, | ||
| }, | ||
| line: add_token.line, | ||
| column: add_token.column, | ||
| }) | ||
| } | ||
| _ => Ok(Statement::AddToListStatement { | ||
| value, | ||
| list_name: target_name, | ||
| line: add_token.line, | ||
| column: add_token.column, | ||
| }), | ||
| } | ||
| } else { | ||
| Err(ParseError::new( | ||
| "Expected 'to' after value in add statement".to_string(), | ||
| add_token.line, | ||
| add_token.column, | ||
| )) | ||
| } | ||
| } else { | ||
| Err(ParseError::new( | ||
| "Unexpected end of input after add value".to_string(), | ||
| add_token.line, | ||
| add_token.column, | ||
| )) | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify ‘add’ semantics: number addition vs list append heuristic
Dispatch is based only on literal numeric values. If a numeric variable/expression is used (not a literal), this will append to list instead of arithmetic add. Ensure this matches the spec and prior behavior.
Please run a quick audit of tests/spec for “add … to …” semantics, especially with non-literal numeric expressions.
🏁 Script executed:
#!/bin/bash
# Search examples/tests that exercise 'add X to Y' with variables/expressions
rg -n --glob 'TestPrograms/**' -e $'^\\s*add\\s+.+\\s+to\\s+'
rg -n --glob 'Docs/**' -e 'add .* to ' -A 2Length of output: 77099
Support non-literal numeric expressions in add … to …
The current parser only treats integer/float literals as arithmetic adds and everything else (including Expression::BinaryOperation) as list-appends. However the spec and examples (e.g. “add (x times x) to squares”) expect any numeric expression to perform arithmetic addition.
Please update parse_add_operation in src/parser/statements.rs to include non-literal numeric expressions (e.g. parse trees for x plus y, parenthesized numeric expressions, etc.) in the arithmetic branch, not only Expression::Literal. For instance:
• File: src/parser/statements.rs, fn parse_add_operation
– Change the match &value arm from only literals to also match Expression::BinaryOperation { operator: Operator::Plus, … }.
– Add a test in TestPrograms/ (e.g. a new file or in an existing numeric test) for “add (x times x) to count” or similar to confirm arithmetic behavior.
Example diff sketch:
--- a/src/parser/statements.rs
+++ b/src/parser/statements.rs
@@ pub(crate) fn parse_add_operation(&mut self) -> Result<Statement, ParseError> {
- match &value {
- Expression::Literal(Literal::Integer(_), _, _)
- | Expression::Literal(Literal::Float(_), _, _) => {
+ match &value {
+ // integer/float literals and any “plus” binary expression count as numeric add
+ Expression::Literal(Literal::Integer(_), _, _)
+ | Expression::Literal(Literal::Float(_), _, _)
+ | Expression::BinaryOperation { operator: Operator::Plus, .. } => {
// existing arithmetic assignment branch…And add a TestPrograms snippet like:
let x to 2
let y to 3
add (x times y) to x
display x // should show 8
This ensures both parser and test coverage align with the language spec.
🤖 Prompt for AI Agents
In src/parser/statements.rs between lines 385 and 439, the parse_add_operation
function currently treats only integer and float literals as arithmetic
additions and all other expressions as list appends. Modify the match arm on the
value expression to also include Expression::BinaryOperation nodes that
represent numeric operations (e.g., with operator Operator::Plus) as arithmetic
additions. This means expanding the pattern match to recognize these binary
numeric expressions and handle them in the arithmetic addition branch.
Additionally, add a test case in the TestPrograms directory that uses a numeric
expression like "add (x times y) to x" to verify that the parser correctly
interprets such expressions as arithmetic additions.
| let operator = match self.tokens.peek() { | ||
| Some(t) => match t.token { | ||
| Token::KeywordAdd => Operator::Plus, | ||
| Token::KeywordSubtract => Operator::Minus, | ||
| Token::KeywordMultiply => Operator::Multiply, | ||
| Token::KeywordDivide => Operator::Divide, | ||
| _ => Operator::Plus, | ||
| }, | ||
| None => Operator::Plus, | ||
| }; | ||
| self.tokens.next(); | ||
| let value = self.parse_expression()?; | ||
| self.expect_token(Token::KeywordTo, "Expected 'to' after value")?; | ||
| let name = self.parse_variable_name_simple()?; | ||
| let binary_expr = Expression::BinaryOperation { | ||
| left: Box::new(Expression::Variable(name.clone(), 0, 0)), | ||
| operator, | ||
| right: Box::new(value), | ||
| line: 0, | ||
| column: 0, | ||
| }; | ||
| Ok(Statement::Assignment { | ||
| name, | ||
| value: binary_expr, | ||
| line: 0, | ||
| column: 0, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Arithmetic ops: set correct spans and avoid 0/0 line/column
Capture the operator token for accurate statement and expression spans.
- let operator = match self.tokens.peek() {
+ let operator = match self.tokens.peek() {
Some(t) => match t.token {
Token::KeywordAdd => Operator::Plus,
Token::KeywordSubtract => Operator::Minus,
Token::KeywordMultiply => Operator::Multiply,
Token::KeywordDivide => Operator::Divide,
_ => Operator::Plus,
},
None => Operator::Plus,
};
- self.tokens.next();
- let value = self.parse_expression()?;
+ let op_tok = self.tokens.next().unwrap();
+ let value = self.parse_expression()?;
self.expect_token(Token::KeywordTo, "Expected 'to' after value")?;
let name = self.parse_variable_name_simple()?;
let binary_expr = Expression::BinaryOperation {
- left: Box::new(Expression::Variable(name.clone(), 0, 0)),
+ left: Box::new(Expression::Variable(name.clone(), op_tok.line, op_tok.column)),
operator,
right: Box::new(value),
- line: 0,
- column: 0,
+ line: op_tok.line,
+ column: op_tok.column,
};
Ok(Statement::Assignment {
name,
value: binary_expr,
- line: 0,
- column: 0,
+ line: op_tok.line,
+ column: op_tok.column,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let operator = match self.tokens.peek() { | |
| Some(t) => match t.token { | |
| Token::KeywordAdd => Operator::Plus, | |
| Token::KeywordSubtract => Operator::Minus, | |
| Token::KeywordMultiply => Operator::Multiply, | |
| Token::KeywordDivide => Operator::Divide, | |
| _ => Operator::Plus, | |
| }, | |
| None => Operator::Plus, | |
| }; | |
| self.tokens.next(); | |
| let value = self.parse_expression()?; | |
| self.expect_token(Token::KeywordTo, "Expected 'to' after value")?; | |
| let name = self.parse_variable_name_simple()?; | |
| let binary_expr = Expression::BinaryOperation { | |
| left: Box::new(Expression::Variable(name.clone(), 0, 0)), | |
| operator, | |
| right: Box::new(value), | |
| line: 0, | |
| column: 0, | |
| }; | |
| Ok(Statement::Assignment { | |
| name, | |
| value: binary_expr, | |
| line: 0, | |
| column: 0, | |
| }) | |
| let operator = match self.tokens.peek() { | |
| Some(t) => match t.token { | |
| Token::KeywordAdd => Operator::Plus, | |
| Token::KeywordSubtract => Operator::Minus, | |
| Token::KeywordMultiply => Operator::Multiply, | |
| Token::KeywordDivide => Operator::Divide, | |
| _ => Operator::Plus, | |
| }, | |
| None => Operator::Plus, | |
| }; | |
| let op_tok = self.tokens.next().unwrap(); | |
| let value = self.parse_expression()?; | |
| self.expect_token(Token::KeywordTo, "Expected 'to' after value")?; | |
| let name = self.parse_variable_name_simple()?; | |
| let binary_expr = Expression::BinaryOperation { | |
| left: Box::new(Expression::Variable(name.clone(), op_tok.line, op_tok.column)), | |
| operator, | |
| right: Box::new(value), | |
| line: op_tok.line, | |
| column: op_tok.column, | |
| }; | |
| Ok(Statement::Assignment { | |
| name, | |
| value: binary_expr, | |
| line: op_tok.line, | |
| column: op_tok.column, | |
| }) |
🤖 Prompt for AI Agents
In src/parser/statements.rs around lines 1220 to 1246, the operator token is not
captured, causing the expression and statement spans to default to line 0 and
column 0. Modify the code to capture the operator token from self.tokens.peek()
before calling self.tokens.next(), then use the operator token's line and column
values to set the correct spans in the Expression::BinaryOperation and
Statement::Assignment structs instead of using 0/0.
| if let Some(token) = self.tokens.peek().cloned() | ||
| && let Token::Identifier(id) = &token.token | ||
| && id.to_lowercase() == "loop" | ||
| { | ||
| self.tokens.next(); | ||
| } |
There was a problem hiding this comment.
Support both Identifier(“loop”) and KeywordLoop in ‘exit loop’
Currently only identifiers are accepted. If the lexer emits KeywordLoop, the token is not consumed.
- if let Some(token) = self.tokens.peek().cloned()
- && let Token::Identifier(id) = &token.token
- && id.to_lowercase() == "loop"
- {
- self.tokens.next();
- }
+ if let Some(token) = self.tokens.peek().cloned() {
+ match &token.token {
+ Token::Identifier(id) if id.to_lowercase() == "loop" => {
+ self.tokens.next();
+ }
+ Token::KeywordLoop => {
+ self.tokens.next();
+ }
+ _ => {}
+ }
+ }🤖 Prompt for AI Agents
In src/parser/statements.rs around lines 1712 to 1717, the code only checks for
Token::Identifier with value "loop" but does not handle the case where the token
is KeywordLoop. Update the condition to also accept Token::KeywordLoop and
consume the token in that case to properly support both Identifier("loop") and
KeywordLoop tokens.
| #[allow(dead_code)] | ||
| pub fn synchronize(&mut self) { | ||
| while let Some(token) = self.tokens.peek().cloned() { | ||
| match &token.token { | ||
| Token::KeywordStore | ||
| | Token::KeywordCreate | ||
| | Token::KeywordDisplay | ||
| | Token::KeywordCheck | ||
| | Token::KeywordCount | ||
| | Token::KeywordFor | ||
| | Token::KeywordDefine | ||
| | Token::KeywordIf | ||
| | Token::KeywordPush => { | ||
| break; | ||
| } | ||
| Token::KeywordEnd => { | ||
| self.tokens.next(); | ||
| if let Some(next_token) = self.tokens.peek() { | ||
| match &next_token.token { | ||
| Token::KeywordAction | ||
| | Token::KeywordCheck | ||
| | Token::KeywordFor | ||
| | Token::KeywordCount | ||
| | Token::KeywordRepeat | ||
| | Token::KeywordTry | ||
| | Token::KeywordLoop | ||
| | Token::KeywordWhile => { | ||
| self.tokens.next(); | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| break; | ||
| } | ||
| _ => { | ||
| self.tokens.next(); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
synchronize: incomplete end-token coverage and duplicated “statement starter” logic.
- Missing block types after 'end': include If (common “end if”), and likely Pattern, Container, Interface, Event to align with the modular parser.
- This re-encodes statement starters; prefer delegating to the single source of truth (Parser::is_statement_starter) to avoid drift.
Apply this minimal fix to include “end if”; consider adding other block types similarly:
Token::KeywordEnd => {
self.tokens.next();
if let Some(next_token) = self.tokens.peek() {
match &next_token.token {
Token::KeywordAction
- | Token::KeywordCheck
+ | Token::KeywordCheck
+ | Token::KeywordIf
| Token::KeywordFor
| Token::KeywordCount
| Token::KeywordRepeat
| Token::KeywordTry
| Token::KeywordLoop
| Token::KeywordWhile => {
self.tokens.next();
}
_ => {}
}
}
break;
}🏁 Script executed:
#!/bin/bash
# Confirm if these tokens exist and what other 'end <X>' pairs are supported:
rg -n "Keyword(Action|Check|If|For|Count|Repeat|Try|Loop|While|Pattern|Container|Interface|Event)" src/lexer src/parserLength of output: 13687
🏁 Script executed:
#!/bin/bash
# Search for is_statement_starter definition and usage:
rg -n "is_statement_starter" -A3 src/parser/util.rs src/parserLength of output: 1084
🏁 Script executed:
#!/bin/bash
# Display the definition of is_statement_starter in src/parser/mod.rs
rg -n "fn is_statement_starter" -A12 -B2 src/parser/mod.rsLength of output: 589
🏁 Script executed:
#!/bin/bash
# Display the full is_statement_starter definition
rg -n "fn is_statement_starter" -A20 -B2 src/parser/mod.rs
# Show the top of util.rs to see how Parser is referenced
sed -n '1,30p' src/parser/util.rsLength of output: 1848
Refactor synchronize in src/parser/util.rs to use Parser::is_statement_starter and consume every block-end pair
- Remove the hard-coded “statement starter” list and delegate both the initial break and the post-
endpeek toParser::is_statement_starter - That automatically covers all current starters (Store, Create, Display, Check, If, Count, For, Define, Change, Try, Repeat, Exit, Push, Break, Continue, Skip, Open, Close) and any future ones (e.g.
pattern,container,interface,event) - Ensures “end if” (and any other
end <Keyword>) is consistently skipped
Suggested diff in src/parser/util.rs:
#[allow(dead_code)]
pub fn synchronize(&mut self) {
- while let Some(token) = self.tokens.peek().cloned() {
- match &token.token {
- Token::KeywordStore
- | Token::KeywordCreate
- | Token::KeywordDisplay
- | Token::KeywordCheck
- | Token::KeywordCount
- | Token::KeywordFor
- | Token::KeywordDefine
- | Token::KeywordIf
- | Token::KeywordPush => {
- break;
- }
- Token::KeywordEnd => {
- self.tokens.next();
- if let Some(next_token) = self.tokens.peek() {
- match &next_token.token {
- Token::KeywordAction
- | Token::KeywordCheck
- | Token::KeywordFor
- | Token::KeywordCount
- | Token::KeywordRepeat
- | Token::KeywordTry
- | Token::KeywordLoop
- | Token::KeywordWhile => {
- self.tokens.next();
- }
- _ => {}
- }
- }
- break;
- }
- _ => {
- self.tokens.next();
- }
- }
- }
+ while let Some(token) = self.tokens.peek().cloned() {
+ // stop at any statement-starter
+ if Parser::is_statement_starter(&token.token) {
+ break;
+ }
+ // handle 'end <Keyword>' pairs uniformly
+ if token.token == Token::KeywordEnd {
+ self.tokens.next(); // consume 'end'
+ if let Some(next) = self.tokens.peek() {
+ if Parser::is_statement_starter(&next.token) {
+ self.tokens.next();
+ }
+ }
+ break;
+ }
+ // skip everything else
+ self.tokens.next();
+ }This centralizes your skip logic on the single source of truth, avoids drift, and ensures you won’t miss “end if” or any other future block-end keywords.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[allow(dead_code)] | |
| pub fn synchronize(&mut self) { | |
| while let Some(token) = self.tokens.peek().cloned() { | |
| match &token.token { | |
| Token::KeywordStore | |
| | Token::KeywordCreate | |
| | Token::KeywordDisplay | |
| | Token::KeywordCheck | |
| | Token::KeywordCount | |
| | Token::KeywordFor | |
| | Token::KeywordDefine | |
| | Token::KeywordIf | |
| | Token::KeywordPush => { | |
| break; | |
| } | |
| Token::KeywordEnd => { | |
| self.tokens.next(); | |
| if let Some(next_token) = self.tokens.peek() { | |
| match &next_token.token { | |
| Token::KeywordAction | |
| | Token::KeywordCheck | |
| | Token::KeywordFor | |
| | Token::KeywordCount | |
| | Token::KeywordRepeat | |
| | Token::KeywordTry | |
| | Token::KeywordLoop | |
| | Token::KeywordWhile => { | |
| self.tokens.next(); | |
| } | |
| _ => {} | |
| } | |
| } | |
| break; | |
| } | |
| _ => { | |
| self.tokens.next(); | |
| } | |
| } | |
| } | |
| } | |
| #[allow(dead_code)] | |
| pub fn synchronize(&mut self) { | |
| while let Some(token) = self.tokens.peek().cloned() { | |
| // stop at any statement-starter | |
| if Parser::is_statement_starter(&token.token) { | |
| break; | |
| } | |
| // handle 'end <Keyword>' pairs uniformly | |
| if token.token == Token::KeywordEnd { | |
| self.tokens.next(); // consume 'end' | |
| if let Some(next) = self.tokens.peek() { | |
| if Parser::is_statement_starter(&next.token) { | |
| self.tokens.next(); | |
| } | |
| } | |
| break; | |
| } | |
| // skip everything else | |
| self.tokens.next(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/parser/util.rs between lines 31 and 70, refactor the synchronize function
to replace the hard-coded list of statement starters with calls to
Parser::is_statement_starter for both the initial break condition and the check
after encountering a Token::KeywordEnd. Modify the logic to consume every
block-end pair consistently by skipping the "end" token and the following token
if it is a statement starter according to Parser::is_statement_starter. This
change centralizes the statement starter logic, covers all current and future
starters, and ensures consistent skipping of "end <Keyword>" constructs.
|
@claude can you run pattern.wfl in syntax_test and verify it still works? |
|
Claude finished @logbie's task —— View job Pattern.wfl verification resultsTodo List:
|
1 participant
Parser: modular refactor complete; orchestrator mod.rs, stable behavior, docs + TODOs
Summary
This PR completes the modularization of the monolithic WFL parser (~5000 lines in
mod.rs) into a clean modular architecture. The mainmod.rsfile now serves as a slim orchestrator (under 300 lines) that delegates parsing responsibilities to specialized modules:expect_token,synchronize, etc.)ParseErrordefinitions and formattingKey behavioral preservation measures:
known_actionsisolation within expressions.rsReview & Testing Checklist for Human
Test Programs/directory) to verify no regressions in parsing behaviorscripts/run_heaptrack.shon larger programs to ensure no performance regressions (target: <800MB peak usage)Testing Notes
The defensive "end X" token handling was added during development to resolve a specific test failure (
test_return_from_loop_in_action). This suggests there may be other similar synchronization edge cases that could surface with complex nested structures.Diagram
%%{ init : { "theme" : "default" }}%% graph TD subgraph Legend L1[Major Edit]:::major-edit L2[Minor Edit]:::minor-edit L3[Context/No Edit]:::context end mod_rs["src/parser/mod.rs<br/>(orchestrator)"]:::major-edit statements["src/parser/statements.rs<br/>(NEW)"]:::major-edit expressions["src/parser/expressions.rs<br/>(NEW)"]:::major-edit containers["src/parser/container_parser.rs<br/>(NEW)"]:::major-edit patterns["src/parser/pattern_parser.rs<br/>(NEW)"]:::major-edit utils["src/parser/util.rs<br/>(NEW)"]:::major-edit errors["src/parser/error.rs<br/>(NEW)"]:::major-edit docs["Docs/technical/wfl-parser.md"]:::minor-edit todos["Docs/TODOs/parser-modularization.md<br/>(NEW)"]:::minor-edit mod_rs -->|delegates to| statements mod_rs -->|delegates to| expressions mod_rs -->|delegates to| containers statements -->|uses| utils expressions -->|uses| utils containers -->|uses| utils statements -->|uses| errors expressions -->|uses| errors classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
Docs/TODOs/parser-modularization.mdfor future refinementknown_actionslogic remains isolated in expressions.rs as requestedSummary by CodeRabbit
New Features
Documentation
Bug Fixes
Refactor
Style
Chores