From f34c42cd91f24d9c359256a1988e068c0d40bcd7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:06:20 +0000 Subject: [PATCH 1/5] test: red tests for issues #698, #699, #700 Failing tests first, against unmodified source (16 of 19 fail): * #698 `replace with in ` returns the input unchanged, exit 0, no diagnostic; no literal string replacement is reachable from WFL source at all. * #699 the `count` loop's trip cap is keyed on the loop's end *value*, so `count from 1 to 20000` aborts at 10001 while `count from 1 to 1000001` runs uncapped, and a downward loop is capped by the value it counts down to. * #700 `exit program` does not parse; no spelling terminates the program. The three that already pass 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. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWCp6cVEGQ5pVfKC7FDdUh --- tests/issues_698_700_test.rs | 303 +++++++++++++++++++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 tests/issues_698_700_test.rs diff --git a/tests/issues_698_700_test.rs b/tests/issues_698_700_test.rs new file mode 100644 index 00000000..d3ed3ae7 --- /dev/null +++ b/tests/issues_698_700_test.rs @@ -0,0 +1,303 @@ +//! 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_src; + +// --------------------------------------------------------------------------- +// #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}"); +} From 3e16447643a61a75816008577288aa5c35b64cdb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:17:42 +0000 Subject: [PATCH 2/5] fix: replace actually replaces, count loops run every trip, exit program stops the program MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #698, #699, #700. #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. `native_pattern_replace` now replaces every match, walking character offsets (the pattern VM reports characters, so byte slicing would panic on multibyte input) in one forward pass over the non-overlapping matches. The needle may now also be a plain text, matched verbatim, which is what makes literal string replacement reachable at all: `replace` lexes as a keyword, so the 3-argument native `replace` in stdlib/text.rs could never be called from WFL source and the language shipped with no reachable string replacement. The type checker accepts Pattern or Text there, so the beginner form and the expert form are the same statement. #699 — the count loop's trip guard was keyed on the loop's end *value*: `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. The cap is gone; `count` is bounded by the same execution timeout as `repeat` and `for each`, which check_time() already enforces on every trip. What replaces it is narrower: a step that cannot move the counter toward the end value (`by 0`, or negative) is refused up front with a diagnostic that names the step, instead of spinning until the timeout. #700 — `exit program`, the spelling in both keyword reference pages, did not parse ("Variable 'program' is not defined", exit 3), and bare `exit` was ignored at top level, so program termination had no working spelling. `ExitStatement` now carries an `ExitScope`: `exit` / `exit loop` keep their existing meaning, and `exit program` raises an `ErrorKind::ExitProgram` sentinel — an error only so it unwinds action calls and expression evaluation, neither of which carries a control-flow channel. It is deliberately uncatchable: `when error` re-raises it (`finally` 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 `main` are skipped, streams and pending requests are still finalized, exit status 0. Docs ship with the change: new "Replacing Matches" (pattern-matching), `pattern_replace` (pattern module), `replace` (text module), new "Stopping the Program" with the break / exit loop / exit program table (control flow), the count-loop step rule and trip-limit wording plus `exit loop` alongside `break` (loops), and both keyword reference pages. End-to-end WFL coverage in TestPrograms/replace_and_count_loop_test.wfl and TestPrograms/exit_program_test.wfl; Dev Diary entry added. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWCp6cVEGQ5pVfKC7FDdUh --- Docs/03-language-basics/control-flow.md | 45 +++++++ .../03-language-basics/loops-and-iteration.md | 43 ++++++- Docs/04-advanced-features/pattern-matching.md | 45 +++++++ Docs/05-standard-library/pattern-module.md | 47 +++++++ Docs/05-standard-library/text-module.md | 51 ++++++++ Docs/reference/keyword-reference.md | 4 +- Docs/reference/reserved-keywords.md | 4 +- ...-698-700-replace-count-cap-exit-program.md | 116 ++++++++++++++++++ TestPrograms/exit_program_test.wfl | 47 +++++++ TestPrograms/replace_and_count_loop_test.wfl | 84 +++++++++++++ src/interpreter/error.rs | 24 ++++ src/interpreter/mod.rs | 107 ++++++++++++---- src/parser/ast.rs | 14 +++ src/parser/stmt/actions.rs | 26 +++- src/stdlib/pattern.rs | 63 ++++++++-- src/typechecker/mod.rs | 12 +- tests/issues_698_700_test.rs | 19 ++- 17 files changed, 697 insertions(+), 54 deletions(-) create mode 100644 History/dev-diary/2026/2026-08-14-issues-698-700-replace-count-cap-exit-program.md create mode 100644 TestPrograms/exit_program_test.wfl create mode 100644 TestPrograms/replace_and_count_loop_test.wfl diff --git a/Docs/03-language-basics/control-flow.md b/Docs/03-language-basics/control-flow.md index 8e7533fc..d351efee 100644 --- a/Docs/03-language-basics/control-flow.md +++ b/Docs/03-language-basics/control-flow.md @@ -523,6 +523,51 @@ 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. + +**`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 | + +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..1a395ade 100644 --- a/Docs/03-language-basics/loops-and-iteration.md +++ b/Docs/03-language-basics/loops-and-iteration.md @@ -57,6 +57,19 @@ 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. + +### 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. + ### Count Examples **Count to 100 by tens:** @@ -268,7 +281,7 @@ end repeat ### Break (Exit Loop) -Exit a loop early (if supported): +Exit a loop early: ```wfl count from 1 to 100: @@ -281,9 +294,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..d23f4dee --- /dev/null +++ b/History/dev-diary/2026/2026-08-14-issues-698-700-replace-count-cap-exit-program.md @@ -0,0 +1,116 @@ +# 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 `describe`/`test` 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. + +## 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. + +## 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..a9ab0a09 --- /dev/null +++ b/TestPrograms/exit_program_test.wfl @@ -0,0 +1,47 @@ +// `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 "" + +display "UNREACHABLE: statements after exit program must not run" 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..f00bf75d 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); @@ -7377,6 +7404,25 @@ impl Interpreter { 1.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. + if !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, + )); + } + let mut count = start_num; let should_continue: Box bool> = if *downward { @@ -7385,22 +7431,19 @@ 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; - *self.in_count_loop.borrow_mut() = true; // Determine the variable name to use - custom name or default "count" 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); @@ -7459,21 +7502,11 @@ impl Interpreter { } else { count += step_num; } - - iterations += 1; } *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 +7872,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 +8596,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 +8777,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 +9132,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; 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..8ab494a0 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,9 +8501,15 @@ 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) + { self.type_error( - format!("Expected Pattern for pattern replacement, got {pattern_type}"), + format!("Expected Pattern or Text to replace, got {pattern_type}"), Some(Type::Pattern), Some(pattern_type), 0, diff --git a/tests/issues_698_700_test.rs b/tests/issues_698_700_test.rs index d3ed3ae7..8ec3c70e 100644 --- a/tests/issues_698_700_test.rs +++ b/tests/issues_698_700_test.rs @@ -177,9 +177,7 @@ 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", - ); + 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"), @@ -198,7 +196,10 @@ fn count_loop_rejects_a_step_that_never_reaches_the_end() { #[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("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}"); } @@ -217,7 +218,10 @@ fn exit_program_stops_the_program_from_inside_a_loop() { 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!( + !out.contains("after"), + "code after the loop does not run: {out}" + ); assert_eq!(code, Some(0), "a normal exit is status 0: {out}"); } @@ -232,7 +236,10 @@ fn exit_program_stops_the_program_from_inside_an_action() { call bail\n\ display \"after\"\n", ); - assert!(out.contains("before"), "statements before it still run: {out}"); + 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}"); From 4e6c413886d0c68f43c872d5e7d5cda5b5d84a3a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:20:34 +0000 Subject: [PATCH 3/5] docs: say plainly that `exit loop` outside a loop does nothing The bare/loop spelling has nothing to leave at top level, exactly like a `break` written outside a loop. Naming that next to the exit table stops a reader reaching for `exit` when they mean `exit program` (#700). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWCp6cVEGQ5pVfKC7FDdUh --- Docs/03-language-basics/control-flow.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Docs/03-language-basics/control-flow.md b/Docs/03-language-basics/control-flow.md index d351efee..977bf1ad 100644 --- a/Docs/03-language-basics/control-flow.md +++ b/Docs/03-language-basics/control-flow.md @@ -566,6 +566,10 @@ Because stopping is not a failure, a `when error:` handler never catches | `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 From d2b6bea2e1874e24c5505f8ce52d8f4ebca8a9b4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 15:43:39 +0000 Subject: [PATCH 4/5] test: red tests for the review follow-ups on #698-#700 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six failing tests, one per defect found reviewing the fix: * `exit program` inside a websocket handler is printed as a handler error and the event pump carries on (the process runs its `wait for` window out and then executes the statements after it). * `exit program` inside a `test` block is recorded as a test failure and the run continues with a nonzero status. * `count from 5 to 1 by 0` — an empty range whose body never runs — is now a runtime error, where it used to complete silently. Same for the downward form. This is a backward-compatibility break. * A step below the counter's floating-point resolution (`100000000000000000 by 1`) passes validation and never advances, so the loop spins until the execution timeout instead of being refused up front. * The replace-needle diagnostic renders "Expected Pattern or Text to replace, got Number - Expected Pattern but found Number", contradicting itself. Three further tests pin behaviour the fixes must not change: a negative step is refused, bare `exit` leaves a loop like `exit loop`, and bare `exit` outside a loop leaves the program running. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWCp6cVEGQ5pVfKC7FDdUh --- tests/issues_698_700_test.rs | 161 ++++++++++++++++++++++++++++++++++- tests/websocket_test.rs | 110 ++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 1 deletion(-) diff --git a/tests/issues_698_700_test.rs b/tests/issues_698_700_test.rs index 8ec3c70e..ee28d6be 100644 --- a/tests/issues_698_700_test.rs +++ b/tests/issues_698_700_test.rs @@ -14,7 +14,8 @@ //! and #700 describe. mod common; -use common::run_src; +use common::{run_file_status, run_src}; +use tempfile::TempDir; // --------------------------------------------------------------------------- // #698 — replace actually replaces @@ -308,3 +309,161 @@ fn bare_exit_still_leaves_the_loop_and_keeps_going() { 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})" + ); +} From c3448d107842eef5a3fbd24c122d3beb5592b3b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 16:07:51 +0000 Subject: [PATCH 5/5] fix: stop the program everywhere, and only reject steps that matter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups from review of the #698-#700 fix. `exit program` was still absorbed at two boundaries that swallow a `Result`: * the websocket event dispatcher reported every handler error and returned `Ok(())`, so stopping inside `on websocket message:` printed a bogus handler error and the pump kept running; * a `test` block records any non-assertion error as a test failure, so stopping inside a test produced a failing test and a nonzero exit. Both now use the same `is_exit_program()` pass-through as the try/when, module, include and top-of-run boundaries. Count loops: * the new step validation ran unconditionally, so `count from 5 to 1 by 0` — an empty range whose body never runs, and which completed fine before — became a runtime error. It is now checked only when the loop is entered, restoring the old behaviour for ranges that never run. * a positive, finite step is not always a step: past 2^53 the counter's resolution exceeds it, `count + step == count`, and the loop can never reach its end. The execution timeout would not save it inside a `main loop`, where the deadline is suspended, so the loop now stops the moment the counter provably cannot move. The replace-needle type error accepted two types while naming one, so rendering appended a contradicting "Expected Pattern but found Number"; `expected` is now left unset. Docs: the count-loop page states the `main loop` deadline exemption instead of implying the timeout covers every runaway loop, and documents the precision floor on steps and that an empty range ignores its step. The control-flow page says what "the program" means for `exit program` — `load module`/`include` are part of it, an `execute file` child is not — and that a websocket handler stops the server rather than logging an error. `TestPrograms/exit_program_test.wfl` now ends its unreachable branch in a read of a file that cannot exist, so continuing past the stop fails the gated run instead of just printing a line. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PWCp6cVEGQ5pVfKC7FDdUh --- Docs/03-language-basics/control-flow.md | 10 ++- .../03-language-basics/loops-and-iteration.md | 15 ++++- ...-698-700-replace-count-cap-exit-program.md | 53 +++++++++++++++- TestPrograms/exit_program_test.wfl | 5 ++ src/interpreter/mod.rs | 61 +++++++++++++++---- src/typechecker/mod.rs | 5 +- 6 files changed, 131 insertions(+), 18 deletions(-) diff --git a/Docs/03-language-basics/control-flow.md b/Docs/03-language-basics/control-flow.md index 977bf1ad..ecb41645 100644 --- a/Docs/03-language-basics/control-flow.md +++ b/Docs/03-language-basics/control-flow.md @@ -556,7 +556,15 @@ 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. +`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`:** diff --git a/Docs/03-language-basics/loops-and-iteration.md b/Docs/03-language-basics/loops-and-iteration.md index 1a395ade..8d401525 100644 --- a/Docs/03-language-basics/loops-and-iteration.md +++ b/Docs/03-language-basics/loops-and-iteration.md @@ -60,7 +60,14 @@ 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. +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 @@ -70,6 +77,12 @@ As many times as you ask it to. A count loop has no built-in trip limit — 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:** 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 index d23f4dee..a22f0027 100644 --- 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 @@ -88,9 +88,11 @@ 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 `describe`/`test` 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. +(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 @@ -105,6 +107,51 @@ that a clean stop is a successful stop. 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: diff --git a/TestPrograms/exit_program_test.wfl b/TestPrograms/exit_program_test.wfl index a9ab0a09..4656ca94 100644 --- a/TestPrograms/exit_program_test.wfl +++ b/TestPrograms/exit_program_test.wfl @@ -44,4 +44,9 @@ 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/src/interpreter/mod.rs b/src/interpreter/mod.rs index f00bf75d..e3eb2762 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -7404,12 +7404,25 @@ impl Interpreter { 1.0 }; + let mut count = start_num; + + let should_continue: Box bool> = if *downward { + Box::new(|count, end_num| count >= end_num) + } else { + Box::new(|count, end_num| count <= end_num) + }; + // 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. - if !step_num.is_finite() || step_num <= 0.0 { + // + // 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( @@ -7423,14 +7436,6 @@ impl Interpreter { )); } - let mut count = start_num; - - let should_continue: Box bool> = if *downward { - Box::new(|count, end_num| count >= end_num) - } else { - Box::new(|count, end_num| count <= end_num) - }; - *self.in_count_loop.borrow_mut() = true; // Determine the variable name to use - custom name or default "count" @@ -7497,11 +7502,31 @@ 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, + )); } + count = next; } *self.current_count.borrow_mut() = previous_count; @@ -13124,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; @@ -13412,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/typechecker/mod.rs b/src/typechecker/mod.rs index 8ab494a0..d33db924 100644 --- a/src/typechecker/mod.rs +++ b/src/typechecker/mod.rs @@ -8508,9 +8508,12 @@ impl TypeChecker { && 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 or Text to replace, got {pattern_type}"), - Some(Type::Pattern), + None, Some(pattern_type), 0, 0,