diff --git a/Docs/03-language-basics/control-flow.md b/Docs/03-language-basics/control-flow.md index 8e7533fc..ecb41645 100644 --- a/Docs/03-language-basics/control-flow.md +++ b/Docs/03-language-basics/control-flow.md @@ -523,6 +523,63 @@ end check This is automatic and helps performance! +## Stopping the Program + +`exit program` stops the whole program where it stands, with a successful +status: + +```wfl +store name as "world" + +check if name is equal to "": + display "Please give me a name." + exit program +end check + +display "Hello, " with name +``` + +It works anywhere — at top level, inside a loop, inside an action, or inside a +`try` block — and nothing after it runs: + +```wfl +define action called require with parameters value and message: + check if value is equal to "": + display message + exit program + end check +end action + +store user_name as "" +call require with user_name and "usage: greet " +display "Hello, " with user_name +``` + +Because stopping is not a failure, a `when error:` handler never catches +`exit program`. A `finally:` block still runs, so cleanup is not skipped. In a +websocket handler it stops the server too, rather than being reported as a +handler error. + +**What counts as "the program".** Code you bring in with `load module from` or +`include from` becomes part of the program that included it, so stopping inside +it stops everything. A program you launch with `execute file` is a *separate* +program: `exit program` there ends that run and hands control back to the +caller, which carries on. + +**`exit program` vs `exit loop`:** + +| Spelling | What it leaves | +|---|---| +| `break` | The innermost loop | +| `exit loop` (or bare `exit`) | Every enclosing loop | +| `exit program` | The program | + +`exit loop` is about loops only: outside one it has nothing to leave and does +nothing, exactly like a `break` written outside a loop. Write `exit program` +when you mean "stop here". + +To stop with a *failure* status instead, raise an error rather than exiting. + ## Common Mistakes ### Forgetting `end check` diff --git a/Docs/03-language-basics/loops-and-iteration.md b/Docs/03-language-basics/loops-and-iteration.md index cc536620..8d401525 100644 --- a/Docs/03-language-basics/loops-and-iteration.md +++ b/Docs/03-language-basics/loops-and-iteration.md @@ -57,6 +57,32 @@ count from to by : end count ``` +The step is a *distance*, so it is always a positive number: a downward loop +(`count from 10 down to 1 by 2`) subtracts it. A step of `0` or a negative step +could never reach the end value, so it is reported as an error instead of +looping forever. A loop whose range is already empty never runs its body, so +its step is never used and never checked — `count from 5 to 1` does nothing, +whatever step you give it. + +A step also has to be big enough to actually move the counter. Past about 9 +quadrillion, numbers lose the precision to add 1 to them, so +`count from 100000000000000000 to 100000000000000100 by 1` would sit on the +same value forever. WFL reports that instead of running it. + +### How Many Times a Count Loop May Run + +As many times as you ask it to. A count loop has no built-in trip limit — +`count from 1 to 20000` runs 20,000 times, exactly like the equivalent +`repeat while` or `for each`. A loop that never finishes is stopped by the +execution timeout (`timeout_seconds` in `.wflcfg`, 60 seconds by default), +which is the same protection every other loop form gets. + +One exception is worth knowing if you write servers: inside a `main loop` the +execution timeout is suspended, because a server must not time out on its own +uptime. Every loop form is unbounded there, count loops included, so a loop +inside a request handler is only as bounded as the values you give it. Keep +handler loop ranges under your own control rather than a caller's. + ### Count Examples **Count to 100 by tens:** @@ -268,7 +294,7 @@ end repeat ### Break (Exit Loop) -Exit a loop early (if supported): +Exit a loop early: ```wfl count from 1 to 100: @@ -281,9 +307,35 @@ end count display "Loop exited at 5" ``` +`exit loop` (or a bare `exit`) does the same thing, except that it leaves +*every* enclosing loop rather than only the innermost one: + +```wfl +count from 1 to 3 as row: + count from 1 to 3 as col: + display row with "," with col + check if col is equal to 2: + exit loop // leaves both loops + end check + end count +end count + +display "Done" +``` + +**Output:** +``` +1,1 +1,2 +Done +``` + +To stop the whole program rather than a loop, use `exit program` — see +[Stopping the Program](control-flow.md#stopping-the-program). + ### Continue (Skip) -Skip to the next iteration (if supported): +Skip to the next iteration: ```wfl count from 1 to 10: diff --git a/Docs/04-advanced-features/pattern-matching.md b/Docs/04-advanced-features/pattern-matching.md index a60ea9ac..cfe8f606 100644 --- a/Docs/04-advanced-features/pattern-matching.md +++ b/Docs/04-advanced-features/pattern-matching.md @@ -288,6 +288,51 @@ otherwise: end check ``` +## Replacing Matches + +`replace ... with ... in ...` returns a new text with every match replaced: + +```wfl +create pattern separator: + "-" or "_" +end pattern + +store raw as "first-name_last" +store cleaned as replace separator with " " in raw +display cleaned +``` + +**Output:** +``` +first name last +``` + +**Syntax:** +```wfl +replace with in +``` + +The thing being replaced may be a pattern *or* a plain text, which is matched +verbatim — the same statement whether you have grown into patterns yet or not: + +```wfl +store greeting as "hello world world" +display replace "world" with "there" in greeting +``` + +**Output:** +``` +hello there there +``` + +Matches are replaced left to right and never overlap; the original text is +unchanged, so store the result if you need it. A pattern that matches nothing +returns the text as-is. + +The replacement text is inserted literally. Referring to a capture group from +the replacement is not supported yet — build the result with `find all` and +text concatenation when you need that. + ## Real-World Patterns ### Email Validation diff --git a/Docs/05-standard-library/pattern-module.md b/Docs/05-standard-library/pattern-module.md index 042fb9a2..5426ffac 100644 --- a/Docs/05-standard-library/pattern-module.md +++ b/Docs/05-standard-library/pattern-module.md @@ -152,6 +152,53 @@ end for --- +### pattern_replace + +**Purpose:** Replace every match of a pattern with a replacement text. + +**Signature:** +```wfl +replace with in +``` + +**Parameters:** +- `pattern` (Pattern or Text): What to look for. A compiled pattern, or a + literal text matched verbatim (see [replace in the Text module](text-module.md#replace)) +- `replacement` (Text): What each match becomes +- `text` (Text): Text to search + +**Returns:** Text - a new text; the original is unchanged + +**Example:** +```wfl +create pattern spaces: + one or more whitespace +end pattern + +store messy as "too many spaces" +store tidy as replace spaces with " " in messy +display tidy +``` + +**Output:** +``` +too many spaces +``` + +Every match is replaced, left to right, and the scan resumes after each match +— matches never overlap. A pattern that matches nothing returns the text +unchanged. + +**Use Cases:** +- Normalize whitespace or separators +- Redact matched text +- Rewrite formats in place + +**Note:** The replacement is inserted literally; there is no syntax yet for +referring to a capture group from the replacement text. + +--- + ## Pattern in Conditions The most common usage is in conditionals: diff --git a/Docs/05-standard-library/text-module.md b/Docs/05-standard-library/text-module.md index 2c6d45b3..d57ef2d5 100644 --- a/Docs/05-standard-library/text-module.md +++ b/Docs/05-standard-library/text-module.md @@ -395,6 +395,57 @@ store directories as split of filepath by "/" --- +### replace + +**Purpose:** Replace every occurrence of one text with another. + +**Signature:** +```wfl +replace with in +``` + +**Parameters:** +- `needle` (Text or Pattern): What to look for. A plain text is matched + verbatim — characters that mean something in a pattern are just characters + here. A [pattern](pattern-module.md#pattern_replace) may be used instead +- `replacement` (Text): What each occurrence becomes +- `text` (Text): The text to search + +**Returns:** Text - a new text; the original is unchanged + +**Example:** +```wfl +store path as "home/user/documents" +store windows_path as replace "/" with "\\" in path +display windows_path +``` + +**Output:** +``` +home\user\documents +``` + +**More examples:** +```wfl +store greeting as "hello world world" +display replace "world" with "there" in greeting +// Output: hello there there + +store spaced as "a.b.c" +display replace "." with "-" in spaced +// Output: a-b-c +``` + +Occurrences are replaced left to right and never overlap. A needle that does +not occur returns the text unchanged. + +**Use Cases:** +- Swap separators +- Redact or mask text +- Normalize input before comparison + +--- + ### format_number **Purpose:** Format a number as text with a fixed number of decimal places. diff --git a/Docs/reference/keyword-reference.md b/Docs/reference/keyword-reference.md index d2f6505b..12a91ed4 100644 --- a/Docs/reference/keyword-reference.md +++ b/Docs/reference/keyword-reference.md @@ -29,7 +29,7 @@ Quick lookup for all WFL reserved keywords. | `downward` | Count loop direction | ✗ | | `each` | For each loop | ✗ | | `end` | Close block | ✗ | -| `exit` | Exit program/loop | ✗ | +| `exit` | Exit loops (`exit loop`) or the program (`exit program`) | ✗ | | `for` | For loop | ✗ | | `forever` | Infinite loop | ✗ | | `from` | Count loop start | ✗ | @@ -152,7 +152,7 @@ Quick lookup for all WFL reserved keywords. | `one` | Quantifier (one or more) | ✗ | | `optional` | Optional quantifier | ✗ | | `pattern` | Pattern definition | ✓ | -| `replace` | Pattern replacement | ✗ | +| `replace` | Pattern or text replacement | ✗ | | `script` | Unicode script | ✗ | | `split` | Split by pattern | ✗ | | `start` | Start anchor | ✗ | diff --git a/Docs/reference/reserved-keywords.md b/Docs/reference/reserved-keywords.md index 95a4eb0a..8d8580fb 100644 --- a/Docs/reference/reserved-keywords.md +++ b/Docs/reference/reserved-keywords.md @@ -607,7 +607,7 @@ Complete reference table of all 181 keywords. | `exactly` | Other | Pattern | ❌ | `exactly 5 times` | | `execute` | Other | Process | ❌ | `execute command` | | `exists` | Other | File I/O | ❌ | `file exists` | -| `exit` | Other | Control Flow | ❌ | `exit program` | +| `exit` | Other | Control Flow | ❌ | `exit loop` / `exit program` | | `extension` | Contextual | File I/O | ✅ | `file extension` | | `extensions` | Contextual | File I/O | ✅ | `file extensions` | | `extends` | Structural | OOP | ❌ | `container extends` | @@ -681,7 +681,7 @@ Complete reference table of all 181 keywords. | `register` | Other | Web/Network | ❌ | `register handler` | | `remove` | Other | Operations | ❌ | `remove item` | | `repeat` | Structural | Control Flow | ❌ | `repeat 10 times` | -| `replace` | Other | Pattern | ❌ | `replace pattern` | +| `replace` | Other | Pattern | ❌ | `replace with in ` | | `request` | Other | Web/Network | ❌ | `HTTP request` | | `requires` | Structural | OOP | ❌ | `requires action` | | `respond` | Other | Web/Network | ❌ | `respond to request` | diff --git a/History/dev-diary/2026/2026-08-14-issues-698-700-replace-count-cap-exit-program.md b/History/dev-diary/2026/2026-08-14-issues-698-700-replace-count-cap-exit-program.md new file mode 100644 index 00000000..a22f0027 --- /dev/null +++ b/History/dev-diary/2026/2026-08-14-issues-698-700-replace-count-cap-exit-program.md @@ -0,0 +1,163 @@ +# 2026-08-14 — replace, the count-loop cap, and `exit program` (#698, #699, #700) + +## What + +Three long-standing bugs, all found in one sitting while porting a 738-line PHP +JavaScript minifier to WFL, all fixed together: + +- **#698** — `replace with in ` validated its three + arguments and then returned the input unchanged. It parsed, type-checked, + ran, exited 0, and produced a wrong answer with no diagnostic. Compounding + it, the literal 3-argument `replace` in `src/stdlib/text.rs` was unreachable + from WFL source (`replace` lexes as a keyword), so the language shipped with + **no reachable string replacement at all**. +- **#699** — the `count` loop's trip guard was keyed on the loop's *end value* + rather than on the trip count: `end_num > 1_000_000` meant "no limit", + anything smaller meant "10001 trips maximum". So `count from 1 to 20000` + aborted while `count from 1 to 1000001` ran uncapped, and + `count from 2000000 down to 1` was refused because the value it counted + *down to* was small. +- **#700** — `exit program`, the spelling in both keyword reference pages, did + not parse (`program` fell through as an operand: "Variable 'program' is not + defined", exit 3). Bare `exit` parsed but was ignored at top level, so + program termination had no working spelling. + +## Why they mattered + +All three fail in the way that costs the most to debug. #698 and #700 are +silent: a correct-looking program gets a wrong answer or skips a stop, and the +exit status says everything is fine. #699 is loud but backwards — it rejects +ordinary small loops, allows the enormous ones, and names a limit that appears +nowhere in the documentation. + +## How they were fixed + +**#698 — `src/stdlib/pattern.rs`.** `native_pattern_replace` now actually +replaces: `find_all_with_budget` gives non-overlapping matches in ascending +order, and the rebuild walks *characters* (the VM reports character offsets, so +slicing on byte offsets would panic on multibyte input) in a single forward +pass. The needle may now also be a plain `Value::Text`, matched verbatim, which +is what makes literal string replacement reachable: `replace "world" with +"there" in s`. The type checker accepts `Pattern` or `Text` in that position. +Beginner form and expert form are the same statement — the no-unlearning +invariant applied to a gap that previously had no beginner form at all. + +Capture-group backreferences in the replacement (`$1`) remain unimplemented; +that is a separate enhancement, and the docs now say so rather than implying +otherwise. + +**#699 — `src/interpreter/mod.rs`.** The end-value-keyed cap is gone. `count` +is now bounded by the same execution timeout as `repeat` and `for each`, which +`check_time()` already enforces on every trip — the cap was never what stopped +a runaway loop (`count from 1 to 100000000000` was already killed by the 60 s +timeout, not by the guard). What replaced it is a much narrower check that +catches the case the cap was accidentally covering: a step that cannot move the +counter toward the end value (`by 0`, or a negative step) is refused up front +with a diagnostic that names the step, instead of spinning for 60 seconds. + +**#700 — parser, AST, interpreter.** `ExitStatement` carries an `ExitScope` +now: `exit` / `exit loop` keep exactly their old meaning (leave every enclosing +loop), and `exit program` is the new form. It is raised as an +`ErrorKind::ExitProgram` sentinel rather than a `ControlFlow` variant, because +it has to unwind action calls and expression evaluation, neither of which +carries a control-flow channel. The sentinel is deliberately not catchable: +`when error:` re-raises it (a `finally:` block still runs), include/module +frames pass it through unwrapped, the concurrent `main loop` propagates it +instead of treating it as a handler failure, and the top of the run turns it +back into a successful finish — remaining top-level statements and the +conventional `main` action are skipped, streams and pending requests are still +finalized, and the process exits 0. + +Bare `exit` at top level is still a no-op, the same as `break` there. That is a +deliberate compatibility choice: making the loop-exit signal terminate the +program would silently change what existing programs do after a loop. The two +reference pages now document the split (`exit loop` vs `exit program`) instead +of a form that errored. + +## Testing + +Red first: `tests/issues_698_700_test.rs` was committed as a test-only commit +in which 16 of its 19 tests fail against unmodified source, each for the +reason its issue describes. The three that already passed pin behaviour the fix +must not change — a non-matching pattern leaves its text alone, a +non-text/non-pattern needle is refused, and bare `exit`/`exit loop` still +leaves the loop and lets the program continue. + +The tests run the real binary, so exit status is part of what is asserted — +which is the whole point for #698 and #700, where the old failure was a wrong +answer with status 0. + +End-to-end WFL coverage lives in `TestPrograms/replace_and_count_loop_test.wfl` +(10 `test` blocks, 11 `expect` assertions) and +`TestPrograms/exit_program_test.wfl`, which stops halfway through and relies on +the gated runner's "exit 0" to assert that a clean stop is a successful stop. +The statements after the stop end in a read of a file that cannot exist, so +unexpected continuation fails the run rather than merely printing a line. + +## Docs shipped with the change + +- `Docs/04-advanced-features/pattern-matching.md` — new "Replacing Matches". +- `Docs/05-standard-library/pattern-module.md` — `pattern_replace`. +- `Docs/05-standard-library/text-module.md` — `replace` (literal form). +- `Docs/03-language-basics/control-flow.md` — new "Stopping the Program", + with the `break` / `exit loop` / `exit program` table. +- `Docs/03-language-basics/loops-and-iteration.md` — the step must be + positive; a count loop has no trip limit; `exit loop` documented next to + `break`, and the "(if supported)" hedges removed now that both are pinned by + tests. +- Both keyword reference pages updated together. + +## What review caught + +Automated review of the fix found five more defects in it, all of the same +family — a change that is right in the common path and wrong at a boundary +nobody walked: + +- **Two more places swallowed the stop sentinel.** The websocket event + dispatcher printed every handler error and returned `Ok(())`, so + `exit program` inside `on websocket message:` printed + `WebSocket message handler error: [Exit program]` and the pump carried on. + A `test` block records any non-assertion error as a failure, so stopping + inside a test was reported as a failing test and the run continued with a + nonzero status — the exact opposite of "a clean stop is a successful stop". + Both now use the same `is_exit_program()` pass-through as the other + boundaries. The lesson is that adding an out-of-band signal means auditing + *every* place that absorbs a `Result`, not just the ones the feature's own + tests walk. +- **The step check broke a working program.** Validating the step + unconditionally turned `count from 5 to 1 by 0` — an empty range whose body + never ran, and which therefore never cared about its step — into a runtime + error. The check now runs only when the loop is actually entered. A fix for + a loud bug quietly became a compatibility break, which is precisely what the + backward-compatibility rule exists to catch. +- **A positive step still is not necessarily a step.** Past 2^53 the counter's + floating-point resolution exceeds the step, so `count + step == count` and + the loop never advances. Removing the trip cap made that endless rather than + merely slow, and inside a `main loop` — where the deadline is suspended — + nothing would have stopped it. The loop now refuses to continue the moment + the counter provably stops moving. +- **A diagnostic contradicted itself.** `TypeError` appends + "Expected X but found Y" whenever both fields are set, so accepting two types + while naming one produced "Expected Pattern or Text to replace, got Number - + Expected Pattern but found Number". With two valid types there is no single + expectation to report, so that field is now left unset. + +Not fixed, deliberately: count loops inside a `main loop` remain unbounded. +The deadline exemption is intentional (a server must not time out on its own +uptime) and applies to every loop form, so `repeat forever` and a `for each` +over a caller-supplied list are equally unbounded there. The removed cap was +never a control against this — it only bit loops whose end value was *below* +1,000,000, so any caller wanting an unbounded loop just passed a larger number. +The real fix is a per-handler deadline, which is a design change rather than a +bug fix; the docs now state the exemption plainly instead of implying the +timeout covers it. + +## Noted, not fixed + +`one or more ` currently matches the *shortest* run, not the longest: +`find digits in "a1b22c333"` returns `1`, and replacing with `one or more +digit` rewrites each digit separately. That is a pattern-VM greediness +question, independent of replacement — `replace` faithfully replaces whatever +`find_all` reports — so it is left for its own issue rather than folded into +this batch. The docs avoid examples that would depend on it, and +`greedy`/`lazy` are lexed but not accepted in that position today. diff --git a/TestPrograms/exit_program_test.wfl b/TestPrograms/exit_program_test.wfl new file mode 100644 index 00000000..4656ca94 --- /dev/null +++ b/TestPrograms/exit_program_test.wfl @@ -0,0 +1,52 @@ +// `exit program` end-to-end (issue #700) +// +// `exit program` is the documented spelling for stopping the whole program. +// It used to fail to parse ("Variable 'program' is not defined", exit 3), and +// bare `exit` did nothing at top level, so program termination had no working +// spelling at all. +// +// This program stops in the middle: everything before the `exit program` runs, +// nothing after it does, and the process still exits 0 — so the runner passing +// this file is itself the assertion that a clean stop is a *successful* stop. + +define action called stop_when_empty with parameters value: + check if value is equal to "": + display "stopping: nothing to do" + exit program + end check + display "continuing with " with value +end action + +define action called main: + display "main must not run after exit program" +end action + +display "step 1: before any loop" + +count from 1 to 5: + display "step 2: trip " with count + check if count is equal to 2: + exit loop + end check +end count + +display "step 3: exit loop left the loop and the program kept going" + +try: + display "step 4: inside try" +when error: + display "step 4 handler must not run" +end try + +call stop_when_empty with "a value" + +display "step 5: about to stop" + +call stop_when_empty with "" + +// Reaching this line means `exit program` did not stop the program. Displaying +// a message would still leave the file exiting 0, so the gated runner would +// call that a pass. Reading a file that cannot exist raises a runtime error +// instead, which is what makes unexpected continuation a *failure*. +display "UNREACHABLE: statements after exit program must not run" +open file at "wfl-exit-program-must-never-open-this.txt" for reading as leaked diff --git a/TestPrograms/replace_and_count_loop_test.wfl b/TestPrograms/replace_and_count_loop_test.wfl new file mode 100644 index 00000000..9ffa6d9f --- /dev/null +++ b/TestPrograms/replace_and_count_loop_test.wfl @@ -0,0 +1,84 @@ +// Replacement and count-loop behaviour (issues #698, #699) +// +// #698: `replace with in ` used to validate its +// arguments and then return the input unchanged, so every program that reached +// for it got a silently wrong answer and a zero exit status. +// +// #699: the count loop's trip guard was keyed on the loop's *end value*, so +// `count from 1 to 20000` aborted at 10001 trips while `count from 1 to +// 1000001` ran uncapped, and a downward loop was capped by the value it +// counted down to. + +create pattern separator: + "-" or "_" +end pattern + +create pattern two_digits: + exactly 2 digit +end pattern + +describe "Replacing text": + + test "a literal needle replaces every occurrence": + store greeting as "hello world world" + expect replace "world" with "there" in greeting to equal "hello there there" + end test + + test "the original text is left alone": + store greeting as "hello world" + store changed as replace "world" with "there" in greeting + expect greeting to equal "hello world" + expect changed to equal "hello there" + end test + + test "a literal needle is matched verbatim": + // Characters that mean something inside a pattern are just characters + // in a literal needle. + expect replace "." with "-" in "a.b.c" to equal "a-b-c" + end test + + test "a needle that never occurs changes nothing": + expect replace "absent" with "x" in "hello world" to equal "hello world" + end test + + test "multibyte text is replaced at the right place": + expect replace "é" with "E" in "aébécé" to equal "aEbEcE" + end test + + test "a pattern replaces every match": + expect replace separator with " " in "first-name_last" to equal "first name last" + end test + + test "a pattern replaces whole matches, not single characters": + expect replace two_digits with "#" in "a12b34c5" to equal "a#b#c5" + end test + +end describe + +describe "Count loops": + + test "a loop runs every trip it was asked for": + store total as 0 + count from 1 to 20000: + change total to total plus 1 + end count + expect total to equal 20000 + end test + + test "a downward loop is not limited by the value it counts down to": + store total as 0 + count from 20000 down to 1: + change total to total plus 1 + end count + expect total to equal 20000 + end test + + test "a stepped loop runs every trip": + store total as 0 + count from 1 to 40000 by 2: + change total to total plus 1 + end count + expect total to equal 20000 + end test + +end describe diff --git a/src/interpreter/error.rs b/src/interpreter/error.rs index 5956ebbd..2ba831cd 100644 --- a/src/interpreter/error.rs +++ b/src/interpreter/error.rs @@ -14,6 +14,12 @@ pub enum ErrorKind { /// Catchable like any other error, but the concurrent `main loop` treats it /// as a normal handler outcome, not a structural failure. Cancelled, + /// Not a failure: an `exit program` statement asking the run to stop where + /// it stands. It travels as an error so it unwinds blocks, loops and + /// action calls alike, but it is deliberately **not** catchable by + /// `when error` and never reaches the user as a diagnostic — the top of + /// the run turns it back into a successful finish. + ExitProgram, FileNotFound, PermissionDenied, ProcessNotFound, @@ -48,6 +54,23 @@ impl RuntimeError { kind, } } + + /// The sentinel raised by `exit program`. See [`ErrorKind::ExitProgram`]: + /// it unwinds like an error but finishes the run successfully. + pub fn exit_program(line: usize, column: usize) -> Self { + RuntimeError { + message: "exit program".to_string(), + line, + column, + kind: ErrorKind::ExitProgram, + } + } + + /// True for the `exit program` sentinel, which must never be caught by + /// `when error`, rewrapped by an include/module frame, or reported. + pub fn is_exit_program(&self) -> bool { + matches!(self.kind, ErrorKind::ExitProgram) + } } impl fmt::Display for RuntimeError { @@ -58,6 +81,7 @@ impl fmt::Display for RuntimeError { ErrorKind::Timeout => "[Timeout] ", ErrorKind::ResourceLimit => "[Resource limit] ", ErrorKind::Cancelled => "[Cancelled] ", + ErrorKind::ExitProgram => "[Exit program] ", ErrorKind::FileNotFound => "[File not found] ", ErrorKind::PermissionDenied => "[Permission denied] ", ErrorKind::ProcessNotFound => "[Process not found] ", diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 26af109d..e3eb2762 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -49,7 +49,7 @@ use crate::exec_var_declare; #[cfg(debug_assertions)] use crate::logging::IndentGuard; use crate::parser::ast::{ - Assertion, Expression, FileOpenMode, Literal, Operator, Program, Statement, Type, + Assertion, ExitScope, Expression, FileOpenMode, Literal, Operator, Program, Statement, Type, UnaryOperator, WsHandlerEvent, }; use crate::pattern::CompiledPattern; @@ -182,6 +182,12 @@ fn classify_concurrent_handler_error( error: &RuntimeError, accepted_request: bool, ) -> ConcurrentHandlerDisposition { + // `exit program` is a deliberate stop, so it must leave the loop and unwind + // to the top of the run — never be absorbed as a request-local outcome, + // even for a handler that already accepted a request. + if error.is_exit_program() { + return ConcurrentHandlerDisposition::Structural; + } let request_wait_timeout = error.kind == ErrorKind::Timeout && error.message.starts_with(REQUEST_WAIT_TIMEOUT_PREFIX); if accepted_request || error.kind == ErrorKind::Cancelled || request_wait_timeout { @@ -6344,6 +6350,11 @@ impl Interpreter { // // Structural pre-request failures feed the breaker. Some((Ok(Err(err)), accepted)) => { + // `exit program` stops the server, so it unwinds out of the + // loop instead of being logged and retried. + if err.is_exit_program() { + return Err(err); + } match classify_concurrent_handler_error(&err, accepted) { ConcurrentHandlerDisposition::RequestLocal => { log::debug!( @@ -6789,6 +6800,10 @@ impl Interpreter { let mut last_value = Value::Null; let mut errors = Vec::new(); + // Set when an `exit program` statement asked the run to stop: the + // remaining top-level statements and the conventional `main` action are + // skipped, but the cleanup below still runs and the run still succeeds. + let mut exited = false; #[allow(unused_variables)] for (i, statement) in program.statements.iter().enumerate() { @@ -6844,6 +6859,13 @@ impl Interpreter { ControlFlow::None => {} } } + // `exit program`: a successful stop, not a failure. Nothing is + // reported and nothing further runs. + Err(err) if err.is_exit_program() => { + exec_trace!("Program exited via `exit program`"); + exited = true; + break; + } Err(err) => { if !self.step_mode { exec_trace!( @@ -6862,7 +6884,7 @@ impl Interpreter { // Run the conventional `main` action (if any) before cleanup, so a // stream/request opened by `main` is finalized by the drain below rather // than leaking. - if errors.is_empty() { + if errors.is_empty() && !exited { let main_func_opt = { match self.global_env.borrow().get("main") { Some(Value::Function(main_func)) => Some(main_func.clone()), @@ -6883,6 +6905,11 @@ impl Interpreter { exec_trace!("Main function returned: {:?}", value); last_value = value } + // `exit program` inside `main` finishes the run cleanly, + // exactly as it does at top level. + Err(err) if err.is_exit_program() => { + exec_trace!("Program exited from main via `exit program`"); + } Err(err) => { exec_trace!("Main function failed: {}", err); errors.push(err); @@ -7385,14 +7412,29 @@ impl Interpreter { Box::new(|count, end_num| count <= end_num) }; - let max_iterations = if end_num > 1000000.0 { - u64::MAX // Effectively no limit for large end values, rely on timeout instead - } else { - // Allow up to 10001 iterations to accommodate loops that need exactly 10000 - // (e.g., "count from 1 to 10000" requires 10000 iterations) - 10001 - }; - let mut iterations = 0; + // A step that cannot move the counter toward the end value would + // spin until the execution timeout, so it is refused up front + // with a diagnostic that names the problem. `count` always moves + // *toward* its end value (a downward loop subtracts the step), so + // the step is a magnitude and must be a positive, finite number. + // + // Only a loop that actually runs is validated: `count from 5 to 1` + // never enters its body, so its step never mattered and a program + // that passed a nonsense one has always completed successfully. + // Validating unconditionally would break those programs. + if should_continue(count, end_num) && (!step_num.is_finite() || step_num <= 0.0) { + *self.current_count.borrow_mut() = previous_count; + *self.in_count_loop.borrow_mut() = was_in_count_loop; + return Err(RuntimeError::new( + format!( + "Count loop step must be a positive number, got {step_num}. \ + A count loop always moves toward its end value, so \ + `count from 10 down to 1 by 2` steps down by 2." + ), + *line, + *column, + )); + } *self.in_count_loop.borrow_mut() = true; @@ -7400,7 +7442,13 @@ impl Interpreter { let loop_var_name = variable_name.as_deref().unwrap_or("count"); let mut loop_env_recycle = None; - while should_continue(count, end_num) && iterations < max_iterations { + // No trip-count ceiling: `count` is bounded by the same + // execution timeout as `repeat` and `for each` (checked below on + // every trip), so an ordinary long loop runs and a runaway one + // still stops. The previous guard keyed on the *end value*, which + // capped `count from 1 to 20000` while letting + // `count from 1 to 1000001` run uncapped (issue #699). + while should_continue(count, end_num) { self.check_time()?; *self.current_count.borrow_mut() = Some(count); @@ -7454,26 +7502,36 @@ impl Interpreter { } } - if *downward { - count -= step_num; + // A positive step is not enough: past 2^53 the counter's + // floating-point resolution exceeds the step, so `count` + // stops changing and the loop can never reach its end. That + // is an endless loop the execution timeout would catch only + // outside a `main loop`, where the deadline is suspended — + // so it is refused here, where the stall is provable. + let next = if *downward { + count - step_num } else { - count += step_num; + count + step_num + }; + if next == count { + *self.current_count.borrow_mut() = previous_count; + *self.in_count_loop.borrow_mut() = was_in_count_loop; + return Err(RuntimeError::new( + format!( + "Count loop step {step_num} is too small to move the counter \ + past {count}, so the loop can never reach {end_num}. \ + Numbers this large lose precision — use a larger step." + ), + *line, + *column, + )); } - - iterations += 1; + count = next; } *self.current_count.borrow_mut() = previous_count; *self.in_count_loop.borrow_mut() = was_in_count_loop; - if iterations >= max_iterations { - return Err(RuntimeError::new( - format!("Count loop exceeded maximum iterations ({max_iterations})"), - *line, - *column, - )); - } - Ok((Value::Null, ControlFlow::None)) } @@ -7839,10 +7897,23 @@ impl Interpreter { Ok((Value::Null, ControlFlow::Continue)) } - Statement::ExitStatement { .. } => { + Statement::ExitStatement { + scope, + line, + column, + } => { #[cfg(debug_assertions)] - exec_trace!("Executing exit statement"); - Ok((Value::Null, ControlFlow::Exit)) + exec_trace!("Executing exit statement ({scope:?})"); + match scope { + // `exit` / `exit loop`: unwind the enclosing loop(s). + ExitScope::Loop => Ok((Value::Null, ControlFlow::Exit)), + // `exit program`: stop the run where it stands. Raised as + // the `ExitProgram` sentinel so it unwinds action calls and + // expression evaluation too — neither of which carries a + // control-flow channel — and is turned back into a + // successful finish at the top of the run. + ExitScope::Program => Err(RuntimeError::exit_program(*line, *column)), + } } Statement::OpenFileStatement { @@ -8550,6 +8621,10 @@ impl Interpreter { *line, *column, )), + // `exit program` inside a module stops the whole run, so it + // travels untouched rather than being retitled as a module + // failure. + Err(e) if e.is_exit_program() => Err(e), Err(e) => { // Capture chain BEFORE guard drops (while current module is still on stack) let chain = _guard.get_chain(); @@ -8727,6 +8802,9 @@ impl Interpreter { *line, *column, )), + // As in module scope: `exit program` stops the whole run and + // must not be retitled as an include failure. + Err(e) if e.is_exit_program() => Err(e), Err(e) => { let chain = _guard.get_chain(); if chain.len() > 1 { @@ -9079,6 +9157,10 @@ impl Interpreter { let primary_result = match self.execute_block(body, Rc::clone(&child_env)).await { Ok(val) => Ok(val), // Success path: just bubble result + // `exit program` is a request to stop, not a failure: no + // `when`/`otherwise` clause may swallow it. (`finally:` + // below still runs, so cleanup is not skipped.) + Err(err) if err.is_exit_program() => Err(err), Err(err) => { // Find matching when clause based on error kind let mut executed = false; @@ -13067,6 +13149,12 @@ impl Interpreter { for stmt in body { match Box::pin(self._execute_statement(stmt, test_env.clone())).await { Ok(_) => {} + Err(e) if e.is_exit_program() => { + // Stopping the program is not a failing test. Pass the + // sentinel through so the run ends cleanly instead of + // recording a bogus failure and continuing. + return Err(e); + } Err(e) => { test_passed = false; @@ -13355,6 +13443,12 @@ impl Interpreter { } if let Err(err) = self.execute_block(&body, child_env).await { + // `exit program` is a stop request, not a handler failure: it must + // leave the pump and unwind to the top of the run rather than be + // reported and swallowed like an ordinary handler error. + if err.is_exit_program() { + return Err(err); + } eprintln!( "WebSocket {} handler error: {}", event.kind.as_str(), diff --git a/src/parser/ast.rs b/src/parser/ast.rs index bff30e90..34eff3a8 100644 --- a/src/parser/ast.rs +++ b/src/parser/ast.rs @@ -230,6 +230,7 @@ pub enum Statement { column: usize, }, ExitStatement { + scope: ExitScope, line: usize, column: usize, }, @@ -1161,6 +1162,19 @@ pub enum WriteMode { Append, } +/// What an `exit` statement leaves. +/// +/// `exit` and `exit loop` leave the enclosing loop(s); `exit program` +/// terminates the whole program with a successful status. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ExitScope { + /// `exit` / `exit loop` + #[default] + Loop, + /// `exit program` + Program, +} + #[derive(Debug, Clone, PartialEq)] pub enum FileOpenMode { Read, diff --git a/src/parser/stmt/actions.rs b/src/parser/stmt/actions.rs index 0c82b85e..43e7d713 100644 --- a/src/parser/stmt/actions.rs +++ b/src/parser/stmt/actions.rs @@ -5,6 +5,7 @@ use super::StmtParser; use super::database::DatabaseParser; use crate::exec_trace; use crate::lexer::token::{Token, TokenWithPosition}; +use crate::parser::ast::ExitScope; use crate::parser::expr::ExprParser; /// Maps a token in type position (`x as `, `returns `) to its @@ -641,15 +642,28 @@ impl<'a> ActionParser<'a> for Parser<'a> { fn parse_exit_statement(&mut self) -> Result { let exit_token = self.bump_sync().unwrap(); // Consume "exit" - // Check for "loop" after "exit" - if let Some(token) = self.cursor.peek() - && let Token::Identifier(id) = &token.token - && id.to_lowercase() == "loop" - { - self.bump_sync(); // Consume "loop" + // `exit loop` leaves the enclosing loop(s) (as does a bare `exit`); + // `exit program` terminates the program. `loop` may arrive as its own + // keyword token or as a plain identifier depending on context. + let mut scope = ExitScope::Loop; + if let Some(token) = self.cursor.peek() { + match &token.token { + Token::KeywordLoop => { + self.bump_sync(); // Consume "loop" + } + Token::Identifier(id) if id.to_lowercase() == "loop" => { + self.bump_sync(); // Consume "loop" + } + Token::Identifier(id) if id.to_lowercase() == "program" => { + self.bump_sync(); // Consume "program" + scope = ExitScope::Program; + } + _ => {} + } } Ok(Statement::ExitStatement { + scope, line: exit_token.line, column: exit_token.column, }) diff --git a/src/stdlib/pattern.rs b/src/stdlib/pattern.rs index 1196758b..4ad265d2 100644 --- a/src/stdlib/pattern.rs +++ b/src/stdlib/pattern.rs @@ -235,30 +235,77 @@ pub fn native_pattern_replace( } }; - let _pattern = match &args[1] { - Value::Pattern(p) => p.as_ref(), + let replacement = match &args[2] { + Value::Text(t) => t.as_ref(), _ => { return Err(RuntimeError::new( - "Second argument must be a pattern".to_string(), + "Third argument must be text".to_string(), line, column, )); } }; - let _replacement = match &args[2] { - Value::Text(t) => t.as_ref(), + let pattern = match &args[1] { + Value::Pattern(p) => p, + // A literal needle is the beginner spelling of the same operation: + // `replace "world" with "there" in s`. It is matched literally, so + // pattern syntax inside it is just characters. + Value::Text(needle) => { + return Ok(Value::Text(Arc::from( + text.replace(needle.as_ref(), replacement).as_str(), + ))); + } _ => { return Err(RuntimeError::new( - "Third argument must be text".to_string(), + "Second argument must be a pattern or text".to_string(), line, column, )); } }; - // TODO: Update to use new pattern system for replacement - Ok(Value::Text(Arc::from(text))) + // Every match is replaced, left to right. Matches come back as *character* + // offsets, so the rebuild walks characters rather than slicing on byte + // offsets, which would panic (or silently mis-cut) on multibyte text. + let budget = ExecutionBudget::current_or_default(); + let matches = pattern + .find_all_with_budget(text, &budget) + .map_err(pattern_err)?; + + if matches.is_empty() { + return Ok(Value::Text(Arc::from(text))); + } + + let mut result = String::with_capacity(text.len()); + let mut chars = text.chars(); + let mut char_idx = 0usize; + + // `find_all` yields non-overlapping matches in ascending order (a + // zero-width match advances the scan by one character), so a single + // forward pass over the input is enough. + for match_result in matches { + while char_idx < match_result.start { + match chars.next() { + Some(c) => { + result.push(c); + char_idx += 1; + } + None => break, + } + } + while char_idx < match_result.end { + match chars.next() { + Some(_) => char_idx += 1, + None => break, + } + } + result.push_str(replacement); + } + + result.extend(chars); + + Ok(Value::Text(Arc::from(result.as_str()))) } /// Native function for pattern splitting (called by interpreter) diff --git a/src/typechecker/mod.rs b/src/typechecker/mod.rs index 020bee0e..d33db924 100644 --- a/src/typechecker/mod.rs +++ b/src/typechecker/mod.rs @@ -4004,7 +4004,7 @@ impl TypeChecker { Type::Any }; } - Statement::ExitStatement { line: _, column: _ } => {} + Statement::ExitStatement { .. } => {} Statement::WaitForStatement { inner, line: _line, @@ -8501,10 +8501,19 @@ impl TypeChecker { ); } - if pattern_type != Type::Pattern && !self.is_gradual_type(&pattern_type) { + // The needle may be a compiled pattern or a literal text to + // match verbatim — the same statement, from the beginner form + // to the expert one. + if pattern_type != Type::Pattern + && pattern_type != Type::Text + && !self.is_gradual_type(&pattern_type) + { + // No single `expected` type to report: two are accepted, and + // naming one would make `TypeError`'s rendering append + // "Expected Pattern but found ...", contradicting the message. self.type_error( - format!("Expected Pattern for pattern replacement, got {pattern_type}"), - Some(Type::Pattern), + format!("Expected Pattern or Text to replace, got {pattern_type}"), + None, Some(pattern_type), 0, 0, diff --git a/tests/issues_698_700_test.rs b/tests/issues_698_700_test.rs new file mode 100644 index 00000000..ee28d6be --- /dev/null +++ b/tests/issues_698_700_test.rs @@ -0,0 +1,469 @@ +//! Regression tests for a batch of GitHub issues fixed together: +//! +//! * #698 — `replace with in ` silently returned the +//! input unchanged (exit 0, no diagnostic), and no literal string +//! replacement was reachable from WFL source at all. +//! * #699 — the `count` loop's iteration cap was keyed on the loop's *end +//! value* rather than on the trip count, so `count from 1 to 20000` aborted +//! while `count from 1 to 1000001` ran uncapped. +//! * #700 — `exit program` (the form in the keyword reference) failed to +//! parse, and no spelling terminated the program. +//! +//! These run the real `wfl` binary so exit status is part of what is asserted +//! — a silently-wrong answer with status 0 is exactly the failure mode #698 +//! and #700 describe. + +mod common; +use common::{run_file_status, run_src}; +use tempfile::TempDir; + +// --------------------------------------------------------------------------- +// #698 — replace actually replaces +// --------------------------------------------------------------------------- + +#[test] +fn pattern_replace_replaces_every_match() { + let (out, code) = run_src( + "create pattern w:\n \"world\"\nend pattern\n\ + store s as \"hello world world\"\n\ + display \"[\" with (replace w with \"there\" in s) with \"]\"\n", + ); + assert!( + out.contains("[hello there there]"), + "every match must be replaced: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn pattern_replace_leaves_a_non_matching_text_alone() { + let (out, code) = run_src( + "create pattern w:\n \"absent\"\nend pattern\n\ + store s as \"hello world\"\n\ + display \"[\" with (replace w with \"x\" in s) with \"]\"\n", + ); + assert!( + out.contains("[hello world]"), + "a pattern that never matches leaves the text unchanged: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn pattern_replace_is_character_correct_on_multibyte_text() { + // The replacement walks character offsets from the pattern VM; multibyte + // input must not be sliced on a byte boundary (which would panic) nor + // replaced at the wrong offset. + let (out, code) = run_src( + "create pattern p:\n \"é\"\nend pattern\n\ + store s as \"aébécé\"\n\ + display \"[\" with (replace p with \"E\" in s) with \"]\"\n", + ); + assert!( + out.contains("[aEbEcE]"), + "multibyte text must be replaced at character offsets: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn pattern_replace_replaces_whole_multi_character_matches() { + // The whole match is what gets replaced, not its first character, and the + // scan resumes after it — so the trailing lone digit is left alone. + let (out, code) = run_src( + "create pattern two_digits:\n exactly 2 digit\nend pattern\n\ + store s as \"a12b34c5\"\n\ + display \"[\" with (replace two_digits with \"#\" in s) with \"]\"\n", + ); + assert!( + out.contains("[a#b#c5]"), + "each whole match is replaced exactly once: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn literal_text_replace_is_reachable_from_wfl_source() { + // #698's compounding half: `replace` lexes as a keyword, so the 3-argument + // native `replace` could never be called. A literal needle in the pattern + // slot is the reachable spelling. + let (out, code) = run_src( + "store s as \"hello world world\"\n\ + display \"[\" with (replace \"world\" with \"there\" in s) with \"]\"\n", + ); + assert!( + out.contains("[hello there there]"), + "a literal text needle must replace every occurrence: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn literal_text_replace_treats_the_needle_as_text_not_a_pattern() { + // A literal needle is compared literally: pattern metacharacters in it are + // just characters. + let (out, code) = run_src( + "store s as \"a.b.c\"\n\ + display \"[\" with (replace \".\" with \"-\" in s) with \"]\"\n", + ); + assert!( + out.contains("[a-b-c]"), + "the needle is matched literally: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn replace_rejects_a_needle_that_is_neither_text_nor_pattern() { + let (out, code) = run_src( + "store s as \"hello\"\n\ + store n as 5\n\ + display replace n with \"x\" in s\n", + ); + assert_ne!(code, Some(0), "a number needle must not be accepted: {out}"); +} + +// --------------------------------------------------------------------------- +// #699 — the count loop runs the iterations it was asked for +// --------------------------------------------------------------------------- + +#[test] +fn count_loop_runs_twenty_thousand_iterations() { + let (out, code) = run_src( + "store total as 0\n\ + count from 1 to 20000:\n change total to total plus 1\nend count\n\ + display \"total: \" with total\n", + ); + assert!(out.contains("total: 20000"), "expected 20000 trips: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn count_loop_runs_just_past_the_old_cap() { + let (out, code) = run_src( + "store total as 0\n\ + count from 1 to 10001:\n change total to total plus 1\nend count\n\ + display \"total: \" with total\n", + ); + assert!(out.contains("total: 10001"), "expected 10001 trips: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn downward_count_loop_is_not_capped_by_its_end_value() { + // The old guard read `end_num`, so a loop counting *down* to 1 was capped + // at 10001 trips however many it actually needed. + let (out, code) = run_src( + "store total as 0\n\ + count from 20000 down to 1:\n change total to total plus 1\nend count\n\ + display \"total: \" with total\n", + ); + assert!(out.contains("total: 20000"), "expected 20000 trips: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn count_loop_with_a_step_still_runs_every_trip() { + let (out, code) = run_src( + "store total as 0\n\ + count from 1 to 40000 by 2:\n change total to total plus 1\nend count\n\ + display \"total: \" with total\n", + ); + assert!(out.contains("total: 20000"), "expected 20000 trips: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn count_loop_rejects_a_step_that_never_reaches_the_end() { + // Removing the trip cap must not turn a typo into a 60-second hang: a step + // that cannot advance the counter toward the end value is refused up front + // with a diagnostic that names the problem. + let (out, code) = run_src("count from 1 to 10 by 0:\n display \"tick\"\nend count\n"); + assert_ne!(code, Some(0), "a zero step must be an error: {out}"); + assert!( + out.to_lowercase().contains("step"), + "the diagnostic must name the step: {out}" + ); + assert!( + !out.contains("tick"), + "the loop body must not run at all: {out}" + ); +} + +// --------------------------------------------------------------------------- +// #700 — `exit program` parses and terminates +// --------------------------------------------------------------------------- + +#[test] +fn exit_program_stops_the_program_at_top_level() { + let (out, code) = run_src("display \"before\"\nexit program\ndisplay \"after\"\n"); + assert!( + out.contains("before"), + "statements before it still run: {out}" + ); + assert!(!out.contains("after"), "nothing after it runs: {out}"); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn exit_program_stops_the_program_from_inside_a_loop() { + let (out, code) = run_src( + "count from 1 to 5:\n\ + \x20 display \"tick \" with count\n\ + \x20 check if count is equal to 2:\n\ + \x20 exit program\n\ + \x20 end check\n\ + end count\n\ + display \"after\"\n", + ); + assert!(out.contains("tick 1"), "the loop starts: {out}"); + assert!(out.contains("tick 2"), "the deciding trip runs: {out}"); + assert!(!out.contains("tick 3"), "later trips do not run: {out}"); + assert!( + !out.contains("after"), + "code after the loop does not run: {out}" + ); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn exit_program_stops_the_program_from_inside_an_action() { + let (out, code) = run_src( + "define action called bail:\n\ + \x20 display \"bailing\"\n\ + \x20 exit program\n\ + end action\n\ + display \"before\"\n\ + call bail\n\ + display \"after\"\n", + ); + assert!( + out.contains("before"), + "statements before it still run: {out}" + ); + assert!(out.contains("bailing"), "the action body runs: {out}"); + assert!(!out.contains("after"), "the caller does not resume: {out}"); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn exit_program_is_not_caught_by_error_handling() { + // Program termination is not a failure, so a `when error` handler must not + // swallow it and carry on. + let (out, code) = run_src( + "try:\n\ + \x20 display \"trying\"\n\ + \x20 exit program\n\ + when error:\n\ + \x20 display \"caught\"\n\ + end try\n\ + display \"after\"\n", + ); + assert!(out.contains("trying"), "the try block runs: {out}"); + assert!(!out.contains("caught"), "the handler must not run: {out}"); + assert!(!out.contains("after"), "nothing after the try runs: {out}"); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn exit_program_does_not_leak_an_error_message() { + let (out, code) = run_src("display \"before\"\nexit program\n"); + assert!( + !out.to_lowercase().contains("error"), + "a clean exit prints no diagnostic: {out}" + ); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn exit_program_skips_the_main_action() { + // `main` runs after the top-level statements; terminating the program must + // cancel that too. + let (out, code) = run_src( + "define action called main:\n\ + \x20 display \"main ran\"\n\ + end action\n\ + display \"before\"\n\ + exit program\n", + ); + assert!(out.contains("before"), "top-level statements run: {out}"); + assert!(!out.contains("main ran"), "main must not run: {out}"); + assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); +} + +#[test] +fn bare_exit_still_leaves_the_loop_and_keeps_going() { + // Backward compatibility: bare `exit` / `exit loop` are the loop-exit + // spelling and keep their existing meaning. + let (out, code) = run_src( + "count from 1 to 5:\n\ + \x20 display \"tick \" with count\n\ + \x20 check if count is equal to 2:\n\ + \x20 exit loop\n\ + \x20 end check\n\ + end count\n\ + display \"after\"\n", + ); + assert!(out.contains("tick 2"), "the deciding trip runs: {out}"); + assert!(!out.contains("tick 3"), "the loop stops: {out}"); + assert!(out.contains("after"), "the program continues: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +// --------------------------------------------------------------------------- +// Follow-ups raised in review of the fix itself +// --------------------------------------------------------------------------- + +#[test] +fn count_loop_with_an_empty_range_ignores_its_step() { + // Backward compatibility: the step only matters if the loop is entered. + // `count from 5 to 1` never runs its body, so a nonsense step was — and + // must remain — harmless. Validating it unconditionally turned a working + // program into a runtime error. + let (out, code) = run_src( + "count from 5 to 1 by 0:\n\ + \x20 display \"never\"\n\ + end count\n\ + display \"done\"\n", + ); + assert!(!out.contains("never"), "the body must not run: {out}"); + assert!(out.contains("done"), "the program continues: {out}"); + assert_eq!(code, Some(0), "an empty range is not an error: {out}"); +} + +#[test] +fn count_loop_with_an_empty_downward_range_ignores_its_step() { + let (out, code) = run_src( + "count from 1 down to 5 by 0:\n\ + \x20 display \"never\"\n\ + end count\n\ + display \"done\"\n", + ); + assert!(!out.contains("never"), "the body must not run: {out}"); + assert!(out.contains("done"), "the program continues: {out}"); + assert_eq!(code, Some(0), "an empty range is not an error: {out}"); +} + +#[test] +fn count_loop_rejects_a_step_too_small_to_advance_the_counter() { + // A positive, finite step is not enough: below the counter's floating-point + // resolution `count + step == count`, so the loop can never reach its end. + // 100000000000000000 plus 1 is still 100000000000000000. + let started = std::time::Instant::now(); + let (out, code) = run_src( + "count from 100000000000000000 to 100000000000000100 by 1:\n\ + \x20 display \"tick\"\n\ + end count\n", + ); + let elapsed = started.elapsed(); + assert_ne!(code, Some(0), "a non-advancing step must be refused: {out}"); + assert!( + out.to_lowercase().contains("step"), + "the diagnostic must name the step: {out}" + ); + assert!( + elapsed < std::time::Duration::from_secs(20), + "the loop must be refused up front, not spin until the timeout \ + (took {elapsed:?}): {out}" + ); +} + +#[test] +fn exit_program_inside_a_test_block_stops_the_run_cleanly() { + // A test block records a runtime error in its body as a test failure. The + // stop sentinel is not a failure: it must pass straight through, leaving a + // clean exit rather than a bogus failing test. + let dir = TempDir::new().expect("tempdir"); + std::fs::write( + dir.path().join("main.wfl"), + "describe \"stopping\":\n\ + \x20 test \"stops the run\":\n\ + \x20 display \"inside the test\"\n\ + \x20 exit program\n\ + \x20 end test\n\ + \x20 test \"must not run\":\n\ + \x20 display \"later test ran\"\n\ + \x20 end test\n\ + end describe\n", + ) + .expect("write program"); + + let (out, code) = run_file_status(&dir, "main.wfl", &["--test"]); + assert!(out.contains("inside the test"), "the test body runs: {out}"); + assert!( + !out.contains("later test ran"), + "the run stops at the exit: {out}" + ); + assert!( + out.contains("Failed: 0"), + "stopping is not a test failure: {out}" + ); + assert_eq!(code, Some(0), "a clean stop is a successful stop: {out}"); +} + +#[test] +fn replace_needle_diagnostic_does_not_contradict_itself() { + // The message says Pattern *or* Text is accepted; the structured + // expected/found pair must not then render "Expected Pattern but found ...". + let (out, code) = run_src("display replace 5 with \"a\" in \"text\"\n"); + assert_ne!(code, Some(0), "a number needle is refused: {out}"); + assert!( + out.contains("Pattern or Text"), + "the diagnostic names both accepted types: {out}" + ); + assert!( + !out.contains("Expected Pattern but found"), + "the diagnostic must not contradict itself: {out}" + ); +} + +#[test] +fn count_loop_rejects_a_negative_step() { + // A negative step moves *away* from the end value. The step is a magnitude + // — a downward loop is spelled `count from 10 down to 1 by 2`. + let (out, code) = run_src( + "count from 1 to 10 by (0 minus 1):\n\ + \x20 display \"tick\"\n\ + end count\n", + ); + assert_ne!(code, Some(0), "a negative step must be refused: {out}"); + assert!(!out.contains("tick"), "the body must not run: {out}"); + assert!( + out.to_lowercase().contains("step"), + "the diagnostic must name the step: {out}" + ); +} + +#[test] +fn bare_exit_leaves_the_loop_like_exit_loop() { + // `exit` with no scope keeps its historic meaning: leave the loop, and let + // the program carry on. Covered separately from `exit loop` so a regression + // in either spelling is visible. + let (out, code) = run_src( + "count from 1 to 5:\n\ + \x20 display \"tick \" with count\n\ + \x20 check if count is equal to 2:\n\ + \x20 exit\n\ + \x20 end check\n\ + end count\n\ + display \"after\"\n", + ); + assert!(out.contains("tick 2"), "the deciding trip runs: {out}"); + assert!(!out.contains("tick 3"), "the loop stops: {out}"); + assert!(out.contains("after"), "the program continues: {out}"); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +#[test] +fn bare_exit_at_top_level_does_not_stop_the_program() { + // Backward compatibility: outside a loop, bare `exit` has always been a + // no-op (like `break`). `exit program` is the spelling that stops the run, + // so a program relying on the old no-op keeps working. + let (out, code) = run_src("display \"before\"\nexit\ndisplay \"after\"\n"); + assert!(out.contains("before"), "statements before run: {out}"); + assert!( + out.contains("after"), + "bare exit outside a loop does not stop the program: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} diff --git a/tests/websocket_test.rs b/tests/websocket_test.rs index 583e05b4..dc042256 100644 --- a/tests/websocket_test.rs +++ b/tests/websocket_test.rs @@ -240,3 +240,113 @@ async fn websocket_close_server_closes_connections() { let _ = child.kill().await; } + +/// `exit program` inside a websocket handler must stop the whole program, not +/// be reported as a handler error while the event pump carries on. +const EXIT_PROGRAM_PROGRAM: &str = r#" +listen for websockets on port 0 as ws_server + +on websocket connect to ws_server as conn: + send websocket message "ready" to conn +end on + +on websocket message from ws_server as msg: + display "handler reached" + exit program +end on + +wait for 30 seconds + +display "UNREACHABLE: the program kept running after exit program" +"#; + +/// Like [`start_ws_server`], but keeps the stdout reader and captures stderr so +/// a test can assert on everything the program printed after startup. +async fn start_ws_server_capturing( + program: &str, +) -> ( + Child, + u16, + TempDir, + tokio::io::Lines>, +) { + let dir = tempfile::tempdir().expect("tempdir"); + let prog_path = dir.path().join("ws_server.wfl"); + std::fs::write(&prog_path, program).expect("write program"); + + let mut child = Command::new(env!("CARGO_BIN_EXE_wfl")) + .arg(&prog_path) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .expect("spawn wfl binary"); + + let stdout = child.stdout.take().expect("child stdout"); + let mut lines = BufReader::new(stdout).lines(); + + let port = tokio::time::timeout(Duration::from_secs(60), async { + while let Ok(Some(line)) = lines.next_line().await { + const MARKER: &str = "listening on port "; + if let Some(idx) = line.rfind(MARKER) + && let Ok(port) = line[idx + MARKER.len()..].trim().parse::() + { + return Some(port); + } + } + None + }) + .await + .expect("timed out waiting for the websocket server to start") + .expect("server did not announce a port"); + + (child, port, dir, lines) +} + +#[tokio::test] +async fn websocket_handler_exit_program_stops_the_program() { + let (mut child, port, _dir, mut lines) = start_ws_server_capturing(EXIT_PROGRAM_PROGRAM).await; + let mut ws = connect(port).await; + assert_eq!(next_text(&mut ws).await, "ready"); + + ws.send(Message::Text("stop".into())) + .await + .expect("send stop"); + + // The program still has ~30 seconds of `wait for` left. Stopping means the + // process goes away well before that window closes; swallowing the sentinel + // means it runs the wait out and then prints the unreachable line. + let status = tokio::time::timeout(Duration::from_secs(15), child.wait()) + .await + .expect("exit program must stop the process, not just print an error") + .expect("wait for the child"); + + let mut stdout = String::new(); + while let Ok(Some(line)) = lines.next_line().await { + stdout.push_str(&line); + stdout.push('\n'); + } + let mut stderr = String::new(); + if let Some(mut err) = child.stderr.take() { + use tokio::io::AsyncReadExt; + let _ = err.read_to_string(&mut stderr).await; + } + + assert!( + stdout.contains("handler reached"), + "the handler body ran: {stdout}" + ); + assert!( + !stdout.contains("UNREACHABLE"), + "statements after the stop must not run: {stdout}" + ); + assert!( + !stderr.contains("handler error"), + "stopping is not a handler error: {stderr}" + ); + assert_eq!( + status.code(), + Some(0), + "a clean stop is a successful stop (stdout: {stdout}, stderr: {stderr})" + ); +}