Skip to content

fix(core): validate nested module required inputs - #2912

Merged
trunk-io[bot] merged 3 commits into
dora-rs:mainfrom
SunSunSun689:fix-module-lint-nested-required-inputs
Aug 13, 2026
Merged

fix(core): validate nested module required inputs#2912
trunk-io[bot] merged 3 commits into
dora-rs:mainfrom
SunSunSun689:fix-module-lint-nested-required-inputs

Conversation

@SunSunSun689

Copy link
Copy Markdown
Contributor

No description provided.

@trunk-io

trunk-io Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

😎 Merged successfully - details.

Copy link
Copy Markdown
Collaborator

Automated review by Claude. No issues found.

check_nested_module_required_inputs makes the standalone module linter (check_module_file) reject a nested-module reference that omits a required input of the nested module, mirroring the expand-time check. It correctly iterates only nested_module.inputs (required, not the optional set) and checks presence by key in the referencing node's inputs. The added test reproduces the gap and asserts on the module / input / node names. This is complementary to the sibling input-validation PRs rather than a duplicate.


🤖 This is a fully automated review by Claude (Claude Code). No human has vetted these findings — please verify before acting on them.

Generated by Claude Code


Generated by Claude Code

@phil-opp

Copy link
Copy Markdown
Collaborator

The check itself is fine. The thing to watch is merge ordering against #2907.

Both PRs insert at the same anchor in check_module_file, immediately after let nested_module = load_module_file(&nested_canonical)?;. If this one's call lands first, #2907's test check_module_file_rejects_nested_module_conflicting_input_declarations fails: its fixture wires x: _mod/data to a nested module declaring required input data, so this check bails with "declares required input data" and the assertion msg.contains("required and optional") never sees its message.

Whichever merges second will need to re-order the checks or re-fixture the test. Worth deciding now rather than discovering it as a red test after a rebase — note that #2907 is a strict subset of #2906, so folding it into #2906 and closing it would make this moot.

@SunSunSun689
SunSunSun689 marked this pull request as draft August 13, 2026 07:28
@phil-opp
phil-opp marked this pull request as ready for review August 13, 2026 10:36
@trunk-io
trunk-io Bot merged commit e91471b into dora-rs:main Aug 13, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants