Fix type inference for RHS forms in included files (issue #553) - #554
Conversation
…ors non-fatal (#553) Follow-on to #551/#552: list index (parts[0]), object index (rec["k"]), and comparison results (a is equal to b) still failed type inference and aborted fatally inside included files. E2E investigation showed the inference failures were not include-specific — the same forms failed in the main file too, where main.rs reports type errors as non-fatal warnings while the include pipeline turned the first one into a fatal RuntimeError. Typechecker fixes (src/typechecker/mod.rs): - ActionDefinition now pushes a scope, defines parameter symbols (declared type, or Unknown when untyped), and body-local variables are recorded with their inferred types. Previously the analyzer's body scope was discarded after analysis, so nothing inside an action body resolved and any expression depending on a local's type (parts[0] after store parts as string_split ...) inferred Unknown. - Comparisons (equals, not-equals, ordered, and/or, contains) infer Boolean even when an operand is Unknown or Any — only arithmetic results depend on operand types. - IndexAccess on an Any collection (e.g. a parse_json result) yields Any instead of "Cannot index into Any"; Unknown/Any index values are tolerated for list and text indexing. Include pipeline (src/interpreter/mod.rs): - Type-check findings in an included file are now reported as warnings and execution continues, exactly like the main-file pipeline. include from runs in the parent scope, so included code is never checked more strictly than the same code in the main program. This closes the class of bug behind #551/#553 rather than individual instances. Parse and semantic errors remain fatal. Analyzer fix (src/analyzer/static_analyzer.rs): - mark_used_variables had no VariableDeclaration arm, so store RHS uses inside action/loop bodies were not counted and the issue's own repro flagged a false "Unused variable 'parts'". Added the arm. Tests: eight regression tests in docs_parser_and_include_fixes_test.rs covering each RHS form in an included action, main-file inference guards, and a guard that a genuinely uninferable store in an included file runs to completion. Docs: modules.md include type-check behavior, error-codes.md refresh, and a Dev Diary entry. No new syntax needed. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiTGMQoi5CLPi8wqmjUqYx
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR fixes type inference failures for included files (Issue ChangesInclude type inference fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Interpreter
participant TypeChecker
participant DiagnosticReporter
participant Stderr
Interpreter->>TypeChecker: check_types(included program)
TypeChecker-->>Interpreter: type errors (if any)
alt type errors found
Interpreter->>DiagnosticReporter: add included file content
loop each type error
Interpreter->>DiagnosticReporter: convert error to diagnostic
DiagnosticReporter->>Stderr: report diagnostic (warning)
end
Interpreter->>Interpreter: continue execution
else no errors
Interpreter->>Interpreter: continue execution
end
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes type inference regressions that surfaced most visibly in include from files (issue #553) by improving action-body scoping in the type checker, ensuring comparison/indexing forms infer correctly, and aligning included-file type checking behavior with the main-file “warnings, not fatal” pipeline.
Changes:
- Re-create action-body scope during type checking and record inferred types for body-local
storevariables so subsequent statements can resolve them. - Ensure comparisons/logical operators always infer
Booleaneven withUnknown/Anyoperands; allow indexing intoAnycollections and tolerateUnknown/Anyindices for list/text indexing. - Make type-check findings in included files non-fatal (reported as warnings) and add regression tests + docs updates describing this behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/docs_parser_and_include_fixes_test.rs |
Adds regression tests for previously-failing RHS forms inside included actions and for non-fatal include type warnings. |
src/typechecker/mod.rs |
Adds action-body scoping + local symbol recording; adjusts boolean-result inference for comparisons; relaxes indexing rules for Any/Unknown. |
src/interpreter/mod.rs |
Changes include pipeline to report type-check failures as warnings and continue execution. |
src/analyzer/static_analyzer.rs |
Counts RHS expressions in store statements as variable “uses” to avoid false unused warnings. |
Docs/reference/error-codes.md |
Clarifies “Could not infer type for variable” is a warning and updates guidance/examples. |
Docs/04-advanced-features/modules.md |
Documents that included-file type-check findings are non-fatal warnings. |
Dev diary/2026-07-03-include-type-inference-issue-553.md |
Adds a dev diary entry describing the investigation and fixes for #553. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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). | ||
| 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, | ||
| }); | ||
| } |
There was a problem hiding this comment.
This scenario can't occur in a valid WFL program: the language forbids shadowing. A store that reuses a name defined in an outer scope is rejected by the analyzer (and independently by the runtime) with Variable 'x' has already been defined in an outer scope. Use 'change x to <value>' to modify it. — so when get_symbol resolves through parent scopes here, the name can only refer to that same outer variable, never a distinct local. I verified this empirically (the shadowing program aborts at analysis before the type checker's result matters) and added a regression test documenting the invariant (action_local_store_reusing_outer_name_is_a_semantic_error) plus a code comment explaining why parent-scope resolution is correct, in 0d28e75.
Generated by Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Docs/04-advanced-features/modules.md`:
- Around line 42-52: Add a language tag to the example code fence in the “Type
Checking in Included Files” section so markdownlint MD040 passes. Update the
bare fenced block around the type checking warning example to use the WFL
language tag, keeping the example content unchanged and matching the surrounding
docs style.
In `@src/typechecker/mod.rs`:
- Around line 662-685: The return-expression diagnostics are being produced
twice because `check_statement_types` already infers `ReturnStatement`
expressions and `check_return_statements` infers them again. Update
`check_return_statements` in `src/typechecker/mod.rs` to avoid re-checking the
same return expression path already handled by `check_statement_types`, and
instead only compare the existing inferred type against the declared
`return_type` or otherwise skip re-emitting expression errors. Use the
`check_statement_types`, `check_return_statements`, and `ReturnStatement` logic
to keep only one diagnostic per failing return expression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4170761-c96e-4d5c-be18-ef42c5f27614
📒 Files selected for processing (7)
Dev diary/2026-07-03-include-type-inference-issue-553.mdDocs/04-advanced-features/modules.mdDocs/reference/error-codes.mdsrc/analyzer/static_analyzer.rssrc/interpreter/mod.rssrc/typechecker/mod.rstests/docs_parser_and_include_fixes_test.rs
- Escape backslashes/quotes when interpolating args into WFL string literals in the run_included_action test helper (Copilot). - Drop duplicate expression diagnostics from check_return_statements: the body pass already infers every return expression, so re-inference for the return-type compatibility check no longer re-emits the same errors (CodeRabbit). - Tag the include-warnings example fence in modules.md as text so markdownlint MD040 passes; the block is console output, not WFL code (CodeRabbit). - Add a regression test documenting that a body-local store reusing an outer variable's name is a fatal semantic error pointing at 'change' — WFL forbids shadowing, so the type checker's parent-scope symbol resolution cannot mis-bind a would-be local (Copilot's shadowing scenario has no valid program). Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TiTGMQoi5CLPi8wqmjUqYx
Summary
Fixes issue #553 where four right-hand-side forms (
list[index],object["key"], comparisons, andlength of) caused fatal type inference failures when used in included files, even though the same code worked in the main file. The root cause was not include-specific: the type checker lacked scope for action bodies, comparisons with unknown operands inferredUnknowninstead ofBoolean, and indexingAnytypes was rejected. The include pipeline also reported type errors fatally instead of as non-fatal warnings like the main file.Key Changes
Type checker scope for action bodies (
src/typechecker/mod.rs):ActionDefinitionnow pushes a scope and defines parameter symbols (with explicitUnknownfor untyped parameters)VariableDeclarationdefines body-local variables with their inferred types so subsequent statements can resolve themparts[0]failing to infer afterstore parts as string_split of ...Comparison type inference (
src/typechecker/mod.rs):Equals,NotEquals, ordered comparisons) and logical operations (And,Or,Contains) now returnBooleaneven when operands areUnknownorAnyBooleanDynamic type indexing (
src/typechecker/mod.rs):Anycollections (e.g.,parse_jsonresults) now yieldsAnyinstead of a type errorUnknownandAnyindex values are tolerated for list and text indexingNon-fatal type warnings in includes (
src/interpreter/mod.rs):RuntimeErrorType checking warnings in included file '...'and execution continuesUnused variable detection fix (
src/analyzer/static_analyzer.rs):VariableDeclarationarm tomark_used_variablesso RHS expressions instorestatements inside action/loop bodies are counted as usesANALYZE-UNUSEDwarnings (e.g.,partsflagged as unused afterstore v as parts[0])Documentation updates (
Docs/04-advanced-features/modules.md,Docs/reference/error-codes.md):Tests
Added comprehensive regression tests in
tests/docs_parser_and_include_fixes_test.rs:included_action_can_store_list_index_result: List indexing in included actionsincluded_action_can_store_object_index_result: Object indexing (parse_json) in included actionsincluded_action_can_store_comparison_result: Equality comparisons in included actionsincluded_action_can_store_ordered_comparison_result: Ordered comparisons in included actionsincluded_action_can_store_length_of_result:length ofin included actionslist_index_result_variable_is_inferable_in_main_file: Guard for main-file inferencecomparison_result_variable_is_inferable_in_main_file: Guard for main-file inferenceinclude_type_errors_are_nonfatal_like_main_file: Ensures genuinely uninferable forms (arithmetic on untyped parameters) run to completion instead of abortingImplementation Notes
This change closes the class of bugs behind #551/#553 rather than just the four instances: any future type inference gap degrades to a warning instead of a fatal abort, aligning with WFL's principle of clear and actionable error reporting.
https://claude.ai/code/session_01TiTGMQoi5CLPi8wqmjUqYx
Summary by CodeRabbit
Bug Fixes
Documentation
Tests