Merge pull request #371 from hjotha/submit/describer-void-endevent-return

fix: preserve void EndEvent returns in describe

Author: ako

mdl-examples/bug-tests/368-void-endevent-return.mdl

@@ -0,0 +1,66 @@
+-- ============================================================================
+-- Bug #368: Empty EndEvent termination paths dropped in void microflow describe
+-- ============================================================================
+--
+-- Symptom (before fix):
+--   The describer skipped every empty `EndEvent` unconditionally. For
+--   void microflows that meant any explicit terminal `EndEvent` (e.g.
+--   the end of an early-exit branch) was lost on describe — even though
+--   `return;` is a valid void microflow termination. The roundtrip then
+--   fabricated a fall-through to the synthetic end and changed semantics
+--   for branches that originally had a deliberate early termination.
+--
+--   Conversely, in value-returning microflows, an empty EndEvent
+--   represented an invalid bare `return;` — emitting it was rejected by
+--   the parser on re-exec.
+--
+-- After fix:
+--   `cmd_microflows_show.go` now tracks whether the microflow has a
+--   return value:
+--     - void microflows describe empty EndEvents as `return;` so explicit
+--       termination paths survive describe → exec → describe.
+--     - value-returning microflows continue to suppress empty EndEvents
+--       so re-exec does not encounter bare `return;` (which is invalid
+--       MDL for value-returning shapes).
+--
+-- Usage:
+--   mxcli exec mdl-examples/bug-tests/368-void-endevent-return.mdl -p app.mpr
+--   mxcli -p app.mpr -c "describe microflow BugTest368.MF_VoidEarlyExit"
+--   The void microflow's describe output must contain `return;` for the
+--   early-exit branch. `mx check` must report 0 errors.
+-- ============================================================================
+
+create module BugTest368;
+
+-- Void microflow with an early-exit branch. The `if` THEN branch must
+-- describe as an explicit `return;` (not be silently dropped).
+create microflow BugTest368.MF_VoidEarlyExit (
+  $Skip: boolean
+)
+begin
+  if $Skip then
+    return;
+  end if;
+
+  log info node 'BugTest368' 'continued path';
+end;
+/
+
+-- Value-returning microflow with the same shape — describer must NOT
+-- emit a bare `return;` for the implicit end EndEvent because that would
+-- be invalid in a value-returning microflow.
+create microflow BugTest368.MF_BoolEarlyExit (
+  $Skip: boolean
+)
+returns boolean as $Done
+begin
+  declare $Done boolean = false;
+
+  if $Skip then
+    return false;
+  end if;
+
+  set $Done = true;
+  return $Done;
+end;
+/

mdl/executor/cmd_microflows_format_action.go

@@ -33,7 +33,10 @@ func formatActivity(
 			}
 			return fmt.Sprintf("return %s;", returnVal)
 		}
-		return "" // Skip end events without return value
+		if ctx != nil && ctx.DescribingMicroflowHasReturnValue {
+			return ""
+		}
+		return "return;"
 
 	case *microflows.ActionActivity:
 		return formatAction(ctx, activity.Action, entityNames, microflowNames)

mdl/executor/cmd_microflows_show.go

