Fix #592: bare zero-arg included action reference now resolves - #637
Merged
Conversation
…#592) A zero-argument action exposed by an `include from` file and referenced by its bare name (`store x as greet`) was a fatal `Variable '<name>' is not defined` analyze error at top level and inside action bodies, while the `of` and `call` forms already worked across an include. A bare reference lowers to `Expression::Variable` with no call node, so it never reached the include-aware relaxation `warn_undefined_callee_if_includes` that #580 unified the `of` (`FunctionCall`) and `call` (`ActionCall`) forms onto — the third and last surviving form of the #548 -> #580 family. Route the `Expression::Variable` arm through that same helper: when the name is unresolved (and not a container property) and the program uses `include from`, emit a non-fatal `Undefined action` warning instead of aborting. With no includes present the helper returns false, so a genuine typo stays fatal. The type checker already returns Unknown for such a name and the interpreter already auto-calls a bare zero-arg action, so this analyzer relaxation is the only change needed. Backward compatible: it only relaxes a currently-fatal error. Production-readiness area: Correctness (#610 Phase 2 — consistent main/include semantics). Tests / docs: - analyzer unit tests: with-include relaxes to a warning; no-include stays fatal - un-ignore the phase1 issue_592_* acceptance tests (top level + action body); add an issue_592 no-include fatal guardrail; update the Phase 1 coverage map - TestPrograms/module_include_bare_zero_arg.wfl end-to-end fixture (+ helper, skip-listed in both integration harness scripts) - Docs/04-advanced-features/modules.md: new "Calling actions from an included file" subsection documenting all three call forms; Dev Diary entry Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ku1nQWnpSBgVXMNNVZ3Qhm
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe analyzer now treats unresolved bare names as possible zero-argument included actions, emitting a non-fatal warning when includes exist while retaining fatal errors otherwise. Regression fixtures, active tests, integration skips, and module documentation cover the behavior. ChangesBare zero-argument include resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes #592 by making bare references to zero-argument actions exposed via include from (e.g. store x as greet) resolve the same way as the existing call greet and greet of ... forms, eliminating a remaining main-vs-include inconsistency in the analyzer.
Changes:
- Analyzer: routes unresolved
Expression::Variablenames through the existing include-aware undefined-callee relaxation helper, falling back to a fatal undefined-name error only when no includes exist. - Tests: enables the previously ignored #592 regression tests, adds guardrails to ensure the relaxation stays fatal when no
include fromis present, and adds an end-to-endTestPrograms/fixture. - Docs/Dev diary: documents the three call forms for included actions and records the fix rationale.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/analyzer/mod.rs |
Extends include-aware undefined-name relaxation to bare Expression::Variable references; adds focused unit tests. |
tests/phase1_correctness_regression_test.rs |
Activates #592 regression tests and adds a no-include fatal guardrail. |
TestPrograms/module_include_bare_zero_arg.wfl |
End-to-end fixture covering bare zero-arg included action references (top-level + inside action body). |
TestPrograms/module_bare_zero_arg_helper.wfl |
Helper module providing a zero-arg action used by the fixture (skip-listed). |
scripts/run_integration_tests.sh |
Skip-lists the helper module so it isn’t executed as a standalone program. |
scripts/run_integration_tests.ps1 |
Same skip-list update for Windows runs. |
Docs/04-advanced-features/modules.md |
Adds a subsection documenting included-action invocation forms (bare / of / call ... with). |
Dev diary/2026-07-18-issue-592-bare-zero-arg-include.md |
Dev diary entry explaining root cause, fix, and regression coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| display line | ||
| ``` | ||
|
|
||
| All three forms work at the top level and inside your own action bodies. Because the analyzer does not read included files, it emits a **non-fatal** `Undefined action '<name>'` note for a name it cannot see statically — the program still runs and the action resolves at runtime. |
Comment on lines
+2569
to
+2573
| // no includes present the helper emits nothing and | ||
| // returns false, so a genuine typo stays fatal. | ||
| if !self.warn_undefined_callee_if_includes(name, *line, *column) { | ||
| self.report_undefined_name( | ||
| format!("Variable '{name}' is not defined"), |
Comment on lines
+39
to
+41
| warning (the action may be exposed by the included file at runtime, which the | ||
| analyzer never reads) and returns `true`; with no includes it emits nothing and | ||
| returns `false`, preserving the fatal path so genuine typos stay caught. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b75bba1501
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // consistently under `include from` (issue #592). With | ||
| // no includes present the helper emits nothing and | ||
| // returns false, so a genuine typo stays fatal. | ||
| if !self.warn_undefined_callee_if_includes(name, *line, *column) { |
There was a problem hiding this comment.
When a program contains any include from, this branch now downgrades every unresolved bare variable expression to a non-fatal Undefined action warning, not just the intended zero-arg action reference. In an include-using file, a genuine typo such as check if no: display misspelled_name end check now only warns and exits 0 because the branch is skipped at runtime, whereas it previously failed analysis with Variable 'misspelled_name' is not defined; this can let undefined-variable bugs pass CI in any program that happens to include a module.
Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/analyzer/mod.rs:2573
warn_undefined_callee_if_includesemitsUndefined action '<name>', but in theExpression::Variablepath the unresolved bare name can also be a variable/constant supplied by an included file (not necessarily an action). This warning text (and the static note that follows from it) can therefore be misleading once includes are present.
// A bare unresolved name may be a zero-argument action
// exposed by an `include from` file and referenced by
// its bare name (e.g. `store x as greet`), which lowers
// to `Expression::Variable` with no call node and so
// never reaches the `of`/`call` forms' include-aware
// relaxation. Route it through the same helper so all
// three call forms of the #548 -> #580 family behave
// consistently under `include from` (issue #592). With
// no includes present the helper emits nothing and
// returns false, so a genuine typo stays fatal.
if !self.warn_undefined_callee_if_includes(name, *line, *column) {
self.report_undefined_name(
format!("Variable '{name}' is not defined"),
Docs/04-advanced-features/modules.md:76
- This sentence says the analyzer does not read included files, but
include fromdoes parse/analyze/type-check the included file at runtime; the real limitation is that symbols from included files are not available when analyzing the including file. Reword to avoid an inaccurate claim.
All three forms work at the top level and inside your own action bodies. Because the analyzer does not read included files, it emits a **non-fatal** `Undefined action '<name>'` note for a name it cannot see statically — the program still runs and the action resolves at runtime.
62 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #592 by making bare references to zero-argument included actions resolve consistently with the
ofandcallforms. Previously,store x as greet(wheregreetis a zero-arg action from an included file) was a fatal analyze error, whilecall greetandgreet of "x"worked. This was the last surviving form of the main-vs-include inconsistency Phase 2 aims to eliminate.Changes
Analyzer (
src/analyzer/mod.rs)Expression::Variablearm inanalyze_expressionto route unresolved, non-container-property names through the existingwarn_undefined_callee_if_includeshelper (the same relaxation used byof/callforms)report_undefined_namewhen the helper returnsfalse(no includes present)include fromTests (
tests/phase1_correctness_regression_test.rs)#[ignore]fromissue_592_bare_zero_arg_included_action_top_levelandissue_592_bare_zero_arg_included_action_in_action_body— these are now active regression guardsissue_592_bare_undefined_without_include_stays_fatalguardrail to pin that the relaxation does not over-broaden into silencing real undefined-name errorsVariable '<name>' is not defined) — the call form #580's fix didn't cover #592 from "High (open) ⏳" to "High (fixed) ✅"Test Programs
TestPrograms/module_include_bare_zero_arg.wfl(end-to-end fixture exercising bare reference at top level and inside an action body)TestPrograms/module_bare_zero_arg_helper.wfl(helper module exposing a zero-arg action)scripts/run_integration_tests.shand.ps1to skip the helper moduleDocumentation (
Docs/04-advanced-features/modules.md)Undefined actionwarning the analyzer emits for statically-invisible namesDev Diary
Dev diary/2026-07-18-issue-592-bare-zero-arg-include.mddocumenting the root cause, fix, regression protection, and compatibilityImplementation Details
The root cause was that a bare reference (
store x as greet) lowers toExpression::Variablewith no call node, so it never reached the include-aware relaxation thatofandcallforms already used. The fix reuses the exact same helper (warn_undefined_callee_if_includes) that #580 unified those two forms onto, ensuring all three call forms cannot drift apart again.Behavior after the fix:
include from→ non-fatalUndefined actionwarning, runs, exit 0include from→ fatalVariable '…' is not defined, exit 3 (unchanged)Backward compatible: only relaxes a currently-fatal error when
include fromis present; preserves the fatal path for all other cases.https://claude.ai/code/session_01Ku1nQWnpSBgVXMNNVZ3Qhm
Summary by CodeRabbit
Bug Fixes
Documentation