Skip to content

Commit ae47faf

Browse files
authored
Merge pull request #323 from hjotha/submit/visitor-preserve-multiline-source-expressions
fix: preserve multiline source expression whitespace
2 parents 49f3400 + 7364284 commit ae47faf

8 files changed

Lines changed: 859 additions & 43 deletions
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
-- ============================================================================
2+
-- Bug #322: Multiline source expression whitespace lost on roundtrip
3+
-- ============================================================================
4+
--
5+
-- Symptom (before fix):
6+
-- The visitor rebuilt expression strings from parsed expression nodes,
7+
-- so original line breaks and whitespace inside DECLARE initial values
8+
-- and LOG template parameters were normalized away. A multiline
9+
-- `declare $X string = ...` statement that authors had carefully
10+
-- formatted across several lines came back as a single very long line
11+
-- after describe → exec → describe. Inter-parameter blank lines in
12+
-- `LOG ... WITH (...)` were similarly collapsed, making real-world
13+
-- microflows non-fixpoint and producing noisy `mxcli diff-local` output.
14+
--
15+
-- After fix:
16+
-- Added a `SourceExpr` AST node that wraps an expression with its
17+
-- original source text. The visitor uses `buildSourceExpression` for
18+
-- source-sensitive declare/log/while expressions, and the executor
19+
-- serializes `SourceExpr` through to MDL output so the original
20+
-- whitespace and line breaks survive every roundtrip.
21+
--
22+
-- Usage:
23+
-- mxcli exec mdl-examples/bug-tests/322-multiline-source-expression-whitespace.mdl -p app.mpr
24+
-- mxcli -p app.mpr -c "describe microflow BugTest322.MF_MultilineDeclare"
25+
-- mxcli -p app.mpr -c "describe microflow BugTest322.MF_MultilineLogTemplate"
26+
-- The describe outputs must keep the multiline shape and must be
27+
-- fixpoints under describe → exec → describe.
28+
-- ============================================================================
29+
30+
create module BugTest322;
31+
32+
-- DECLARE initial value spread across several lines with leading `+`. The
33+
-- describer must preserve the line breaks rather than collapse them onto
34+
-- one line.
35+
create microflow BugTest322.MF_MultilineDeclare (
36+
$Page: integer,
37+
$Token: string
38+
)
39+
returns string as $Endpoint
40+
begin
41+
declare $Endpoint string = '/api/v1'
42+
+ '/items?page=' + toString($Page)
43+
+ '&token=' + $Token;
44+
45+
return $Endpoint;
46+
end;
47+
/
48+
49+
-- LOG template parameters separated by a blank line. The newline-only
50+
-- whitespace between `{1} = toString($Count)` and the next parameter must
51+
-- survive the roundtrip.
52+
create microflow BugTest322.MF_MultilineLogTemplate (
53+
$Count: integer,
54+
$Endpoint: string
55+
)
56+
begin
57+
log info node 'BugTest322' 'Processed {1} items for {2}' with ({1} = toString($Count)
58+
59+
, {2} = $Endpoint);
60+
end;
61+
/

mdl/ast/ast_expression.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ type IfThenElseExpr struct {
125125

126126
func (e *IfThenElseExpr) isExpression() {}
127127

128+
// SourceExpr preserves original expression source text while keeping the parsed
129+
// expression tree available for callers that need semantic inspection.
130+
type SourceExpr struct {
131+
Expression Expression
132+
Source string
133+
}
134+
135+
func (e *SourceExpr) isExpression() {}
136+
128137
// ============================================================================
129138
// XPath-Specific Expression Types
130139
// ============================================================================

mdl/executor/bugfix_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,40 @@ func TestResolveAssociationPaths(t *testing.T) {
515515
}
516516
}
517517

518+
func TestResolveAssociationPathsUnwrapsEmptySourceExpr(t *testing.T) {
519+
fb := &flowBuilder{}
520+
resolved := fb.resolveAssociationPaths(&ast.SourceExpr{
521+
Expression: &ast.VariableExpr{Name: "CurrentObject"},
522+
})
523+
524+
if _, ok := resolved.(*ast.SourceExpr); ok {
525+
t.Fatalf("empty SourceExpr should unwrap to resolved inner expression, got %T", resolved)
526+
}
527+
if got := expressionToString(resolved); got != "$CurrentObject" {
528+
t.Fatalf("resolved expression = %q, want $CurrentObject", got)
529+
}
530+
}
531+
532+
func TestResolveAssociationPathsKeepsNonEmptySourceExprVerbatim(t *testing.T) {
533+
source := "$CurrentObject/Module.Assoc/Name\n"
534+
fb := &flowBuilder{}
535+
resolved := fb.resolveAssociationPaths(&ast.SourceExpr{
536+
Expression: &ast.AttributePathExpr{
537+
Variable: "CurrentObject",
538+
Path: []string{"Module.Assoc", "Name"},
539+
},
540+
Source: source,
541+
})
542+
543+
sourceExpr, ok := resolved.(*ast.SourceExpr)
544+
if !ok {
545+
t.Fatalf("non-empty SourceExpr should remain SourceExpr, got %T", resolved)
546+
}
547+
if sourceExpr.Source != source {
548+
t.Fatalf("source = %q, want %q", sourceExpr.Source, source)
549+
}
550+
}
551+
518552
// TestExprToStringNoSpaces verifies that association navigation expressions
519553
// produce no extra spaces around separators after parsing.
520554
// Issue #120: generated $Order / Module.Assoc / Name instead of $Order/Module.Assoc/Name

mdl/executor/cmd_microflows_builder.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,15 @@ func (fb *flowBuilder) resolveAssociationPaths(expr ast.Expression) ast.Expressi
329329
ThenExpr: fb.resolveAssociationPaths(e.ThenExpr),
330330
ElseExpr: fb.resolveAssociationPaths(e.ElseExpr),
331331
}
332+
case *ast.SourceExpr:
333+
if e.Source != "" {
334+
// Non-empty Source is the exact expression text to write back.
335+
// Rebuilding it here would defeat the whitespace-preservation
336+
// purpose of SourceExpr, so keep the parsed tree only for callers
337+
// that need semantic inspection.
338+
return e
339+
}
340+
return fb.resolveAssociationPaths(e.Expression)
332341
default:
333342
return expr
334343
}
@@ -443,6 +452,8 @@ func unwrapParenCall(expr ast.Expression) *ast.FunctionCallExpr {
443452
return e
444453
case *ast.ParenExpr:
445454
expr = e.Inner
455+
case *ast.SourceExpr:
456+
expr = e.Expression
446457
default:
447458
return nil
448459
}

mdl/executor/cmd_microflows_helpers.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,11 @@ func expressionToString(expr ast.Expression) string {
327327
thenStr := expressionToString(e.ThenExpr)
328328
elseStr := expressionToString(e.ElseExpr)
329329
return "if " + cond + " then " + thenStr + " else " + elseStr
330+
case *ast.SourceExpr:
331+
if e.Source != "" {
332+
return e.Source
333+
}
334+
return expressionToString(e.Expression)
330335
default:
331336
return ""
332337
}
@@ -379,6 +384,11 @@ func expressionToXPath(expr ast.Expression) string {
379384
return expressionToString(expr)
380385
case *ast.QualifiedNameExpr:
381386
return qualifiedNameToXPath(e)
387+
case *ast.SourceExpr:
388+
if e.Source != "" {
389+
return e.Source
390+
}
391+
return expressionToXPath(e.Expression)
382392
default:
383393
// For all other expression types, the standard serialization is correct
384394
return expressionToString(expr)

0 commit comments

Comments
 (0)