@@ -273,6 +273,12 @@ func describeMicroflow(ctx *ExecContext, name ast.QualifiedName) error {
 	// BEGIN block
 	lines = append(lines, "begin")
 
+	prevDescribingReturnValue := ctx.DescribingMicroflowHasReturnValue
+	ctx.DescribingMicroflowHasReturnValue = microflowHasReturnValue(targetMf)
+	defer func() {
+		ctx.DescribingMicroflowHasReturnValue = prevDescribingReturnValue
+	}()
+
 	// Generate activities
 	if targetMf.ObjectCollection != nil && len(targetMf.ObjectCollection.Objects) > 0 {
 		activityLines := formatMicroflowActivities(ctx, targetMf, entityNames, microflowNames)
@@ -487,6 +493,12 @@ func renderMicroflowMDL(
 	microflowNames map[model.ID]string,
 	sourceMap map[string]elkSourceRange,
 ) string {
+	prevDescribingReturnValue := ctx.DescribingMicroflowHasReturnValue
+	ctx.DescribingMicroflowHasReturnValue = microflowHasReturnValue(mf)
+	defer func() {
+		ctx.DescribingMicroflowHasReturnValue = prevDescribingReturnValue
+	}()
+
 	var lines []string
 
 	if mf.Documentation != "" {
@@ -573,6 +585,14 @@ func renderMicroflowMDL(
 	return strings.Join(lines, "\n")
 }
 
+func microflowHasReturnValue(mf *microflows.Microflow) bool {
+	if mf == nil || mf.ReturnType == nil {
+		return false
+	}
+	_, isVoid := mf.ReturnType.(*microflows.VoidType)
+	return !isVoid
+}
+
 // formatMicroflowDataType formats a microflow data type for MDL output.
 func formatMicroflowDataType(ctx *ExecContext, dt microflows.DataType, entityNames map[model.ID]string) string {
 	if dt == nil {

mdl/executor/cmd_microflows_show_helpers_test.go

@@ -322,8 +322,16 @@ func TestFormatActivity_EndEvent_NoReturn(t *testing.T) {
 	e := newTestExecutor()
 	obj := &microflows.EndEvent{BaseMicroflowObject: mkObj("1")}
 	got := e.formatActivity(obj, nil, nil)
+	if got != "return;" {
+		t.Errorf("expected bare return for EndEvent without return, got %q", got)
+	}
+}
+
+func TestFormatActivity_EndEvent_NoReturnValueInReturningMicroflow(t *testing.T) {
+	obj := &microflows.EndEvent{BaseMicroflowObject: mkObj("1")}
+	got := formatActivity(&ExecContext{DescribingMicroflowHasReturnValue: true}, obj, nil, nil)
 	if got != "" {
-		t.Errorf("expected empty for EndEvent without return, got %q", got)
+		t.Errorf("expected empty EndEvent to be skipped in value-returning microflow, got %q", got)
 	}
 }
 

mdl/executor/cmd_microflows_traverse_test.go

@@ -43,15 +43,17 @@ func TestTraverseFlow_LinearSequence(t *testing.T) {
 	visited := make(map[model.ID]bool)
 	e.traverseFlow(mkID("start"), activityMap, flowsByOrigin, nil, visited, nil, nil, &lines, 1, nil, 0, nil)
 
-	// StartEvent produces no output, EndEvent with no return produces no output.
-	// Each activity now has a @position line before it.
-	if len(lines) != 4 {
-		t.Fatalf("expected 4 lines, got %d: %v", len(lines), lines)
+	// StartEvent produces no output. Void EndEvent emits an explicit return.
+	// Each emitted activity has a @position line before it.
+	if len(lines) != 6 {
+		t.Fatalf("expected 6 lines, got %d: %v", len(lines), lines)
 	}
 	assertContains(t, lines[0], "@position(0, 0)")
 	assertContains(t, lines[1], "$Obj = create Mod.Entity;")
 	assertContains(t, lines[2], "@position(0, 0)")
 	assertContains(t, lines[3], "commit $Obj;")
+	assertContains(t, lines[4], "@position(0, 0)")
+	assertContains(t, lines[5], "return;")
 }
 
 // =============================================================================
@@ -206,11 +208,12 @@ func TestCollectErrorHandlerStatements_Simple(t *testing.T) {
 	}
 
 	stmts := e.collectErrorHandlerStatements(mkID("err_log"), activityMap, flowsByOrigin, nil, nil)
-	if len(stmts) != 1 {
-		t.Fatalf("expected 1 statement, got %d: %v", len(stmts), stmts)
+	if len(stmts) != 2 {
+		t.Fatalf("expected 2 statements, got %d: %v", len(stmts), stmts)
 	}
 	assertContains(t, stmts[0], "log error")
 	assertContains(t, stmts[0], "Something failed")
+	assertContains(t, stmts[1], "return;")
 }
 
 func TestCollectErrorHandlerStatements_StopsAtMerge(t *testing.T) {

mdl/executor/exec_context.go

@@ -82,6 +82,11 @@ type ExecContext struct {
 	// Executor. Used by REFRESH CATALOG BACKGROUND so the goroutine can
 	// deliver the result after syncBack has already run.
 	SyncCatalog func(*catalog.Catalog)
+
+	// DescribingMicroflowHasReturnValue is set while rendering a microflow body.
+	// It lets activity formatting distinguish a terminal void EndEvent from an
+	// empty EndEvent in a value-returning microflow, where bare `return;` is invalid.
+	DescribingMicroflowHasReturnValue bool
 }
 
 // Connected returns true if a project is connected via the Backend.