Skip to content

Commit 109c010

Browse files
hjothamendixhjothamendix
committed
fix: preserve no-merge branch continuations
Symptom: when an if/else did not need an ExclusiveMerge because one branch was terminal, the builder could expose the original split as the continuation point instead of the tail of the non-terminal branch. Root cause: addIfStatement only carried a deferred split case to the parent flow. It did not remember the actual tail activity, pending branch case, or anchor from the branch that continued without a merge. Fix: track the no-merge continuation endpoint while building each branch, including pending case and anchor state from nested compound statements, and hand that endpoint to the parent flow instead of the stale split. Tests: add a synthetic builder regression where the else branch continues after the then branch returns, proving the following statement is wired from the branch tail rather than from the split.
1 parent d871691 commit 109c010

2 files changed

Lines changed: 197 additions & 19 deletions

File tree

‎mdl/executor/cmd_microflows_builder_control.go‎

Lines changed: 122 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
106106
}
107107

108108
thenStartX := splitX + SplitWidth + HorizontalSpacing/2
109+
var noMergeExitID model.ID
110+
var noMergeExitCase string
111+
var noMergeExitAnchor *ast.FlowAnchors
109112

110113
if hasElseBody {
111114
// IF WITH ELSE: TRUE path horizontal (happy path), FALSE path below
@@ -115,6 +118,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
115118

116119
var lastThenID model.ID
117120
var prevThenAnchor *ast.FlowAnchors
121+
var pendingThenCase string
122+
var pendingThenAnchor *ast.FlowAnchors
118123
for _, stmt := range s.ThenBody {
119124
thisAnchor := stmtOwnAnchor(stmt)
120125
actID := fb.addStatement(stmt)
@@ -131,7 +136,17 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
131136
}
132137
fb.flows = append(fb.flows, flow)
133138
} else {
134-
flow := newHorizontalFlow(lastThenID, actID)
139+
var flow *microflows.SequenceFlow
140+
if pendingThenCase != "" {
141+
flow = newHorizontalFlowWithCase(lastThenID, actID, pendingThenCase)
142+
if pendingThenAnchor != nil {
143+
prevThenAnchor = pendingThenAnchor
144+
}
145+
pendingThenCase = ""
146+
pendingThenAnchor = nil
147+
} else {
148+
flow = newHorizontalFlow(lastThenID, actID)
149+
}
135150
applyUserAnchors(flow, prevThenAnchor, thisAnchor)
136151
fb.flows = append(fb.flows, flow)
137152
}
@@ -140,6 +155,10 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
140155
if fb.nextConnectionPoint != "" {
141156
lastThenID = fb.nextConnectionPoint
142157
fb.nextConnectionPoint = ""
158+
pendingThenCase = fb.nextFlowCase
159+
fb.nextFlowCase = ""
160+
pendingThenAnchor = fb.nextFlowAnchor
161+
fb.nextFlowAnchor = nil
143162
} else {
144163
lastThenID = actID
145164
}
@@ -151,7 +170,15 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
151170
// nextConnectionPoint/nextFlowCase, so we must not emit a dangling flow here.
152171
if !thenReturns && needMerge {
153172
if lastThenID != "" {
154-
flow := newHorizontalFlow(lastThenID, mergeID)
173+
var flow *microflows.SequenceFlow
174+
if pendingThenCase != "" {
175+
flow = newHorizontalFlowWithCase(lastThenID, mergeID, pendingThenCase)
176+
if pendingThenAnchor != nil {
177+
prevThenAnchor = pendingThenAnchor
178+
}
179+
} else {
180+
flow = newHorizontalFlow(lastThenID, mergeID)
181+
}
155182
applyUserAnchors(flow, prevThenAnchor, nil)
156183
fb.flows = append(fb.flows, flow)
157184
} else {
@@ -170,6 +197,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
170197

171198
var lastElseID model.ID
172199
var prevElseAnchor *ast.FlowAnchors
200+
var pendingElseCase string
201+
var pendingElseAnchor *ast.FlowAnchors
173202
for _, stmt := range s.ElseBody {
174203
thisAnchor := stmtOwnAnchor(stmt)
175204
actID := fb.addStatement(stmt)
@@ -184,7 +213,17 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
184213
}
185214
fb.flows = append(fb.flows, flow)
186215
} else {
187-
flow := newHorizontalFlow(lastElseID, actID)
216+
var flow *microflows.SequenceFlow
217+
if pendingElseCase != "" {
218+
flow = newHorizontalFlowWithCase(lastElseID, actID, pendingElseCase)
219+
if pendingElseAnchor != nil {
220+
prevElseAnchor = pendingElseAnchor
221+
}
222+
pendingElseCase = ""
223+
pendingElseAnchor = nil
224+
} else {
225+
flow = newHorizontalFlow(lastElseID, actID)
226+
}
188227
applyUserAnchors(flow, prevElseAnchor, thisAnchor)
189228
fb.flows = append(fb.flows, flow)
190229
}
@@ -193,6 +232,10 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
193232
if fb.nextConnectionPoint != "" {
194233
lastElseID = fb.nextConnectionPoint
195234
fb.nextConnectionPoint = ""
235+
pendingElseCase = fb.nextFlowCase
236+
fb.nextFlowCase = ""
237+
pendingElseAnchor = fb.nextFlowAnchor
238+
fb.nextFlowAnchor = nil
196239
} else {
197240
lastElseID = actID
198241
}
@@ -205,10 +248,50 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
205248
if !elseReturns && needMerge {
206249
if lastElseID != "" {
207250
flow := newUpwardFlow(lastElseID, mergeID)
251+
if pendingElseCase != "" {
252+
flow.CaseValue = microflows.EnumerationCase{
253+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
254+
Value: pendingElseCase,
255+
}
256+
if pendingElseAnchor != nil {
257+
prevElseAnchor = pendingElseAnchor
258+
}
259+
}
208260
applyUserAnchors(flow, prevElseAnchor, nil)
209261
fb.flows = append(fb.flows, flow)
210262
}
211263
}
264+
if !needMerge {
265+
if thenReturns && !elseReturns {
266+
if lastElseID != "" {
267+
noMergeExitID = lastElseID
268+
noMergeExitCase = pendingElseCase
269+
if pendingElseAnchor != nil {
270+
noMergeExitAnchor = pendingElseAnchor
271+
} else {
272+
noMergeExitAnchor = prevElseAnchor
273+
}
274+
} else {
275+
noMergeExitID = splitID
276+
noMergeExitCase = "false"
277+
noMergeExitAnchor = falseBranchAnchor
278+
}
279+
} else if elseReturns && !thenReturns {
280+
if lastThenID != "" {
281+
noMergeExitID = lastThenID
282+
noMergeExitCase = pendingThenCase
283+
if pendingThenAnchor != nil {
284+
noMergeExitAnchor = pendingThenAnchor
285+
} else {
286+
noMergeExitAnchor = prevThenAnchor
287+
}
288+
} else {
289+
noMergeExitID = splitID
290+
noMergeExitCase = "true"
291+
noMergeExitAnchor = trueBranchAnchor
292+
}
293+
}
294+
}
212295
} else {
213296
// IF WITHOUT ELSE: FALSE path horizontal (happy path), TRUE path below
214297
// This keeps the "do nothing" path straight and the "do something" path below
@@ -230,6 +313,8 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
230313

231314
var lastThenID model.ID
232315
var prevThenAnchor *ast.FlowAnchors
316+
var pendingThenCase string
317+
var pendingThenAnchor *ast.FlowAnchors
233318
for _, stmt := range s.ThenBody {
234319
thisAnchor := stmtOwnAnchor(stmt)
235320
actID := fb.addStatement(stmt)
@@ -243,7 +328,17 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
243328
}
244329
fb.flows = append(fb.flows, flow)
245330
} else {
246-
flow := newHorizontalFlow(lastThenID, actID)
331+
var flow *microflows.SequenceFlow
332+
if pendingThenCase != "" {
333+
flow = newHorizontalFlowWithCase(lastThenID, actID, pendingThenCase)
334+
if pendingThenAnchor != nil {
335+
prevThenAnchor = pendingThenAnchor
336+
}
337+
pendingThenCase = ""
338+
pendingThenAnchor = nil
339+
} else {
340+
flow = newHorizontalFlow(lastThenID, actID)
341+
}
247342
applyUserAnchors(flow, prevThenAnchor, thisAnchor)
248343
fb.flows = append(fb.flows, flow)
249344
}
@@ -252,6 +347,10 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
252347
if fb.nextConnectionPoint != "" {
253348
lastThenID = fb.nextConnectionPoint
254349
fb.nextConnectionPoint = ""
350+
pendingThenCase = fb.nextFlowCase
351+
fb.nextFlowCase = ""
352+
pendingThenAnchor = fb.nextFlowAnchor
353+
fb.nextFlowAnchor = nil
255354
} else {
256355
lastThenID = actID
257356
}
@@ -264,6 +363,15 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
264363
if !thenReturns && needMerge {
265364
if lastThenID != "" {
266365
flow := newUpwardFlow(lastThenID, mergeID)
366+
if pendingThenCase != "" {
367+
flow.CaseValue = microflows.EnumerationCase{
368+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
369+
Value: pendingThenCase,
370+
}
371+
if pendingThenAnchor != nil {
372+
prevThenAnchor = pendingThenAnchor
373+
}
374+
}
267375
applyUserAnchors(flow, prevThenAnchor, nil)
268376
fb.flows = append(fb.flows, flow)
269377
} else {
@@ -273,6 +381,11 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
273381
fb.flows = append(fb.flows, flow)
274382
}
275383
}
384+
if !needMerge {
385+
noMergeExitID = splitID
386+
noMergeExitCase = "false"
387+
noMergeExitAnchor = falseBranchAnchor
388+
}
276389
}
277390

