fix(visitor): store text expressions with their whitespace

.claude/skills/fix-issue/findings/mdl-visitor.jsonl

@@ -34,3 +34,4 @@
 {"date": "2026-09-24", "area": "mdl-visitor", "symptom": "upstream #1174: DESCRIBE printed a view whose join is `AS ROLE`, and exec'ing that output failed — `line 7:62 mismatched input 'ROLE' expecting IDENTIFIER` then a knock-on `mismatched input ')' expecting {SELECT, HAVING}`. The model was valid (mx check 0 errors on 11.12.1)", "cause": "`tableReference` / `joinClause` took the source alias as `AS? IDENTIFIER`, and `associationPath`'s leading alias as `IDENTIFIER`, so any MDL keyword (ROLE, STATUS, VALUE…) was refused although OQL does not reserve it. The select alias already accepted `keyword`; the source alias had never been given the same", "file": "`mdl/grammar/domains/MDLCatalog.g4` (new `oqlSourceAlias`: `AS (IDENTIFIER | keyword) | IDENTIFIER`; `associationPath` leading `(IDENTIFIER | keyword)`); `mdl/visitor/visitor_entity.go` (`oqlSourceAliasText`); tests `mdl/visitor/oql_keyword_alias_test.go`; example `mdl-examples/bug-tests/1174-oql-keyword-source-alias.mdl`", "insight": "**Accept the keyword only after an explicit AS.** Copying `selectAlias`'s `IDENTIFIER | keyword` into `AS? …` would let `from M.Sale s left join …` read LEFT as the alias — the bare form has to stay IDENTIFIER-only, and the test keeps a control for exactly that. MDL's keyword list is not OQL's: rule 4 of `mxcli syntax domain-model.view-entity.oql` (rename a reserved alias) is about OQL's own words (Month, Year) and does not apply to an MDL-only keyword, so do not send the user to rename. Measured end-to-end on 11.12.1: exec + `mx check` 0 errors, DESCRIBE prints `AS ROLE`, and the described text re-parses and execs. Two gaps met on the way, both independent of the keyword and filed separately: an uppercase `AS` on an association-path join leaves that alias unresolved in `extractAliasMap` (case-sensitive `TrimSuffix(path, \"as\")`), so its columns get no type check — first misread as 'System entity lengths are not checked' until bisecting the query text against a fresh project (ako/mxcli#652); and describe -> exec adds 2 spaces to OQL lines 2..n on EVERY cycle — re-running the same described file said Unchanged and hid it, only describing again between runs shows the drift (ako/mxcli#653)", "refs": ["mendixlabs/mxcli#1174", "ako/mxcli#652", "ako/mxcli#653"]}
 {"area": "mdl/visitor", "date": "2026-09-24", "symptom": "`datagrid dg (DataSource: Mod.Car, …)` — the bare-entity shorthand — passed `check` (no project), `exec --no-check` printed \"Created page\", `describe page` showed `datagrid dg (onClick: …)` with the source gone, and mxbuild 11.14.0 reported CE0488 \"No entity configured for the data source of this widgets container\" + CE1571 + two column-attribute errors on that one grid. With `-p`, the reference pass refused it instead, but with the misleading \"Attribute 'Name' is bound but there is no enclosing data container providing entity context\" on a grid whose source the script did name.", "cause": "Every dataSourceExprV3 alternative starts with a keyword (DATABASE/MICROFLOW/…) or a VARIABLE, so `DataSource: Mod.Car` matched none and fell through to the generic `keyword COLON propertyValueV3` branch at the end of widgetPropertyV3 (DATASOURCE is in `keyword`). The visitor stored the string \"Mod.Car\"; GetDataSource() only type-asserts *ast.DataSourceV3, so nothing downstream saw it.", "file": "`mdl/visitor/visitor_page_v3.go` (bareEntityDataSource, bareEntityWidgets)", "insight": "Resolve the shorthand in the VISITOR, keyed on widget type (datagrid/listview/gallery/dataview), not as a grammar alternative. The first cut added `DATASOURCE COLON qualifiedName` to widgetPropertyV3 and broke the Barcode Scanner: a pluggable widget's generic keys are its own .mpk keys, and its `datasource: Module.Entity.Code` binds an ATTRIBUTE (Image's `datasource` is an enum) — only the widget type says what the key means. Grep `\"propertyKey\": \"datasource\"` in modelsdk/widgets/definitions before giving a common word a meaning. The tell for the class is a `map[string]any` property whose readers type-assert: a value of the wrong Go type is invisible rather than wrong. Side effects worth knowing: a data view with the shorthand (maint2-editable-never-create-page.mdl had one, unbound and unnoticed) is now refused as MDL-WIDGET09 instead of written unbound; `-p` reference checking on main already refused the grid case but blamed the column ('no enclosing data container'), a downstream symptom reading as user error; ALTER `set DataSource = M.E` was never silent (refuses 'must be a datasource expression'). Control: pre-fix binary + `exec --no-check` on a fresh 11.14.0 app reproduced the issue's four mxbuild errors verbatim; fixed binary, same script, 0 errors.", "refs": ["ako/mxcli#576", "ako/mxcli#552"], "ce": ["CE0488", "CE1571"]}
 {"area":"mdl-visitor","date":"2026-09-24","refs":["#653"],"symptom":"describe entity on a view entity, exec'd back, was never idempotent: \"Each cycle reports `Modified view entity` and stores the query with every line after the first indented two spaces further.\" Re-exec of the SAME described file reported Unchanged, so it looked stable until you described again","cause":"The two directions did not mirror: describe (cmd_entities_describe.go, and cmd_diff_mdl.go) prefixes two spaces to every stored OQL line; the visitor stored extractOriginalText(oqlCtx), which starts at the query's first token, so line 1 lost its indentation and lines 2…n kept all of it — +2 per cycle","file":"`mdl/visitor/visitor_entity.go` (dedentOQL, leadingLineWhitespace); tests `mdl/visitor/visitor_view_entity_oql_indent_test.go`, `mdl/executor/view_entity_oql_roundtrip_test.go`; bug-test `mdl-examples/bug-tests/653-view-entity-oql-indent-drift.mdl`","insight":"**Any verbatim-source capture that starts at the first token is asymmetric**: line 1 is dedented for free, the continuation lines are not. Fix it on the way IN (exec), not by making describe emit less: strip the common leading-whitespace prefix of the non-blank lines, counting line 1 at its column when only whitespace precedes it (read it from the input stream, start.GetStart()-GetColumn()). Compare prefixes byte-wise, not by width, so a Studio Pro query indented with tabs comes back byte-identical under describe's two spaces. A round-trip test must run describe → exec at least twice AND start from stored text mxcli did not write (flat, tabs, blank lines, comments): one pass from a script is exactly how this went unnoticed. Control: stubbing dedentOQL to return raw fails every round-trip case with lines 2…n two spaces deeper; a real 11.12.1 project with the old binary printed Modified ×3 with growing indent, the fixed one Unchanged ×3. Separate, not fixed here: a comment AFTER the query's last token is outside the captured span and is dropped on exec"}
