Skip to content

Commit 90f6ca4

Browse files
committed
fix: report XPath true()/false() in filter/find conditions (MDL044)
`filter($L, $currentObject/Flag = true())` passed `mxcli check` without a project, reported an unrelated E014 with one, and failed mx check with CE0117. true()/false() are XPath spellings, valid only inside a retrieve's where [...]. exprcheck now parses them as calls, and MDL044 walks list-operation conditions, offsets and limits, suggesting the bare literal. The reported hang was the catalog load of a large project, not the parser; a timed test guards every malformed shape. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 57259e8 commit 90f6ca4

10 files changed

Lines changed: 296 additions & 5 deletions

File tree

‎.claude/skills/fix-issue/findings/mdl-executor.jsonl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,3 +680,4 @@
680680
{"area": "mdl/executor", "date": "2026-09-23", "symptom": "`describe page` on a page with several galleries emits MDL that mxcli's own `check --references` rejects: `duplicate widget name 'template1' (used 4 times) — Mendix requires unique widget names per page (CE0495)` (and `filter1` when the galleries have filters). The row/col half of the same report was already fixed; this third name survived it", "cause": "`template`/`filter` inside a gallery are CHILD SLOTS of its definition (gallery.def.json childSlots). `applyChildSlots` builds only the block's children into the slot property and discards the block's name, so DESCRIBE has to synthesise `template1` for every gallery. `checkDuplicateWidgetNames` already exempted the parent's object-list containers but not its child-slot containers", "file": "`mdl/executor/validate_page_context.go` (`unstoredContainerKinds`, was `objectListContainerKinds`; `checkDuplicateWidgetNames`); test `mdl/executor/validate_page_slot_containers_test.go`", "insight": "**When a multi-symptom report is marked fixed, re-run every symptom in it, not the first** — the earlier fix covered `row1`/`col1`/`FullName` and its test never mentioned `template1`, so the issue stayed open with a third name nobody had reproduced. Cheapest proof a name is not stored: author a distinctive one (`template contentZebra`), `grep -rl` it in `mprcontents/` (0 files) and DESCRIBE it back (`template1`). Don't exempt the keyword `template` globally — outside a slot-declaring widget, `template` is `buildTemplateV3`, a `Forms$DivContainer` that DOES store its name; resolve it from the enclosing widget's def (both `ChildSlots` and every mode's), and cover the `container <slotName>` spelling that `applyChildSlots` routes by name. Measured on 11.12.2: four galleries each `template template1` → `mx check` 0 errors", "refs": ["mendixlabs/mxcli#978"], "ce": ["CE0495"]}
681681
{"area":"mdl/executor","date":"2026-09-23","symptom":"\"List views are written with `Editable: false` by mxcli, so every input inside a list view renders disabled — even with `editable: Always` on the text box, and even inside a nested data view.\" Valid BSON, `mxcli check` clean, build succeeds; only the rendered app shows it","cause":"Not a writer bug: false IS Mendix's default for Pages$ListView.editable. The 2026-09-06 fix made an explicit `editable: true` reach the document, but nothing told an author who never wrote it that the list view's read-only context overrides the inputs' own `editable:`","file":"`mdl/executor/validate_listview_editable_inputs.go` (MDL-WIDGET31, hooked in `validate_widgets.go`); test `validate_listview_editable_inputs_test.go`; example `mdl-examples/bug-tests/631-listview-inputs-not-editable.mdl`","insight":"**Measure the default before changing it — and the measurement is cheap.** The issue parked \"default to true\" on a Studio Pro comparison it could not run; `npm pack mendixmodelsdk` and reading the class in `src/gen/pages.js` settles it in a minute: `new PrimitiveProperty(ListView, this, \"editable\", false, …)` and `_initializeDefaultProperties` never sets it, so flipping the default would have made mxcli disagree with every Studio Pro-authored list view. A Studio Pro project confirms it (ako/TestApp, 11.14.0): 30 of 32 list views stored false, none holding an input, and the one with inputs set to true by its author; `describe` of that page checks clean, deleting `Editable: true` from it fires MDL-WIDGET31, and describe -> exec preserves it. The fix is therefore a check for the COMBINATION (not-editable list view + input not marked `editable: Never`). Read editability with the SAME accessor the builder uses (`GetBoolProp`): a quoted `editable: 'true'` parses as a string, the builder writes false, and a rule that read it leniently would stay quiet on a page that renders disabled. Stop the descent at a nested list view — its own Editable governs its inputs and it is visited on its own — but NOT at a nested data view, which the reporter measured does not escape the context","refs":["ako/mxcli#631"],"rules":["MDL-WIDGET31"]}
682682
{"area": "mdl/executor", "date": "2026-09-23", "symptom": "`captionparams` on an action button is accepted by `mxcli check` but not written; a container with `onclick` and a dynamic text does the job. Measured: `actionbutton b (caption: 'Save {1}', captionparams: [{1} = Title])` in a dataview wrote the LITERAL 'Title' (describe: `ContentParams: [{1} = 'Title']`), check --references and mx check both clean; and describe -> exec of any button with params dropped them all -> 4x CE0720 'Place holder index 1 is greater than 0' at mx check.", "cause": "buildButtonV3 carried its own copy of the template-parameter resolver (bare name without `.`/`$` -> quoted literal) instead of the shared buildClientTemplateParams that dynamictext/datagrid columns use; and DESCRIBE emitted a button's params as `ContentParams:`, a key the button builder never read. MDL-WIDGET04's orphan-placeholder check covered dynamictext only, so the round-tripped `{1}` with no parameter reached the build.", "file": "`mdl/executor/cmd_pages_builder_v3_widgets.go` (buildButtonV3 -> buildClientTemplateParams, ContentParams fallback), `mdl/executor/cmd_pages_describe_output.go` (ActionButton emits CaptionParams), `mdl/executor/validate_widgets.go` (validateButtonCaptionPlaceholders)", "insight": "**The report's workaround is the diagnosis**: 'a dynamic text does the job' means the SAME parameter text works on one widget and not another, so diff the two builders before touching grammar or the writer -- the writer was fine (clientTemplateToGen is shared). Two duplicate resolvers had drifted: the shared one gained SourceVariable, toString, association steps and format blocks over a year of fixes; the button copy got none. Do not trust 'check passed' for the literal case -- a literal holding the attribute's name is a valid model, so nothing short of describe (or looking at the rendered button) shows it. The describe->exec round trip is the cheap detector: run it once and the ContentParams/CaptionParams mismatch drops every param and mx check reports CE0720. Keep reading `ContentParams:` on buttons -- scripts dumped before this fix use it. Remaining gap, NOT this bug: `check --references` does not flag an unknown BARE attribute in contentparams/captionparams on either widget (`[{1} = Titel]` passes).", "refs": ["ako/mxcli#632"], "ce": ["CE0720"], "rules": ["MDL-WIDGET04"]}
683+
{"area": "mdl/executor", "date": "2026-09-23", "symptom": "`$R = filter($L, $currentObject/Flag = true())` (or `false()`, or bare `filter($L, true())`) passed `mxcli check` with no project, reported E014 with advice about `empty` with -p, and was reported to hang check; mx check then fails CE0117 'Error(s) in expression'.", "cause": "`true()`/`false()` are XPath's Boolean constants, valid only inside a retrieve's `where [...]`. MDL044's unknown-function walk did not visit list-operation (filter/find) conditions at all, and the exprcheck parser treated `true(` as the literal followed by stray tokens, which surfaced as the unrelated E014. The 'hang' was the 158 MB project's catalog load, not the parser: `timeout 20 mxcli check` on the statement alone returns at once.", "file": "`mdl/exprcheck/unknown_funcs.go` + `parser.go` (true()/false() as a call), `mdl/executor/validate_microflow.go` (MDL044 walks filter/find conditions)", "insight": "Reproduce a reported hang with `timeout 20` against the statement alone BEFORE suspecting the parser: against the real project the time was catalog build, and the grammar returned instantly on every malformed variant (a timed test now guards that). The misleading E014 came from the literal being recognised and the `()` left over; teaching the checker the XPath spelling turns a puzzling error into one naming the fix (`write true without parentheses`). Walking filter/find conditions also made MDL044 catch any unknown call there, which it silently skipped before.", "ce": ["CE0117"], "rules": ["MDL044"]}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
-- The valid spelling of filter-xpath-true-call.fail.mdl: Boolean literals, no
2+
-- parentheses. `mxcli check` is clean and mx check reports no CE0117.
3+
4+
create or modify module FilterTrueProbe;
5+
6+
create or modify persistent entity FilterTrueProbe.Item (
7+
IsActive: Boolean default false
8+
);
9+
10+
create or replace microflow FilterTrueProbe.ActiveItems () returns Boolean
11+
begin
12+
retrieve $L from FilterTrueProbe.Item;
13+
$Active = filter($L, $currentObject/IsActive = true);
14+
$First = find($L, $currentObject/IsActive = false);
15+
return true;
16+
end;
17+
/
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- filter()/find() with `true()` / `false()`: XPath's spelling of the Boolean
2+
-- constants, valid inside a retrieve's `where [...]` and CE0117 "Error(s) in
3+
-- expression" anywhere a microflow expression is stored (mx check 11.12.2).
4+
--
5+
-- `mxcli check mdl-examples/bug-tests/filter-xpath-true-call.fail.mdl` reports
6+
-- MDL044 on each statement below, suggesting the literal `true` / `false`.
7+
-- Before the fix check with no project passed both, and with -p reported E014
8+
-- with advice about `empty`. The reported hang was the 158 MB project's catalog
9+
-- load, not the parser: `timeout 20 mxcli check` on this file returns at once.
10+
11+
create or modify module FilterTrueProbe;
12+
13+
create or modify persistent entity FilterTrueProbe.Item (
14+
IsActive: Boolean default false
15+
);
16+
17+
create or replace microflow FilterTrueProbe.ActiveItems () returns Boolean
18+
begin
19+
retrieve $L from FilterTrueProbe.Item;
20+
$Active = filter($L, $currentObject/IsActive = true());
21+
$First = find($L, $currentObject/IsActive = false());
22+
return true;
23+
end;
24+
/
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package executor
4+
5+
import (
6+
"strings"
7+
"testing"
8+
"time"
9+
10+
"github.com/mendixlabs/mxcli/mdl/ast"
11+
"github.com/mendixlabs/mxcli/mdl/visitor"
12+
)
13+
14+
// TestValidateMicroflow_XPathBooleanCall covers `true()` / `false()` in a
15+
// microflow expression. That is XPath's spelling of the Boolean constants — it is
16+
// valid inside a retrieve's `where [...]`, and CE0117 "Error(s) in expression"
17+
// anywhere a microflow expression is stored (measured with mx check 11.12.2 on a
18+
// filter condition and on a declare). Before the fix `mxcli check` with no project
19+
// passed it, and with -p reported E014 with advice about `empty`, which sent the
20+
// reader looking for a keyword they had not written.
21+
//
22+
// The filter/find condition is covered explicitly: list operations were not
23+
// among the expression sites MDL044 walked at all.
24+
func TestValidateMicroflow_XPathBooleanCall(t *testing.T) {
25+
cases := []struct {
26+
name string
27+
body string
28+
wantMDL bool
29+
}{
30+
{"true() in filter", "$R = filter($L, $currentObject/Flag = true());", true},
31+
{"false() in find", "$R = find($L, $currentObject/Flag = false());", true},
32+
{"bare true() as filter condition", "$R = filter($L, true());", true},
33+
{"true() in declare", "declare $b Boolean = true();", true},
34+
{"true() inside and", "$R = filter($L, $currentObject/Flag = true() and $currentObject != $G);", true},
35+
// Controls: the literal spelling is valid and stays silent.
36+
{"true literal in filter", "$R = filter($L, $currentObject/Flag = true);", false},
37+
{"object compare in filter", "$R = filter($L, $currentObject != $G);", false},
38+
{"true literal in declare", "declare $b Boolean = true;", false},
39+
// An unknown call inside a filter condition is caught too, now that the
40+
// condition is walked.
41+
{"unknown func in filter", "$R = filter($L, isBlank($currentObject/Name));", true},
42+
}
43+
for _, tc := range cases {
44+
t.Run(tc.name, func(t *testing.T) {
45+
src := "create microflow M.F ($L: list of M.E, $G: M.E)\nbegin\n " + tc.body + "\nend;"
46+
ids := mdlIDs(buildMF(t, src))
47+
msg, got := ids["MDL044"]
48+
if got != tc.wantMDL {
49+
t.Fatalf("MDL044 fired=%v, want %v (body %q) all=%v", got, tc.wantMDL, tc.body, ids)
50+
}
51+
if got && strings.Contains(tc.body, "true()") && !strings.Contains(msg, "write `true`") {
52+
t.Errorf("suggestion should name the literal spelling, got %q", msg)
53+
}
54+
})
55+
}
56+
}
57+
58+
// TestFilterPredicateParsingTerminates guards the report that one `filter(...,
59+
// true…)` variant hung `mxcli check`. Every malformed shape of the statement
60+
// must come back — with errors or without — well inside the deadline, through
61+
// both the MDL visitor and the validator. A parser must never hang.
62+
func TestFilterPredicateParsingTerminates(t *testing.T) {
63+
bodies := []string{
64+
"$R = filter($L, true;",
65+
"$R = filter($L, true));",
66+
"$R = filter($L, true true);",
67+
"$R = filter($L, );",
68+
"$R = filter($L, = true);",
69+
"$R = filter($L, $currentObject != );",
70+
"$R = filter($L, $currentObject/Flag = true()",
71+
"$R = filter($L, true) and true;",
72+
"$R = filter($L, true or);",
73+
"$R = filter($L, (true);",
74+
"$R = filter($L, true() = true());",
75+
"$R = filter($L, true()()());",
76+
"$R = filter($L, true(true));",
77+
"$R = filter($L, if true() then true() else false());",
78+
"$R = filter($L, [Flag = true()]);",
79+
"$R = filter($L, true",
80+
"$R = filter($L, true(",
81+
}
82+
for _, b := range bodies {
83+
src := "create microflow M.F ($L: list of M.E, $G: M.E)\nbegin\n " + b + "\nend;"
84+
done := make(chan struct{})
85+
go func() {
86+
defer close(done)
87+
prog, _ := visitor.Build(src)
88+
if prog == nil {
89+
return
90+
}
91+
for _, s := range prog.Statements {
92+
if mf, ok := s.(*ast.CreateMicroflowStmt); ok {
93+
_ = ValidateMicroflow(mf)
94+
}
95+
}
96+
}()
97+
select {
98+
case <-done:
99+
case <-time.After(5 * time.Second):
100+
t.Fatalf("parsing/validating did not terminate within 5s: %q", b)
101+
}
102+
}
103+
}

