Skip to content

fix: scope duplicate output checks by branch path#490

Merged
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/branch-scoped-output-validation
May 2, 2026
Merged

fix: scope duplicate output checks by branch path#490
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/branch-scoped-output-validation

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented May 2, 2026

Closes #489.

Summary

  • validate IF and enum-split branch bodies with cloned output-variable scopes
  • make duplicate-output DESCRIBE warnings track variables per normal sequence-flow path
  • keep linear duplicate-output errors/warnings intact while avoiding false positives for mutually exclusive branches

Validation

  • make build
  • make test
  • make lint-go

Symptom: duplicate-output validation and DESCRIBE warnings flagged variables declared in mutually exclusive branches as CE0111-style duplicates, even though only one branch can execute on a given path.

Root cause: both checks accumulated output variable names globally across the entire microflow body/object collection instead of tracking declarations per validation branch or per control-flow path.

Fix: validate IF and enum-split bodies with cloned branch scopes, and make duplicate-output DESCRIBE warnings walk normal sequence-flow paths with path-local state. Linear duplicates still warn/fail, while sibling branch outputs do not share scope.

Tests: make build, make test, make lint-go.
@ako
Copy link
Copy Markdown
Collaborator

ako commented May 2, 2026

Review: fix: scope duplicate output checks by branch path

Verdict: Approve with minor notes

What the PR does

Two coordinated fixes for false-positive duplicate output variable warnings/errors when the same output variable name is used in mutually exclusive branches (IF/ELSE, enum split cases):

  1. validateScopedStatements — clones varTypes and declaredVars before validating branch bodies so cross-branch variable declarations don't bleed into sibling branches.
  2. duplicateOutputVariableWarnings rewrite — replaces flat occurrence-counting with a path-following graph traversal that clones seen and path maps at each step, making each execution path independent.

No blockers

Minor issues

O(N²) map cloning in duplicateOutputVariableWarnings
cloneIDBoolMap(path) is called at every node in the graph. For a linear flow of N nodes this creates N maps of sizes 1, 2, ..., N — O(N²) total allocations. For typical microflows (tens of nodes) this is fine, but worth a comment if microflows with hundreds of generated nodes become common.

starts fallback uses all objects
When no StartEvent is found, starts is seeded from all NodesByID values. For a large microflow this triggers O(N) separate DFS traversals from every node, many of which will be immediately terminated by the path guard. Correct and safe, just slightly surprising — a brief comment would help future readers.

Positive notes

  • Clean PR: no bundled unrelated changes (unlike several recent PRs on this branch — this is the right scoping)
  • Four focused tests that directly reproduce the reported false-positive scenarios
  • validateScopedStatements is the minimal correct fix (8 lines, no API surface change)
  • Path-scoped traversal correctly models exclusive-branch semantics: a variable assigned in the true branch cannot be "seen again" in the false branch

@ako ako merged commit 58d907a into mendixlabs:main May 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate microflow output checks should be scoped to exclusive control-flow paths

3 participants