From 71efe7ca6877d970b331fb8a3b47562aa6a92f1b Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Sun, 26 Apr 2026 11:47:40 +0200 Subject: [PATCH 1/9] fix: preserve multiline source expression whitespace Symptom: describe/exec round-trips collapsed multiline declaration and log-template expressions, producing cosmetic drift and losing source layout that users intentionally wrote. Root cause: declare initial values and log message/node expressions were rebuilt from parsed expression nodes instead of retaining source text, and template parameter gaps discarded newline-only whitespace between parameters. Fix: add SourceExpr as a small expression wrapper for retained source text, use it for declare/log/while expressions that need source preservation, serialize it back through the executor, and append newline-only trailing whitespace between log template parameters to the captured source expression. Tests: added parser coverage for multiline declare values and log template parameters with trailing blank lines; validated make build, make test, and make lint-go. --- mdl/ast/ast_expression.go | 9 +++ mdl/executor/cmd_microflows_builder.go | 8 +++ mdl/executor/cmd_microflows_helpers.go | 10 +++ mdl/visitor/visitor_microflow_actions.go | 76 +++++++++++++++++++-- mdl/visitor/visitor_microflow_statements.go | 38 ++++++++++- mdl/visitor/visitor_test.go | 48 +++++++++++++ 6 files changed, 182 insertions(+), 7 deletions(-) diff --git a/mdl/ast/ast_expression.go b/mdl/ast/ast_expression.go index 72115cea..7a3acb24 100644 --- a/mdl/ast/ast_expression.go +++ b/mdl/ast/ast_expression.go @@ -125,6 +125,15 @@ type IfThenElseExpr struct { func (e *IfThenElseExpr) isExpression() {} +// SourceExpr preserves original expression source text while keeping the parsed +// expression tree available for callers that need semantic inspection. +type SourceExpr struct { + Expression Expression + Source string +} + +func (e *SourceExpr) isExpression() {} + // ============================================================================ // XPath-Specific Expression Types // ============================================================================ diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 99b5d724..8091e566 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -329,6 +329,14 @@ func (fb *flowBuilder) resolveAssociationPaths(expr ast.Expression) ast.Expressi ThenExpr: fb.resolveAssociationPaths(e.ThenExpr), ElseExpr: fb.resolveAssociationPaths(e.ElseExpr), } + case *ast.SourceExpr: + if e.Source != "" { + return e + } + return &ast.SourceExpr{ + Expression: fb.resolveAssociationPaths(e.Expression), + Source: e.Source, + } default: return expr } diff --git a/mdl/executor/cmd_microflows_helpers.go b/mdl/executor/cmd_microflows_helpers.go index 18ba523c..f7bf4d55 100644 --- a/mdl/executor/cmd_microflows_helpers.go +++ b/mdl/executor/cmd_microflows_helpers.go @@ -327,6 +327,11 @@ func expressionToString(expr ast.Expression) string { thenStr := expressionToString(e.ThenExpr) elseStr := expressionToString(e.ElseExpr) return "if " + cond + " then " + thenStr + " else " + elseStr + case *ast.SourceExpr: + if e.Source != "" { + return e.Source + } + return expressionToString(e.Expression) default: return "" } @@ -379,6 +384,11 @@ func expressionToXPath(expr ast.Expression) string { return expressionToString(expr) case *ast.QualifiedNameExpr: return qualifiedNameToXPath(e) + case *ast.SourceExpr: + if e.Source != "" { + return e.Source + } + return expressionToXPath(e.Expression) default: // For all other expression types, the standard serialization is correct return expressionToString(expr) diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index bd8460fe..799900ce 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -51,10 +51,10 @@ func buildLogStatement(ctx parser.ILogStatementContext) *ast.LogStmt { // LOG always has a message expression; when NODE is present the first // expression is the node and the second is the message. if logCtx.NODE() != nil && len(exprs) > 1 { - stmt.Node = buildExpression(exprs[0]) - stmt.Message = buildExpression(exprs[1]) + stmt.Node = buildSourceExpression(exprs[0]) + stmt.Message = buildSourceExpression(exprs[1]) } else if len(exprs) > 0 { - stmt.Message = buildExpression(exprs[0]) + stmt.Message = buildSourceExpression(exprs[0]) } // Parse template parameters: WITH ({1} = expr, {2} = expr, ...) @@ -79,7 +79,8 @@ func buildTemplateParams(ctx parser.ITemplateParamsContext) []ast.TemplateParam var result []ast.TemplateParam // Handle WITH ({1} = expr, {2} = expr, ...) syntax - for _, param := range paramsCtx.AllTemplateParam() { + allParams := paramsCtx.AllTemplateParam() + for i, param := range allParams { paramCtx := param.(*parser.TemplateParamContext) indexStr := paramCtx.NUMBER_LITERAL().GetText() index, _ := strconv.Atoi(indexStr) @@ -89,7 +90,8 @@ func buildTemplateParams(ctx parser.ITemplateParamsContext) []ast.TemplateParam // Parse the expression and check for data source attribute reference if exprCtx := paramCtx.Expression(); exprCtx != nil { - expr := buildExpression(exprCtx) + expr := buildSourceExpression(exprCtx) + expr = appendTemplateParamTrailingWhitespace(paramsCtx, allParams, i, exprCtx, expr) tp.Value = expr // Check if this is a $Widget.Attr pattern (AttributePathExpr with Path) @@ -117,6 +119,70 @@ func buildTemplateParams(ctx parser.ITemplateParamsContext) []ast.TemplateParam return result } +func appendTemplateParamTrailingWhitespace( + paramsCtx *parser.TemplateParamsContext, + allParams []parser.ITemplateParamContext, + index int, + exprCtx parser.IExpressionContext, + expr ast.Expression, +) ast.Expression { + trailing := templateParamTrailingWhitespace(paramsCtx, allParams, index, exprCtx) + if trailing == "" || !strings.ContainsAny(trailing, "\r\n") { + return expr + } + + source := strings.TrimSpace(extractOriginalText(exprCtx.(antlr.ParserRuleContext))) + innerExpr := expr + if sourceExpr, ok := expr.(*ast.SourceExpr); ok { + source = sourceExpr.Source + innerExpr = sourceExpr.Expression + } + return &ast.SourceExpr{Expression: innerExpr, Source: source + trailing} +} + +func templateParamTrailingWhitespace( + paramsCtx *parser.TemplateParamsContext, + allParams []parser.ITemplateParamContext, + index int, + exprCtx parser.IExpressionContext, +) string { + exprRule, ok := exprCtx.(antlr.ParserRuleContext) + if !ok || exprRule.GetStop() == nil { + return "" + } + input := exprRule.GetStop().GetInputStream() + if input == nil { + return "" + } + + start := exprRule.GetStop().GetStop() + 1 + end := -1 + delimiter := byte(')') + if index+1 < len(allParams) { + nextParam := allParams[index+1].(antlr.ParserRuleContext) + end = nextParam.GetStart().GetStart() - 1 + delimiter = ',' + } else if paramsCtx.GetStop() != nil { + end = paramsCtx.GetStop().GetStart() - 1 + } + if start < 0 || end < start { + return "" + } + + gap := input.GetText(start, end) + if delimiter == ',' { + comma := strings.IndexByte(gap, ',') + if comma == -1 { + return "" + } + gap = gap[:comma] + } + if strings.TrimSpace(gap) != "" { + return "" + } + return gap +} + // buildCallMicroflowStatement converts CALL MICROFLOW statement context to CallMicroflowStmt. // Grammar: (VARIABLE EQUALS)? CALL MICROFLOW qualifiedName LPAREN callArgumentList? RPAREN func buildCallMicroflowStatement(ctx parser.ICallMicroflowStatementContext) *ast.CallMicroflowStmt { diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 6ae3a0c8..46d57a77 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -5,6 +5,7 @@ package visitor import ( "strings" + "github.com/antlr4-go/antlr/v4" "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/grammar/parser" ) @@ -530,7 +531,7 @@ func buildDeclareStatement(ctx parser.IDeclareStatementContext) *ast.DeclareStmt // Get optional initial value if expr := declCtx.Expression(); expr != nil { - stmt.InitialValue = buildExpression(expr) + stmt.InitialValue = buildSourceExpression(expr) } return stmt @@ -1104,7 +1105,7 @@ func buildWhileStatement(ctx parser.IWhileStatementContext) *ast.WhileStmt { // Get condition expression if expr := wsCtx.Expression(); expr != nil { - stmt.Condition = buildExpression(expr) + stmt.Condition = buildSourceExpression(expr) } // Get body @@ -1115,6 +1116,39 @@ func buildWhileStatement(ctx parser.IWhileStatementContext) *ast.WhileStmt { return stmt } +func buildSourceExpression(ctx parser.IExpressionContext) ast.Expression { + if ctx == nil { + return nil + } + expr := buildExpression(ctx) + if prc, ok := ctx.(antlr.ParserRuleContext); ok { + if source := strings.TrimSpace(extractOriginalText(prc)); source != "" { + if shouldPreserveExpressionSource(source) { + return &ast.SourceExpr{Expression: expr, Source: source} + } + } + } + return expr +} + +func shouldPreserveExpressionSource(source string) bool { + if strings.ContainsAny(source, "\r\n") { + return true + } + for i := 0; i < len(source); i++ { + switch source[i] { + case '=', '!', '<', '>', '+', '-', '*', ':': + if i > 0 && source[i-1] != ' ' && source[i-1] != '\t' { + return true + } + if i+1 < len(source) && source[i+1] != ' ' && source[i+1] != '\t' && source[i+1] != '=' { + return true + } + } + } + return false +} + // buildReturnStatement converts RETURN statement context to ReturnStmt. func buildReturnStatement(ctx parser.IReturnStatementContext) *ast.ReturnStmt { if ctx == nil { diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index bfe9e1f9..5742f005 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1685,3 +1685,51 @@ END;` t.Fatalf("free annotation = %q, want empty", logStmt.Annotations.FreeAnnotation) } } + +func TestDeclareAndLogTemplatePreserveMultilineSourceWhitespace(t *testing.T) { + input := `CREATE MICROFLOW Synthetic.Check () +RETURNS Boolean AS $Success +BEGIN + DECLARE $Endpoint String = @Synthetic.Endpoint ++ '/items?page=' + toString($Page) ++ '&token=' + (if $Token!=empty then $Token/Value +else +''); + LOG INFO NODE 'SyntheticLog' 'Processed {1} items for {2}' WITH ({1} = toString($Count) + +, {2} = $Endpoint); + RETURN true; +END;` + + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + + stmt := prog.Statements[0].(*ast.CreateMicroflowStmt) + decl, ok := stmt.Body[0].(*ast.DeclareStmt) + if !ok { + t.Fatalf("Expected DeclareStmt, got %T", stmt.Body[0]) + } + logStmt, ok := stmt.Body[1].(*ast.LogStmt) + if !ok { + t.Fatalf("Expected LogStmt, got %T", stmt.Body[1]) + } + + declSource, ok := decl.InitialValue.(*ast.SourceExpr) + if !ok { + t.Fatalf("Expected SourceExpr declare value, got %T", decl.InitialValue) + } + wantDecl := "@Synthetic.Endpoint\n+ '/items?page=' + toString($Page)\n+ '&token=' + (if $Token!=empty then $Token/Value\nelse\n'')" + if declSource.Source != wantDecl { + t.Fatalf("declare source = %q, want %q", declSource.Source, wantDecl) + } + + firstParam, ok := logStmt.Template[0].Value.(*ast.SourceExpr) + if !ok { + t.Fatalf("Expected SourceExpr first log template param, got %T", logStmt.Template[0].Value) + } + if firstParam.Source != "toString($Count)\n\n" { + t.Fatalf("template param source = %q, want trailing blank line", firstParam.Source) + } +} From f7a4600b7d81a353724657a89639b197fb6c923b Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 00:01:39 +0200 Subject: [PATCH 2/9] test: add bug-test reproducer for multiline source expression preservation 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 both multiline DECLARE initial values and inter-parameter blank lines in LOG template parameters. The describe → exec → describe fixpoint confirms SourceExpr-wrapped expressions retain their original whitespace. Co-Authored-By: Claude Opus 4.7 --- ...multiline-source-expression-whitespace.mdl | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl diff --git a/mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl b/mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl new file mode 100644 index 00000000..25ef890d --- /dev/null +++ b/mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl @@ -0,0 +1,61 @@ +-- ============================================================================ +-- Bug #322: Multiline source expression whitespace lost on roundtrip +-- ============================================================================ +-- +-- Symptom (before fix): +-- The visitor rebuilt expression strings from parsed expression nodes, +-- so original line breaks and whitespace inside DECLARE initial values +-- and LOG template parameters were normalized away. A multiline +-- `declare $X string = ...` statement that authors had carefully +-- formatted across several lines came back as a single very long line +-- after describe → exec → describe. Inter-parameter blank lines in +-- `LOG ... WITH (...)` were similarly collapsed, making real-world +-- microflows non-fixpoint and producing noisy `mxcli diff-local` output. +-- +-- After fix: +-- Added a `SourceExpr` AST node that wraps an expression with its +-- original source text. The visitor uses `buildSourceExpression` for +-- source-sensitive declare/log/while expressions, and the executor +-- serializes `SourceExpr` through to MDL output so the original +-- whitespace and line breaks survive every roundtrip. +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow BugTest322.MF_MultilineDeclare" +-- mxcli -p app.mpr -c "describe microflow BugTest322.MF_MultilineLogTemplate" +-- The describe outputs must keep the multiline shape and must be +-- fixpoints under describe → exec → describe. +-- ============================================================================ + +create module BugTest322; + +-- DECLARE initial value spread across several lines with leading `+`. The +-- describer must preserve the line breaks rather than collapse them onto +-- one line. +create microflow BugTest322.MF_MultilineDeclare ( + $Page: integer, + $Token: string +) +returns string as $Endpoint +begin + declare $Endpoint string = '/api/v1' ++ '/items?page=' + toString($Page) ++ '&token=' + $Token; + + return $Endpoint; +end; +/ + +-- LOG template parameters separated by a blank line. The newline-only +-- whitespace between `{1} = toString($Count)` and the next parameter must +-- survive the roundtrip. +create microflow BugTest322.MF_MultilineLogTemplate ( + $Count: integer, + $Endpoint: string +) +begin + log info node 'BugTest322' 'Processed {1} items for {2}' with ({1} = toString($Count) + +, {2} = $Endpoint); +end; +/ From 3507a7f12c06deb780c0cee2789c908a2bdf0e43 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 03:02:43 +0200 Subject: [PATCH 3/9] fix: ignore string literal punctuation in source preservation scan Symptom: a plain string literal containing punctuation such as `!` or `:` could be wrapped as a SourceExpr, causing downstream log-template formatting to emit extra quoting. Root cause: shouldPreserveExpressionSource scanned every operator-like byte without tracking whether it was inside a single-quoted string literal. Fix: skip compact-operator detection while inside single-quoted literals, including doubled single-quote escapes. Tests: add visitor coverage for punctuation and escaped quotes inside string literals while keeping compact operators outside literals preserved. --- mdl/visitor/visitor_microflow_statements.go | 12 ++++++++++++ mdl/visitor/visitor_test.go | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 46d57a77..9d988ae5 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -1135,7 +1135,19 @@ func shouldPreserveExpressionSource(source string) bool { if strings.ContainsAny(source, "\r\n") { return true } + inString := false for i := 0; i < len(source); i++ { + if source[i] == '\'' { + if inString && i+1 < len(source) && source[i+1] == '\'' { + i++ + continue + } + inString = !inString + continue + } + if inString { + continue + } switch source[i] { case '=', '!', '<', '>', '+', '-', '*', ':': if i > 0 && source[i-1] != ' ' && source[i-1] != '\t' { diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 5742f005..84cf5c03 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1733,3 +1733,15 @@ END;` t.Fatalf("template param source = %q, want trailing blank line", firstParam.Source) } } + +func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testing.T) { + if shouldPreserveExpressionSource("'Processed {1} items!'") { + t.Fatal("plain string literal punctuation should not force SourceExpr preservation") + } + if shouldPreserveExpressionSource("'Owner''s item: {1}'") { + t.Fatal("escaped quotes and colon inside a string literal should not force SourceExpr preservation") + } + if !shouldPreserveExpressionSource("$Token!=empty") { + t.Fatal("compact operators outside string literals should preserve source") + } +} From e7414a10c4c88eec70a7eda1d9ae4ba519eb6dd1 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 19:47:40 +0200 Subject: [PATCH 4/9] fix: preserve source whitespace across expression slots Symptom: microflow roundtrips still normalized expression source text in common action slots such as call arguments, member assignments, show-message arguments, retrieve WHERE expressions, IF conditions, and RETURN values. Root cause: the visitor only wrapped a small subset of expressions in SourceExpr. Other expression-bearing fields still used buildExpression directly, so the writer re-rendered them with canonical spacing and collapsed line breaks. Fix: route expression slots through buildSourceExpression, preserve newline-only trailing whitespace between list elements, and unwrap SourceExpr when detecting rule split calls so source preservation does not hide the structured expression. Tests: added a visitor regression covering create/change assignments, Java action arguments, IF conditions, show-message arguments, and RETURN values; ran make build, make lint-go, and make test. --- mdl/executor/cmd_microflows_builder.go | 2 + mdl/visitor/visitor_microflow_actions.go | 135 ++++++++++++++++---- mdl/visitor/visitor_microflow_statements.go | 20 ++- mdl/visitor/visitor_test.go | 82 ++++++++++++ 4 files changed, 206 insertions(+), 33 deletions(-) diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index 8091e566..c2639656 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -451,6 +451,8 @@ func unwrapParenCall(expr ast.Expression) *ast.FunctionCallExpr { return e case *ast.ParenExpr: expr = e.Inner + case *ast.SourceExpr: + expr = e.Expression default: return nil } diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index 799900ce..36b7374c 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -183,6 +183,73 @@ func templateParamTrailingWhitespace( return gap } +func nextParserRuleContext[T any](items []T, index int) antlr.ParserRuleContext { + if index+1 >= len(items) { + return nil + } + next, _ := any(items[index+1]).(antlr.ParserRuleContext) + return next +} + +func appendExpressionListTrailingWhitespace( + parent antlr.ParserRuleContext, + next antlr.ParserRuleContext, + exprCtx parser.IExpressionContext, + expr ast.Expression, +) ast.Expression { + trailing := expressionListTrailingWhitespace(parent, next, exprCtx) + if trailing == "" || !strings.ContainsAny(trailing, "\r\n") { + return expr + } + + source := strings.TrimSpace(extractOriginalText(exprCtx.(antlr.ParserRuleContext))) + innerExpr := expr + if sourceExpr, ok := expr.(*ast.SourceExpr); ok { + source = sourceExpr.Source + innerExpr = sourceExpr.Expression + } + return &ast.SourceExpr{Expression: innerExpr, Source: source + trailing} +} + +func expressionListTrailingWhitespace( + parent antlr.ParserRuleContext, + next antlr.ParserRuleContext, + exprCtx parser.IExpressionContext, +) string { + exprRule, ok := exprCtx.(antlr.ParserRuleContext) + if !ok || exprRule.GetStop() == nil { + return "" + } + input := exprRule.GetStop().GetInputStream() + if input == nil { + return "" + } + + start := exprRule.GetStop().GetStop() + 1 + end := -1 + if next != nil { + end = next.GetStart().GetStart() - 1 + } else if parent != nil && parent.GetStop() != nil { + end = parent.GetStop().GetStart() - 1 + } + if start < 0 || end < start { + return "" + } + + gap := input.GetText(start, end) + if next != nil { + comma := strings.IndexByte(gap, ',') + if comma == -1 { + return "" + } + gap = gap[:comma] + } + if strings.TrimSpace(gap) != "" { + return "" + } + return gap +} + // buildCallMicroflowStatement converts CALL MICROFLOW statement context to CallMicroflowStmt. // Grammar: (VARIABLE EQUALS)? CALL MICROFLOW qualifiedName LPAREN callArgumentList? RPAREN func buildCallMicroflowStatement(ctx parser.ICallMicroflowStatementContext) *ast.CallMicroflowStmt { @@ -423,7 +490,8 @@ func buildCallArgumentList(ctx parser.ICallArgumentListContext) []ast.CallArgume listCtx := ctx.(*parser.CallArgumentListContext) var args []ast.CallArgument - for _, argCtx := range listCtx.AllCallArgument() { + allArgs := listCtx.AllCallArgument() + for i, argCtx := range allArgs { arg := argCtx.(*parser.CallArgumentContext) ca := ast.CallArgument{} @@ -434,7 +502,8 @@ func buildCallArgumentList(ctx parser.ICallArgumentListContext) []ast.CallArgume ca.Name = parameterNameText(pn) } if expr := arg.Expression(); expr != nil { - ca.Value = buildExpression(expr) + value := buildSourceExpression(expr) + ca.Value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allArgs, i), expr, value) } args = append(args, ca) @@ -451,7 +520,8 @@ func buildMemberAssignmentList(ctx parser.IMemberAssignmentListContext) []ast.Ch listCtx := ctx.(*parser.MemberAssignmentListContext) var items []ast.ChangeItem - for _, assignCtx := range listCtx.AllMemberAssignment() { + allAssignments := listCtx.AllMemberAssignment() + for i, assignCtx := range allAssignments { assign := assignCtx.(*parser.MemberAssignmentContext) ci := ast.ChangeItem{} @@ -460,7 +530,8 @@ func buildMemberAssignmentList(ctx parser.IMemberAssignmentListContext) []ast.Ch ci.Attribute = memberAttributeNameText(name) } if expr := assign.Expression(); expr != nil { - ci.Value = buildExpression(expr) + value := buildSourceExpression(expr) + ci.Value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allAssignments, i), expr, value) } items = append(items, ci) @@ -477,7 +548,8 @@ func buildChangeList(ctx parser.IChangeListContext) []ast.ChangeItem { listCtx := ctx.(*parser.ChangeListContext) var items []ast.ChangeItem - for _, itemCtx := range listCtx.AllChangeItem() { + allItems := listCtx.AllChangeItem() + for i, itemCtx := range allItems { item := itemCtx.(*parser.ChangeItemContext) ci := ast.ChangeItem{} @@ -485,7 +557,8 @@ func buildChangeList(ctx parser.IChangeListContext) []ast.ChangeItem { ci.Attribute = id.GetText() } if expr := item.Expression(); expr != nil { - ci.Value = buildExpression(expr) + value := buildSourceExpression(expr) + ci.Value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allItems, i), expr, value) } items = append(items, ci) @@ -537,7 +610,7 @@ func buildListOperationStatement(ctx parser.IListOperationStatementContext) *ast stmt.InputVariable = strings.TrimPrefix(vars[0].GetText(), "$") } if expr := op.Expression(0); expr != nil { - stmt.Condition = buildExpression(expr) + stmt.Condition = buildSourceExpression(expr) } } else if op.FILTER() != nil { stmt.Operation = ast.ListOpFilter @@ -545,7 +618,7 @@ func buildListOperationStatement(ctx parser.IListOperationStatementContext) *ast stmt.InputVariable = strings.TrimPrefix(vars[0].GetText(), "$") } if expr := op.Expression(0); expr != nil { - stmt.Condition = buildExpression(expr) + stmt.Condition = buildSourceExpression(expr) } } else if op.SORT() != nil { stmt.Operation = ast.ListOpSort @@ -602,10 +675,10 @@ func buildListOperationStatement(ctx parser.IListOperationStatementContext) *ast } exprs := op.AllExpression() if len(exprs) >= 1 { - stmt.OffsetExpr = buildExpression(exprs[0]) + stmt.OffsetExpr = buildSourceExpression(exprs[0]) } if len(exprs) >= 2 { - stmt.LimitExpr = buildExpression(exprs[1]) + stmt.LimitExpr = buildSourceExpression(exprs[1]) } } } @@ -669,7 +742,7 @@ func buildAggregateListStatement(ctx parser.IAggregateListStatementContext) *ast stmt.Operation = ast.AggregateSum if exprCtx := op.Expression(); exprCtx != nil { stmt.IsExpression = true - stmt.Expression = buildExpression(exprCtx) + stmt.Expression = buildSourceExpression(exprCtx) if v := op.VARIABLE(); v != nil { stmt.InputVariable = strings.TrimPrefix(v.GetText(), "$") } @@ -680,7 +753,7 @@ func buildAggregateListStatement(ctx parser.IAggregateListStatementContext) *ast stmt.Operation = ast.AggregateAverage if exprCtx := op.Expression(); exprCtx != nil { stmt.IsExpression = true - stmt.Expression = buildExpression(exprCtx) + stmt.Expression = buildSourceExpression(exprCtx) if v := op.VARIABLE(); v != nil { stmt.InputVariable = strings.TrimPrefix(v.GetText(), "$") } @@ -691,7 +764,7 @@ func buildAggregateListStatement(ctx parser.IAggregateListStatementContext) *ast stmt.Operation = ast.AggregateMinimum if exprCtx := op.Expression(); exprCtx != nil { stmt.IsExpression = true - stmt.Expression = buildExpression(exprCtx) + stmt.Expression = buildSourceExpression(exprCtx) if v := op.VARIABLE(); v != nil { stmt.InputVariable = strings.TrimPrefix(v.GetText(), "$") } @@ -702,7 +775,7 @@ func buildAggregateListStatement(ctx parser.IAggregateListStatementContext) *ast stmt.Operation = ast.AggregateMaximum if exprCtx := op.Expression(); exprCtx != nil { stmt.IsExpression = true - stmt.Expression = buildExpression(exprCtx) + stmt.Expression = buildSourceExpression(exprCtx) if v := op.VARIABLE(); v != nil { stmt.InputVariable = strings.TrimPrefix(v.GetText(), "$") } @@ -893,7 +966,7 @@ func buildShowPageArgList(ctx parser.IShowPageArgListContext) []ast.ShowPageArg // Widget-style: Param: $value spa.ParamName = identifierOrKeywordText(iok) if expr := arg.Expression(); expr != nil { - spa.Value = buildExpression(expr) + spa.Value = buildSourceExpression(expr) } } else { // Canonical: $Param = $value @@ -904,7 +977,7 @@ func buildShowPageArgList(ctx parser.IShowPageArgListContext) []ast.ShowPageArg if len(vars) >= 2 { spa.Value = &ast.VariableExpr{Name: strings.TrimPrefix(vars[1].GetText(), "$")} } else if expr := arg.Expression(); expr != nil { - spa.Value = buildExpression(expr) + spa.Value = buildSourceExpression(expr) } } @@ -927,7 +1000,7 @@ func buildShowMessageStatement(ctx parser.IShowMessageStatementContext) *ast.Sho } if expr := smCtx.Expression(); expr != nil { - stmt.Message = buildExpression(expr) + stmt.Message = buildSourceExpression(expr) } if id := smCtx.IdentifierOrKeyword(); id != nil { @@ -937,8 +1010,11 @@ func buildShowMessageStatement(ctx parser.IShowMessageStatementContext) *ast.Sho // Build template arguments (optional) if exprList := smCtx.ExpressionList(); exprList != nil { listCtx := exprList.(*parser.ExpressionListContext) - for _, expr := range listCtx.AllExpression() { - stmt.TemplateArgs = append(stmt.TemplateArgs, buildExpression(expr)) + allExprs := listCtx.AllExpression() + for i, expr := range allExprs { + value := buildSourceExpression(expr) + value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allExprs, i), expr, value) + stmt.TemplateArgs = append(stmt.TemplateArgs, value) } } @@ -979,14 +1055,17 @@ func buildValidationFeedbackStatement(ctx parser.IValidationFeedbackStatementCon // Build message expression if msgExpr := vfCtx.Expression(); msgExpr != nil { - stmt.Message = buildExpression(msgExpr) + stmt.Message = buildSourceExpression(msgExpr) } // Build template arguments (optional) if exprList := vfCtx.ExpressionList(); exprList != nil { listCtx := exprList.(*parser.ExpressionListContext) - for _, expr := range listCtx.AllExpression() { - stmt.TemplateArgs = append(stmt.TemplateArgs, buildExpression(expr)) + allExprs := listCtx.AllExpression() + for i, expr := range allExprs { + value := buildSourceExpression(expr) + value = appendExpressionListTrailingWhitespace(listCtx, nextParserRuleContext(allExprs, i), expr, value) + stmt.TemplateArgs = append(stmt.TemplateArgs, value) } } @@ -1081,7 +1160,7 @@ func buildRestCallStatement(ctx parser.IRestCallStatementContext) *ast.RestCallS Value: unquoteString(strLit.GetText()), } } else if expr := urlC.Expression(); expr != nil { - stmt.URL = buildExpression(expr) + stmt.URL = buildSourceExpression(expr) } } @@ -1104,7 +1183,7 @@ func buildRestCallStatement(ctx parser.IRestCallStatementContext) *ast.RestCallS header.Name = unquoteString(strLit.GetText()) } if expr := hdrCtx.Expression(); expr != nil { - header.Value = buildExpression(expr) + header.Value = buildSourceExpression(expr) } stmt.Headers = append(stmt.Headers, header) } @@ -1115,8 +1194,8 @@ func buildRestCallStatement(ctx parser.IRestCallStatementContext) *ast.RestCallS exprs := authCtx.AllExpression() if len(exprs) >= 2 { stmt.Auth = &ast.RestAuth{ - Username: buildExpression(exprs[0]), - Password: buildExpression(exprs[1]), + Username: buildSourceExpression(exprs[0]), + Password: buildSourceExpression(exprs[1]), } } } @@ -1144,7 +1223,7 @@ func buildRestCallStatement(ctx parser.IRestCallStatementContext) *ast.RestCallS Value: unquoteString(strLit.GetText()), } } else if expr := bodyCtx.Expression(); expr != nil { - body.Template = buildExpression(expr) + body.Template = buildSourceExpression(expr) } // Get template parameters if tplParams := bodyCtx.TemplateParams(); tplParams != nil { @@ -1159,7 +1238,7 @@ func buildRestCallStatement(ctx parser.IRestCallStatementContext) *ast.RestCallS if timeoutClause := restCtx.RestCallTimeoutClause(); timeoutClause != nil { timeoutCtx := timeoutClause.(*parser.RestCallTimeoutClauseContext) if expr := timeoutCtx.Expression(); expr != nil { - stmt.Timeout = buildExpression(expr) + stmt.Timeout = buildSourceExpression(expr) } } diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 9d988ae5..21becaa5 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -554,9 +554,12 @@ func buildSetStatement(ctx parser.ISetStatementContext) ast.MicroflowStatement { targetVar = ap.GetText() } - // Get value expression + // Get value expression. Keep the structured expression for list/aggregate + // detection, then preserve source text for plain SET statements. var valueExpr ast.Expression + var valueExprCtx parser.IExpressionContext if expr := setCtx.Expression(); expr != nil { + valueExprCtx = expr valueExpr = buildExpression(expr) } @@ -691,6 +694,10 @@ func buildSetStatement(ctx parser.ISetStatementContext) ast.MicroflowStatement { } } + if valueExprCtx != nil { + valueExpr = buildSourceExpression(valueExprCtx) + } + // Default: regular SET statement return &ast.MfSetStmt{ Target: targetVar, @@ -983,7 +990,7 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS stmt.Where = result } } else if expr := retrCtx.Expression(0); expr != nil { - stmt.Where = buildExpression(expr) + stmt.Where = buildSourceExpression(expr) } } @@ -1052,7 +1059,7 @@ func buildIfStatement(ctx parser.IIfStatementContext) *ast.IfStmt { // Get all expressions (condition for IF and ELSIFs) exprs := ifCtx.AllExpression() if len(exprs) > 0 { - stmt.Condition = buildExpression(exprs[0]) + stmt.Condition = buildSourceExpression(exprs[0]) } // Get all microflow bodies (THEN, ELSIF THENs, ELSE) @@ -1149,7 +1156,7 @@ func shouldPreserveExpressionSource(source string) bool { continue } switch source[i] { - case '=', '!', '<', '>', '+', '-', '*', ':': + case '=', '!', '<', '>', '+', '-', '*', ':', ',': if i > 0 && source[i-1] != ' ' && source[i-1] != '\t' { return true } @@ -1158,6 +1165,9 @@ func shouldPreserveExpressionSource(source string) bool { } } } + if strings.Contains(strings.ToLower(source), "not(") { + return true + } return false } @@ -1172,7 +1182,7 @@ func buildReturnStatement(ctx parser.IReturnStatementContext) *ast.ReturnStmt { // Get optional return value if expr := retCtx.Expression(); expr != nil { - stmt.Value = buildExpression(expr) + stmt.Value = buildSourceExpression(expr) } return stmt diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 84cf5c03..8e9175d5 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1734,6 +1734,82 @@ END;` } } +func TestActionExpressionSlotsPreserveSourceWhitespace(t *testing.T) { + input := `CREATE MICROFLOW Synthetic.PreserveExpressionSlots ( + $Input: String, + $Count: Integer, + $Flag: Boolean, + $OtherFlag: Boolean +) +RETURNS Boolean AS $Result +BEGIN + $Created = CREATE Synthetic.Entity (Name = if $Count=0 then 'zero' +else 'many' +, Description = 'sample'); + CHANGE $Created (Name = 'updated' +, Description = 'sample'); + $Loaded = CALL JAVA ACTION Synthetic.LoadValue(envVarName = 'SYNTHETIC_VALUE' +, defaultValue = @Synthetic.DefaultValue); + IF isMatch( $Input, '[0-9]+') THEN + SHOW MESSAGE 'Value {1}' TYPE Information OBJECTS [if $Flag then 'enabled' +else 'disabled' +, $Input]; + END IF; + RETURN $Flag +and +$OtherFlag; +END;` + + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + + createStmt := mf.Body[0].(*ast.CreateObjectStmt) + if source, ok := createStmt.Changes[0].Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected create assignment SourceExpr, got %T", createStmt.Changes[0].Value) + } else if !strings.Contains(source.Source, "\nelse 'many'\n") { + t.Fatalf("create assignment source lost line breaks: %q", source.Source) + } + + changeStmt := mf.Body[1].(*ast.ChangeObjectStmt) + if source, ok := changeStmt.Changes[0].Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected change assignment SourceExpr, got %T", changeStmt.Changes[0].Value) + } else if !strings.Contains(source.Source, "\n") { + t.Fatalf("change assignment source lost trailing separator whitespace: %q", source.Source) + } + + callStmt := mf.Body[2].(*ast.CallJavaActionStmt) + if source, ok := callStmt.Arguments[0].Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected call argument SourceExpr, got %T", callStmt.Arguments[0].Value) + } else if !strings.Contains(source.Source, "\n") { + t.Fatalf("call argument source lost trailing separator whitespace: %q", source.Source) + } + + ifStmt := mf.Body[3].(*ast.IfStmt) + if source, ok := ifStmt.Condition.(*ast.SourceExpr); !ok { + t.Fatalf("expected if condition SourceExpr, got %T", ifStmt.Condition) + } else if source.Source != "isMatch( $Input, '[0-9]+')" { + t.Fatalf("if condition source = %q", source.Source) + } + + showStmt := ifStmt.ThenBody[0].(*ast.ShowMessageStmt) + if source, ok := showStmt.TemplateArgs[0].(*ast.SourceExpr); !ok { + t.Fatalf("expected show-message argument SourceExpr, got %T", showStmt.TemplateArgs[0]) + } else if !strings.Contains(source.Source, "\nelse 'disabled'\n") { + t.Fatalf("show-message argument source lost line breaks: %q", source.Source) + } + + returnStmt := mf.Body[4].(*ast.ReturnStmt) + if source, ok := returnStmt.Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected return SourceExpr, got %T", returnStmt.Value) + } else if source.Source != "$Flag\nand\n$OtherFlag" { + t.Fatalf("return source = %q", source.Source) + } +} + func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testing.T) { if shouldPreserveExpressionSource("'Processed {1} items!'") { t.Fatal("plain string literal punctuation should not force SourceExpr preservation") @@ -1744,4 +1820,10 @@ func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testin if !shouldPreserveExpressionSource("$Token!=empty") { t.Fatal("compact operators outside string literals should preserve source") } + if !shouldPreserveExpressionSource("substring($Text,0,find($Text, '.'))") { + t.Fatal("compact comma-separated arguments should preserve source") + } + if !shouldPreserveExpressionSource("not($Object/Flag)") { + t.Fatal("compact not() expressions should preserve source") + } } From b4b3a53595bc6165a651c052015dd657fbc6aadc Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 22:58:42 +0200 Subject: [PATCH 5/9] fix: preserve trailing expression slot whitespace Symptom: describe/exec/describe still drifted for expression slots that had layout whitespace immediately before delimiters such as semicolons, closing parentheses, or LOG message expressions. Root cause: source-preservation helpers kept multiline expression bodies but ignored delimiter-adjacent whitespace unless it contained a newline, and LOG node/message gaps were not captured at all. Fix: preserve whitespace-only gaps before expression-list delimiters and statement semicolons, and carry multiline LOG node-to-message gaps through the node expression without duplicating formatter spacing. Tests: add a visitor regression covering DECLARE, SET, CHANGE, LOG node gaps, and LOG template parameter trailing whitespace; make build and make test pass. --- mdl/visitor/visitor_microflow_actions.go | 134 +++++++++++++++++--- mdl/visitor/visitor_microflow_statements.go | 2 + mdl/visitor/visitor_test.go | 56 ++++++++ 3 files changed, 176 insertions(+), 16 deletions(-) diff --git a/mdl/visitor/visitor_microflow_actions.go b/mdl/visitor/visitor_microflow_actions.go index 36b7374c..7e03e1cf 100644 --- a/mdl/visitor/visitor_microflow_actions.go +++ b/mdl/visitor/visitor_microflow_actions.go @@ -52,6 +52,7 @@ func buildLogStatement(ctx parser.ILogStatementContext) *ast.LogStmt { // expression is the node and the second is the message. if logCtx.NODE() != nil && len(exprs) > 1 { stmt.Node = buildSourceExpression(exprs[0]) + stmt.Node = appendLogNodeTrailingWhitespace(exprs[0], exprs[1], stmt.Node) stmt.Message = buildSourceExpression(exprs[1]) } else if len(exprs) > 0 { stmt.Message = buildSourceExpression(exprs[0]) @@ -127,17 +128,11 @@ func appendTemplateParamTrailingWhitespace( expr ast.Expression, ) ast.Expression { trailing := templateParamTrailingWhitespace(paramsCtx, allParams, index, exprCtx) - if trailing == "" || !strings.ContainsAny(trailing, "\r\n") { + if trailing == "" { return expr } - source := strings.TrimSpace(extractOriginalText(exprCtx.(antlr.ParserRuleContext))) - innerExpr := expr - if sourceExpr, ok := expr.(*ast.SourceExpr); ok { - source = sourceExpr.Source - innerExpr = sourceExpr.Expression - } - return &ast.SourceExpr{Expression: innerExpr, Source: source + trailing} + return appendSourceExpressionSuffix(exprCtx, expr, trailing) } func templateParamTrailingWhitespace( @@ -198,17 +193,11 @@ func appendExpressionListTrailingWhitespace( expr ast.Expression, ) ast.Expression { trailing := expressionListTrailingWhitespace(parent, next, exprCtx) - if trailing == "" || !strings.ContainsAny(trailing, "\r\n") { + if trailing == "" { return expr } - source := strings.TrimSpace(extractOriginalText(exprCtx.(antlr.ParserRuleContext))) - innerExpr := expr - if sourceExpr, ok := expr.(*ast.SourceExpr); ok { - source = sourceExpr.Source - innerExpr = sourceExpr.Expression - } - return &ast.SourceExpr{Expression: innerExpr, Source: source + trailing} + return appendSourceExpressionSuffix(exprCtx, expr, trailing) } func expressionListTrailingWhitespace( @@ -233,6 +222,9 @@ func expressionListTrailingWhitespace( end = parent.GetStop().GetStart() - 1 } if start < 0 || end < start { + if next == nil { + return whitespaceUntilDelimiter(input, start, ")]") + } return "" } @@ -250,6 +242,116 @@ func expressionListTrailingWhitespace( return gap } +func appendStatementExpressionTrailingWhitespace( + exprCtx parser.IExpressionContext, + expr ast.Expression, +) ast.Expression { + trailing := statementExpressionTrailingWhitespace(exprCtx) + if trailing == "" { + return expr + } + return appendSourceExpressionSuffix(exprCtx, expr, trailing) +} + +func statementExpressionTrailingWhitespace(exprCtx parser.IExpressionContext) string { + exprRule, ok := exprCtx.(antlr.ParserRuleContext) + if !ok || exprRule.GetStop() == nil { + return "" + } + input := exprRule.GetStop().GetInputStream() + if input == nil { + return "" + } + + start := exprRule.GetStop().GetStop() + 1 + if start < 0 || start >= input.Size() { + return "" + } + + return whitespaceUntilDelimiter(input, start, ";") +} + +func appendLogNodeTrailingWhitespace( + nodeCtx parser.IExpressionContext, + messageCtx parser.IExpressionContext, + expr ast.Expression, +) ast.Expression { + trailing := expressionGapWhitespace(nodeCtx, messageCtx) + if trailing == "" || !strings.ContainsAny(trailing, "\r\n") { + return expr + } + // formatAction always writes one space between the node and message slots. + // Preserve the source line break but avoid duplicating indentation spaces. + trailing = strings.TrimRight(trailing, " \t") + if trailing == "" { + return expr + } + return appendSourceExpressionSuffix(nodeCtx, expr, trailing) +} + +func expressionGapWhitespace( + leftCtx parser.IExpressionContext, + rightCtx parser.IExpressionContext, +) string { + leftRule, ok := leftCtx.(antlr.ParserRuleContext) + if !ok || leftRule.GetStop() == nil { + return "" + } + rightRule, ok := rightCtx.(antlr.ParserRuleContext) + if !ok || rightRule.GetStart() == nil { + return "" + } + input := leftRule.GetStop().GetInputStream() + if input == nil { + return "" + } + start := leftRule.GetStop().GetStop() + 1 + end := rightRule.GetStart().GetStart() - 1 + if start < 0 || end < start { + return "" + } + gap := input.GetText(start, end) + if strings.TrimSpace(gap) != "" { + return "" + } + return gap +} + +func whitespaceUntilDelimiter(input antlr.CharStream, start int, delimiters string) string { + if start < 0 || start >= input.Size() { + return "" + } + end := start + for end < input.Size() { + ch := input.GetText(end, end) + if strings.Contains(delimiters, ch) { + break + } + if strings.TrimSpace(ch) != "" { + return "" + } + end++ + } + if end >= input.Size() || end == start { + return "" + } + return input.GetText(start, end-1) +} + +func appendSourceExpressionSuffix( + exprCtx parser.IExpressionContext, + expr ast.Expression, + suffix string, +) ast.Expression { + source := strings.TrimSpace(extractOriginalText(exprCtx.(antlr.ParserRuleContext))) + innerExpr := expr + if sourceExpr, ok := expr.(*ast.SourceExpr); ok { + source = sourceExpr.Source + innerExpr = sourceExpr.Expression + } + return &ast.SourceExpr{Expression: innerExpr, Source: source + suffix} +} + // buildCallMicroflowStatement converts CALL MICROFLOW statement context to CallMicroflowStmt. // Grammar: (VARIABLE EQUALS)? CALL MICROFLOW qualifiedName LPAREN callArgumentList? RPAREN func buildCallMicroflowStatement(ctx parser.ICallMicroflowStatementContext) *ast.CallMicroflowStmt { diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 21becaa5..52d76afa 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -532,6 +532,7 @@ func buildDeclareStatement(ctx parser.IDeclareStatementContext) *ast.DeclareStmt // Get optional initial value if expr := declCtx.Expression(); expr != nil { stmt.InitialValue = buildSourceExpression(expr) + stmt.InitialValue = appendStatementExpressionTrailingWhitespace(expr, stmt.InitialValue) } return stmt @@ -696,6 +697,7 @@ func buildSetStatement(ctx parser.ISetStatementContext) ast.MicroflowStatement { if valueExprCtx != nil { valueExpr = buildSourceExpression(valueExprCtx) + valueExpr = appendStatementExpressionTrailingWhitespace(valueExprCtx, valueExpr) } // Default: regular SET statement diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 8e9175d5..7583cd5c 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1810,6 +1810,62 @@ END;` } } +func TestTrailingExpressionWhitespacePreservedForRoundtripSlots(t *testing.T) { + input := `CREATE MICROFLOW Synthetic.PreserveTrailingExpressionWhitespace ( + $Object: Synthetic.Entity +) +RETURNS Boolean AS $Result +BEGIN + DECLARE $Prefix String = 'Hello' +; + SET $Prefix = substring($Prefix, 0, 1) +; + CHANGE $Object (Name = $Prefix ); + LOG INFO NODE @Synthetic.LogNode + 'Processed {1}' WITH ({1} = $Prefix ); + RETURN true; +END;` + + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + decl := mf.Body[0].(*ast.DeclareStmt) + if source, ok := decl.InitialValue.(*ast.SourceExpr); !ok { + t.Fatalf("expected declare initial value SourceExpr, got %T", decl.InitialValue) + } else if source.Source != "'Hello'\n" { + t.Fatalf("declare source = %q", source.Source) + } + + setStmt := mf.Body[1].(*ast.MfSetStmt) + if source, ok := setStmt.Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected set value SourceExpr, got %T", setStmt.Value) + } else if source.Source != "substring($Prefix, 0, 1)\n" { + t.Fatalf("set source = %q", source.Source) + } + + changeStmt := mf.Body[2].(*ast.ChangeObjectStmt) + if source, ok := changeStmt.Changes[0].Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected change value SourceExpr, got %T", changeStmt.Changes[0].Value) + } else if source.Source != "$Prefix " { + t.Fatalf("change source = %q", source.Source) + } + + logStmt := mf.Body[3].(*ast.LogStmt) + if source, ok := logStmt.Node.(*ast.SourceExpr); !ok { + t.Fatalf("expected log node SourceExpr, got %T", logStmt.Node) + } else if source.Source != "@Synthetic.LogNode\n" { + t.Fatalf("log node source = %q", source.Source) + } + if source, ok := logStmt.Template[0].Value.(*ast.SourceExpr); !ok { + t.Fatalf("expected log template SourceExpr, got %T", logStmt.Template[0].Value) + } else if source.Source != "$Prefix " { + t.Fatalf("template source = %q", source.Source) + } +} + func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testing.T) { if shouldPreserveExpressionSource("'Processed {1} items!'") { t.Fatal("plain string literal punctuation should not force SourceExpr preservation") From 1a647f5de04bc6a98f1e652f6ec3a79714830d9b Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Mon, 27 Apr 2026 23:23:12 +0200 Subject: [PATCH 6/9] fix: preserve retrieve range expression whitespace Symptom: database retrieve roundtrips could collapse blank lines before OFFSET and move a semicolon onto the OFFSET expression line. Root cause: retrieve LIMIT and OFFSET used GetText(), which kept the expression value but discarded delimiter-adjacent source whitespace that the describer can emit from existing model strings. Fix: parse retrieve range expressions from original source text and preserve only whitespace that belongs to the range expression slots, accounting for the formatter's built-in OFFSET clause prefix. Tests: add a visitor regression covering multiline LIMIT/OFFSET source whitespace and run make build plus make test. --- mdl/visitor/visitor_microflow_statements.go | 72 ++++++++++++++++++++- mdl/visitor/visitor_test.go | 37 +++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 52d76afa..9798f741 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -1008,10 +1008,10 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS // Get LIMIT and OFFSET expressions if limitExpr := retrCtx.GetLimitExpr(); limitExpr != nil { - stmt.Limit = limitExpr.GetText() + stmt.Limit = retrieveRangeExpressionSource(limitExpr) + retrieveLimitTrailingWhitespace(retrCtx, limitExpr) } if offsetExpr := retrCtx.GetOffsetExpr(); offsetExpr != nil { - stmt.Offset = offsetExpr.GetText() + stmt.Offset = retrieveRangeExpressionSource(offsetExpr) + retrieveRangeExpressionTrailingWhitespace(offsetExpr) } // Check for ON ERROR clause @@ -1022,6 +1022,74 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS return stmt } +func retrieveRangeExpressionSource(exprCtx parser.IExpressionContext) string { + if exprCtx == nil { + return "" + } + if prc, ok := exprCtx.(antlr.ParserRuleContext); ok { + if source := strings.TrimSpace(extractOriginalText(prc)); source != "" { + return source + } + } + return exprCtx.GetText() +} + +func retrieveLimitTrailingWhitespace(retrCtx *parser.RetrieveStatementContext, limitExpr parser.IExpressionContext) string { + if retrCtx == nil || limitExpr == nil { + return "" + } + exprRule, ok := limitExpr.(antlr.ParserRuleContext) + if !ok || exprRule.GetStop() == nil { + return "" + } + input := exprRule.GetStop().GetInputStream() + if input == nil { + return "" + } + + start := exprRule.GetStop().GetStop() + 1 + if offset := retrCtx.OFFSET(); offset != nil && offset.GetSymbol() != nil { + gap := whitespaceBetween(input, start, offset.GetSymbol().GetStart()-1) + return retrieveInterClauseWhitespaceSuffix(gap) + } + return whitespaceUntilDelimiter(input, start, ";") +} + +func retrieveRangeExpressionTrailingWhitespace(exprCtx parser.IExpressionContext) string { + exprRule, ok := exprCtx.(antlr.ParserRuleContext) + if !ok || exprRule.GetStop() == nil { + return "" + } + input := exprRule.GetStop().GetInputStream() + if input == nil { + return "" + } + return whitespaceUntilDelimiter(input, exprRule.GetStop().GetStop()+1, ";") +} + +func whitespaceBetween(input antlr.CharStream, start, end int) string { + if start < 0 || end < start || start >= input.Size() { + return "" + } + gap := input.GetText(start, end) + if strings.TrimSpace(gap) != "" { + return "" + } + return gap +} + +func retrieveInterClauseWhitespaceSuffix(gap string) string { + if gap == "" { + return "" + } + for _, suffix := range []string{"\r\n ", "\n "} { + if strings.HasSuffix(gap, suffix) { + return strings.TrimSuffix(gap, suffix) + } + } + return "" +} + // buildSortColumnMicroflow builds a sort column definition from a SortColumnContext. // This is a duplicate of buildSortColumn in visitor_page_widgets.go but in a different file. func buildSortColumnMicroflow(ctx parser.ISortColumnContext) *ast.SortColumnDef { diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 7583cd5c..154aa0e0 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1866,6 +1866,43 @@ END;` } } +func TestRetrieveRangeExpressionsPreserveTrailingWhitespace(t *testing.T) { + input := `CREATE MICROFLOW Synthetic.PreserveRetrieveRangeWhitespace ( + $UseFirstPage: Boolean, + $PageSize: Integer, + $PageNumber: Integer +) +RETURNS List OF Synthetic.Entity +BEGIN + RETRIEVE $Items FROM Synthetic.Entity + LIMIT $PageSize + + OFFSET IF $UseFirstPage THEN 0 +ELSE +$PageSize * ($PageNumber - 1) +; + RETURN $Items; +END;` + + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + retrieveStmt, ok := mf.Body[0].(*ast.RetrieveStmt) + if !ok { + t.Fatalf("expected RetrieveStmt, got %T", mf.Body[0]) + } + if retrieveStmt.Limit != "$PageSize\n" { + t.Fatalf("limit source = %q", retrieveStmt.Limit) + } + wantOffset := "IF $UseFirstPage THEN 0\nELSE\n$PageSize * ($PageNumber - 1)\n" + if retrieveStmt.Offset != wantOffset { + t.Fatalf("offset source = %q, want %q", retrieveStmt.Offset, wantOffset) + } +} + func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testing.T) { if shouldPreserveExpressionSource("'Processed {1} items!'") { t.Fatal("plain string literal punctuation should not force SourceExpr preservation") From 38d28b9cfd9095b3e4c87a86b5e93788aec79d3c Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Tue, 28 Apr 2026 15:28:46 +0200 Subject: [PATCH 7/9] fix: preserve XPath path source spacing in retrieve clauses Symptom: retrieve WHERE clauses using compact XPath paths could roundtrip as canonicalized paths with spaces around '/', producing cosmetic drift. Root cause: source-expression preservation did not treat retrieve XPath paths as source-sensitive, and XPath constraints were built directly from normalized XPath AST nodes. Fix: preserve original source text for XPath constraints and retrieve WHERE expressions that contain path separators without changing normal expression parsing. Tests: added a retrieve XPath path-spacing regression and ran make test. --- mdl/visitor/visitor_microflow_statements.go | 34 +++++++++++++++++++-- mdl/visitor/visitor_test.go | 24 +++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 9798f741..c537dd8e 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -970,7 +970,7 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS if len(xpathConstraints) == 1 { xcCtx := xpathConstraints[0].(*parser.XpathConstraintContext) if xpathExpr := xcCtx.XpathExpr(); xpathExpr != nil { - stmt.Where = buildXPathExpr(xpathExpr) + stmt.Where = buildXPathSourceExpression(xpathExpr) } } else if len(xpathConstraints) > 1 { // Multiple predicates [cond1][cond2] — combine with AND @@ -978,7 +978,7 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS for _, xc := range xpathConstraints { xcCtx := xc.(*parser.XpathConstraintContext) if xpathExpr := xcCtx.XpathExpr(); xpathExpr != nil { - andExprs = append(andExprs, buildXPathExpr(xpathExpr)) + andExprs = append(andExprs, buildXPathSourceExpression(xpathExpr)) } } if len(andExprs) == 1 { @@ -992,7 +992,7 @@ func buildRetrieveStatement(ctx parser.IRetrieveStatementContext) *ast.RetrieveS stmt.Where = result } } else if expr := retrCtx.Expression(0); expr != nil { - stmt.Where = buildSourceExpression(expr) + stmt.Where = buildRetrieveWhereExpression(expr) } } @@ -1208,6 +1208,34 @@ func buildSourceExpression(ctx parser.IExpressionContext) ast.Expression { return expr } +func buildXPathSourceExpression(ctx parser.IXpathExprContext) ast.Expression { + if ctx == nil { + return nil + } + expr := buildXPathExpr(ctx) + if prc, ok := ctx.(antlr.ParserRuleContext); ok { + if source := strings.TrimSpace(extractOriginalText(prc)); source != "" { + return &ast.SourceExpr{Expression: expr, Source: source} + } + } + return expr +} + +func buildRetrieveWhereExpression(ctx parser.IExpressionContext) ast.Expression { + if ctx == nil { + return nil + } + expr := buildExpression(ctx) + if prc, ok := ctx.(antlr.ParserRuleContext); ok { + if source := strings.TrimSpace(extractOriginalText(prc)); source != "" { + if shouldPreserveExpressionSource(source) || strings.Contains(source, "/") { + return &ast.SourceExpr{Expression: expr, Source: source} + } + } + } + return expr +} + func shouldPreserveExpressionSource(source string) bool { if strings.ContainsAny(source, "\r\n") { return true diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index 154aa0e0..4012678c 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1810,6 +1810,30 @@ END;` } } +func TestRetrieveXPathPreservesOriginalPathSpacing(t *testing.T) { + input := `CREATE MICROFLOW Synthetic.PreserveXPathPathSpacing () +BEGIN + RETRIEVE $Items FROM Synthetic.Item + WHERE Synthetic.Item_Group/Synthetic.Group/Synthetic.Group_Company = $Company; +END;` + + prog, errs := Build(input) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + retrieveStmt := mf.Body[0].(*ast.RetrieveStmt) + source, ok := retrieveStmt.Where.(*ast.SourceExpr) + if !ok { + t.Fatalf("expected retrieve XPath SourceExpr, got %T", retrieveStmt.Where) + } + want := "Synthetic.Item_Group/Synthetic.Group/Synthetic.Group_Company = $Company" + if source.Source != want { + t.Fatalf("XPath source = %q, want %q", source.Source, want) + } +} + func TestTrailingExpressionWhitespacePreservedForRoundtripSlots(t *testing.T) { input := `CREATE MICROFLOW Synthetic.PreserveTrailingExpressionWhitespace ( $Object: Synthetic.Entity From 9aa2248d8462d0150978cde50d8062bd5df1632e Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Wed, 29 Apr 2026 08:17:18 +0200 Subject: [PATCH 8/9] fix: unwrap empty source expressions during association resolution Symptom: resolveAssociationPaths could return a SourceExpr wrapper with an empty Source field after resolving the inner expression. Root cause: the SourceExpr branch rebuilt the wrapper even when there was no source text to preserve. That made the wrapper semantically redundant and left future callers dependent on expressionToString peeling it away. Fix: keep non-empty SourceExpr values verbatim for whitespace-preserving roundtrips, but unwrap empty SourceExpr values to the resolved inner expression. Tests: make build; focused SourceExpr association-resolution tests; make test. --- mdl/executor/bugfix_test.go | 34 ++++++++++++++++++++++++++ mdl/executor/cmd_microflows_builder.go | 9 ++++--- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/mdl/executor/bugfix_test.go b/mdl/executor/bugfix_test.go index fc1b19d7..e807cdc6 100644 --- a/mdl/executor/bugfix_test.go +++ b/mdl/executor/bugfix_test.go @@ -515,6 +515,40 @@ func TestResolveAssociationPaths(t *testing.T) { } } +func TestResolveAssociationPathsUnwrapsEmptySourceExpr(t *testing.T) { + fb := &flowBuilder{} + resolved := fb.resolveAssociationPaths(&ast.SourceExpr{ + Expression: &ast.VariableExpr{Name: "CurrentObject"}, + }) + + if _, ok := resolved.(*ast.SourceExpr); ok { + t.Fatalf("empty SourceExpr should unwrap to resolved inner expression, got %T", resolved) + } + if got := expressionToString(resolved); got != "$CurrentObject" { + t.Fatalf("resolved expression = %q, want $CurrentObject", got) + } +} + +func TestResolveAssociationPathsKeepsNonEmptySourceExprVerbatim(t *testing.T) { + source := "$CurrentObject/Module.Assoc/Name\n" + fb := &flowBuilder{} + resolved := fb.resolveAssociationPaths(&ast.SourceExpr{ + Expression: &ast.AttributePathExpr{ + Variable: "CurrentObject", + Path: []string{"Module.Assoc", "Name"}, + }, + Source: source, + }) + + sourceExpr, ok := resolved.(*ast.SourceExpr) + if !ok { + t.Fatalf("non-empty SourceExpr should remain SourceExpr, got %T", resolved) + } + if sourceExpr.Source != source { + t.Fatalf("source = %q, want %q", sourceExpr.Source, source) + } +} + // TestExprToStringNoSpaces verifies that association navigation expressions // produce no extra spaces around separators after parsing. // Issue #120: generated $Order / Module.Assoc / Name instead of $Order/Module.Assoc/Name diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index c2639656..bd1d2c52 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -331,12 +331,13 @@ func (fb *flowBuilder) resolveAssociationPaths(expr ast.Expression) ast.Expressi } case *ast.SourceExpr: if e.Source != "" { + // Non-empty Source is the exact expression text to write back. + // Rebuilding it here would defeat the whitespace-preservation + // purpose of SourceExpr, so keep the parsed tree only for callers + // that need semantic inspection. return e } - return &ast.SourceExpr{ - Expression: fb.resolveAssociationPaths(e.Expression), - Source: e.Source, - } + return fb.resolveAssociationPaths(e.Expression) default: return expr } From 736428431f643e8d1a560dd45d88fb76ad2834c7 Mon Sep 17 00:00:00 2001 From: Henrique Costa Date: Thu, 30 Apr 2026 09:15:56 +0200 Subject: [PATCH 9/9] fix: make retrieve inter-clause whitespace robust to indent changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the hard-coded "\n " / "\r\n " suffix matches with a backwards walk that strips a trailing newline plus whatever indent (spaces or tabs) follows it. The formatter re-emits its own indent before each clause, so the original source's trailing newline+indent is structural and would duplicate after a roundtrip — but the old 4-space match silently dropped the entire whitespace gap if the formatter or the source used a different width. Adds a doc comment explaining the contract. Also documents the `not(` heuristic in shouldPreserveExpressionSource so future readers know it preserves the compact form against the parser's `not ()` re-emission. Addresses ako's review on PR #323. Co-Authored-By: Claude Opus 4.7 --- mdl/visitor/visitor_microflow_statements.go | 37 +++++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index c537dd8e..ca788eea 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -1078,14 +1078,38 @@ func whitespaceBetween(input antlr.CharStream, start, end int) string { return gap } +// retrieveInterClauseWhitespaceSuffix returns the whitespace gap between a +// retrieve expression and the next clause keyword (LIMIT/OFFSET), with the +// trailing newline + indent that the formatter will re-emit stripped off. +// +// The formatter writes each subsequent clause on its own line indented by +// `formatRetrieveContinuationIndent` spaces, so the original source's trailing +// "\n" is structural and would duplicate after a roundtrip if kept. +// Anything before that final newline (blank lines, comments, additional +// indentation) is preserved as authored. When the gap does not end in a +// recognisable line-break-then-indent sequence we return "" — the formatter +// will lay out the clause normally. func retrieveInterClauseWhitespaceSuffix(gap string) string { if gap == "" { return "" } - for _, suffix := range []string{"\r\n ", "\n "} { - if strings.HasSuffix(gap, suffix) { - return strings.TrimSuffix(gap, suffix) + // Trim the trailing newline + structural indentation the formatter will + // re-emit. We strip whatever indent (spaces or tabs) follows the final + // newline so this stays robust if the formatter changes its indent width. + for i := len(gap) - 1; i >= 0; i-- { + c := gap[i] + if c == ' ' || c == '\t' { + continue + } + if c == '\n' { + // Include a preceding \r in the strip so CRLF line endings work. + cut := i + if cut > 0 && gap[cut-1] == '\r' { + cut-- + } + return gap[:cut] } + break } return "" } @@ -1263,6 +1287,13 @@ func shouldPreserveExpressionSource(source string) bool { } } } + // Mendix's `not()` function call has no surrounding spaces in + // idiomatic source, but the parser would re-emit it as `not ()` + // (function-call AST node loses the no-space affordance). Preserving the + // original source keeps the compact form across describe → exec → + // describe. The substring check is intentionally loose; false positives + // (e.g. an attribute name containing "not(") only over-preserve and have + // no semantic effect since the parsed expression is what runs. if strings.Contains(strings.ToLower(source), "not(") { return true }