From e107f614abf8ad0221c40bc0f18978a2bb5d372e Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 08:23:55 +0200 Subject: [PATCH 1/2] fix: preserve duplicate implicit output aliases Symptom: when a described microflow contained repeated implicit output variables at the same canvas position, rebuilding the MDL could reuse the first variable name for later activities and make downstream changes or returns point at the wrong object. Root cause: the builder tracked variable declarations by name only and did not distinguish same-position duplicate implicit outputs that need a deterministic local alias. Fix: record implicit output positions, assign numeric aliases such as `Foo_2` for same-position collisions, and resolve downstream variable references, attribute paths, change targets, and retrieve start variables through the active alias. Tests: added a builder regression for duplicate create-object outputs, added a doctype fixture, and documented the aliasing rule in the proposal index and quick reference. --- docs/01-project/MDL_QUICK_REFERENCE.md | 1 + ...OSAL_microflow_variable_alias_collision.md | 62 +++++++++++++++ docs/11-proposals/README.md | 1 + .../variable_alias_collision.test.mdl | 17 +++++ mdl/executor/cmd_microflows_builder.go | 52 ++++++++++++- .../cmd_microflows_builder_actions.go | 38 +++++----- ..._microflows_builder_variable_alias_test.go | 76 +++++++++++++++++++ 7 files changed, 229 insertions(+), 18 deletions(-) create mode 100644 docs/11-proposals/PROPOSAL_microflow_variable_alias_collision.md create mode 100644 mdl-examples/doctype-tests/variable_alias_collision.test.mdl create mode 100644 mdl/executor/cmd_microflows_builder_variable_alias_test.go diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 3733d30e..0a4117d5 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -215,6 +215,7 @@ authentication basic, session | List declaration | `declare $list list of Module.Entity = empty;` | | | Assignment | `set $Var = expression;` | Variable must be declared first | | Create object | `$Var = create Module.Entity (attr = value);` | | +| Duplicate implicit output | `$Var`, `$Var_2`, `$Var_3` | Describe may alias same-position duplicate outputs for round-trip preservation | | Change object | `change $entity (attr = value);` | | | Commit | `commit $entity [with events] [refresh];` | | | Delete | `delete $entity;` | | diff --git a/docs/11-proposals/PROPOSAL_microflow_variable_alias_collision.md b/docs/11-proposals/PROPOSAL_microflow_variable_alias_collision.md new file mode 100644 index 00000000..a5dd14f2 --- /dev/null +++ b/docs/11-proposals/PROPOSAL_microflow_variable_alias_collision.md @@ -0,0 +1,62 @@ +# Microflow Variable Alias Collision + +Status: Draft + +## Summary + +Document the round-trip-only aliasing rule used when `describe` encounters +multiple implicit output variables with the same name at the same microflow +position. + +When the builder detects a duplicate implicit output at the same canvas point, +the later output is renamed with a numeric suffix: + +```mdl +$Item = create SampleModule.Item (); +$Item = create SampleModule.Item (); -- becomes Item_2 internally +``` + +References emitted after the aliased activity are rewritten to the generated +name (`$Item_2`, `$Item_3`, and so on) so the generated Mendix model remains +valid. + +## Motivation + +Some legacy projects contain duplicated or ambiguous implicit output variables +that Studio Pro can keep in the model but MDL cannot represent as repeated +variables in the same scope without ambiguity. Failing the round-trip would +block describe/exec use on those projects. Silently reusing the first variable +would also be wrong because later changes, returns, or association paths would +target the wrong object. + +The aliasing rule preserves a valid model while making the generated MDL +deterministic and reviewable. + +## Semantics + +- Aliasing is position-scoped. A duplicate implicit output only aliases when + the same variable name is produced at the same `@position(x, y)`. +- The first output keeps its original name. +- Later outputs are renamed to the first available suffix: + `Foo_2`, `Foo_3`, and so on. +- Subsequent variable references and attribute paths are rewritten to the active + alias. +- Moving to a different position resets the alias for that variable name. + +This is primarily a describe/round-trip preservation rule. Authored MDL should +prefer explicit unique variable names. + +## Tests And Examples + +- Builder coverage verifies duplicate implicit outputs are emitted as + `SelectedItem` and `SelectedItem_2`, and that downstream references follow + the alias. +- Example script: + `mdl-examples/doctype-tests/variable_alias_collision.test.mdl`. + +## Open Questions + +- Should the builder fail with an explicit disambiguation error instead of + aliasing silently when authored MDL contains this pattern? +- Should `describe` emit a comment near generated aliases so users can + distinguish model-preservation aliases from names authored manually? diff --git a/docs/11-proposals/README.md b/docs/11-proposals/README.md index 3f997476..923b30de 100644 --- a/docs/11-proposals/README.md +++ b/docs/11-proposals/README.md @@ -43,6 +43,7 @@ BSON schema Registry ◄──── multi-version Support |----------|--------|---------|------------| | [MDL Syntax Improvements v1](PROPOSAL_mdl_syntax_improvements.md) | Draft | Go-style assignment, C-style braces, fluent list APIs | — | | [MDL Syntax Improvements v2](PROPOSAL_mdl_syntax_improvements_v2.md) | Proposed | Consolidated v2: unified variable declaration, C-style braces, fluent list ops | Syntax Improvements v1 | +| [Microflow Variable Alias Collision](PROPOSAL_microflow_variable_alias_collision.md) | Draft | Deterministic `Foo_2` aliases for duplicate implicit outputs at the same microflow position | — | | [Page Syntax V2](PROPOSAL_page_syntax_v2.md) | Superseded | Page/widget syntax with `{}` blocks and `->` binding. Superseded by V3 (archived) | — | | [Page Styling Support](page-styling-support.md) | Partial | CSS classes, inline styles, dynamic classes, design properties. Phase 1 (Class/Style) done | — | | [Page Composition](proposal_page_composition.md) | Proposed | Fragment definitions and ALTER PAGE for partial page editing | Page Syntax V2, Page Styling | diff --git a/mdl-examples/doctype-tests/variable_alias_collision.test.mdl b/mdl-examples/doctype-tests/variable_alias_collision.test.mdl new file mode 100644 index 00000000..9b5ac9f4 --- /dev/null +++ b/mdl-examples/doctype-tests/variable_alias_collision.test.mdl @@ -0,0 +1,17 @@ +create microflow SampleAliases.ACT_DuplicateOutputPosition () +returns SampleAliases.Item as $Item +begin + @position(100, 200) + $Item = create SampleAliases.Item ( + Name = 'first'); + + @position(100, 200) + $Item = create SampleAliases.Item ( + Name = 'second'); + + change $Item ( + Name = $Item/Name + ' updated'); + + return $Item; +end; +/ diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index f4d5f4fd..be8f6fb1 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -46,6 +46,8 @@ type flowBuilder struct { // just emitted an activity, so the next flow's OriginConnectionIndex can // be overridden by the user. Cleared after each flow is created. previousStmtAnchor *ast.FlowAnchors + variableAliases map[string]string + outputVarPositions map[string]model.Point } // addError records a validation error during flow building. @@ -133,6 +135,52 @@ func (fb *flowBuilder) registerResultVariableType(varName string, dt microflows. } } +func (fb *flowBuilder) resolveVariableName(varName string) string { + if varName == "" || fb.variableAliases == nil { + return varName + } + if alias := fb.variableAliases[varName]; alias != "" { + return alias + } + return varName +} + +func (fb *flowBuilder) resolveVariablePath(path string) string { + if before, after, ok := strings.Cut(path, "/"); ok { + return fb.resolveVariableName(before) + "/" + after + } + return fb.resolveVariableName(path) +} + +func (fb *flowBuilder) uniqueImplicitOutputVariable(varName string) string { + if varName == "" { + return "" + } + if fb.outputVarPositions == nil { + fb.outputVarPositions = make(map[string]model.Point) + } + position := model.Point{X: fb.posX, Y: fb.posY} + if previous, ok := fb.outputVarPositions[varName]; ok && + previous.X == position.X && previous.Y == position.Y && + fb.isVariableDeclared(varName) { + if fb.variableAliases == nil { + fb.variableAliases = make(map[string]string) + } + for i := 2; ; i++ { + candidate := fmt.Sprintf("%s_%d", varName, i) + if !fb.isVariableDeclared(candidate) { + fb.variableAliases[varName] = candidate + return candidate + } + } + } + if fb.variableAliases != nil { + delete(fb.variableAliases, varName) + } + fb.outputVarPositions[varName] = position + return varName +} + // lookupMicroflowReturnType resolves the return type of a called microflow by // qualified name so downstream activities can infer variable types. func (fb *flowBuilder) lookupMicroflowReturnType(qualifiedName string) microflows.DataType { @@ -231,10 +279,12 @@ func (fb *flowBuilder) resolveAssociationPaths(expr ast.Expression) ast.Expressi } switch e := expr.(type) { + case *ast.VariableExpr: + return &ast.VariableExpr{Name: fb.resolveVariableName(e.Name)} case *ast.AttributePathExpr: resolved := fb.resolvePathSegments(e.Path) return &ast.AttributePathExpr{ - Variable: e.Variable, + Variable: fb.resolveVariableName(e.Variable), Path: resolved, Segments: e.Segments, } diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 9559eea2..5a54ae33 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -54,16 +54,17 @@ func (fb *flowBuilder) addCreateVariableAction(s *ast.DeclareStmt) model.ID { // addChangeVariableAction creates a SET statement as a ChangeVariableAction. func (fb *flowBuilder) addChangeVariableAction(s *ast.MfSetStmt) model.ID { + target := fb.resolveVariablePath(s.Target) // Validate that the variable has been declared - if !fb.isVariableDeclared(s.Target) { + if !fb.isVariableDeclared(target) { fb.addErrorWithExample( - fmt.Sprintf("variable '%s' is not declared", s.Target), - errorExampleDeclareVariable(s.Target)) + fmt.Sprintf("variable '%s' is not declared", target), + errorExampleDeclareVariable(target)) } action := µflows.ChangeVariableAction{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - VariableName: s.Target, + VariableName: target, Value: fb.exprToString(s.Value), } @@ -86,9 +87,10 @@ func (fb *flowBuilder) addChangeVariableAction(s *ast.MfSetStmt) model.ID { // addCreateObjectAction creates a CREATE OBJECT statement. func (fb *flowBuilder) addCreateObjectAction(s *ast.CreateObjectStmt) model.ID { + outputVariable := fb.uniqueImplicitOutputVariable(s.Variable) action := µflows.CreateObjectAction{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - OutputVariable: s.Variable, + OutputVariable: outputVariable, Commit: microflows.CommitTypeNo, } // Set entity reference as qualified name (BY_NAME_REFERENCE) @@ -100,7 +102,7 @@ func (fb *flowBuilder) addCreateObjectAction(s *ast.CreateObjectStmt) model.ID { // Register variable type for CHANGE statements if fb.varTypes != nil && entityQN != "" { - fb.varTypes[s.Variable] = entityQN + fb.varTypes[outputVariable] = entityQN } // Build InitialMembers for each SET assignment @@ -239,9 +241,10 @@ func (fb *flowBuilder) addRollbackAction(s *ast.RollbackStmt) model.ID { // addChangeObjectAction creates a CHANGE statement. func (fb *flowBuilder) addChangeObjectAction(s *ast.ChangeObjectStmt) model.ID { + changeVariable := fb.resolveVariableName(s.Variable) action := µflows.ChangeObjectAction{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - ChangeVariable: s.Variable, + ChangeVariable: changeVariable, Commit: microflows.CommitTypeNo, RefreshInClient: false, } @@ -249,7 +252,7 @@ func (fb *flowBuilder) addChangeObjectAction(s *ast.ChangeObjectStmt) model.ID { // Look up entity type from variable scope entityQN := "" if fb.varTypes != nil { - entityQN = fb.varTypes[s.Variable] + entityQN = fb.varTypes[changeVariable] } // Build MemberChange items for each SET assignment @@ -283,6 +286,7 @@ func (fb *flowBuilder) addChangeObjectAction(s *ast.ChangeObjectStmt) model.ID { // addRetrieveAction creates a RETRIEVE statement. func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { var source microflows.RetrieveSource + outputVariable := fb.uniqueImplicitOutputVariable(s.Variable) if s.StartVariable != "" { // Association retrieve: RETRIEVE $List FROM $Parent/Module.AssocName @@ -296,7 +300,7 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { assocInfo := fb.lookupAssociation(s.Source.Module, s.Source.Name) startVarType := "" if fb.varTypes != nil { - startVarType = fb.varTypes[s.StartVariable] + startVarType = fb.varTypes[fb.resolveVariableName(s.StartVariable)] } if assocInfo != nil && assocInfo.Type == domainmodel.AssociationTypeReference && @@ -306,17 +310,17 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { dbSource := µflows.DatabaseRetrieveSource{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, EntityQualifiedName: assocInfo.parentEntityQN, - XPathConstraint: "[" + assocQN + " = $" + s.StartVariable + "]", + XPathConstraint: "[" + assocQN + " = $" + fb.resolveVariableName(s.StartVariable) + "]", } source = dbSource if fb.varTypes != nil { - fb.varTypes[s.Variable] = "List of " + assocInfo.parentEntityQN + fb.varTypes[outputVariable] = "List of " + assocInfo.parentEntityQN } } else { // Forward traversal or ReferenceSet: use AssociationRetrieveSource source = µflows.AssociationRetrieveSource{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - StartVariable: s.StartVariable, + StartVariable: fb.resolveVariableName(s.StartVariable), AssociationQualifiedName: assocQN, } if fb.varTypes != nil { @@ -326,10 +330,10 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { if startVarType == assocInfo.childEntityQN { otherEntity = assocInfo.parentEntityQN } - fb.varTypes[s.Variable] = otherEntity + fb.varTypes[outputVariable] = otherEntity } else { // ReferenceSet or unknown: returns a list - fb.varTypes[s.Variable] = "List of " + assocQN + fb.varTypes[outputVariable] = "List of " + assocQN } } } @@ -403,17 +407,17 @@ func (fb *flowBuilder) addRetrieveAction(s *ast.RetrieveStmt) model.ID { if fb.varTypes != nil { if s.Limit == "1" { // LIMIT 1 returns a single entity - fb.varTypes[s.Variable] = entityQN + fb.varTypes[outputVariable] = entityQN } else { // No LIMIT or LIMIT > 1 returns a list - fb.varTypes[s.Variable] = "List of " + entityQN + fb.varTypes[outputVariable] = "List of " + entityQN } } } action := µflows.RetrieveAction{ BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())}, - OutputVariable: s.Variable, + OutputVariable: outputVariable, Source: source, } diff --git a/mdl/executor/cmd_microflows_builder_variable_alias_test.go b/mdl/executor/cmd_microflows_builder_variable_alias_test.go new file mode 100644 index 00000000..d52ab00e --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_variable_alias_test.go @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +func TestBuildFlowGraph_DuplicateImplicitOutputAtSamePositionGetsLocalAlias(t *testing.T) { + entityRef := ast.QualifiedName{Module: "Sample", Name: "Item"} + sharedPosition := &ast.ActivityAnnotations{Position: &ast.Position{X: 400, Y: 100}} + body := []ast.MicroflowStatement{ + &ast.CreateObjectStmt{ + Variable: "SelectedItem", + EntityType: entityRef, + Annotations: sharedPosition, + }, + &ast.CreateObjectStmt{ + Variable: "SelectedItem", + EntityType: entityRef, + Annotations: sharedPosition, + }, + &ast.ChangeObjectStmt{ + Variable: "SelectedItem", + Changes: []ast.ChangeItem{{ + Attribute: "Name", + Value: &ast.VariableExpr{Name: "SelectedItem"}, + }}, + }, + &ast.ReturnStmt{Value: &ast.VariableExpr{Name: "SelectedItem"}}, + } + + fb := &flowBuilder{ + posX: 100, posY: 100, baseY: 100, spacing: HorizontalSpacing, + varTypes: map[string]string{}, + declaredVars: map[string]string{}, + } + oc := fb.buildFlowGraph(body, &ast.MicroflowReturnType{Type: ast.DataType{Kind: ast.TypeEntity, EntityRef: &entityRef}}) + + createOutputs := map[string]bool{} + var changeVariable, changeValue string + var returnValue string + for _, obj := range oc.Objects { + switch o := obj.(type) { + case *microflows.ActionActivity: + switch action := o.Action.(type) { + case *microflows.CreateObjectAction: + createOutputs[action.OutputVariable] = true + case *microflows.ChangeObjectAction: + changeVariable = action.ChangeVariable + if len(action.Changes) == 1 { + changeValue = action.Changes[0].Value + } + } + case *microflows.EndEvent: + returnValue = strings.TrimSpace(o.ReturnValue) + } + } + + if !createOutputs["SelectedItem"] || !createOutputs["SelectedItem_2"] { + t.Fatalf("duplicate implicit output should be aliased, got create outputs %#v", createOutputs) + } + if changeVariable != "SelectedItem_2" { + t.Fatalf("change target = %q, want SelectedItem_2", changeVariable) + } + if changeValue != "$SelectedItem_2" { + t.Fatalf("change value = %q, want $SelectedItem_2", changeValue) + } + if returnValue != "$SelectedItem_2" { + t.Fatalf("return value = %q, want $SelectedItem_2", returnValue) + } +} From 4266168b0727e1cf4020ef2f5862bc694d5d9fc3 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 16:49:44 +0200 Subject: [PATCH 2/2] test: add bug-test reproducer for variable alias collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an MDL script under mdl-examples/bug-tests/ exercising two implicit `$Item` outputs at the same `@position(100, 200)`. The describe → exec → describe fixpoint plus `mx check` (0 errors) confirm the alias `$Item_2` is generated and downstream references resolve through the active alias. Co-Authored-By: Claude Opus 4.7 --- .../bug-tests/variable-alias-collision.mdl | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 mdl-examples/bug-tests/variable-alias-collision.mdl diff --git a/mdl-examples/bug-tests/variable-alias-collision.mdl b/mdl-examples/bug-tests/variable-alias-collision.mdl new file mode 100644 index 00000000..36073117 --- /dev/null +++ b/mdl-examples/bug-tests/variable-alias-collision.mdl @@ -0,0 +1,52 @@ +-- ============================================================================ +-- Variable alias collision: duplicate implicit output at same position +-- ============================================================================ +-- +-- Symptom (before fix): +-- When a Studio Pro project contained two activities producing the same +-- implicit output variable at the same canvas position (e.g. two +-- `Create Object` actions both writing `$Item` at `@position(100, 200)`), +-- describing it produced MDL with two `$Item =` assignments and the +-- builder collapsed both into one shared variable. Downstream `change` +-- and `return` statements then operated on the wrong object, the +-- resulting BSON was non-equivalent to the original, and roundtrip +-- verification failed. +-- +-- After fix: +-- The flow builder now tracks implicit output positions. When it sees +-- the same variable name produced again at the same `@position(x, y)`, +-- it assigns a deterministic suffix (`Foo_2`, `Foo_3`, …) and rewrites +-- subsequent variable references, attribute paths, change targets, and +-- retrieve start variables through the active alias. Roundtrip +-- describe→exec→describe is a fixpoint and the resulting model is valid. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/variable-alias-collision.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow SampleAliases.ACT_DuplicateOutputPosition" +-- The describe output must contain `$Item_2` for the second create and +-- for the downstream `change`/`return`. Studio Pro `mx check` must +-- report 0 errors. +-- ============================================================================ + +create module SampleAliases; + +create entity SampleAliases.Item ( + Name : string(100) +); +/ + +create microflow SampleAliases.ACT_DuplicateOutputPosition () +returns SampleAliases.Item as $Item +begin + @position(100, 200) + $Item = create SampleAliases.Item (Name = 'first'); + + @position(100, 200) + $Item = create SampleAliases.Item (Name = 'second'); + + change $Item ( + Name = $Item/Name + ' updated'); + + return $Item; +end; +/