Fix issue #560 residuals: returns in try blocks and container methods

Dev diary/2026-07-10-issue-560-residuals-try-returns-container-methods.md

@@ -0,0 +1,115 @@
+# Issue #560 Residuals: Returns Inside `try` Blocks and Container Method Results No Longer Typed `Nothing`
+
+**Date:** 2026-07-10
+
+## Background
+
+Issue #560 ("unannotated action return types default to `Nothing`") was fixed
+in two rounds: #575 added post-body return-type inference for top-level
+actions, and #591 seeded the provisional return type as `Unknown` so
+self-recursive calls degrade gracefully. Working on Scriptorium surfaced the
+same false `Cannot index into Nothing` diagnostic again, so this pass hunted
+down the remaining shapes that still slipped through. Two were found and
+fixed; both are static-diagnostics-only bugs (runtime was always correct).
+
+## Residual 1 — `return` inside a `try:` block was invisible to inference
+
+`collect_return_types` (`src/typechecker/mod.rs`) descends into conditionals
+and loops to find an action's `return` statements, but it never descended into
+`Statement::TryStatement`. An action whose only returns live inside a `try:`
+body, a `when error` clause, an `otherwise` clause, or a `finally` block was
+inferred as returning `Nothing`, and indexing its call result raised the false
+error:
+
+```wfl
+define action called load_data:
+    try:
+        return [1 and 2]
+    when error:
+        return [3 and 4]
+    end try
+end action
+
+store xs as call load_data
+store x0 as xs[0]      // error[ERROR]: Cannot index into Nothing (false)
+```
+
+This is a very common shape — try blocks wrap exactly the file/database/parse
+work that helper actions return values from, which is why Scriptorium hit it.
+
+**Fix:** `collect_return_types` now descends into `TryStatement` (body, every
+`when` clause, `otherwise`, `finally`) and `WaitForStatement` (its wrapped
+inner statement). Its sibling `check_return_statements` — the traversal used
+when an annotation *is* present — got the same arms so the two stay in sync.
+
+## Residual 2 — container method results were registered as `Nothing`
+
+The analyzer registers container methods in its container registry with
+
+```rust
+return_type: return_type.as_ref().cloned().unwrap_or(Type::Nothing),
+```
+
+and the type checker's `Expression::MethodCall` arm reads that registry to
+type `instance.method()`. Unannotated value-returning methods (the norm) were
+therefore typed `Nothing` at every call site — the container-flavored twin of
+the original #560, which #575 only fixed for top-level actions:
+
+```wfl
+create container Store:
+    property label: Text
+    action get_items:
+        return [1 and 2]
+    end
+end
+...
+store xs as s.get_items()
+store x0 as xs[0]      // error[ERROR]: Cannot index into Nothing (false)
+```
+
+**Fix,** mirroring the #575 + #591 design for top-level actions:
+
+- The analyzer seeds unannotated instance *and* static methods with a
+  provisional `Type::Unknown` (degrades gracefully after #588/#589) instead of
+  `Type::Nothing`.
+- The type checker's `ContainerDefinition` arm now checks each method body —
+  instance *and* static — with the method's parameters in scope (mirroring the
+  top-level action arm's #553 handling), infers the real return type from the
+  body's `return` statements, and writes it back into the registry through a
+  new `Analyzer::get_container_mut`. Void methods still end up `Nothing`,
+  exactly as before.
+- Static methods matter here even though static method *calls*
+  (`Container.method()`) are still a future feature at runtime: the
+  refinement keeps `Container.method` member access reporting an accurate
+  function type, and without it a void static method would have stayed
+  `Unknown` forever instead of `Nothing` (a strictness regression flagged by
+  review on the first revision of this change).
+- Annotated container methods (e.g. `action get_info: Text`) now also get
+  their `return` statements validated against the annotation via
+  `check_return_statements`, mirroring the top-level action arm.
+- Inherited methods get the fix for free: the `MethodCall` parent-walk reads
+  the same registry entries.
+
+## Verification (TDD)
+
+`tests/action_return_type_residuals_test.rs` was written first and confirmed
+failing (4/4) before the fix:
+
+- return inside `try`/`when error` infers the return type
+- return inside `try`/`when`/`otherwise` infers the return type
+- unannotated container method result is not typed `Nothing`
+- inherited container method result is not typed `Nothing`
+
+All four pass after the fix, alongside the full `cargo test --all` suite,
+`cargo clippy --all-targets --all-features -- -D warnings`, and the
+`TestPrograms/` backward-compatibility run. A combined Scriptorium-shaped
+stress case (recursive action indexing a map returned from inside `try`,
+wrapped by a container method) type-checks completely clean.
+
+## Notes
+
+- No syntax, keyword, or runtime behavior changed — this is purely a
+  false-positive-diagnostics fix, so no user-facing docs needed updating.
+- Remaining known limitation (unchanged by this pass): a method call typed
+  *before* its container's definition statement is reached in program order
+  still sees the provisional `Unknown` — safe (permissive) but not concrete.

src/analyzer/mod.rs

@@ -1277,7 +1277,14 @@ impl Analyzer {
                         let method_info = MethodInfo {
                             name: method_name.clone(),
                             parameters: parameters.clone(),
-                            return_type: return_type.as_ref().cloned().unwrap_or(Type::Nothing),
+                            // Unannotated methods get a provisional `Unknown`
+                            // (not `Nothing`) so uses of a method result checked
+                            // before the type checker refines it degrade
+                            // gracefully instead of raising false "found
+                            // Nothing" errors (issue #560 residual). The type
+                            // checker infers the real return type from the body
+                            // and writes it back via `get_container_mut`.
+                            return_type: return_type.as_ref().cloned().unwrap_or(Type::Unknown),
                             is_public: true, // Default to public for now
                             line: *method_line,
                             column: *method_column,
@@ -1336,7 +1343,9 @@ impl Analyzer {
                         let method_info = MethodInfo {
                             name: method_name.clone(),
                             parameters: parameters.clone(),
-                            return_type: return_type.as_ref().cloned().unwrap_or(Type::Nothing),
+                            // Same provisional `Unknown` seed as instance
+                            // methods above (issue #560 residual).
+                            return_type: return_type.as_ref().cloned().unwrap_or(Type::Unknown),
                             is_public: true, // Default to public for now
                             line: *method_line,
                             column: *method_column,
@@ -2144,6 +2153,13 @@ impl Analyzer {
         self.containers.get(name)
     }
 
+    /// Mutable access to a registered container, used by the type checker to
+    /// refine an unannotated method's provisional return type after inferring
+    /// it from the method body (issue #560 residual).
+    pub fn get_container_mut(&mut self, name: &str) -> Option<&mut ContainerInfo> {
+        self.containers.get_mut(name)
+    }
+
     pub fn get_containers(&self) -> &HashMap<String, ContainerInfo> {
         &self.containers
     }

src/typechecker/mod.rs

@@ -1657,7 +1657,7 @@ impl TypeChecker {
                 methods,
                 events: _events,
                 static_properties: _static_properties,
-                static_methods: _static_methods,
+                static_methods,
                 line,
                 column,
             } => {
@@ -1726,21 +1726,96 @@ impl TypeChecker {
                     }
                 }
 
-                for method in methods {
-                    if let Statement::ActionDefinition { body, .. } = method {
+                // Check method bodies (instance and static) and, for
+                // unannotated methods, infer the real return type from the
+                // body's `return` statements — the analyzer only registered a
+                // provisional `Unknown` (issue #560 residual: leaving the
+                // provisional type in place made `instance.method()` results
+                // degrade to `Unknown`, and the old `Nothing` default produced
+                // false "Cannot index into Nothing" errors). Static methods get
+                // the same refinement so `Container.method` member access
+                // reports an accurate function type and void statics resolve
+                // back to `Nothing`. Inferred types are collected first and
+                // written back after the loop so the registry borrow doesn't
+                // overlap the body checks.
+                let mut inferred_method_returns: Vec<(String, Type)> = Vec::new();
+                let mut inferred_static_method_returns: Vec<(String, Type)> = Vec::new();
+                for (method, is_static) in methods
+                    .iter()
+                    .map(|m| (m, false))
+                    .chain(static_methods.iter().map(|m| (m, true)))
+                {
+                    if let Statement::ActionDefinition {
+                        name: method_name,
+                        parameters,
+                        body,
+                        return_type,
+                        line: method_line,
+                        column: method_column,
+                    } = method
+                    {
                         // Set container context for method body analysis
                         let previous_container = self.current_container.clone();
                         self.current_container = Some(_name.clone());
 
+                        // Parameters must be resolvable while checking the body
+                        // and inferring return expressions, mirroring the
+                        // top-level `ActionDefinition` arm (issue #553).
+                        self.analyzer.push_scope();
+                        for param in parameters {
+                            let param_symbol = Symbol {
+                                name: param.name.clone(),
+                                kind: SymbolKind::Variable { mutable: false },
+                                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);
                         }
 
+                        if let Some(ret_type) = return_type {
+                            self.check_return_statements(
+                                body,
+                                ret_type,
+                                *method_line,
+                                *method_column,
+                            );
+                        } else {
+                            let inferred = self.infer_action_return_type(body);
+                            if is_static {
+                                inferred_static_method_returns
+                                    .push((method_name.clone(), inferred));
+                            } else {
+                                inferred_method_returns.push((method_name.clone(), inferred));
+                            }
+                        }
+
+                        self.analyzer.pop_scope();
+
                         // Restore previous container context
                         self.current_container = previous_container;
                     }
                 }
 
+                if let Some(container_info) = self.analyzer.get_container_mut(_name) {
+                    for (method_name, inferred) in inferred_method_returns {
+                        if let Some(method_info) = container_info.methods.get_mut(&method_name) {
+                            method_info.return_type = inferred;
+                        }
+                    }
+                    for (method_name, inferred) in inferred_static_method_returns {
+                        if let Some(method_info) =
+                            container_info.static_methods.get_mut(&method_name)
+                        {
+                            method_info.return_type = inferred;
+                        }
+                    }
+                }
+
                 // Container type registration would be handled by analyzer
             }
             Statement::ContainerInstantiation {
@@ -3836,6 +3911,32 @@ impl TypeChecker {
                 | Statement::MainLoop { body, .. } => {
                     self.collect_return_types(body, out);
                 }
+                // Actions commonly return from inside error handling — a `try:`
+                // body, its `when error` clauses, `otherwise`, or `finally`.
+                // Skipping these blocks inferred such actions as `Nothing` and
+                // produced false "Cannot index into Nothing" diagnostics at the
+                // call site (issue #560 residual).
+                Statement::TryStatement {
+                    body,
+                    when_clauses,
+                    otherwise_block,
+                    finally_block,
+                    ..
+                } => {
+                    self.collect_return_types(body, out);
+                    for clause in when_clauses {
+                        self.collect_return_types(&clause.body, out);
+                    }
+                    if let Some(otherwise_stmts) = otherwise_block {
+                        self.collect_return_types(otherwise_stmts, out);
+                    }
+                    if let Some(finally_stmts) = finally_block {
+                        self.collect_return_types(finally_stmts, out);
+                    }
+                }
+                Statement::WaitForStatement { inner, .. } => {
+                    self.collect_return_types(std::slice::from_ref(inner), out);
+                }
                 _ => {}
             }
         }
@@ -3926,6 +4027,34 @@ impl TypeChecker {
                 | Statement::MainLoop { body, .. } => {
                     self.check_return_statements(body, expected_type, line, column);
                 }
+                // Keep in sync with `collect_return_types`: returns inside
+                // `try:` blocks and `wait for` wrappers count too.
+                Statement::TryStatement {
+                    body,
+                    when_clauses,
+                    otherwise_block,
+                    finally_block,
+                    ..
+                } => {
+                    self.check_return_statements(body, expected_type, line, column);
+                    for clause in when_clauses {
+                        self.check_return_statements(&clause.body, expected_type, line, column);
+                    }
+                    if let Some(otherwise_stmts) = otherwise_block {
+                        self.check_return_statements(otherwise_stmts, expected_type, line, column);
+                    }
+                    if let Some(finally_stmts) = finally_block {
+                        self.check_return_statements(finally_stmts, expected_type, line, column);
+                    }
+                }
+                Statement::WaitForStatement { inner, .. } => {
+                    self.check_return_statements(
+                        std::slice::from_ref(inner),
+                        expected_type,
+                        line,
+                        column,
+                    );
+                }
                 _ => {}
             }
         }
@@ -4946,4 +5075,54 @@ mod tests {
             "Inferred type should be precisely Number (not widened to Any), got: {errors:?}"
         );
     }
+
+    /// Issue #560 residual: unannotated static container methods must be
+    /// refined in the container registry just like instance methods — a
+    /// value-returning one gets its inferred type (so `Container.method`
+    /// member access reports an accurate function type) and a void one goes
+    /// back to `Nothing` instead of keeping the provisional `Unknown` seed.
+    /// Static method *calls* are still a future feature at runtime, so the
+    /// registry is the observable surface to pin down here.
+    #[test]
+    fn test_static_method_return_types_refined_in_registry() {
+        let code = r#"
+create container MathUtils:
+    static action get_pair:
+        return [1 and 2]
+    end
+    static action log_it:
+        display "hi"
+    end
+end
+"#;
+        let tokens = crate::lexer::lex_wfl_with_positions(code);
+        let mut parser = crate::parser::Parser::new(&tokens);
+        let program = parser.parse().expect("Should parse");
+
+        let mut type_checker = TypeChecker::new();
+        let _ = type_checker.check_types(&program);
+
+        let container = type_checker
+            .analyzer
+            .get_container("MathUtils")
+            .expect("container should be registered");
+        let get_pair = container
+            .static_methods
+            .get("get_pair")
+            .expect("static method get_pair should be registered");
+        assert!(
+            matches!(get_pair.return_type, Type::List(_)),
+            "value-returning static method should have an inferred List return type, got {:?}",
+            get_pair.return_type
+        );
+        let log_it = container
+            .static_methods
+            .get("log_it")
+            .expect("static method log_it should be registered");
+        assert_eq!(
+            log_it.return_type,
+            Type::Nothing,
+            "void static method should be refined to Nothing, not left as the Unknown seed"
+        );
+    }
 }