‎mdl/executor/validate_microflow.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,14 @@ func (v *microflowValidator) checkStmtExprFunctions(s ast.MicroflowStatement) {
503503
for _, ch := range stmt.Changes {
504504
v.checkExprFunctions(fmt.Sprintf("change '%s' attribute '%s'", stmt.Variable, ch.Attribute), ch.Value)
505505
}
506+
case *ast.ListOperationStmt:
507+
// A filter/find predicate and a range's offset/limit are microflow
508+
// expressions as well; an unknown call or XPath's `true()` there fails
509+
// CE0117 exactly as it does in a declare, but was never walked.
510+
label := fmt.Sprintf("list operation '$%s'", stmt.OutputVariable)
511+
v.checkExprFunctions(label+" condition", stmt.Condition)
512+
v.checkExprFunctions(label+" offset", stmt.OffsetExpr)
513+
v.checkExprFunctions(label+" limit", stmt.LimitExpr)
506514
case *ast.LogStmt:
507515
// The #1033 repro put the unknown call in a log message, which was never
508516
// walked — in a microflow either.
@@ -525,7 +533,13 @@ func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expressio
525533
}
526534
for _, u := range exprcheck.UnknownFunctionCalls(src) {
527535
var suggestion string
528-
if mendixAggregateFuncs[strings.ToLower(u.Name)] {
536+
if lname := strings.ToLower(u.Name); lname == "true" || lname == "false" {
537+
// XPath spells the literals as calls (valid inside a retrieve's
538+
// where [...]); a microflow expression does not.
539+
suggestion = fmt.Sprintf(
540+
"'%s()' is XPath's spelling (valid only in a retrieve's where [...]); in a microflow expression write `true` / `false` without parentheses.",
541+
lname)
542+
} else if mendixAggregateFuncs[strings.ToLower(u.Name)] {
529543
// count/sum/average/minimum/maximum are aggregate ACTIVITIES, not
530544
// expression functions — a did-you-mean against an unrelated math
531545
// function (e.g. count→round) sends the author the wrong way. Tell them

‎mdl/exprcheck/ast.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ type NumberLit struct {
3131
type BoolLit struct {
3232
baseNode
3333
Value bool
34+
// Called is set when the literal was written with XPath's call spelling,
35+
// `true()` / `false()`. The parser accepts it so the rest of the expression
36+
// still parses (it used to stop at the '(' and report E014, whose advice is
37+
// about `empty`), and UnknownFunctionCalls reports it, because in a microflow
38+
// expression it is CE0117 "Error(s) in expression".
39+
Called bool
3440
}
3541

3642
type EmptyExpr struct{ baseNode }

‎mdl/exprcheck/parser.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -439,10 +439,18 @@ func parseIdentLed(s *Stream, ctx Context) (RobustExpr, []Hint) {
439439
t := s.Consume()
440440
name := t.Text
441441
switch strings.ToLower(name) {
442-
case "true":
443-
return &BoolLit{baseNode: baseNode{P: t.Pos}, Value: true}, nil
444-
case "false":
445-
return &BoolLit{baseNode: baseNode{P: t.Pos}, Value: false}, nil
442+
case "true", "false":
443+
lit := &BoolLit{baseNode: baseNode{P: t.Pos}, Value: strings.EqualFold(name, "true")}
444+
// `true()` / `false()` is XPath's spelling. Consume an empty argument
445+
// list so the expression around it keeps parsing; the literal is marked
446+
// so UnknownFunctionCalls (MDL044) can report it. Only an EMPTY list is
447+
// taken: `true(x)` is left for the trailing-token check.
448+
if s.Peek().Kind == TokLParen && s.PeekAt(1).Kind == TokRParen {
449+
s.Consume()
450+
s.Consume()
451+
lit.Called = true
452+
}
453+
return lit, nil
446454
case "empty":
447455
return &EmptyExpr{baseNode: baseNode{P: t.Pos}}, nil
448456
case "null":

‎mdl/exprcheck/stream.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,16 @@ func (s *Stream) Consume() Token {
2929
return t
3030
}
3131

32+
// PeekAt returns the token n positions ahead of the cursor (PeekAt(0) == Peek()).
33+
func (s *Stream) PeekAt(n int) Token {
34+
if i := s.pos + n; i >= 0 && i < len(s.toks) {
35+
return s.toks[i]
36+
}
37+
if n := len(s.toks); n > 0 {
38+
return Token{Kind: TokEOF, Pos: s.toks[n-1].Pos}
39+
}
40+
return Token{Kind: TokEOF}
41+
}
42+
3243
func (s *Stream) Mark() int { return s.pos }
3344
func (s *Stream) Reset(mark int) { s.pos = mark }

‎mdl/exprcheck/unknown_funcs.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,45 @@ func UnknownFunctionCalls(src string) []FuncRef {
4444
Suggestion: nearestFunc(c.Name),
4545
})
4646
})
47+
// `true()` / `false()` are XPath's spelling of the boolean literals. The
48+
// parser keeps them as a BoolLit (so the rest of the tree types correctly),
49+
// but a microflow expression rejects the call form with CE0117 — measured
50+
// with mx check 11.12.2 on a filter predicate and a declare.
51+
walkBoolCalls(root, func(b *BoolLit) {
52+
name := "false"
53+
if b.Value {
54+
name = "true"
55+
}
56+
out = append(out, FuncRef{Name: name, Line: b.Pos().Line, Column: b.Pos().Column})
57+
})
4758
return out
4859
}
4960

61+
// walkBoolCalls invokes fn for every BoolLit written in call form (`true()`).
62+
func walkBoolCalls(e RobustExpr, fn func(*BoolLit)) {
63+
switch n := e.(type) {
64+
case *BoolLit:
65+
if n.Called {
66+
fn(n)
67+
}
68+
case *CallExpr:
69+
for _, a := range n.Args {
70+
walkBoolCalls(a, fn)
71+
}
72+
case *BinExpr:
73+
walkBoolCalls(n.L, fn)
74+
walkBoolCalls(n.R, fn)
75+
case *UnaryExpr:
76+
walkBoolCalls(n.Operand, fn)
77+
case *ParenExpr:
78+
walkBoolCalls(n.Inner, fn)
79+
case *IfThenElseExpr:
80+
walkBoolCalls(n.Cond, fn)
81+
walkBoolCalls(n.Then, fn)
82+
walkBoolCalls(n.Else, fn)
83+
}
84+
}
85+
5086
// walkCalls invokes fn for every CallExpr in the tree (depth-first).
5187
func walkCalls(e RobustExpr, fn func(*CallExpr)) {
5288
switch n := e.(type) {

0 commit comments

Comments
 (0)