diff --git a/Dev diary/2026-07-03-include-type-inference-issue-553.md b/Dev diary/2026-07-03-include-type-inference-issue-553.md new file mode 100644 index 00000000..a8ea9056 --- /dev/null +++ b/Dev diary/2026-07-03-include-type-inference-issue-553.md @@ -0,0 +1,87 @@ +# Include Type Inference: Remaining RHS Forms (Issue #553) + +**Date:** 2026-07-03 + +## What Changed + +Issue #553 (follow-on to #551/#552) reported that four right-hand-side forms +still aborted with a fatal `Could not infer type for variable 'v'` when they +appeared inside an `include from` file: + +```wfl +store v as parts[0] # list index +store v as rec["k"] # object index (parse_json result) +store v as a is equal to b # comparison result +store v as length of a # length of (already fixed by #552's builtin table) +``` + +Investigating end to end showed the failures were not include-specific at all. +The same forms failed inference in the **main file** too — but `main.rs` +reports type errors as non-fatal warnings and keeps running, while the include +pipeline turned the first type error into a fatal `RuntimeError`. That +asymmetry is what made includes look uniquely broken. + +### Root causes and fixes + +1. **The type checker had no scope for action bodies** + (`src/typechecker/mod.rs`). The analyzer creates a child scope for each + action body during analysis and then discards it, so when the type checker + later walked the AST, neither parameters nor body-local variables resolved + to any symbol. `store parts as string_split of a and "-"` inferred + `List` but had nowhere to record it, so `parts[0]` on the next line + inferred `Unknown`. The `ActionDefinition` branch now pushes a scope, + defines parameter symbols (declared type, or `Unknown` when untyped), and + the `VariableDeclaration` branch defines body-local symbols with their + inferred types so later statements can see them. + +2. **Comparisons with `Unknown` operands inferred `Unknown`.** A comparison + yields a Boolean no matter what the operand types turn out to be; only + arithmetic results depend on the operand types. `BinaryOperation` inference + now returns `Boolean` for `Equals`/`NotEquals`/ordered comparisons/ + `And`/`Or`/`Contains` even when an operand is `Unknown` or `Any`. + +3. **Indexing an `Any` collection was a type error.** `parse_json` results + are typed `Any` (the shape is only known at runtime), but `IndexAccess` + had no `Any` arm and fell through to `Cannot index into Any`. Indexing + `Any` now yields `Any`, and `Unknown`/`Any` index values are tolerated + for list and text indexing. + +4. **Include type errors are now non-fatal warnings** + (`src/interpreter/mod.rs`). `include from` executes in the parent scope — + the code is semantically part of the main program — so its type-check + findings are now reported exactly like the main file's: printed as + `Type checking warnings in included file '...'` and execution continues. + This closes the class of bug behind #551/#553 rather than the four + instances: any future inference gap degrades to a warning instead of a + show-stopping abort. Parse and semantic errors in included files remain + fatal. + +5. **Bonus: false `ANALYZE-UNUSED` fix** (`src/analyzer/static_analyzer.rs`). + The issue's own repro flagged `Unused variable 'parts'` after + `store v as parts[0]` inside an action: `mark_used_variables` had no + `VariableDeclaration` arm, so right-hand sides of `store` statements inside + action/loop bodies never counted as uses. Added the arm. + +### No new syntax + +No new syntax was needed, so nothing was added to the language surface. The +change is purely semantic and aligns with the WFL foundation principles of +clear and actionable error reporting (principle 4) and type safety with +inference where practical (principle 5): inference now understands the +bread-and-butter forms, and what it cannot infer degrades to a readable +warning instead of a fatal abort. + +## Tests + +`tests/docs_parser_and_include_fixes_test.rs` gained regression tests: each +of the four RHS forms inside an included action called from a main program, +main-file inference guards for list-index and comparison results, and a guard +that a genuinely uninferable store (`a plus b` on untyped parameters) in an +included file runs to completion instead of aborting. + +## Docs + +- `Docs/04-advanced-features/modules.md`: documented include type-check + behavior (warnings, not fatal). +- `Docs/reference/error-codes.md`: refreshed the "Could not infer type" + entry. diff --git a/Docs/04-advanced-features/modules.md b/Docs/04-advanced-features/modules.md index 6e5ea06b..2389c442 100644 --- a/Docs/04-advanced-features/modules.md +++ b/Docs/04-advanced-features/modules.md @@ -39,6 +39,17 @@ include from "containers.wfl" This reads, parses, and executes the specified file in the parent scope, making all definitions available to the parent. +### Type Checking in Included Files + +Included files go through the same pipeline as the main program (parse, analyze, type check). Because `include from` runs the file in the parent scope — as if the code were written in the main program — type-check findings in an included file are reported the same way as in the main file: as **non-fatal warnings**. The program still runs. + +```text +Type checking warnings in included file 'mod.wfl': +error[ERROR]: Could not infer type for variable 'v' +``` + +Parse errors and semantic errors (for example, an undefined variable) in an included file remain fatal. + ## When to Use Each Approach ### Use `load module from` for: diff --git a/Docs/reference/error-codes.md b/Docs/reference/error-codes.md index ac27bca0..53af40a8 100644 --- a/Docs/reference/error-codes.md +++ b/Docs/reference/error-codes.md @@ -141,7 +141,7 @@ store result as abs of -5 // Numbers only ### "Could not infer type for variable" -**Cause:** Type checker can't determine type (usually in actions with parameters). +**Cause:** Type checker can't determine type (usually arithmetic on untyped action parameters). **Example:** ```wfl @@ -151,7 +151,9 @@ define action called process with parameters x: end action ``` -**Fix:** This is typically a warning, not an error. Code still works. +**Fix:** This is a warning, not an error — the code still runs. This applies everywhere, including files pulled in with `include from` (included files are never type-checked more strictly than the main program). + +Note: comparison results (`a is equal to b`), list/object indexing (`parts[0]`, `rec["k"]`), `length of`, and builtin call results are all inferable and do not produce this warning. --- diff --git a/src/analyzer/static_analyzer.rs b/src/analyzer/static_analyzer.rs index c8fd884a..b3100296 100644 --- a/src/analyzer/static_analyzer.rs +++ b/src/analyzer/static_analyzer.rs @@ -552,6 +552,14 @@ impl Analyzer { self.mark_used_in_expression(value, usages); } + Statement::VariableDeclaration { value, .. } => { + // Variables referenced on the right-hand side of a `store` + // are uses — including inside action/loop bodies, which the + // top-level declaration pass does not reach (issue #553's + // repro flagged `parts` as unused after `store v as parts[0]` + // inside an action). + self.mark_used_in_expression(value, usages); + } Statement::ActionDefinition { body, .. } => { for stmt in body { self.mark_used_variables(stmt, usages); diff --git a/src/interpreter/mod.rs b/src/interpreter/mod.rs index 347aa77e..0f4dcfbb 100644 --- a/src/interpreter/mod.rs +++ b/src/interpreter/mod.rs @@ -3522,23 +3522,30 @@ impl Interpreter { )); } - // 7. Type check + // 7. Type check. Type errors are reported as non-fatal + // warnings, exactly like the main-file pipeline (main.rs + // prints them and continues): `include from` executes in the + // parent scope, so included code must never be checked more + // strictly than the same code written in the main program + // (issues #551/#553). + use crate::diagnostics::DiagnosticReporter; use crate::typechecker::TypeChecker; let mut tc = TypeChecker::with_analyzer(analyzer); if let Err(type_errors) = tc.check_types(&program) { - let first_error = type_errors.first(); - let (error_line, error_column) = - first_error.map(|e| (e.line, e.column)).unwrap_or((1, 1)); - return Err(RuntimeError::new( - format!( - "Type error in included file '{}': {}", - resolved_path.display(), - first_error.map(|e| e.to_string()).unwrap_or_default() - ), - error_line, - error_column, - )); + eprintln!( + "Type checking warnings in included file '{}':", + resolved_path.display() + ); + let mut reporter = DiagnosticReporter::new(); + let file_id = + reporter.add_file(resolved_path.display().to_string(), content.clone()); + for error in &type_errors { + let diagnostic = reporter.convert_type_error(file_id, error); + if reporter.report_diagnostic(file_id, &diagnostic).is_err() { + eprintln!("{error}"); + } + } } // 8. Create guard for context tracking diff --git a/src/typechecker/mod.rs b/src/typechecker/mod.rs index 4180463a..43efbca0 100644 --- a/src/typechecker/mod.rs +++ b/src/typechecker/mod.rs @@ -579,7 +579,31 @@ impl TypeChecker { && let Some(symbol) = self.analyzer.get_symbol_mut(name) && symbol.symbol_type.is_none() { - symbol.symbol_type = Some(inferred_type); + symbol.symbol_type = Some(inferred_type.clone()); + } + + // Locals declared inside an action body have no symbol left + // over from analysis (the analyzer discards body scopes), so + // record them in the type checker's re-created scope; later + // statements in the body can then see their inferred types + // (issue #553). Resolving through parent scopes is correct + // here because WFL forbids shadowing: a `store` reusing an + // outer variable's name is a fatal semantic error ("Use + // 'change x to '"), so a resolved outer symbol can + // only mean the store refers to that same variable. + if self.analyzer.get_symbol(name).is_none() { + let recorded_type = if inferred_type == Type::Error { + Type::Unknown + } else { + inferred_type + }; + let _ = self.analyzer.define_symbol(Symbol { + name: name.clone(), + kind: SymbolKind::Variable { mutable: true }, + symbol_type: Some(recorded_type), + line: *_line, + column: *_column, + }); } } Statement::Assignment { @@ -633,6 +657,27 @@ impl TypeChecker { }); } + // The analyzer's action-body scope is discarded when analysis + // finishes, so re-create one here: parameters and body-local + // variables must be resolvable while checking the body, + // otherwise expressions that depend on a local's type (e.g. + // `parts[0]` after `store parts as ...`) infer Unknown + // (issue #553). + self.analyzer.push_scope(); + for param in parameters { + let param_symbol = Symbol { + name: param.name.clone(), + kind: SymbolKind::Variable { mutable: false }, + // Untyped parameters get an explicit Unknown so + // references resolve without a "cannot determine + // type" diagnostic, matching prior behavior. + symbol_type: param.param_type.clone().or(Some(Type::Unknown)), + line: param.line, + column: param.column, + }; + let _ = self.analyzer.define_symbol(param_symbol); + } + for stmt in body { self.check_statement_types(stmt); } @@ -640,6 +685,8 @@ impl TypeChecker { if let Some(ret_type) = return_type { self.check_return_statements(body, ret_type, *_line, *_column); } + + self.analyzer.pop_scope(); } Statement::IfStatement { condition, @@ -2211,7 +2258,39 @@ impl TypeChecker { } if left_type == Type::Unknown || right_type == Type::Unknown { - return Type::Unknown; + // Comparisons and logical operations always produce a + // Boolean, no matter what the operand types turn out to + // be at runtime — only arithmetic results depend on the + // operand types (issue #553). + return match operator { + Operator::Equals + | Operator::NotEquals + | Operator::GreaterThan + | Operator::LessThan + | Operator::GreaterThanOrEqual + | Operator::LessThanOrEqual + | Operator::And + | Operator::Or + | Operator::Contains => Type::Boolean, + _ => Type::Unknown, + }; + } + + // Dynamically-typed (Any) operands are checked at runtime; + // comparisons on them still produce a Boolean. + if left_type == Type::Any || right_type == Type::Any { + match operator { + Operator::Equals + | Operator::NotEquals + | Operator::GreaterThan + | Operator::LessThan + | Operator::GreaterThanOrEqual + | Operator::LessThanOrEqual + | Operator::And + | Operator::Or + | Operator::Contains => return Type::Boolean, + _ => {} + } } match operator { @@ -2542,7 +2621,10 @@ impl TypeChecker { match collection_type { Type::List(item_type) => { - if index_type != Type::Number { + if index_type != Type::Number + && index_type != Type::Unknown + && index_type != Type::Any + { self.type_error( format!("List index must be a number, got {index_type}"), Some(Type::Number), @@ -2570,7 +2652,10 @@ impl TypeChecker { } } Type::Text => { - if index_type != Type::Number { + if index_type != Type::Number + && index_type != Type::Unknown + && index_type != Type::Any + { self.type_error( format!("Text index must be a number, got {index_type}"), Some(Type::Number), @@ -2584,6 +2669,10 @@ impl TypeChecker { } } Type::Unknown => Type::Unknown, + // A dynamically-typed collection (e.g. a parse_json + // result) is indexable; the element type is only known + // at runtime (issue #553). + Type::Any => Type::Any, _ => { self.type_error( format!("Cannot index into {collection_type}"), @@ -3351,7 +3440,14 @@ impl TypeChecker { column, } => { if let Some(expr) = value { + // The body pass (check_statement_types) has already + // inferred every return expression and reported any + // diagnostics inside it; re-inferring here is only to + // learn the type for the compatibility check, so drop + // the duplicate expression diagnostics it produces. + let errors_before = self.errors.len(); let return_type = self.infer_expression_type(expr); + self.errors.truncate(errors_before); if !self.are_types_compatible(expected_type, &return_type) { self.type_error( "Return statement has incorrect type".to_string(), diff --git a/tests/docs_parser_and_include_fixes_test.rs b/tests/docs_parser_and_include_fixes_test.rs index 9524ea5f..95bcb9f2 100644 --- a/tests/docs_parser_and_include_fixes_test.rs +++ b/tests/docs_parser_and_include_fixes_test.rs @@ -383,3 +383,156 @@ fn parse_json_result_variable_is_inferable_in_main_file() { ); assert!(out.contains("[1]"), "expected '[1]', got: {out}"); } + +// --------------------------------------------------------------------------- +// #553 — remaining RHS forms inside included files: list index, object index, +// comparison results, and `length of`. Each used to abort with a fatal +// "Could not infer type for variable 'v'" when it appeared in an included +// file, even though the same code ran fine in the main file. +// --------------------------------------------------------------------------- + +/// Escape a Rust string for interpolation into a WFL double-quoted literal. +fn wfl_escape(s: &str) -> String { + s.replace('\\', "\\\\").replace('"', "\\\"") +} + +/// Write `mod.wfl` with an included action whose body is `body`, call it from +/// `main.wfl` with two text arguments, and return the combined output. +fn run_included_action(body: &str, arg1: &str, arg2: &str) -> String { + let dir = TempDir::new().expect("tempdir"); + fs::write( + dir.path().join("mod.wfl"), + format!("define action called f with parameters a and b:\n{body}\nend action\n"), + ) + .unwrap(); + let (arg1, arg2) = (wfl_escape(arg1), wfl_escape(arg2)); + fs::write( + dir.path().join("main.wfl"), + format!( + "include from \"mod.wfl\"\nstore r as call f with \"{arg1}\" and \"{arg2}\"\ndisplay \"R=\" with r\n" + ), + ) + .unwrap(); + run_file(&dir, "main.wfl") +} + +#[test] +fn included_action_can_store_list_index_result() { + let out = run_included_action( + " store parts as string_split of a and \"-\"\n store v as parts[0]\n return v", + "x-y", + "z", + ); + assert!( + !out.contains("Could not infer type"), + "list-index result must be inferable in an included file: {out}" + ); + assert!(out.contains("R=x"), "expected 'R=x', got: {out}"); +} + +#[test] +fn included_action_can_store_object_index_result() { + let out = run_included_action( + " store rec as parse_json of \"{\\\"k\\\":1}\"\n store v as rec[\"k\"]\n return v", + "x", + "z", + ); + assert!( + !out.contains("Could not infer type"), + "object-index result must be inferable in an included file: {out}" + ); + assert!( + !out.contains("Cannot index into"), + "indexing a parse_json (Any-typed) value must not be a type error: {out}" + ); + assert!(out.contains("R=1"), "expected 'R=1', got: {out}"); +} + +#[test] +fn included_action_can_store_comparison_result() { + let out = run_included_action(" store v as a is equal to b\n return v", "x", "z"); + assert!( + !out.contains("Could not infer type"), + "comparison result must be inferable in an included file: {out}" + ); + assert!(out.contains("R=no"), "expected 'R=no', got: {out}"); +} + +#[test] +fn included_action_can_store_ordered_comparison_result() { + let out = run_included_action( + " store v as a is greater than or equal to b\n return v", + "b", + "a", + ); + assert!( + !out.contains("Could not infer type"), + "ordered-comparison result must be inferable in an included file: {out}" + ); + assert!(out.contains("R=yes"), "expected 'R=yes', got: {out}"); +} + +#[test] +fn included_action_can_store_length_of_result() { + let out = run_included_action(" store v as length of a\n return v", "hello", "z"); + assert!( + !out.contains("Could not infer type"), + "`length of` result must be inferable in an included file: {out}" + ); + assert!(out.contains("R=5"), "expected 'R=5', got: {out}"); +} + +#[test] +fn list_index_result_variable_is_inferable_in_main_file() { + // Guard: the same forms must not produce spurious "Could not infer type" + // warnings in the main file either (they were warnings there, not fatal). + let out = run_wfl( + "define action called f with parameters a and b:\n store parts as string_split of a and \"-\"\n store v as parts[0]\n return v\nend action\nstore r as call f with \"x-y\" and \"z\"\ndisplay r\n", + ); + assert!( + !out.contains("Could not infer type"), + "list-index result must be inferable in the main file: {out}" + ); + assert!(out.contains("x"), "expected 'x', got: {out}"); +} + +#[test] +fn comparison_result_variable_is_inferable_in_main_file() { + let out = run_wfl( + "define action called f with parameters a and b:\n store v as a is equal to b\n return v\nend action\nstore r as call f with \"x\" and \"z\"\ndisplay r\n", + ); + assert!( + !out.contains("Could not infer type"), + "comparison result must be inferable in the main file: {out}" + ); + assert!(out.contains("no"), "expected 'no', got: {out}"); +} + +#[test] +fn action_local_store_reusing_outer_name_is_a_semantic_error() { + // WFL forbids shadowing: a body-local `store` that reuses an outer + // variable's name is rejected by the analyzer with a pointer to + // `change`. This guards the type checker's scope handling assumption + // that a name resolving to an outer symbol always refers to that same + // variable (there is no valid program where it is a distinct local). + let out = run_wfl( + "store parts as 5\ndefine action called f with parameters a and b:\n store parts as string_split of a and \"-\"\n return parts\nend action\nstore r as call f with \"x-y\" and \"z\"\ndisplay r\n", + ); + assert!( + out.contains("already been defined") && out.contains("change parts to"), + "shadowing store must be rejected with a 'change' suggestion: {out}" + ); +} + +#[test] +fn include_type_errors_are_nonfatal_like_main_file() { + // Deeper guarantee behind #551/#553: the include pipeline must never be + // stricter than the main-file pipeline. A form the type checker genuinely + // cannot infer (untyped parameters in arithmetic) is a non-fatal warning + // in the main file, so an included file must also run, not abort. + let out = run_included_action(" store v as a plus b\n return v", "1", "2"); + assert!( + out.contains("R="), + "included file with an uninferable store must still run: {out}" + ); +}