Add actionable hint for undefined symbols in load-module programs

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:
+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:**
 
-1. **Load Module** - Isolated execution (existing behavior)
-2. **Include** - Parent scope execution (NEW in V2)
+| 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
 
@@ -162,7 +176,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 +187,24 @@ 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
+  = 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.
+```
+
+If you meant to share the definition, switch that line to
+`include from "setup.wfl"`.
+
 ❌ **Modify parent variables:**
 ```wfl
 # main.wfl

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<SemanticError>,

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,31 @@ 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(
+                        "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 {
+                    None::<String>
+                };
+
                 diagnostics.push(WflDiagnostic::new(
                     Severity::Error,
                     error.message.clone(),
-                    None::<String>,
+                    note,
                     "ANALYZE-SEMANTIC".to_string(),
                     file_id,
                     error.line,

tests/load_module_undefined_hint_test.rs

@@ -0,0 +1,191 @@
+//! 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<i32>) {
+    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<i32>) {
+    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.
+// ---------------------------------------------------------------------------
+
+/// 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]
+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}");
+}