+{"area": "mdl/visitor", "date": "2026-09-25", "symptom": "A page action's microflow argument `Flag: true and false` was stored as the expression \"trueandfalse\", `Mode: if true then 'a' else 'b'` as \"iftruethen'a'else'b'\", and a REST call parameter `$OrderId = if $x then $id else 'none'` as \"if$xthen$idelse'none'\" (measured by decoding the units on a copy of ako/TestApp). `check -p --references` passed; describe printed the fused text back", "cause": "Four sites stored an expression as text via ANTLR's ctx.GetText(), which concatenates tokens without the hidden-channel whitespace: microflowArgV3 values (page/nanoflow call arguments), contentparams values, send-rest-request WITH parameters, and a dynamic `execute database query`. Literals, `+` and a lone $currentObject are single tokens or need no spacing, so every common case looked correct", "file": "`mdl/visitor/visitor_helpers.go` (`expressionSourceText`), `mdl/visitor/visitor_page_v3.go` (`buildMicroflowArgV3`, `buildParamAssignmentV3`), `mdl/visitor/visitor_microflow_actions.go` (dynamic query, send rest params)", "insight": "**GetText() on an expression context is always a bug** — grep `Expression().*GetText()` / `expr.GetText()` in mdl/visitor; each hit either builds the AST or must use expressionSourceText (whitespace kept, MDL comments stripped). The earlier comment-leak fix moved the six microflow sites to extractExpressionText and missed these four because they lived in page/REST code, not microflow statements: fix a text-extraction defect by searching for the call, not the feature. The tell that hid it: tests used `$currentObject` and `'a' + 'b'`, both immune; a probe with `and` / `if…then` exposed all four at once. Found while writing PROPOSAL_first_class_expressions.md §5.1. Tests `visitor_expression_source_text_test.go`", "refs": ["mendixlabs/mxcli#750"]}

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **Keyword operators were fused in some stored expressions** — a page action's microflow argument `Flag: $a and $b` was stored as `$aand$b`, `if $x then 'a' else 'b'` as `if$xthen'a'else'b'`; the same in `contentparams` values, `send rest request … with (…)` parameters and a dynamic `execute database query`. Those four places took the expression's text without its whitespace; literals, `+` and `$currentObject` were unaffected, which is why it went unnoticed, and `check --references` passed. Measured by decoding the stored units on a Mendix 11.14.0 project. The expression is now stored as written, with MDL comments removed, as the microflow expression sites already did.
 - **An expression property written in brackets was silently dropped** (mendixlabs/mxcli#750) — `dynamicclasses: [ if $currentObject/Featured then 'a' else 'b' ]`, the spelling #750 proposes, parsed as a list that no writer reads: `check` was clean, `exec` said `Created page`, and the widget was stored with no dynamic class. `alter page … set DynamicClasses = [ … ]` said `Altered page` and changed nothing, and a column's `DynamicCellClass` stored the list's text — tokens fused, `[if$x/Ythen'a'else'b']` — as its expression. Measured on a copy of a Mendix 11.14.0 project with the pre-fix binary. `mxcli check` now reports **MDL-WIDGET32** for `DynamicClasses` and `DynamicCellClass` written as a list (no project needed), and ALTER refuses it, so `check -p` reports that too. Write the expression quoted.
 - **`describe odata client` lost a quote level on a literal credential** — Studio Pro stores a literal user name as the expression `'abc'`, quotes included. `describe` printed `HttpUsername: 'abc'`, and re-executing that output stored `abc`, an identifier. `ClientCertificate`, header keys, `Version`, `MetadataUrl` and `Folder` were printed unescaped and did not re-parse when they held a quote. Every value is now quoted so a re-exec stores exactly what was read; measured against a Studio Pro-authored client decoded before and after a round trip.
 - **An OData client's proxy constant written `@Module.Const` was stored with the `@`** — `ProxyHost` / `ProxyPort` / `ProxyUsername` / `ProxyPassword` are by-name references to a constant, and Studio Pro stores the bare name (with `ProxyType: Override`). `"@Module.Const"` named no constant, so the proxy resolved to nothing. `create`, `create or modify` and `alter` now store the bare name for the bare, `@` and quoted-`@` spellings. The constant may be a String or an Integer.

docs/11-proposals/PROPOSAL_first_class_expressions.md

@@ -211,8 +211,10 @@ proposals compose rather than compete.
    tokens *without* the hidden-channel whitespace — `if $x then 'a' else ''`
    becomes `if$xthen'a'else''`. Literals survive (one token each); keywords and
    operators fuse. Every new slot must go through `buildExpression` →
-   `expressionToString`, never `GetText()`. Whether the microflow-argument path
-   is live-broken for `if`-expressions is untested.
+   `expressionToString`, never `GetText()`. *(Resolved 2026-09-25: it was live —
+   page microflow arguments, contentparams, send-rest-request parameters and a
+   dynamic database query stored the fused text. All four now use
+   `expressionSourceText`.)*
 
 2. **Does `expressionToString` round-trip Studio Pro's spelling?** A stored
    `if $currentObject/Featured then 'x' else ''` re-emitted through

mdl-examples/bug-tests/expression-text-keyword-operators.mdl

@@ -0,0 +1,28 @@
+-- An expression stored as TEXT lost its whitespace, fusing keyword operators.
+--
+-- Four places took an expression's text with ANTLR's GetText(), which joins the
+-- tokens without the whitespace between them. Literals, `+` and a lone
+-- $currentObject survived, so the common cases looked fine; a keyword operator
+-- fused with its neighbours and that text was written into the model. Measured
+-- on a copy of ako/TestApp, pre-fix:
+--
+--   action: microflow GT.ACT(Flag: true and false, …)   -> "trueandfalse"
+--   action: microflow GT.ACT(Mode: if true then 'a' else 'b')
+--                                                      -> "iftruethen'a'else'b'"
+--   send rest request … with ($OrderId = if $x then $id else 'none')
+--                                                      -> "if$xthen$idelse'none'"
+--
+-- `check -p --references` passed on all of it. Fix: expressionSourceText, the
+-- same whitespace-preserving, comment-stripping extraction the microflow
+-- expression sites already use. Also: contentparams values and a dynamic
+-- `execute database query`.
+
+create microflow BugExpr.ACT ($Flag: boolean, $Mode: string) begin
+end;
+/
+
+create page BugExpr.P (title: 'P', layout: Atlas_Core.Atlas_Default) {
+  actionbutton b (caption: 'Go', action: microflow BugExpr.ACT(Flag: true and false, Mode: if true then 'a' else 'b'))
+  dynamictext t (content: 'x {1}', contentparams: [{1} = if true then 'a' else 'b'])
+}
+/

mdl/visitor/visitor_expression_source_text_test.go

@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package visitor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+)
+
+// Four places took an expression's text with ANTLR's GetText(), which joins the
+// tokens WITHOUT the whitespace between them. Literals survive (one token each)
+// and so do operators like `+`, so `'a' + 'b'` and `$currentObject` looked fine
+// — but a keyword operator fuses with its neighbours: `$a and $b` became
+// `$aand$b`, `if $x then 'a' else 'b'` became `if$xthen'a'else'b'`. The fused
+// text was written into the model as the expression (measured on a copy of
+// ako/TestApp: a page action's microflow argument stored "trueandfalse", a REST
+// call parameter "if$xthen$idelse'none'"), while check -p --references passed.
+//
+// Same class as the MDL-comment leak (stripMDLComments): those six microflow
+// sites were moved to extractExpressionText, and these four were missed.
+
+func buildExprSrc(t *testing.T, src string) *ast.Program {
+	t.Helper()
+	prog, errs := Build(src)
+	if len(errs) > 0 {
+		t.Fatalf("parse %q: %v", src, errs)
+	}
+	return prog
+}
+
+func TestExpressionSourceText_PageMicroflowArguments(t *testing.T) {
+	prog := buildExprSrc(t, `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) {
+  actionbutton b (caption: 'Go', action: microflow M.ACT(Flag: $a and $b, Mode: if $x then 'a' else 'b', Obj: $currentObject))
+}`)
+	btn := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0]
+	act, ok := btn.Properties["Action"].(*ast.ActionV3)
+	if !ok {
+		t.Fatalf("no action on the button: %#v", btn.Properties)
+	}
+	want := map[string]string{"Flag": "$a and $b", "Mode": "if $x then 'a' else 'b'", "Obj": "$currentObject"}
+	for _, a := range act.Args {
+		if a.Value != want[a.Name] {
+			t.Errorf("argument %s stored %q, want %q", a.Name, a.Value, want[a.Name])
+		}
+	}
+}
+
+func TestExpressionSourceText_ContentParams(t *testing.T) {
+	prog := buildExprSrc(t, `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) {
+  dynamictext t (content: 'x {1}', contentparams: [{1} = if $x then 'a' else 'b'])
+}`)
+	w := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0]
+	params := w.GetContentParams()
+	if len(params) != 1 || params[0].Value != "if $x then 'a' else 'b'" {
+		t.Errorf("contentparams stored %#v, want the expression with its spacing", params)
+	}
+}
+
+func TestExpressionSourceText_SendRestRequestParameters(t *testing.T) {
+	prog := buildExprSrc(t, `create microflow M.F ($x: boolean, $a: boolean, $b: boolean) begin
+  send rest request M.C.Op with ($Id = if $x then 1 else 2, $Flag = $a and $b);
+end;`)
+	mf := prog.Statements[0].(*ast.CreateMicroflowStmt)
+	var got []ast.SendRestParamDef
+	for _, s := range mf.Body {
+		if r, ok := s.(*ast.SendRestRequestStmt); ok {
+			got = r.Parameters
+		}
+	}
+	want := map[string]string{"Id": "if $x then 1 else 2", "Flag": "$a and $b"}
+	if len(got) != 2 {
+		t.Fatalf("parameters = %#v", got)
+	}
+	for _, p := range got {
+		if p.Expression != want[p.Name] {
+			t.Errorf("parameter %s stored %q, want %q", p.Name, p.Expression, want[p.Name])
+		}
+	}
+}
+
+func TestExpressionSourceText_DynamicDatabaseQuery(t *testing.T) {
+	prog := buildExprSrc(t, `create microflow M.F ($x: boolean) begin
+  $r = execute database query M.C.Q dynamic if $x then 'select 1' else 'select 2';
+end;`)
+	mf := prog.Statements[0].(*ast.CreateMicroflowStmt)
+	for _, s := range mf.Body {
+		if q, ok := s.(*ast.ExecuteDatabaseQueryStmt); ok {
+			if q.DynamicQuery != "if $x then 'select 1' else 'select 2'" {
+				t.Errorf("dynamic query stored %q, want the expression with its spacing", q.DynamicQuery)
+			}
+			return
+		}
+	}
+	t.Fatal("no execute database query statement")
+}
+
+// Control: a single-token value was never affected, and must stay exactly as is.
+// And an MDL comment between operands must not end up inside the expression.
+func TestExpressionSourceText_ControlsAndComments(t *testing.T) {
+	prog := buildExprSrc(t, `create page M.P (title: 'P', layout: Atlas_Core.Atlas_Default) {
+  actionbutton b (caption: 'Go', action: microflow M.ACT(Obj: $currentObject, N: 'a' + 'b', C: $a -- why
+    and $b))
+}`)
+	act := prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0].Properties["Action"].(*ast.ActionV3)
+	want := map[string]string{"Obj": "$currentObject", "N": "'a' + 'b'", "C": "$a \n    and $b"}
+	for _, a := range act.Args {
+		if a.Name == "C" {
+			v, _ := a.Value.(string)
+			if v == "$aand$b" || v == "" || containsComment(v) {
+				t.Errorf("argument C stored %q: fused or carrying the comment", a.Value)
+			}
+			continue
+		}
+		if a.Value != want[a.Name] {
+			t.Errorf("argument %s stored %q, want %q", a.Name, a.Value, want[a.Name])
+		}
+	}
+}
+
+func containsComment(s string) bool {
+	for i := 0; i+1 < len(s); i++ {
+		if s[i] == '-' && s[i+1] == '-' {
+			return true
+		}
+	}
+	return false
+}