278391
// If both branches end with RETURN, the flow terminates here
@@ -300,22 +413,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
300413
fb.posX = max(afterSplit, afterBranch)
301414
}
302415
fb.posY = centerY
303-
fb.nextConnectionPoint = splitID
304-
// Tell parent to attach the case value on the next flow, and pass the
305-
// matching branch anchor so @anchor(true: ..., false: ...) survives to
306-
// the deferred splitID→nextActivity flow.
307-
if hasElseBody {
308-
if thenReturns {
309-
fb.nextFlowCase = "false"
310-
fb.nextFlowAnchor = falseBranchAnchor
311-
} else {
312-
fb.nextFlowCase = "true"
313-
fb.nextFlowAnchor = trueBranchAnchor
314-
}
416+
if noMergeExitID != "" {
417+
fb.nextConnectionPoint = noMergeExitID
418+
fb.nextFlowCase = noMergeExitCase
419+
fb.nextFlowAnchor = noMergeExitAnchor
315420
} else {
316-
// IF without ELSE: false is the continuing path
317-
fb.nextFlowCase = "false"
318-
fb.nextFlowAnchor = falseBranchAnchor
421+
fb.nextConnectionPoint = splitID
319422
}
320423
}
321424

‎mdl/executor/cmd_microflows_builder_no_merge_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package executor
4+
5+
import (
6+
"testing"
7+
8+
"github.com/mendixlabs/mxcli/mdl/ast"
9+
"github.com/mendixlabs/mxcli/model"
10+
"github.com/mendixlabs/mxcli/sdk/microflows"
11+
)
12+
13+
func TestBuildFlowGraph_NoMergeIfElseContinuesFromBranchTail(t *testing.T) {
14+
body := []ast.MicroflowStatement{
15+
&ast.IfStmt{
16+
Condition: &ast.VariableExpr{Name: "Done"},
17+
ThenBody: []ast.MicroflowStatement{
18+
&ast.ReturnStmt{Value: &ast.LiteralExpr{Kind: ast.LiteralBoolean, Value: true}},
19+
},
20+
ElseBody: []ast.MicroflowStatement{
21+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "else branch"}},
22+
},
23+
},
24+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "after branch"}},
25+
}
26+
27+
fb := &flowBuilder{
28+
posX: 100,
29+
posY: 100,
30+
spacing: HorizontalSpacing,
31+
declaredVars: map[string]string{"Done": "Boolean"},
32+
measurer: &layoutMeasurer{},
33+
}
34+
oc := fb.buildFlowGraph(body, &ast.MicroflowReturnType{Type: ast.DataType{Kind: ast.TypeBoolean}})
35+
36+
elseID := findLogActivityIDByMessage(t, oc, "else branch")
37+
afterID := findLogActivityIDByMessage(t, oc, "after branch")
38+
39+
if !hasSequenceFlow(oc.Flows, elseID, afterID) {
40+
t.Fatal("continuing ELSE branch tail must connect to the following statement")
41+
}
42+
for _, flow := range oc.Flows {
43+
if flow.DestinationID == afterID && flow.OriginID != elseID {
44+
t.Fatalf("following statement must not be wired from stale split origin %q", flow.OriginID)
45+
}
46+
}
47+
}
48+
49+
func findLogActivityIDByMessage(t *testing.T, oc *microflows.MicroflowObjectCollection, message string) model.ID {
50+
t.Helper()
51+
for _, obj := range oc.Objects {
52+
activity, ok := obj.(*microflows.ActionActivity)
53+
if !ok {
54+
continue
55+
}
56+
action, ok := activity.Action.(*microflows.LogMessageAction)
57+
if !ok || action.MessageTemplate == nil {
58+
continue
59+
}
60+
if action.MessageTemplate.GetTranslation("en_US") == message {
61+
return activity.ID
62+
}
63+
}
64+
t.Fatalf("missing log activity %q", message)
65+
return ""
66+
}
67+
68+
func hasSequenceFlow(flows []*microflows.SequenceFlow, originID, destinationID model.ID) bool {
69+
for _, flow := range flows {
70+
if flow.OriginID == originID && flow.DestinationID == destinationID {
71+
return true
72+
}
73+
}
74+
return false
75+
}

0 commit comments

Comments
 (0)