Skip to content

Commit 58d907a

Browse files
akoako
authored
Merge pull request #490 from hjotha/submit/branch-scoped-output-validation
fix: scope duplicate output checks by branch path
2 parents 248c305 + bc501c6 commit 58d907a

3 files changed

Lines changed: 219 additions & 28 deletions

File tree

‎mdl/executor/cmd_microflows_builder_validate.go‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ func (fb *flowBuilder) validateStatements(stmts []ast.MicroflowStatement) {
6767
}
6868
}
6969

70+
func (fb *flowBuilder) validateScopedStatements(stmts []ast.MicroflowStatement) {
71+
scoped := *fb
72+
scoped.varTypes = cloneStringMap(fb.varTypes)
73+
scoped.declaredVars = cloneStringMap(fb.declaredVars)
74+
scoped.validateStatements(stmts)
75+
fb.errors = scoped.errors
76+
}
77+
7078
// validateStatement validates a single statement for semantic errors.
7179
func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) {
7280
switch s := stmt.(type) {
@@ -93,22 +101,20 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) {
93101
}
94102

95103
case *ast.IfStmt:
96-
// Validate then branch
97-
fb.validateStatements(s.ThenBody)
98-
// Validate else branch if present
104+
fb.validateScopedStatements(s.ThenBody)
99105
if len(s.ElseBody) > 0 {
100-
fb.validateStatements(s.ElseBody)
106+
fb.validateScopedStatements(s.ElseBody)
101107
}
102108

103109
case *ast.EnumSplitStmt:
104110
if count := enumSplitBranchCount(s); count > maxEnumSplitBranches {
105111
fb.addError("enum split has %d branches; at most %d branches are supported", count, maxEnumSplitBranches)
106112
}
107113
for _, c := range s.Cases {
108-
fb.validateStatements(c.Body)
114+
fb.validateScopedStatements(c.Body)
109115
}
110116
if len(s.ElseBody) > 0 {
111-
fb.validateStatements(s.ElseBody)
117+
fb.validateScopedStatements(s.ElseBody)
112118
}
113119

114120
case *ast.LoopStmt:

‎mdl/executor/cmd_microflows_duplicate_output_test.go‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,68 @@ func TestValidateMicroflowBodyRejectsDuplicateCallOutputs(t *testing.T) {
5959
}
6060
}
6161

