From 74af40824a8c5639c4f7b6beab220ada12d526e4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 13:17:12 +0000 Subject: [PATCH 1/3] fix(analyzer): actionable error for load-module symbols, pointing to include from (#584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `load module from "..."` runs a file in an isolated child scope and, by design, does not expose its actions/containers/variables to the caller — `include from` is the mechanism that shares definitions. A caller that references a load-module-defined action was therefore correctly rejected, but with an opaque `Variable '...' is not defined` (exit 3) that gave no hint toward the fix. The issue's suggested fix (register module symbols in the analyzer, or relax the error to a warning like the include path) is wrong here: verified empirically, such a reference also fails at runtime, so relaxing analysis would let the program past the analyzer only to crash later — trading a clear compile-time error for a confusing runtime one. Instead keep the fatal error (the program genuinely cannot run) but make it actionable: when a file uses `load module`, an undefined action/variable diagnostic now carries a note explaining the isolation and pointing at `include from`. No semantics change, no backward-compat risk — all 103 integration tests still pass. - add `program_has_load_module` helper alongside `program_has_includes` - attach the guidance note in `analyze_static` for undefined-symbol errors - tests/load_module_undefined_hint_test.rs: fatal+hint for of/call forms, hint under --analyze, include-from resolves and runs, and no hint when no load module is present - Docs/04-advanced-features/modules.md: document the diagnostic guidance Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RAtsuADakMwNuDy6n3967r --- Docs/04-advanced-features/modules.md | 19 ++- src/analyzer/mod.rs | 15 +++ src/analyzer/static_analyzer.rs | 25 +++- tests/load_module_undefined_hint_test.rs | 163 +++++++++++++++++++++++ 4 files changed, 220 insertions(+), 2 deletions(-) create mode 100644 tests/load_module_undefined_hint_test.rs diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index de59ec80..05dcfdf3 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -162,7 +162,7 @@ create file at log_file with "Application started" ### What Modules Cannot Do -❌ **Define variables visible to parent:** +❌ **Define actions, containers, or variables visible to parent:** ```wfl # main.wfl load module from "setup.wfl" @@ -173,6 +173,23 @@ store utility_function as "some value" # This variable is local to the module ``` +Because these definitions never reach the caller, referencing one is an +undefined-name error — reported by the analyzer *and* raised at runtime. WFL's +diagnostics make this actionable: when a file uses `load module`, an +"is not defined" / "Undefined action" error carries a note reminding you that +`load module` is isolated and pointing you at `include from`, which *does* +share definitions across files: + +```text +error[ANALYZE-SEMANTIC]: Variable 'mod_double' is not defined + = `load module from "..."` runs a file in an isolated scope and does not + expose its actions, containers, or variables to the caller ... To share + definitions across files, use `include from "..."` instead. +``` + +If you meant to share the definition, switch that line to +`include from "setup.wfl"`. + ❌ **Modify parent variables:** ```wfl # main.wfl diff --git a/src/analyzer/mod.rs b/src/analyzer/mod.rs index fe4e0dde..7a5bc1e0 100644 --- a/src/analyzer/mod.rs +++ b/src/analyzer/mod.rs @@ -175,6 +175,21 @@ pub fn program_has_includes(program: &Program) -> bool { .any(|s| matches!(s, Statement::IncludeStatement { .. })) } +/// True when the program contains a top-level `load module from` statement. +/// +/// `load module` runs a file in an *isolated* child scope and does not expose +/// its actions/containers/variables to the caller (unlike `include from`). When +/// a caller references such a symbol it is genuinely undefined — fatal at +/// analysis *and* at runtime — so this is used to attach an actionable +/// "use `include from`" note to the undefined-name diagnostic rather than to +/// relax it (see issue #584). +pub fn program_has_load_module(program: &Program) -> bool { + program + .statements + .iter() + .any(|s| matches!(s, Statement::LoadModuleStatement { .. })) +} + pub struct Analyzer { current_scope: Scope, errors: Vec, diff --git a/src/analyzer/static_analyzer.rs b/src/analyzer/static_analyzer.rs index 2bb5b3f1..5302500a 100644 --- a/src/analyzer/static_analyzer.rs +++ b/src/analyzer/static_analyzer.rs @@ -155,6 +155,14 @@ impl StaticAnalyzer for Analyzer { } if let Err(errors) = analyze_result { + // `load module` runs a file in an isolated scope and does not expose + // its actions/containers/variables to the caller — so an undefined + // action/variable here (whose definition lives in a loaded module) + // fails at runtime too, not only in the analyzer. Keep the error + // fatal, but point the user at `include from`, which actually shares + // definitions across files (issue #584). + let has_load_module = crate::analyzer::program_has_load_module(program); + for error in errors { // Skip errors about undefined variables that are actually action parameters if error.message.starts_with("Variable '") @@ -172,10 +180,25 @@ impl StaticAnalyzer for Analyzer { } } + let is_undefined_symbol = error.message.starts_with("Undefined action '") + || (error.message.starts_with("Variable '") + && error.message.ends_with("' is not defined")); + let note = if has_load_module && is_undefined_symbol { + Some( + "`load module from \"...\"` runs a file in an isolated scope and does not \ + expose its actions, containers, or variables to the caller, so this name \ + is not defined here even though the module loads successfully. To share \ + definitions across files, use `include from \"...\"` instead." + .to_string(), + ) + } else { + None:: + }; + diagnostics.push(WflDiagnostic::new( Severity::Error, error.message.clone(), - None::, + note, "ANALYZE-SEMANTIC".to_string(), file_id, error.line, diff --git a/tests/load_module_undefined_hint_test.rs b/tests/load_module_undefined_hint_test.rs new file mode 100644 index 00000000..39aa9e4a --- /dev/null +++ b/tests/load_module_undefined_hint_test.rs @@ -0,0 +1,163 @@ +//! Issue #584 — referencing an action defined in a `load module` file. +//! +//! `load module from "..."` runs a file in an *isolated* child scope; by design +//! it does NOT expose the module's actions/containers/variables to the caller +//! (that is what `include from` is for — see `Docs/04-advanced-features/modules.md`). +//! Empirically, a caller that references a `load module`-defined action fails at +//! *runtime* too, not only in the static analyzer — so relaxing the analyzer to a +//! warning (as the include path does) would be wrong: it would let analysis pass +//! and then crash at runtime. +//! +//! The correct, backward-compatible fix keeps the fatal analyzer error (the +//! program genuinely cannot run) but makes it *actionable*: when a file uses +//! `load module`, an undefined action/variable diagnostic carries a note that +//! points the user at `include from`, the mechanism that actually shares +//! definitions. This suite pins that behavior and guards the boundaries. + +use std::fs; +use std::process::Command; +use tempfile::TempDir; + +fn wfl_exe() -> &'static str { + if cfg!(target_os = "windows") { + "target/release/wfl.exe" + } else { + "target/release/wfl" + } +} + +/// Run a WFL file inside `dir` (so module paths resolve to sibling files), +/// returning (combined stdout+stderr, exit code). +fn run_file_status(dir: &TempDir, name: &str, extra_args: &[&str]) -> (String, Option) { + let path = dir.path().join(name); + let output = Command::new(wfl_exe()) + .args(extra_args) + .arg(&path) + .output() + .expect("Failed to execute WFL"); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + (combined, output.status.code()) +} + +/// Write a `lib_mod.wfl` exposing a one-arg `mod_double` action next to a +/// `main_mod.wfl` whose body is `main_body`, then run `main_mod.wfl`. +fn with_double_module(main_body: &str, extra_args: &[&str]) -> (String, Option) { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("lib_mod.wfl"), + "define action called mod_double with parameters n:\n return n plus n\nend action\n", + ) + .unwrap(); + fs::write(dir.path().join("main_mod.wfl"), main_body).unwrap(); + let out = run_file_status(&dir, "main_mod.wfl", extra_args); + drop(dir); + out +} + +// --------------------------------------------------------------------------- +// The exact repro from issue #584 — `load module` + `of`-form call. +// --------------------------------------------------------------------------- + +/// The undefined-action error must stay fatal (the call cannot resolve at +/// runtime either), and it must carry an actionable note pointing at +/// `include from`. +#[test] +fn load_module_of_form_is_fatal_with_include_hint() { + let (out, code) = with_double_module( + "load module from \"lib_mod.wfl\"\ndisplay mod_double of 5\n", + &[], + ); + assert!( + out.contains("is not defined"), + "reference to a load-module action must stay fatal: {out}" + ); + assert!( + out.contains("include from"), + "the diagnostic should guide the user to `include from`: {out}" + ); + assert_eq!( + code, + Some(3), + "should exit 3 (fatal analyze error), not run: {out}" + ); +} + +/// Same for the `call ... with` form (which surfaces as "Undefined action"). +#[test] +fn load_module_call_form_is_fatal_with_include_hint() { + let (out, code) = with_double_module( + "load module from \"lib_mod.wfl\"\nstore d as call mod_double with 5\ndisplay d\n", + &[], + ); + assert!( + out.contains("Undefined action") || out.contains("is not defined"), + "call-form reference to a load-module action must stay fatal: {out}" + ); + assert!( + out.contains("include from"), + "the diagnostic should guide the user to `include from`: {out}" + ); + assert_eq!(code, Some(3), "should exit 3: {out}"); +} + +/// The hint also appears under `--analyze`. +#[test] +fn load_module_hint_under_analyze() { + let (out, _code) = with_double_module( + "load module from \"lib_mod.wfl\"\ndisplay mod_double of 5\n", + &["--analyze"], + ); + assert!( + out.contains("include from"), + "`--analyze` should also surface the `include from` guidance: {out}" + ); +} + +// --------------------------------------------------------------------------- +// The documented resolution: `include from` shares the action and runs. +// --------------------------------------------------------------------------- + +/// Swapping `load module` for `include from` makes the same program work — this +/// is the path the hint steers users toward. +#[test] +fn include_from_shares_the_action_and_runs() { + let (out, code) = with_double_module( + "include from \"lib_mod.wfl\"\ndisplay mod_double of 5\n", + &[], + ); + assert!( + out.contains("10"), + "include-from should share `mod_double` and print 10: {out}" + ); + assert_eq!(code, Some(0), "program should exit 0: {out}"); +} + +// --------------------------------------------------------------------------- +// Guard rails — the hint is specific to `load module` programs. +// --------------------------------------------------------------------------- + +/// Without any `load module` (or `include`), an undefined `of` callee stays +/// fatal but must NOT carry the `include from` hint (it would be misleading). +#[test] +fn undefined_without_load_module_has_no_include_hint() { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("main.wfl"), + "display \"BEFORE\"\nstore g as missing_action of \"x\"\ndisplay g\n", + ) + .unwrap(); + let (out, code) = run_file_status(&dir, "main.wfl", &[]); + assert!( + out.contains("is not defined"), + "undefined callee must stay fatal: {out}" + ); + assert!( + !out.contains("include from"), + "no load module present, so no `include from` hint should appear: {out}" + ); + assert_eq!(code, Some(3), "should exit 3: {out}"); +} From d937f4a7dbc5effaf2e4ca9b22a5ae0827b3a78e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 13:20:34 +0000 Subject: [PATCH 2/3] docs(modules): lead with load-module vs include-from decision guide (#584) Add an at-a-glance table and a prominent "common mistake" callout at the top of the modules guide, so readers pick `include from` for shared libraries and `load module` for side-effect-only files. This is the confusion behind #584 (referencing an action from a `load module`d file), now also surfaced by an actionable analyzer diagnostic. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RAtsuADakMwNuDy6n3967r --- Docs/04-advanced-features/modules.md | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 05dcfdf3..5a477ae7 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -14,10 +14,24 @@ WFL's module system allows you to organize code across multiple files, enabling ## Basic Module Loading -WFL provides two ways to include code from other files: - -1. **Load Module** - Isolated execution (existing behavior) -2. **Include** - Parent scope execution (NEW in V2) +WFL provides two ways to pull in code from other files, and **choosing the right +one is the single most important thing to get right:** + +| You want to… | Use | Why | +| --- | --- | --- | +| **Share a library** — call actions, use containers, or read constants defined in another file | **`include from "lib.wfl"`** | Runs the file *in your scope*, so its definitions become available to you | +| **Run a file for its side effects** — initialization, setup, logging — *without* exposing its definitions | **`load module from "setup.wfl"`** | Runs the file in an *isolated* scope; its actions/containers/variables stay private to that file | + +> **⚠️ Common mistake.** `load module` does **not** share the loaded file's +> actions, containers, or variables with the caller — that isolation is the +> whole point of `load module`. If you `load module` a library and then try to +> call one of its actions, you'll get an `is not defined` error (at analysis +> *and* at runtime). **To build a multi-file program out of shared library +> files, use `include from`.** WFL's diagnostics will remind you of this if you +> reach for the wrong one (see [What Modules Cannot Do](#what-modules-cannot-do)). + +1. **Include** - Parent scope execution; **use this to share libraries** +2. **Load Module** - Isolated execution; use this for side-effect-only files ## Load Module Statement From d3b4368d55373fbbba67f0ad29a00845ed224107 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 13:31:36 +0000 Subject: [PATCH 3/3] refactor(analyzer): reword load-module hint to be safe for unrelated typos (#584) Addresses CodeRabbit review on #586: the include-from note fires for any undefined-symbol error whenever a file uses `load module`, without parsing the loaded file to confirm the missing name is one of its exports. Rather than add that heavyweight module resolution to the analyzer, reword the note conditionally ("If you expected this name to come from a file loaded with `load module` ...") so it is a correct fix for a real module symbol while not misleading a plain typo in a side-effect-only load. - reword the note in static_analyzer.rs (behavior/trigger unchanged) - pin the mixed load-module + unrelated-typo case with a new guard-rail test - update the documented error snippet to match Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RAtsuADakMwNuDy6n3967r --- Docs/04-advanced-features/modules.md | 5 +++-- src/analyzer/static_analyzer.rs | 14 ++++++++---- tests/load_module_undefined_hint_test.rs | 28 ++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 5a477ae7..a513ad5c 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -196,8 +196,9 @@ share definitions across files: ```text error[ANALYZE-SEMANTIC]: Variable 'mod_double' is not defined - = `load module from "..."` runs a file in an isolated scope and does not - expose its actions, containers, or variables to the caller ... To share + = If you expected this name to come from a file loaded with `load module`, + note that `load module from "..."` runs a file in an isolated scope and does + not expose its actions, containers, or variables to the caller. To share definitions across files, use `include from "..."` instead. ``` diff --git a/src/analyzer/static_analyzer.rs b/src/analyzer/static_analyzer.rs index 5302500a..1f9ebb67 100644 --- a/src/analyzer/static_analyzer.rs +++ b/src/analyzer/static_analyzer.rs @@ -183,12 +183,18 @@ impl StaticAnalyzer for Analyzer { let is_undefined_symbol = error.message.starts_with("Undefined action '") || (error.message.starts_with("Variable '") && error.message.ends_with("' is not defined")); + // The note fires for any undefined symbol when the file uses + // `load module` — the analyzer does not parse the loaded file to + // check whether this specific name is one of its exports, so the + // wording is conditional ("if you expected ... from a loaded + // module"). That keeps it a correct fix for a real module symbol + // while not misleading a plain typo in a side-effect-only load. let note = if has_load_module && is_undefined_symbol { Some( - "`load module from \"...\"` runs a file in an isolated scope and does not \ - expose its actions, containers, or variables to the caller, so this name \ - is not defined here even though the module loads successfully. To share \ - definitions across files, use `include from \"...\"` instead." + "If you expected this name to come from a file loaded with `load module`, \ + note that `load module from \"...\"` runs a file in an isolated scope and \ + does not expose its actions, containers, or variables to the caller. To \ + share definitions across files, use `include from \"...\"` instead." .to_string(), ) } else { diff --git a/tests/load_module_undefined_hint_test.rs b/tests/load_module_undefined_hint_test.rs index 39aa9e4a..64001361 100644 --- a/tests/load_module_undefined_hint_test.rs +++ b/tests/load_module_undefined_hint_test.rs @@ -140,6 +140,34 @@ fn include_from_shares_the_action_and_runs() { // Guard rails — the hint is specific to `load module` programs. // --------------------------------------------------------------------------- +/// A file that uses `load module` for side effects but has an *unrelated* +/// undefined reference still gets the hint. The analyzer does not parse the +/// loaded file to check whether the missing name is one of its exports, so the +/// trigger is deliberately coarse — but the note is conditionally worded +/// ("if you expected this name to come from a file loaded with `load module`"), +/// so it does not mislead when the real problem is a plain typo. This pins that +/// intentional behavior (CodeRabbit review on PR #586). +#[test] +fn load_module_plus_unrelated_typo_is_hinted_but_not_misleading() { + let (out, code) = with_double_module( + "load module from \"lib_mod.wfl\"\ndisplay totally_unrelated_typo\n", + &[], + ); + assert!( + out.contains("is not defined"), + "the unrelated typo must stay fatal: {out}" + ); + assert!( + out.contains("include from"), + "the hint fires whenever `load module` is present (coarse but safe): {out}" + ); + assert!( + out.to_lowercase().contains("if you expected"), + "the hint is conditionally framed so it does not mislead for a typo: {out}" + ); + assert_eq!(code, Some(3), "should exit 3: {out}"); +} + /// Without any `load module` (or `include`), an undefined `of` callee stays /// fatal but must NOT carry the `include from` hint (it would be misleading). #[test]