diff --git a/mdl-examples/bug-tests/262-terminal-nested-if-no-dangling-merge.mdl b/mdl-examples/bug-tests/262-terminal-nested-if-no-dangling-merge.mdl new file mode 100644 index 00000000..98f78252 --- /dev/null +++ b/mdl-examples/bug-tests/262-terminal-nested-if-no-dangling-merge.mdl @@ -0,0 +1,53 @@ +-- ============================================================================ +-- Bug #262: Treat terminal nested IF as returning in flow builder +-- ============================================================================ +-- +-- Symptom (before fix): +-- A microflow whose body ends in `if { return } else { return }` was not +-- detected as terminal. The outer flow builder created a merge node and a +-- dangling continuation edge that pointed at no activity. Studio Pro raised +-- "KeyNotFoundException" / "Sequence contains no matching element" on open. +-- +-- After fix: +-- lastStmtIsReturn recurses through IfStmt via the new isTerminalStmt helper, +-- so an IF with an ELSE where both branches return is treated as terminal — +-- no merge emitted, no dangling edge. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/262-terminal-nested-if-no-dangling-merge.mdl -p app.mpr +-- Open in Studio Pro — no dangling edge, flow graph is clean. +-- ============================================================================ + +create module BugTest262; + +create microflow BugTest262.MF_BothBranchesReturn ( + $input: string +) +returns string as $result +begin + declare $result string = empty; + + if $input = 'a' then + return 'was a'; + else + return 'not a'; + end if; +end; +/ + +create microflow BugTest262.MF_ElseIfChainAllReturn ( + $input: string +) +returns string as $result +begin + declare $result string = empty; + + if $input = 'a' then + return 'a'; + elsif $input = 'b' then + return 'b'; + else + return 'other'; + end if; +end; +/ diff --git a/mdl/executor/cmd_microflows_builder_flows.go b/mdl/executor/cmd_microflows_builder_flows.go index 385a21bc..f0691356 100644 --- a/mdl/executor/cmd_microflows_builder_flows.go +++ b/mdl/executor/cmd_microflows_builder_flows.go @@ -172,11 +172,37 @@ func newUpwardFlow(originID, destinationID model.ID) *microflows.SequenceFlow { } } -// lastStmtIsReturn checks if the last statement in a body is a RETURN statement. +// lastStmtIsReturn reports whether execution of a body is guaranteed to terminate +// (via RETURN or RAISE ERROR) on every path — i.e. control can never fall off the +// end of the body into the parent flow. +// +// Terminal statements: ReturnStmt, RaiseErrorStmt. An IfStmt is terminal iff it +// has an ELSE and both branches are terminal (recursively). A LoopStmt is never +// terminal — BREAK can exit the loop even if the body returns. +// +// Naming kept for history; the predicate is really "last stmt is a guaranteed +// terminator". Missing this case causes the outer IF to emit a dangling +// continuation flow (duplicate "true" edge + orphan EndEvent), which Studio Pro +// rejects as "Sequence contains no matching element" when diffing. func lastStmtIsReturn(stmts []ast.MicroflowStatement) bool { if len(stmts) == 0 { return false } - _, ok := stmts[len(stmts)-1].(*ast.ReturnStmt) - return ok + return isTerminalStmt(stmts[len(stmts)-1]) +} + +func isTerminalStmt(stmt ast.MicroflowStatement) bool { + switch s := stmt.(type) { + case *ast.ReturnStmt: + return true + case *ast.RaiseErrorStmt: + return true + case *ast.IfStmt: + if len(s.ElseBody) == 0 { + return false + } + return lastStmtIsReturn(s.ThenBody) && lastStmtIsReturn(s.ElseBody) + default: + return false + } } diff --git a/mdl/executor/cmd_microflows_builder_terminal_test.go b/mdl/executor/cmd_microflows_builder_terminal_test.go new file mode 100644 index 00000000..3fe6ac66 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_terminal_test.go @@ -0,0 +1,99 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +func TestLastStmtIsReturn_EmptyBody(t *testing.T) { + if lastStmtIsReturn(nil) { + t.Error("empty body must not be terminal") + } +} + +func TestLastStmtIsReturn_PlainReturn(t *testing.T) { + body := []ast.MicroflowStatement{&ast.ReturnStmt{}} + if !lastStmtIsReturn(body) { + t.Error("body ending in ReturnStmt must be terminal") + } +} + +func TestLastStmtIsReturn_RaiseError(t *testing.T) { + body := []ast.MicroflowStatement{&ast.RaiseErrorStmt{}} + if !lastStmtIsReturn(body) { + t.Error("body ending in RaiseErrorStmt must be terminal") + } +} + +func TestLastStmtIsReturn_IfWithoutElse_NotTerminal(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}}, + } + if lastStmtIsReturn(body) { + t.Error("IF without ELSE must not be terminal (false path falls through)") + } +} + +func TestLastStmtIsReturn_IfBothBranchesReturn_Terminal(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ + ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + ElseBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + }, + } + if !lastStmtIsReturn(body) { + t.Error("IF/ELSE where both branches return must be terminal") + } +} + +func TestLastStmtIsReturn_IfOnlyThenReturns_NotTerminal(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ + ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + ElseBody: []ast.MicroflowStatement{&ast.LogStmt{}}, // non-terminal + }, + } + if lastStmtIsReturn(body) { + t.Error("IF/ELSE where only THEN terminates must not be terminal") + } +} + +func TestLastStmtIsReturn_NestedIfChain_Terminal(t *testing.T) { + // if { return } else if { return } else { return } + inner := &ast.IfStmt{ + ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + ElseBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + } + outer := &ast.IfStmt{ + ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + ElseBody: []ast.MicroflowStatement{inner}, + } + if !lastStmtIsReturn([]ast.MicroflowStatement{outer}) { + t.Error("else-if chain where every terminal branch returns must be terminal") + } +} + +func TestLastStmtIsReturn_RaiseErrorMixed_Terminal(t *testing.T) { + body := []ast.MicroflowStatement{ + &ast.IfStmt{ + ThenBody: []ast.MicroflowStatement{&ast.ReturnStmt{}}, + ElseBody: []ast.MicroflowStatement{&ast.RaiseErrorStmt{}}, + }, + } + if !lastStmtIsReturn(body) { + t.Error("IF/ELSE with return on one side and raise error on the other must be terminal") + } +} + +func TestLastStmtIsReturn_LoopNotTerminal(t *testing.T) { + // A LOOP whose body only returns is still non-terminal — BREAK can exit. + body := []ast.MicroflowStatement{ + &ast.LoopStmt{Body: []ast.MicroflowStatement{&ast.ReturnStmt{}}}, + } + if lastStmtIsReturn(body) { + t.Error("LOOP must never be terminal (BREAK path)") + } +}