62+
func TestValidateMicroflowBodyAllowsDuplicateOutputsInExclusiveBranches(t *testing.T) {
63+
entityRef := ast.QualifiedName{Module: "Synthetic", Name: "Item"}
64+
stmt := &ast.CreateMicroflowStmt{
65+
Body: []ast.MicroflowStatement{
66+
&ast.IfStmt{
67+
Condition: &ast.VariableExpr{Name: "UsePrimaryPath"},
68+
ThenBody: []ast.MicroflowStatement{
69+
&ast.CreateObjectStmt{Variable: "Result", EntityType: entityRef},
70+
&ast.ReturnStmt{},
71+
},
72+
ElseBody: []ast.MicroflowStatement{
73+
&ast.RetrieveStmt{Variable: "Result", Source: entityRef, Limit: "1"},
74+
&ast.ReturnStmt{},
75+
},
76+
},
77+
},
78+
}
79+
80+
errs := ValidateMicroflowBody(stmt)
81+
for _, err := range errs {
82+
if strings.Contains(err, "duplicate variable name '$Result'") {
83+
t.Fatalf("exclusive branches must not share duplicate-output scope: %#v", errs)
84+
}
85+
}
86+
}
87+
88+
func TestValidateMicroflowBodyAllowsDuplicateOutputsInEnumCases(t *testing.T) {
89+
entityRef := ast.QualifiedName{Module: "Synthetic", Name: "Item"}
90+
stmt := &ast.CreateMicroflowStmt{
91+
Body: []ast.MicroflowStatement{
92+
&ast.EnumSplitStmt{
93+
Variable: "Route",
94+
Cases: []ast.EnumSplitCase{
95+
{
96+
Value: "First",
97+
Body: []ast.MicroflowStatement{
98+
&ast.CallJavaActionStmt{OutputVariable: "GeneratedID", ActionName: ast.QualifiedName{Module: "Synthetic", Name: "Generate"}},
99+
&ast.CreateObjectStmt{Variable: "Result", EntityType: entityRef},
100+
&ast.ReturnStmt{},
101+
},
102+
},
103+
{
104+
Value: "Second",
105+
Body: []ast.MicroflowStatement{
106+
&ast.CallJavaActionStmt{OutputVariable: "GeneratedID", ActionName: ast.QualifiedName{Module: "Synthetic", Name: "Generate"}},
107+
&ast.CreateObjectStmt{Variable: "Result", EntityType: entityRef},
108+
&ast.ReturnStmt{},
109+
},
110+
},
111+
},
112+
},
113+
},
114+
}
115+
116+
errs := strings.Join(ValidateMicroflowBody(stmt), "\n")
117+
for _, name := range []string{"GeneratedID", "Result"} {
118+
if strings.Contains(errs, "duplicate variable name '$"+name+"'") {
119+
t.Fatalf("enum cases must not share duplicate-output scope: %s", errs)
120+
}
121+
}
122+
}
123+
62124
func TestFormatMicroflowActivitiesWarnsAboutDuplicateModelOutputs(t *testing.T) {
63125
oc := &microflows.MicroflowObjectCollection{
64126
Objects: []microflows.MicroflowObject{
@@ -109,3 +171,71 @@ func TestFormatMicroflowActivitiesWarnsAboutDuplicateModelOutputs(t *testing.T)
109171
t.Fatalf("describe output must not invent aliases:\n%s", got)
110172
}
111173
}
174+
175+
func TestFormatMicroflowActivitiesDoesNotWarnForExclusiveBranchOutputs(t *testing.T) {
176+
oc := &microflows.MicroflowObjectCollection{
177+
Objects: []microflows.MicroflowObject{
178+
&microflows.StartEvent{
179+
BaseMicroflowObject: microflows.BaseMicroflowObject{
180+
BaseElement: model.BaseElement{ID: "start"},
181+
Position: model.Point{X: 0, Y: 100},
182+
},
183+
},
184+
&microflows.ExclusiveSplit{
185+
BaseMicroflowObject: microflows.BaseMicroflowObject{
186+
BaseElement: model.BaseElement{ID: "split"},
187+
Position: model.Point{X: 100, Y: 100},
188+
},
189+
SplitCondition: &microflows.ExpressionSplitCondition{Expression: "$UsePrimaryPath"},
190+
},
191+
&microflows.ActionActivity{
192+
BaseActivity: microflows.BaseActivity{
193+
BaseMicroflowObject: microflows.BaseMicroflowObject{
194+
BaseElement: model.BaseElement{ID: "then_create"},
195+
Position: model.Point{X: 200, Y: 100},
196+
},
197+
},
198+
Action: &microflows.CreateObjectAction{OutputVariable: "Result", EntityQualifiedName: "Synthetic.Item"},
199+
},
200+
&microflows.ActionActivity{
201+
BaseActivity: microflows.BaseActivity{
202+
BaseMicroflowObject: microflows.BaseMicroflowObject{
203+
BaseElement: model.BaseElement{ID: "else_retrieve"},
204+
Position: model.Point{X: 200, Y: 200},
205+
},
206+
},
207+
Action: &microflows.RetrieveAction{
208+
OutputVariable: "Result",
209+
Source: &microflows.DatabaseRetrieveSource{
210+
EntityQualifiedName: "Synthetic.Item",
211+
},
212+
},
213+
},
214+
&microflows.EndEvent{
215+
BaseMicroflowObject: microflows.BaseMicroflowObject{
216+
BaseElement: model.BaseElement{ID: "then_end"},
217+
Position: model.Point{X: 300, Y: 100},
218+
},
219+
},
220+
&microflows.EndEvent{
221+
BaseMicroflowObject: microflows.BaseMicroflowObject{
222+
BaseElement: model.BaseElement{ID: "else_end"},
223+
Position: model.Point{X: 300, Y: 200},
224+
},
225+
},
226+
},
227+
Flows: []*microflows.SequenceFlow{
228+
{OriginID: "start", DestinationID: "split"},
229+
{OriginID: "split", DestinationID: "then_create", CaseValue: &microflows.ExpressionCase{Expression: "true"}},
230+
{OriginID: "split", DestinationID: "else_retrieve", CaseValue: &microflows.ExpressionCase{Expression: "false"}},
231+
{OriginID: "then_create", DestinationID: "then_end"},
232+
{OriginID: "else_retrieve", DestinationID: "else_end"},
233+
},
234+
}
235+
lines := formatMicroflowActivities(&ExecContext{}, &microflows.Microflow{ObjectCollection: oc}, nil, nil)
236+
got := strings.Join(lines, "\n")
237+
238+
if strings.Contains(got, "-- WARNING: duplicate output variable $Result") {
239+
t.Fatalf("exclusive branch outputs must not be warned as linear duplicates:\n%s", got)
240+
}
241+
}

‎mdl/executor/cmd_microflows_show.go‎

Lines changed: 77 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -783,55 +783,110 @@ func formatMicroflowActivities(
783783
return lines
784784
}
785785

786+
type outputVariableSeen struct {
787+
pos model.Point
788+
}
789+
786790
func duplicateOutputVariableWarnings(oc *microflows.MicroflowObjectCollection) []string {
787-
type occurrence struct {
788-
variable string
789-
pos model.Point
790-
}
791-
var occurrences []occurrence
792-
var walk func(*microflows.MicroflowObjectCollection)
793-
walk = func(collection *microflows.MicroflowObjectCollection) {
791+
warningPositions := make(map[string]model.Point)
792+
793+
var walkCollection func(*microflows.MicroflowObjectCollection, map[string]outputVariableSeen)
794+
walkCollection = func(collection *microflows.MicroflowObjectCollection, inherited map[string]outputVariableSeen) {
794795
if collection == nil {
795796
return
796797
}
798+
activityMap := make(map[model.ID]microflows.MicroflowObject)
799+
flowsByOrigin := make(map[model.ID][]*microflows.SequenceFlow)
800+
var starts []model.ID
797801
for _, obj := range collection.Objects {
802+
activityMap[obj.GetID()] = obj
803+
if _, ok := obj.(*microflows.StartEvent); ok {
804+
starts = append(starts, obj.GetID())
805+
}
806+
}
807+
for _, flow := range collection.Flows {
808+
flowsByOrigin[flow.OriginID] = append(flowsByOrigin[flow.OriginID], flow)
809+
}
810+
if len(starts) == 0 {
811+
for _, obj := range collection.Objects {
812+
starts = append(starts, obj.GetID())
813+
}
814+
}
815+
816+
var walkNode func(model.ID, map[string]outputVariableSeen, map[model.ID]bool)
817+
walkNode = func(currentID model.ID, seen map[string]outputVariableSeen, path map[model.ID]bool) {
818+
if currentID == "" || path[currentID] {
819+
return
820+
}
821+
obj := activityMap[currentID]
822+
if obj == nil {
823+
return
824+
}
825+
path = cloneIDBoolMap(path)
826+
path[currentID] = true
827+
seen = cloneSeenOutputs(seen)
828+
798829
if activity, ok := obj.(*microflows.ActionActivity); ok {
799830
if name := actionOutputVariableName(activity.Action); name != "" {
800-
occurrences = append(occurrences, occurrence{variable: name, pos: obj.GetPosition()})
831+
if first, ok := seen[name]; ok {
832+
if _, recorded := warningPositions[name]; !recorded {
833+
warningPositions[name] = first.pos
834+
}
835+
} else {
836+
seen[name] = outputVariableSeen{pos: obj.GetPosition()}
837+
}
801838
}
802839
}
803840
if loop, ok := obj.(*microflows.LoopedActivity); ok {
804-
walk(loop.ObjectCollection)
841+
walkCollection(loop.ObjectCollection, seen)
842+
}
843+
for _, flow := range findNormalFlows(flowsByOrigin[currentID]) {
844+
walkNode(flow.DestinationID, seen, path)
805845
}
806846
}
807-
}
808-
walk(oc)
809847

810-
counts := make(map[string]int)
811-
firstPos := make(map[string]model.Point)
812-
for _, occ := range occurrences {
813-
counts[occ.variable]++
814-
if _, ok := firstPos[occ.variable]; !ok {
815-
firstPos[occ.variable] = occ.pos
848+
for _, startID := range starts {
849+
walkNode(startID, cloneSeenOutputs(inherited), nil)
816850
}
817851
}
852+
walkCollection(oc, nil)
818853

819854
var names []string
820-
for name, count := range counts {
821-
if count > 1 {
822-
names = append(names, name)
823-
}
855+
for name := range warningPositions {
856+
names = append(names, name)
824857
}
825858
sort.Strings(names)
826859

827860
warnings := make([]string, 0, len(names))
828861
for _, name := range names {
829-
pos := firstPos[name]
862+
pos := warningPositions[name]
830863
warnings = append(warnings, fmt.Sprintf("-- WARNING: duplicate output variable $%s at position (%d, %d) - model is invalid; open in Studio Pro to fix", name, pos.X, pos.Y))
831864
}
832865
return warnings
833866
}
834867

868+
func cloneSeenOutputs(src map[string]outputVariableSeen) map[string]outputVariableSeen {
869+
if src == nil {
870+
return make(map[string]outputVariableSeen)
871+
}
872+
dst := make(map[string]outputVariableSeen, len(src))
873+
for k, v := range src {
874+
dst[k] = v
875+
}
876+
return dst
877+
}
878+
879+
func cloneIDBoolMap(src map[model.ID]bool) map[model.ID]bool {
880+
if src == nil {
881+
return make(map[model.ID]bool)
882+
}
883+
dst := make(map[model.ID]bool, len(src))
884+
for k, v := range src {
885+
dst[k] = v
886+
}
887+
return dst
888+
}
889+
835890
func actionOutputVariableName(action any) string {
836891
switch a := action.(type) {
837892
case *microflows.CreateObjectAction:

0 commit comments

Comments
 (0)