Merge pull request #204 from engalar/fix/connection-index-int64

fix: microflow layout issues (ConnectionIndex, redundant Merge, DESCRIBE roundtrip)

Author: ako

mdl-examples/doctype-tests/keyword-as-identifier.mdl

@@ -62,12 +62,9 @@ CREATE ENUMERATION KeywordTest.MiscKeywords (
   Action,
   Source,
   Target,
-  Owner,
-  Type,
   Name,
   Value,
   Result,
-  Object,
   Index,
   Input,
   Output,
@@ -94,7 +91,6 @@ CREATE ENTITY KeywordTest.Data (
 
 CREATE ENTITY KeywordTest.Activity (
   Name       : String(200),
-  Type       : String(200),
   Result     : String(200)
 );
 
@@ -111,7 +107,6 @@ BEGIN
 
   $Activity = CREATE KeywordTest.Activity (
     Name   = 'test',
-    Type   = 'task',
     Result = 'ok'
   );
 END;

mdl/executor/cmd_microflows_builder.go

@@ -29,6 +29,7 @@ type flowBuilder struct {
 	errors              []string                 // Validation errors collected during build
 	measurer            *layoutMeasurer          // For measuring statement dimensions
 	nextConnectionPoint model.ID                 // For compound statements: the exit point differs from entry point
+	nextFlowCase        string                   // If set, next connecting flow uses this case value (for merge-less splits)
 	reader              *mpr.Reader              // For looking up page/microflow references
 	hierarchy           *ContainerHierarchy      // For resolving container IDs to module names
 	pendingAnnotations  *ast.ActivityAnnotations // Pending annotations to attach to next activity

mdl/executor/cmd_microflows_builder_control.go

@@ -65,9 +65,19 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	// Calculate merge position (after the longest branch)
 	mergeX := splitX + SplitWidth + HorizontalSpacing/2 + branchWidth + HorizontalSpacing/2
 
-	// Only create merge if at least one branch does NOT end with RETURN
-	var mergeID model.ID
+	// Determine if the merge would have 2+ incoming edges (non-redundant).
+	// Skip merge when only one branch flows into it (the other returns).
+	needMerge := false
 	if !bothReturn {
+		if hasElseBody {
+			needMerge = !thenReturns && !elseReturns // both branches continue → 2 inputs
+		} else {
+			needMerge = !thenReturns // THEN continues + FALSE path → 2 inputs
+		}
+	}
+
+	var mergeID model.ID
+	if needMerge {
 		merge := &microflows.ExclusiveMerge{
 			BaseMicroflowObject: microflows.BaseMicroflowObject{
 				BaseElement: model.BaseElement{ID: model.ID(mpr.GenerateID())},
@@ -153,8 +163,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 		// IF WITHOUT ELSE: FALSE path horizontal (happy path), TRUE path below
 		// This keeps the "do nothing" path straight and the "do something" path below
 
-		// FALSE path: connect split directly to merge horizontally
-		fb.flows = append(fb.flows, newHorizontalFlowWithCase(splitID, mergeID, "false"))
+		if needMerge {
+			// FALSE path: connect split directly to merge horizontally
+			fb.flows = append(fb.flows, newHorizontalFlowWithCase(splitID, mergeID, "false"))
+		}
+		// When !needMerge (thenReturns): FALSE flow is deferred — the parent will
+		// connect splitID to the next activity with nextFlowCase="false".
 
 		// TRUE path: goes below the main line
 		thenCenterY := centerY + VerticalSpacing
@@ -202,13 +216,34 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	// Restore endsWithReturn - a single branch returning doesn't end the overall flow
 	fb.endsWithReturn = savedEndsWithReturn
 
-	// Update position to after the merge, on the happy path center line
-	fb.posX = mergeX + MergeSize + HorizontalSpacing/2
-	fb.posY = centerY
-
-	// Set nextConnectionPoint so the next activity connects FROM the merge
-	// while incoming connection goes TO the split (returned below)
-	fb.nextConnectionPoint = mergeID
+	if needMerge {
+		// Update position to after the merge, on the happy path center line
+		fb.posX = mergeX + MergeSize + HorizontalSpacing/2
+		fb.posY = centerY
+		fb.nextConnectionPoint = mergeID
+	} else {
+		// No merge: the split's continuing branch connects directly to the next activity.
+		// Position after the split, past the downward branch's horizontal extent.
+		afterSplit := splitX + SplitWidth + HorizontalSpacing
+		afterBranch := thenStartX + thenBounds.Width + HorizontalSpacing/2
+		if !hasElseBody {
+			fb.posX = max(afterSplit, afterBranch)
+		} else {
+			fb.posX = max(afterSplit, afterBranch)
+		}
+		fb.posY = centerY
+		fb.nextConnectionPoint = splitID
+		// Tell parent to attach the case value on the next flow
+		if hasElseBody {
+			if thenReturns {
+				fb.nextFlowCase = "false"
+			} else {
+				fb.nextFlowCase = "true"
+			}
+		} else {
+			fb.nextFlowCase = "false" // IF without ELSE: false is the continuing path
+		}
+	}
 
 	return splitID
 }

mdl/executor/cmd_microflows_builder_graph.go

@@ -53,6 +53,8 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
 	fb.posX += fb.spacing
 
 	// Process each statement
+	// pendingCase holds the case value for the NEXT flow (set by merge-less splits)
+	pendingCase := ""
 	for _, stmt := range stmts {
 		activityID := fb.addStatement(stmt)
 		if activityID != "" {
@@ -62,11 +64,19 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
 				fb.pendingAnnotations = nil
 			}
 			// Connect to previous object with horizontal SequenceFlow
-			fb.flows = append(fb.flows, newHorizontalFlow(lastID, activityID))
+			if pendingCase != "" {
+				fb.flows = append(fb.flows, newHorizontalFlowWithCase(lastID, activityID, pendingCase))
+				pendingCase = ""
+			} else {
+				fb.flows = append(fb.flows, newHorizontalFlow(lastID, activityID))
+			}
 			// For compound statements (IF, LOOP), the exit point differs from entry point
 			if fb.nextConnectionPoint != "" {
 				lastID = fb.nextConnectionPoint
 				fb.nextConnectionPoint = ""
+				// Save nextFlowCase for the NEXT iteration's flow creation
+				pendingCase = fb.nextFlowCase
+				fb.nextFlowCase = ""
 			} else {
 				lastID = activityID
 			}
@@ -98,7 +108,11 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
 		fb.objects = append(fb.objects, endEvent)
 
 		// Connect last activity to end event
-		fb.flows = append(fb.flows, newHorizontalFlow(lastID, endEvent.ID))
+		if pendingCase != "" {
+			fb.flows = append(fb.flows, newHorizontalFlowWithCase(lastID, endEvent.ID, pendingCase))
+		} else {
+			fb.flows = append(fb.flows, newHorizontalFlow(lastID, endEvent.ID))
+		}
 	}
 
 	return &microflows.MicroflowObjectCollection{

mdl/executor/cmd_microflows_format_action.go

@@ -24,7 +24,9 @@ func (e *Executor) formatActivity(
 	case *microflows.EndEvent:
 		if activity.ReturnValue != "" {
 			returnVal := strings.TrimSuffix(activity.ReturnValue, "\n")
-			if !strings.HasPrefix(returnVal, "$") && !isMendixKeyword(returnVal) && !isQualifiedEnumLiteral(returnVal) {
+			// Only add $ prefix for bare identifiers (no operators, quotes, or parens)
+			if !strings.HasPrefix(returnVal, "$") && !isMendixKeyword(returnVal) && !isQualifiedEnumLiteral(returnVal) &&
+				!strings.ContainsAny(returnVal, "+'\"()") {
 				returnVal = "$" + returnVal
 			}
 			return fmt.Sprintf("RETURN %s;", returnVal)
@@ -97,7 +99,7 @@ func (e *Executor) formatAction(
 		if a.DataType != nil {
 			varType = e.formatMicroflowDataType(a.DataType, entityNames)
 		}
-		initialValue := a.InitialValue
+		initialValue := strings.TrimSuffix(a.InitialValue, "\n")
 		if initialValue == "" {
 			initialValue = "empty"
 		}

mdl/executor/cmd_microflows_show_helpers.go

@@ -211,28 +211,63 @@ func (e *Executor) traverseFlow(
 
 		trueFlow, falseFlow := findBranchFlows(flows)
 
+		// Guard pattern: true branch is a single EndEvent (RETURN),
+		// but only when the false branch does NOT also end directly.
+		// If both branches return, use normal IF/ELSE/END IF.
+		isGuard := false
 		if trueFlow != nil {
-			e.traverseFlowUntilMerge(trueFlow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			if _, isEnd := activityMap[trueFlow.DestinationID].(*microflows.EndEvent); isEnd {
+				isGuard = true
+				// Not a guard if both branches return directly
+				if falseFlow != nil {
+					if _, falseIsEnd := activityMap[falseFlow.DestinationID].(*microflows.EndEvent); falseIsEnd {
+						isGuard = false
+					}
+				}
+			}
 		}
 
-		if falseFlow != nil {
-			*lines = append(*lines, indentStr+"ELSE")
-			visitedFalseBranch := make(map[model.ID]bool)
-			for id := range visited {
-				visitedFalseBranch[id] = true
+		if isGuard {
+			e.traverseFlowUntilMerge(trueFlow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			*lines = append(*lines, indentStr+"END IF;")
+			recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
+
+			// Continue from the false branch (skip through merge if present)
+			if falseFlow != nil {
+				contID := falseFlow.DestinationID
+				if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge {
+					visited[contID] = true
+					for _, flow := range flowsByOrigin[contID] {
+						contID = flow.DestinationID
+						break
+					}
+				}
+				e.traverseFlow(contID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+			}
+		} else {
+			if trueFlow != nil {
+				e.traverseFlowUntilMerge(trueFlow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
 			}
-			e.traverseFlowUntilMerge(falseFlow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visitedFalseBranch, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
-		}
 
-		*lines = append(*lines, indentStr+"END IF;")
-		recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
+			if falseFlow != nil {
+				*lines = append(*lines, indentStr+"ELSE")
+				visitedFalseBranch := make(map[model.ID]bool)
+				for id := range visited {
+					visitedFalseBranch[id] = true
+				}
+				e.traverseFlowUntilMerge(falseFlow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visitedFalseBranch, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			}
+
+			*lines = append(*lines, indentStr+"END IF;")
+			recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
 
-		// Continue after the merge point
-		if mergeID != "" {
-			visited[mergeID] = true
-			nextFlows := flowsByOrigin[mergeID]
-			for _, flow := range nextFlows {
-				e.traverseFlow(flow.DestinationID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+			// Continue after the merge point
+			if mergeID != "" {
+				visited[mergeID] = true
+				nextFlows := flowsByOrigin[mergeID]
+				for _, flow := range nextFlows {
+					e.traverseFlow(flow.DestinationID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+				}
 			}
 		}
 		return
@@ -324,28 +359,61 @@ func (e *Executor) traverseFlowUntilMerge(
 
 		trueFlow, falseFlow := findBranchFlows(flows)
 
+		// Guard pattern: true branch is a single EndEvent (RETURN),
+		// but only when the false branch does NOT also end directly.
+		isGuard := false
 		if trueFlow != nil {
-			e.traverseFlowUntilMerge(trueFlow.DestinationID, nestedMergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			if _, isEnd := activityMap[trueFlow.DestinationID].(*microflows.EndEvent); isEnd {
+				isGuard = true
+				if falseFlow != nil {
+					if _, falseIsEnd := activityMap[falseFlow.DestinationID].(*microflows.EndEvent); falseIsEnd {
+						isGuard = false
+					}
+				}
+			}
 		}
 
-		if falseFlow != nil {
-			*lines = append(*lines, indentStr+"ELSE")
-			visitedFalseBranch := make(map[model.ID]bool)
-			for id := range visited {
-				visitedFalseBranch[id] = true
+		if isGuard {
+			e.traverseFlowUntilMerge(trueFlow.DestinationID, nestedMergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			*lines = append(*lines, indentStr+"END IF;")
+			recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
+
+			// Continue from the false branch (skip through merge if present)
+			if falseFlow != nil {
+				contID := falseFlow.DestinationID
+				if _, isMerge := activityMap[contID].(*microflows.ExclusiveMerge); isMerge {
+					visited[contID] = true
+					for _, flow := range flowsByOrigin[contID] {
+						contID = flow.DestinationID
+						break
+					}
+				}
+				e.traverseFlowUntilMerge(contID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+			}
+		} else {
+			if trueFlow != nil {
+				e.traverseFlowUntilMerge(trueFlow.DestinationID, nestedMergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
 			}
-			e.traverseFlowUntilMerge(falseFlow.DestinationID, nestedMergeID, activityMap, flowsByOrigin, splitMergeMap, visitedFalseBranch, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
-		}
 
-		*lines = append(*lines, indentStr+"END IF;")
-		recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
+			if falseFlow != nil {
+				*lines = append(*lines, indentStr+"ELSE")
+				visitedFalseBranch := make(map[model.ID]bool)
+				for id := range visited {
+					visitedFalseBranch[id] = true
+				}
+				e.traverseFlowUntilMerge(falseFlow.DestinationID, nestedMergeID, activityMap, flowsByOrigin, splitMergeMap, visitedFalseBranch, entityNames, microflowNames, lines, indent+1, sourceMap, headerLineCount, annotationsByTarget)
+			}
+
+			*lines = append(*lines, indentStr+"END IF;")
+			recordSourceMap(sourceMap, currentID, startLine, len(*lines)+headerLineCount-1)
 
-		// Continue after nested merge
-		if nestedMergeID != "" && nestedMergeID != mergeID {
-			visited[nestedMergeID] = true
-			nextFlows := flowsByOrigin[nestedMergeID]
-			for _, flow := range nextFlows {
-				e.traverseFlowUntilMerge(flow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+			// Continue after nested merge
+			if nestedMergeID != "" && nestedMergeID != mergeID {
+				visited[nestedMergeID] = true
+				nextFlows := flowsByOrigin[nestedMergeID]
+				for _, flow := range nextFlows {
+					e.traverseFlowUntilMerge(flow.DestinationID, mergeID, activityMap, flowsByOrigin, splitMergeMap, visited, entityNames, microflowNames, lines, indent, sourceMap, headerLineCount, annotationsByTarget)
+				}
 			}
 		}
 		return

mdl/grammar/MDLParser.g4

@@ -2867,7 +2867,8 @@ alterSettingsClause
     ;
 
 settingsSection
-    : IDENTIFIER   // MODEL, LANGUAGE
+    : IDENTIFIER   // LANGUAGE, etc.
+    | MODEL
     | WORKFLOWS
     ;