mdl/visitor/visitor_helpers.go

@@ -755,3 +755,21 @@ func buildErrorMessage(ctx parser.IErrorMessageClauseContext) string {
 	}
 	return unquoteString(emc.STRING_LITERAL().GetText())
 }
+
+// expressionSourceText is an expression as the author wrote it — whitespace kept,
+// MDL comments removed — for the places that store an expression as TEXT rather
+// than building its AST. Never use ctx.GetText() for this: it joins the tokens
+// without the whitespace between them, so literals and `+` survive but a keyword
+// operator fuses with its neighbours (`$a and $b` -> `$aand$b`, `if $x then 'a'`
+// -> `if$xthen'a'`), and that fused text is what reaches the model.
+func expressionSourceText(expr parser.IExpressionContext) string {
+	if expr == nil {
+		return ""
+	}
+	if prc, ok := expr.(antlr.ParserRuleContext); ok {
+		if source := strings.TrimSpace(extractExpressionText(prc)); source != "" {
+			return source
+		}
+	}
+	return expr.GetText()
+}

mdl/visitor/visitor_microflow_actions.go

@@ -624,7 +624,7 @@ func buildExecuteDatabaseQueryStatement(ctx parser.IExecuteDatabaseQueryStatemen
 		} else if ds := execCtx.DOLLAR_STRING(); ds != nil {
 			stmt.DynamicQuery = unquoteDollarString(ds.GetText())
 		} else if expr := execCtx.Expression(); expr != nil {
-			stmt.DynamicQuery = expr.GetText()
+			stmt.DynamicQuery = expressionSourceText(expr)
 			stmt.DynamicQueryIsExpression = true
 		}
 	}
@@ -1661,7 +1661,7 @@ func buildSendRestRequestStatement(ctx parser.ISendRestRequestStatementContext)
 				param.Name = strings.TrimPrefix(v.GetText(), "$")
 			}
 			if expr := pc.Expression(); expr != nil {
-				param.Expression = expr.GetText()
+				param.Expression = expressionSourceText(expr)
 			}
 			stmt.Parameters = append(stmt.Parameters, param)
 		}