Merge pull request #469 from hjotha/submit/merged-pr-ako-polish

fix: address merged-PR ako follow-up polish items

Author: ako

mdl/executor/cmd_microflows_builder_actions.go

@@ -227,8 +227,13 @@ func (fb *flowBuilder) addRollbackAction(s *ast.RollbackStmt) model.ID {
 
 // addChangeObjectAction creates a CHANGE statement.
 func (fb *flowBuilder) addChangeObjectAction(s *ast.ChangeObjectStmt) model.ID {
-	// Empty non-committing changes need RefreshInClient to satisfy Studio Pro
-	// consistency checks; explicit `refresh` keeps the same flag for all changes.
+	// CE0032 rejects change actions with no items that do not commit the
+	// object. The published error text only mentions items/commit, but
+	// `mx check` also accepts RefreshInClient=true as a third valid escape.
+	// The builder auto-promotes empty changes to refresh-only so describe →
+	// exec of such actions stays valid without requiring authored MDL to say
+	// `refresh` explicitly; when the author wrote `refresh`, we keep the
+	// same flag for non-empty changes too.
 	action := &microflows.ChangeObjectAction{
 		BaseElement:       model.BaseElement{ID: model.ID(types.GenerateID())},
 		ErrorHandlingType: fb.ehType(nil),

mdl/executor/cmd_microflows_builder_calls.go

@@ -357,28 +357,28 @@ func javaActionReturnVarType(returnType javaactions.CodeActionReturnType) string
 	return ""
 }
 
+// inferGenericJavaActionReturnType infers the element type of a generic
+// `ListType{Entity: ""}` Java action return by inspecting the caller's
+// variable-typed arguments. When the action parameters don't determine an
+// element type, the function returns "" and the caller records the declaration
+// as Unknown.
 func (fb *flowBuilder) inferGenericJavaActionReturnType(jaDef *javaactions.JavaAction, s *ast.CallJavaActionStmt) string {
 	if jaDef == nil || fb.varTypes == nil || s == nil {
 		return ""
 	}
-	switch t := jaDef.ReturnType.(type) {
-	case *javaactions.ListType:
-		if t.Entity != "" {
-			return ""
-		}
-	case javaactions.ListType:
-		if t.Entity != "" {
-			return ""
-		}
-	default:
+	// ListType is always stored as a pointer by the parser; there is no value
+	// form in the SDK. Only the generic (Entity == "") case reaches the
+	// variable-type lookup below.
+	t, ok := jaDef.ReturnType.(*javaactions.ListType)
+	if !ok || t.Entity != "" {
 		return ""
 	}
 	for _, arg := range s.Arguments {
-		valueExpr := strings.TrimPrefix(strings.Trim(fb.exprToString(arg.Value), "'"), "$")
-		if valueExpr == "" {
+		varExpr, ok := arg.Value.(*ast.VariableExpr)
+		if !ok {
 			continue
 		}
-		if typ := fb.varTypes[valueExpr]; strings.HasPrefix(typ, "List of ") {
+		if typ := fb.varTypes[varExpr.Name]; strings.HasPrefix(typ, "List of ") {
 			return typ
 		}
 	}

mdl/executor/cmd_microflows_builder_flows.go

@@ -783,12 +783,15 @@ func isTerminalStmt(stmt ast.MicroflowStatement) bool {
 				return false
 			}
 		}
-		// Both paths return true intentionally. isTerminalStmt diverges from
-		// bodyReturns here: bodyReturns requires an ELSE to guarantee all paths
-		// return (a valid Mendix requirement), but isTerminalStmt only needs to
-		// know whether the flow builder must thread a continuation edge past this
-		// statement. When every explicit case terminates the split has no outgoing
-		// merge edge regardless of whether an ELSE exists, so we are always terminal.
+		// Every reachable branch terminates, so the split has no continuation
+		// to thread into the parent flow. This intentionally diverges from
+		// `bodyReturns` in validate_microflow.go: that predicate treats an
+		// enum split without an `else` as non-terminal because authored MDL
+		// is expected to supply a default branch covering missing values.
+		// Here we also accept described-from-MPR graphs where Studio Pro
+		// produced an exhaustive set of value cases without a default flow —
+		// in both no-else and with-else forms the split terminates once we
+		// reach this point.
 		return true
 	default:
 		return false

mdl/executor/cmd_microflows_builder_java_return_test.go

@@ -63,6 +63,37 @@ func TestAddJavaAction_ConcreteListReturnRegistersListType(t *testing.T) {
 	}
 }
 
+// TestAddJavaAction_EntityReturnRegistersEntityType pins ako's review follow-up
+// for PR #357: the most common Java action return shape — a bare
+// `*javaactions.EntityType{Entity: "Mod.Ent"}` — must register the output
+// variable as the entity's qualified name so downstream attribute access
+// resolves against the right domain-model entry.
+func TestAddJavaAction_EntityReturnRegistersEntityType(t *testing.T) {
+	backend := &mock.MockBackend{
+		ReadJavaActionByNameFunc: func(qualifiedName string) (*javaactions.JavaAction, error) {
+			return &javaactions.JavaAction{
+				ReturnType: &javaactions.EntityType{Entity: "Orders.Order"},
+			}, nil
+		},
+	}
+
+	fb := &flowBuilder{
+		backend:      backend,
+		varTypes:     map[string]string{},
+		declaredVars: map[string]string{},
+		measurer:     &layoutMeasurer{},
+	}
+
+	fb.addCallJavaActionAction(&ast.CallJavaActionStmt{
+		OutputVariable: "CreatedOrder",
+		ActionName:     ast.QualifiedName{Module: "Orders", Name: "CreateFromPayload"},
+	})
+
+	if got := fb.varTypes["CreatedOrder"]; got != "Orders.Order" {
+		t.Fatalf("CreatedOrder type = %q, want Orders.Order", got)
+	}
+}
+
 func TestAddJavaAction_GenericListReturnInheritsInputListType(t *testing.T) {
 	backend := &mock.MockBackend{
 		ReadJavaActionByNameFunc: func(qualifiedName string) (*javaactions.JavaAction, error) {

mdl/executor/cmd_microflows_has_explicit_false_anchor_test.go

@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// TestHasExplicitFalseBranchAnchor pins ako's review follow-up for PR #364:
+// the heuristic that decides whether a false-branch flow carries an explicit
+// anchor annotation should fire only on the AnchorTop→AnchorBottom pair and
+// must stay dormant for nil flows, builder defaults, and any other side
+// combination.
+func TestHasExplicitFalseBranchAnchor(t *testing.T) {
+	tests := []struct {
+		name string
+		flow *microflows.SequenceFlow
+		want bool
+	}{
+		{name: "nil flow", flow: nil, want: false},
+		{
+			name: "builder default right→left",
+			flow: &microflows.SequenceFlow{OriginConnectionIndex: AnchorRight, DestinationConnectionIndex: AnchorLeft},
+			want: false,
+		},
+		{
+			name: "split default bottom→top",
+			flow: &microflows.SequenceFlow{OriginConnectionIndex: AnchorBottom, DestinationConnectionIndex: AnchorTop},
+			want: false,
+		},
+		{
+			name: "authored top→bottom",
+			flow: &microflows.SequenceFlow{OriginConnectionIndex: AnchorTop, DestinationConnectionIndex: AnchorBottom},
+			want: true,
+		},
+		{
+			name: "only origin customised",
+			flow: &microflows.SequenceFlow{OriginConnectionIndex: AnchorTop, DestinationConnectionIndex: AnchorTop},
+			want: false,
+		},
+		{
+			name: "only destination customised",
+			flow: &microflows.SequenceFlow{OriginConnectionIndex: AnchorBottom, DestinationConnectionIndex: AnchorBottom},
+			want: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := hasExplicitFalseBranchAnchor(tc.flow)
+			if got != tc.want {
+				t.Fatalf("hasExplicitFalseBranchAnchor(%s) = %v, want %v", tc.name, got, tc.want)
+			}
+		})
+	}
+}

mdl/executor/cmd_microflows_show_helpers.go

@@ -1473,12 +1473,16 @@ func branchFlowStartsAtTerminal(flow *microflows.SequenceFlow, activityMap map[m
 	}
 }
 
-// hasExplicitFalseBranchAnchor reports whether a flow is the false-branch anchor
-// of a boolean ExclusiveSplit (origin=top, destination=bottom). This heuristic
-// distinguishes boolean false-branch flows from enum-split flows inside isGuard,
-// because both share a nil CaseValue but differ in their anchor positions.
-// Limitation: a custom @anchor that happens to use (top, bottom) would be
-// misclassified; this is accepted because it is an unlikely user choice.
+// hasExplicitFalseBranchAnchor reports whether a false-branch sequence flow
+// carries anchor metadata that the user explicitly authored. Top→Bottom is
+// the non-default pair produced by `@anchor(false: (from: top, to: bottom))`;
+// any other combination is either a builder default or a different author
+// intention that should not trigger the guard-pattern describer.
+//
+// Used by the `isGuard` paths in traverseFlow / traverseFlowUntilMerge to
+// distinguish a real guard continuation from a branch whose layout should
+// stay visible as an explicit `else` in the described MDL. See
+// TestHasExplicitFalseBranchAnchor for the exhaustive cases.
 func hasExplicitFalseBranchAnchor(flow *microflows.SequenceFlow) bool {
 	if flow == nil {
 		return false