feat: add the route construct for natural-language value dispatch - #576
Conversation
`route` is WFL's natural-language "dispatch on a value" form — the match/switch it previously lacked. Its flagship use is web-server request routing, but the subject is any expression so it is general. Implemented as pure parser-level desugaring: a `route … end route` block lowers to the same `check if / otherwise check if / otherwise` (`Statement::IfStatement`) chain a hand-written dispatch produces, so the analyzer, type checker, and interpreter are unchanged and every existing program is unaffected (No-Unlearning Invariant: a `when` arm is ordinary WFL statements, and the block can always be rewritten back to `check if`). Pattern heads (all validated under the full pipeline, not just --analyze): - `when V` equality - `when V1 or V2` or-chained equalities - `when contains V` substring / list membership - `when one of L` list membership - `when starts with V` starts_with builtin - `when ends with V` ends_with builtin - `otherwise` default arm (optional, must be last) `starts with` / `ends with` sidestep the fragile bareword-operator path (issue #566) by consuming the operator words as tokens directly in the route head; `starts_with` / `ends_with` are now also registered in the type checker. Tests: parser unit tests, a lexer token test, an end-to-end pipeline suite (tests/route_test.rs), and a runnable demo (TestPrograms/route_comprehensive.wfl). Docs: new Docs/04-advanced-features/routing.md cross-linked from control flow; keyword references updated (`route` is the 53rd structural keyword; total 179). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgYFmuEgJnk1dAXBiQhZwv
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a new ChangesRoute construct implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Source as WFL source
participant Lexer
participant Parser
participant RouteParser
participant IfStatement as Desugared AST
Source->>Lexer: route ... when ... otherwise ... end route
Lexer->>Parser: KeywordRoute / KeywordWhen tokens
Parser->>RouteParser: parse_route()
RouteParser->>RouteParser: parse_route_pattern()
RouteParser->>RouteParser: parse_route_arm_body()
RouteParser->>IfStatement: fold arms into nested if/else chain
IfStatement-->>Parser: return Statement
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/parser/tests.rs (1)
1952-1977: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStrengthen
starts_with/ends_withtest to assert arguments, not just callee name.Unlike the sibling
contains/one oftests, this test never checks that the arguments are(subject, literal)in the right order — only that the function name matches. A regression that swapped argument order or hardcoded the subject would slip through.♻️ Suggested strengthening
if let Statement::IfStatement { condition, .. } = starts { assert!( - matches!(condition, Expression::FunctionCall { ref function, .. } - if matches!(**function, Expression::Variable(ref n, ..) if n == "starts_with")) + matches!(condition, Expression::FunctionCall { ref function, ref arguments, .. } + if matches!(**function, Expression::Variable(ref n, ..) if n == "starts_with") + && matches!(arguments[0].value, Expression::Variable(ref n, ..) if n == "path") + && matches!(arguments[1].value, Expression::Literal(Literal::String(ref s), ..) if s.as_ref() == "/api/")) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser/tests.rs` around lines 1952 - 1977, The `route_starts_and_ends_with_desugar_to_builtins` test only checks that `starts_with` and `ends_with` are the callee names, so it can miss regressions in argument ordering. Update the assertions in `parse_single`/`Statement::IfStatement` checks to verify the `Expression::FunctionCall` arguments are exactly the subject expression and the string literal in the expected order, matching the stronger pattern used by the `contains`/`one of` tests. Keep the existing function-name checks, but add argument-shape assertions so a swapped or hardcoded argument fails the test.tests/route_test.rs (1)
15-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport the shared test helpers instead of duplicating them here.
tests/route_test.rsrepeatsget_wfl_binary_path,get_unique_test_file_path, andrun_wfl_programeven thoughtests/test_helpers.rsalready provides the same harness; using the shared module would reduce drift across the e2e tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/route_test.rs` around lines 15 - 85, The test harness in route_test is duplicating shared helpers that already exist in test_helpers, so update the test to use the shared module instead of maintaining local copies. Replace the local get_wfl_binary_path, get_unique_test_file_path, and run_wfl_program usage in route_test with imports or calls to the equivalents from tests/test_helpers.rs, keeping run_ok as the thin wrapper if needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Docs/04-advanced-features/routing.md`:
- Around line 49-57: Update the routing syntax example to match the parser
behavior in route-related docs: the trailing colons after route, when, and
otherwise are optional, not mandatory. Adjust the syntax block in routing.md to
show the optional colon form or explicitly note both forms, and align the
wording with the parser contract in route handling so readers do not think the
colon is required.
In `@Docs/development/route-construct-design.md`:
- Around line 15-24: Reconcile the implementation-status summary in the route
design doc so it consistently reflects the actual change set: the status block
should no longer say there are no type-checker changes if `starts_with` and
`ends_with` are now registered in `typechecker.rs`. Update the bullets around
`route`, `starts_with`, and `ends_with` so the parser/desugaring and
stdlib/type-checker work are described with matching scope and wording.
In `@src/parser/stmt/route.rs`:
- Around line 88-89: `parse_route_pattern` is reusing the parsed subject
expression inside each generated arm, so it gets reevaluated every time an
`IfStatement` condition is checked. Change the route lowering in
`parse_route_pattern` to bind `subject` to a temporary once before building the
nested condition chain, and have each arm compare against that temporary instead
of cloning the original expression. Keep the fix localized to the route
statement generation in `stmt/route.rs`, using `parse_expression` and the
`IfStatement` construction path.
In `@tests/route_test.rs`:
- Around line 54-69: The helper run_wfl_program currently waits on
Command::output() with no timeout, so a stuck wfl run can hang the test suite.
Update this helper to launch the process and poll it with try_wait (or
equivalent timeout logic) using the existing binary_path/test_file setup,
killing the child and failing the test if it does not exit within a reasonable
time. Keep the cleanup of the temporary test file in place after the process
finishes or is terminated.
---
Nitpick comments:
In `@src/parser/tests.rs`:
- Around line 1952-1977: The `route_starts_and_ends_with_desugar_to_builtins`
test only checks that `starts_with` and `ends_with` are the callee names, so it
can miss regressions in argument ordering. Update the assertions in
`parse_single`/`Statement::IfStatement` checks to verify the
`Expression::FunctionCall` arguments are exactly the subject expression and the
string literal in the expected order, matching the stronger pattern used by the
`contains`/`one of` tests. Keep the existing function-name checks, but add
argument-shape assertions so a swapped or hardcoded argument fails the test.
In `@tests/route_test.rs`:
- Around line 15-85: The test harness in route_test is duplicating shared
helpers that already exist in test_helpers, so update the test to use the shared
module instead of maintaining local copies. Replace the local
get_wfl_binary_path, get_unique_test_file_path, and run_wfl_program usage in
route_test with imports or calls to the equivalents from tests/test_helpers.rs,
keeping run_ok as the thin wrapper if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ab0ada4-2a1d-4144-9ed7-dc76103c8734
📒 Files selected for processing (16)
Docs/03-language-basics/control-flow.mdDocs/04-advanced-features/index.mdDocs/04-advanced-features/routing.mdDocs/development/route-construct-design.mdDocs/reference/keyword-reference.mdDocs/reference/reserved-keywords.mdTestPrograms/route_comprehensive.wflsrc/lexer/tests.rssrc/lexer/token.rssrc/parser/helpers.rssrc/parser/mod.rssrc/parser/stmt/mod.rssrc/parser/stmt/route.rssrc/parser/tests.rssrc/stdlib/typechecker.rstests/route_test.rs
- tests/route_test.rs: reuse the shared tests/test_helpers harness instead of duplicating get_wfl_binary_path/get_unique_test_file_path/run_wfl_program. This also gives the e2e runs a 30s timeout so a hung interpreter fails fast rather than stalling CI. - src/parser/tests.rs: strengthen the starts_with/ends_with desugaring test to assert the (subject, literal) argument shape and order, not just the callee name, matching the sibling contains/one-of tests. - Docs/development/route-construct-design.md: reconcile the implementation-status summary — the route lowering itself needs no analyzer/type-checker/interpreter changes; the starts_with/ends_with type-signature registration is a separate, independent stdlib change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgYFmuEgJnk1dAXBiQhZwv
Address CodeRabbit's observation that a `route` subject is evaluated per `when` arm tested (an inherent property of the documented check-if desugaring). Rather than change the desugaring semantics, document the behavior and recommend binding a side-effectful/expensive subject to a variable first and routing on that variable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgYFmuEgJnk1dAXBiQhZwv
Resolve conflicts from main's #571/#577 (finally/modulo/division/precedence) against the route construct work: - Docs/reference/keyword-reference.md, reserved-keywords.md: combine both keyword additions. route + finally are structural, modulo is an operations keyword, so the totals become: 181 keywords (was 178 base), 54 structural, 96 other, 24 contextual-usable, 7 literals (54 + 24 + 96 + 7 = 181). Fixed the structural count that both branches independently bumped 52->53 (the true combined value is 54). - src/lexer/tests.rs: keep both the route token test and #571's slash/modulo/ finally token tests. - src/parser/tests.rs: keep both the full route desugaring test suite and #571's precedence/of-call/between/above-below/finally/error-binding tests. All lib tests (475), route e2e tests (15), clippy, and the comprehensive route program pass after the merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MgYFmuEgJnk1dAXBiQhZwv
routeis WFL's natural-language "dispatch on a value" form — thematch/switch it previously lacked. Its flagship use is web-server request
routing, but the subject is any expression so it is general.
Implemented as pure parser-level desugaring: a
route … end routeblocklowers to the same
check if / otherwise check if / otherwise(
Statement::IfStatement) chain a hand-written dispatch produces, so theanalyzer, type checker, and interpreter are unchanged and every existing
program is unaffected (No-Unlearning Invariant: a
whenarm is ordinaryWFL statements, and the block can always be rewritten back to
check if).Pattern heads (all validated under the full pipeline, not just --analyze):
when Vequalitywhen V1 or V2or-chained equalitieswhen contains Vsubstring / list membershipwhen one of Llist membershipwhen starts with Vstarts_with builtinwhen ends with Vends_with builtinotherwisedefault arm (optional, must be last)starts with/ends withsidestep the fragile bareword-operator path(issue #566) by consuming the operator words as tokens directly in the
route head;
starts_with/ends_withare now also registered in thetype checker.
Tests: parser unit tests, a lexer token test, an end-to-end pipeline suite
(tests/route_test.rs), and a runnable demo
(TestPrograms/route_comprehensive.wfl). Docs: new
Docs/04-advanced-features/routing.md cross-linked from control flow;
keyword references updated (
routeis the 53rd structural keyword;total 179).
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01MgYFmuEgJnk1dAXBiQhZwv
Summary by CodeRabbit
routecontrol-flow construct for value-based dispatch withwhenarms,otherwisefallback, and “first match wins” semantics.orlists, text matching (starts with/ends with/contains), and membership (one of).routepage and updated advanced-feature learning paths and keyword references to includeroute/when.routeprogram and validation of invalidotherwiseplacement.