From 73f795ba6374204cebefb08c9b60fd878d95ca30 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:52:56 +0000 Subject: [PATCH 01/14] fix(skills): Vega pack writes JSON numbers locale-independently (#982) formatDecimal(x, '0.00') follows the user's language, so a Dutch user got 12,50 and invalid JSON. Use toString(round(x, 2)), or formatDecimal with a hyphenated locale; explain the silently-ignored underscore tag. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/packs/mendix-vega-charts/SKILL.md | 4 ++-- .claude/skills/packs/mendix-vega-charts/specs/README.md | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.claude/skills/packs/mendix-vega-charts/SKILL.md b/.claude/skills/packs/mendix-vega-charts/SKILL.md index 7f1c356ec4..9d494d0ee0 100644 --- a/.claude/skills/packs/mendix-vega-charts/SKILL.md +++ b/.claude/skills/packs/mendix-vega-charts/SKILL.md @@ -162,12 +162,12 @@ begin set $Json = $Json + $Sep + '{"cat":"' + $R/CategoryName + '"' + ',"m":"' + $Month + '"' - + ',"v":' + formatDecimal($R/Total, '0.00') + '}'; + + ',"v":' + toString(round($R/Total, 2)) + '}'; set $Sep = ','; end loop; ``` -`formatDecimal(x, '0.00')` is the right way to write a number into JSON — it emits a plain decimal with no grouping separators. Never write a value that could be empty into an unquoted position; emit `null` instead, and never emit `0` for "no data" (a zero against a full budget reads as maximally under budget, which is a lie the chart tells convincingly). +`toString(round(x, 2))` is the right way to write a number into JSON: it always uses a `.` decimal point, whatever the user's language, and never switches to an exponent (measured on 11.13: `12.35`, and `0.0000001` stays a plain decimal). **Do not use `formatDecimal(x, '0.00')`** — without a locale argument it formats in the *current user's* language, so a Dutch user gets `12,50` and the chart reports "Data is not valid JSON" while it renders fine for you. If you need `formatDecimal` (fixed trailing zeros), pass an explicit locale with a **hyphenated** tag: `formatDecimal(x, '0.00', 'en-US')`. The underscore form `'nl_NL'`/`'en_US'` is not an error — it is silently ignored and falls back to the user's language, so the bug comes back. Never write a value that could be empty into an unquoted position; emit `null` instead, and never emit `0` for "no data" (a zero against a full budget reads as maximally under budget, which is a lie the chart tells convincingly). ## Verifying without running the app diff --git a/.claude/skills/packs/mendix-vega-charts/specs/README.md b/.claude/skills/packs/mendix-vega-charts/specs/README.md index 2eb3379076..fc3a82f3be 100644 --- a/.claude/skills/packs/mendix-vega-charts/specs/README.md +++ b/.claude/skills/packs/mendix-vega-charts/specs/README.md @@ -41,5 +41,7 @@ out any one and the columns drift apart while every panel insists it is correct. ## Emitting the rows One JSON array of flat objects, built by a microflow over an OQL view entity. Numbers via -`formatDecimal(x, '0.00')`; `null` — never `0` — for "no value"; a discriminator column +`toString(round(x, 2))` — locale-independent, unlike `formatDecimal(x, '0.00')`, which +writes `12,50` for a Dutch user (if you need it, pass a hyphenated locale: +`formatDecimal(x, '0.00', 'en-US')`; `'en_US'` is silently ignored); `null` — never `0` — for "no value"; a discriminator column (`k` above) when one payload feeds several panels. From 997256a6bbf08a32f757f37e659ae9ab87a80cc2 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:52:57 +0000 Subject: [PATCH 02/14] fix(cli): oql and log read the run --local admin port from run-local.json (#982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The run --local hint 'mxcli oql -p …' failed on a non-default --admin-port (cannot connect … localhost:8090), and mxcli log did the same. Both now take the port and password from a live .mxcli/run-local.json for any flag not given. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/cmd_log.go | 23 +++-- cmd/mxcli/devloop_admin.go | 44 ++++++++++ cmd/mxcli/devloop_admin_test.go | 147 ++++++++++++++++++++++++++++++++ cmd/mxcli/docker/runlocal.go | 5 +- cmd/mxcli/docker_oql.go | 19 +++-- 5 files changed, 226 insertions(+), 12 deletions(-) create mode 100644 cmd/mxcli/devloop_admin.go create mode 100644 cmd/mxcli/devloop_admin_test.go diff --git a/cmd/mxcli/cmd_log.go b/cmd/mxcli/cmd_log.go index 74ed9f71e2..a8dcd116e3 100644 --- a/cmd/mxcli/cmd_log.go +++ b/cmd/mxcli/cmd_log.go @@ -44,7 +44,10 @@ Levels, most to least severe: NONE CRITICAL ERROR WARNING INFO DEBUG TRACE This talks to the M2EE admin API, so it needs a running app whose admin port is -reachable — typically one started by 'mxcli run --local'. +reachable — typically one started by 'mxcli run --local'. With -p, the admin port +and password that loop recorded in .mxcli/run-local.json are used for any of +--admin-host/--admin-port/--admin-pass not given, so 'run --local --admin-port' +needs no matching flag here. Examples: # What nodes exist, and at what level @@ -177,11 +180,20 @@ func parseLogSetArgs(args []string) ([]docker.LogNodeLevel, error) { return out, nil } +// logAdminOptions builds the admin connection from the flags, taking the port +// and password of a `mxcli run --local` serving -p for any flag not given +// (ako/mxcli#982) — the flag defaults are only right for a loop on 8090. func logAdminOptions(cmd *cobra.Command) docker.M2EEOptions { host, _ := cmd.Flags().GetString("admin-host") port, _ := cmd.Flags().GetInt("admin-port") pass, _ := cmd.Flags().GetString("admin-pass") - return docker.M2EEOptions{Host: host, Port: port, Token: pass, Direct: true} + projectPath, _ := cmd.Flags().GetString("project") + opts := docker.M2EEOptions{Host: host, Port: port, Token: pass, Direct: true} + return devLoopAdminOptions(projectPath, opts, adminFlagsSet{ + host: cmd.Flags().Changed("admin-host"), + port: cmd.Flags().Changed("admin-port"), + token: cmd.Flags().Changed("admin-pass") || os.Getenv("MXCLI_ADMIN_PASS") != "", + }) } // logConnectionHint names the most likely cause when nothing is listening — but @@ -192,11 +204,10 @@ func logConnectionHint(cmd *cobra.Command, err error) string { if !errors.Is(err, docker.ErrAdminUnreachable) { return "" } - host, _ := cmd.Flags().GetString("admin-host") - port, _ := cmd.Flags().GetInt("admin-port") + opts := logAdminOptions(cmd) return fmt.Sprintf(" Log levels come from a RUNNING app's admin API (%s:%d).\n"+ - " Start one with 'mxcli run --local -p ', or point at another with --admin-host/--admin-port/--admin-pass.\n", - host, port) + " Start one with 'mxcli run --local -p ' (and pass the same -p here), or point at another with --admin-host/--admin-port/--admin-pass.\n", + opts.Host, opts.Port) } func init() { diff --git a/cmd/mxcli/devloop_admin.go b/cmd/mxcli/devloop_admin.go new file mode 100644 index 0000000000..2c4f2ade3d --- /dev/null +++ b/cmd/mxcli/devloop_admin.go @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + +// adminFlagsSet records which admin connection settings the user gave +// explicitly. Those always win; only the rest are taken from a dev loop. +type adminFlagsSet struct { + host, port, token bool +} + +// devLoopAdminOptions points admin-API options at the `mxcli run --local` +// serving projectPath, when one is live and the user did not say otherwise. +// +// The loop records its admin port and password in .mxcli/run-local.json; it is +// the only place a second process can learn them. Without this, `run --local +// --admin-port 8091` printed a `mxcli oql -p …` hint that failed with "cannot +// connect … localhost:8090", and `mxcli log` failed the same way (ako/mxcli#982). +// +// Precedence: explicit flags > live run-local.json > environment > .docker/.env +// > defaults. A handshake whose process is gone is ignored (readDevLoopHandshake +// refuses it), so a crashed loop cannot redirect a query to whatever took its +// port since. A live loop's admin API is loopback HTTP, never docker exec. +func devLoopAdminOptions(projectPath string, opts docker.M2EEOptions, set adminFlagsSet) docker.M2EEOptions { + // An explicit host means "not the loop on this machine"; so do both port + // and password, since nothing would be left to take from the handshake. + if projectPath == "" || set.host || (set.port && set.token) { + return opts + } + hs, err := readDevLoopHandshake(projectPath) + if err != nil || hs.AdminPort == 0 { + return opts + } + if !set.port { + opts.Port = hs.AdminPort + } + if !set.token && hs.AdminPass != "" { + opts.Token = hs.AdminPass + } + opts.Host = "127.0.0.1" + opts.Direct = true + return opts +} diff --git a/cmd/mxcli/devloop_admin_test.go b/cmd/mxcli/devloop_admin_test.go new file mode 100644 index 0000000000..f927d2d246 --- /dev/null +++ b/cmd/mxcli/devloop_admin_test.go @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: Apache-2.0 + +// `mxcli oql` and `mxcli log` find the admin API of a `mxcli run --local` through +// the dev-loop handshake (ako/mxcli#982). Before this, both assumed port 8090, +// so a loop started with --admin-port printed a query hint that failed with +// "cannot connect … localhost:8090". +package main + +import ( + "encoding/base64" + "net" + "net/http" + "net/http/httptest" + "os" + "strconv" + "strings" + "testing" + "time" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + "github.com/spf13/cobra" +) + +// fakeAdminAPI answers the 11.11+ OQL preview route, but only for one password. +func fakeAdminAPI(t *testing.T, pass string) (port int) { + t.Helper() + want := base64.StdEncoding.EncodeToString([]byte(pass)) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-M2EE-Authentication") != want { + w.WriteHeader(http.StatusUnauthorized) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":[{"n":"1"}]}`)) + })) + t.Cleanup(srv.Close) + _, p, err := net.SplitHostPort(srv.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + port, _ = strconv.Atoi(p) + return port +} + +func publishFakeLoop(t *testing.T, project string, port int, pass string) { + t.Helper() + if err := writeDevLoopHandshake(project, devLoopHandshake{ + Project: project, PID: os.Getpid(), AppPort: 18080, + AdminPort: port, AdminPass: pass, Started: time.Now(), + }); err != nil { + t.Fatal(err) + } +} + +// The reported symptom, end to end: the hint `mxcli oql -p ` must reach the +// admin port the loop recorded, with the password it recorded. +func TestOQL_UsesRunLocalAdminPort(t *testing.T) { + t.Setenv("ADMIN_PORT", "") + t.Setenv("M2EE_ADMIN_PASS", "") + p := handshakeProject(t) + port := fakeAdminAPI(t, "loop-secret") + publishFakeLoop(t, p, port, "loop-secret") + + opts := devLoopAdminOptions(p, docker.M2EEOptions{ProjectPath: p}, adminFlagsSet{}) + res, err := docker.ExecuteOQL(docker.OQLOptions{ + Host: opts.Host, Port: opts.Port, Token: opts.Token, + ProjectPath: opts.ProjectPath, Direct: opts.Direct, + }, "SELECT 1") + if err != nil { + t.Fatalf("oql against the run --local admin port: %v", err) + } + if len(res.Rows) != 1 { + t.Fatalf("rows = %d, want 1", len(res.Rows)) + } +} + +// Control: without a handshake the defaults stand, so the test above is +// measuring the handshake and not something else that happens to work. +func TestDevLoopAdminOptions_NoHandshakeKeepsDefaults(t *testing.T) { + p := handshakeProject(t) + in := docker.M2EEOptions{Host: "127.0.0.1", Port: 8090, Token: "x", Direct: true} + got := devLoopAdminOptions(p, in, adminFlagsSet{}) + if got != in { + t.Fatalf("no handshake must leave options alone: got %+v", got) + } +} + +// Explicit flags win over the handshake. +func TestDevLoopAdminOptions_FlagsWin(t *testing.T) { + p := handshakeProject(t) + publishFakeLoop(t, p, 18091, "loop-secret") + in := docker.M2EEOptions{Host: "127.0.0.1", Port: 9999, Token: "mine", Direct: true} + got := devLoopAdminOptions(p, in, adminFlagsSet{port: true, token: true}) + if got.Port != 9999 || got.Token != "mine" { + t.Fatalf("explicit flags overridden: %+v", got) + } + got = devLoopAdminOptions(p, in, adminFlagsSet{}) + if got.Port != 18091 || got.Token != "loop-secret" || !got.Direct { + t.Fatalf("handshake not applied: %+v", got) + } +} + +// A handshake left behind by a dead loop is ignored, not trusted. +func TestDevLoopAdminOptions_StaleHandshakeIgnored(t *testing.T) { + p := handshakeProject(t) + if err := writeDevLoopHandshake(p, devLoopHandshake{PID: 1 << 30, AdminPort: 18091, AdminPass: "s"}); err != nil { + t.Fatal(err) + } + in := docker.M2EEOptions{Port: 8090} + if got := devLoopAdminOptions(p, in, adminFlagsSet{}); got.Port != 8090 { + t.Fatalf("stale handshake used: %+v", got) + } +} + +// `mxcli log` goes through the same resolution: the options it builds from its +// flags must pick up the loop's port when --admin-port was not given. +func TestLogAdminOptions_UsesRunLocalAdminPort(t *testing.T) { + p := handshakeProject(t) + publishFakeLoop(t, p, 18092, "loop-secret") + cmd := newLogFlagsCmd() + if err := cmd.ParseFlags([]string{"-p", p}); err != nil { + t.Fatal(err) + } + got := logAdminOptions(cmd) + if got.Port != 18092 || got.Token != "loop-secret" { + t.Fatalf("log ignores run-local.json: %+v", got) + } + cmd = newLogFlagsCmd() + if err := cmd.ParseFlags([]string{"-p", p, "--admin-port", "9999"}); err != nil { + t.Fatal(err) + } + if got := logAdminOptions(cmd); got.Port != 9999 { + t.Fatalf("--admin-port overridden: %+v", got) + } + if hint := logConnectionHint(cmd, docker.ErrAdminUnreachable); !strings.Contains(hint, ":9999") { + t.Fatalf("hint names the wrong port: %q", hint) + } +} + +func newLogFlagsCmd() *cobra.Command { + c := &cobra.Command{Use: "list"} + c.Flags().StringP("project", "p", "", "") + c.Flags().String("admin-host", "127.0.0.1", "") + c.Flags().Int("admin-port", 8090, "") + c.Flags().String("admin-pass", "mxcli-local-dev", "") + return c +} diff --git a/cmd/mxcli/docker/runlocal.go b/cmd/mxcli/docker/runlocal.go index 4490daec43..1c133e688b 100644 --- a/cmd/mxcli/docker/runlocal.go +++ b/cmd/mxcli/docker/runlocal.go @@ -825,8 +825,11 @@ func RunLocal(opts LocalRunOptions) error { // The local runtime boots with the live-preview dev flags (see // LocalRuntimeOptions.jvmArgs), so `mxcli oql` can query it directly — and it // now defaults to the local admin password, so no M2EE_ADMIN_PASS is needed - // (findings #36). + // (findings #36). Neither hint names the admin port: both commands read it, + // and the password, from the .mxcli/run-local.json that OnReady just + // published, so a non-default --admin-port needs no flag (ako/mxcli#982). fmt.Fprintf(w, "Query data: mxcli oql -p %s \"SELECT ...\"\n", opts.ProjectPath) + fmt.Fprintf(w, "Log levels: mxcli log list -p %s\n", opts.ProjectPath) if runtimeLog != "" { fmt.Fprintf(w, "Runtime log: %s\n", runtimeLog) } diff --git a/cmd/mxcli/docker_oql.go b/cmd/mxcli/docker_oql.go index 5ac350f6a9..799b410247 100644 --- a/cmd/mxcli/docker_oql.go +++ b/cmd/mxcli/docker_oql.go @@ -20,7 +20,10 @@ By default, when -p is set, the request is routed through "docker compose exec" to reach the container's admin API (which binds to localhost inside the container). Use --direct to bypass docker exec and connect via HTTP directly. -Connection settings are resolved in order: flags > environment variables > .docker/.env > defaults. +Connection settings are resolved in order: flags > the 'mxcli run --local' serving +the project (.mxcli/run-local.json) > environment variables > .docker/.env > defaults. +A live 'run --local' is reached over loopback HTTP on the admin port it recorded, +so a loop started with --admin-port needs no --port here. Examples: # Query with project path (reads .docker/.env for credentials) @@ -42,12 +45,18 @@ Examples: jsonOutput, _ := cmd.Flags().GetBool("json") direct, _ := cmd.Flags().GetBool("direct") + // A `mxcli run --local` serving this project records its admin port and + // password; use them unless the flags say otherwise (ako/mxcli#982). + admin := devLoopAdminOptions(projectPath, + docker.M2EEOptions{Host: host, Port: port, Token: token, ProjectPath: projectPath, Direct: direct}, + adminFlagsSet{host: host != "", port: port != 0, token: token != ""}) + opts := docker.OQLOptions{ - Host: host, - Port: port, - Token: token, + Host: admin.Host, + Port: admin.Port, + Token: admin.Token, ProjectPath: projectPath, - Direct: direct, + Direct: admin.Direct, Stdout: os.Stdout, Stderr: os.Stderr, } From a6a8074c9f7924254f0d36b98b6c8bd7c52ca099 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:53:08 +0000 Subject: [PATCH 03/14] feat(check): MDL-JSONNUM01 notes locale-less formatDecimal in hand-built JSON (#982) Co-Authored-By: Claude Opus 5.5 --- docs-site/src/appendixes/error-messages.md | 21 +++ mdl/executor/validate_json_locale_number.go | 120 ++++++++++++++++++ .../validate_json_locale_number_test.go | 62 +++++++++ mdl/executor/validate_microflow.go | 3 + 4 files changed, 206 insertions(+) create mode 100644 mdl/executor/validate_json_locale_number.go create mode 100644 mdl/executor/validate_json_locale_number_test.go diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index 1662742615..fbecfd008e 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -276,6 +276,27 @@ $n = count $Approved; The same applies to both operands of `union`/`intersect`/`subtract`. +### MDL-JSONNUM01: A locale-dependent number in hand-built JSON + +``` +set '$Json' builds JSON with formatDecimal(…) and no locale: it formats in the user's +language, so a Dutch user gets '12,50' and the JSON is invalid for them only [MDL-JSONNUM01] +``` + +**Cause:** `formatDecimal(x, '0.00')` without a locale argument formats in the *current user's* language. The microflow writes `12.50` for an English user and `12,50` for a Dutch one, so JSON built by concatenation (a chart's data, an API payload) is invalid only for some users — typically not the author. An underscore locale tag (`'nl_NL'`, `'en_US'`) is silently ignored and falls back to the user's language; only the hyphenated form applies. Measured on Mendix 11.13. + +The rule is info and heuristic: it fires when the same `+` concatenation has a string literal containing `{` or `":`. A display string such as `'Total: ' + formatDecimal(…)` is not flagged. + +**Solution:** Use `toString(round(x, 2))`, which always writes a `.` and never an exponent, or pass a hyphenated locale. + +```text +-- WRONG +set $Json = $Json + ',"v":' + formatDecimal($R/Total, '0.00') + '}'; +-- RIGHT +set $Json = $Json + ',"v":' + toString(round($R/Total, 2)) + '}'; +set $Json = $Json + ',"v":' + formatDecimal($R/Total, '0.00', 'en-US') + '}'; +``` + ### Mismatched input ``` diff --git a/mdl/executor/validate_json_locale_number.go b/mdl/executor/validate_json_locale_number.go new file mode 100644 index 0000000000..3213c082c8 --- /dev/null +++ b/mdl/executor/validate_json_locale_number.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// checkLocaleNumberInJSON flags formatDecimal without a locale argument inside a +// string concatenation that builds JSON — MDL-JSONNUM01, ako/mxcli#982. +// +// formatDecimal(x, '0.00') formats in the CURRENT USER's language, so the same +// microflow writes `12.50` for an English user and `12,50` for a Dutch one, and +// the JSON is invalid only for the second ("Data is not valid JSON" in a chart +// widget that works for its author). Measured on 11.13: +// +// formatDecimal(1234.5, '0.00', 'nl-NL') → 1234,50 +// formatDecimal(1234.5, '0.00', 'nl_NL') → the user's default: the underscore +// tag is silently ignored +// toString(round(0.0000001, 2)) → a plain decimal, no exponent +// +// It is a heuristic, so it is info: "builds JSON" means the same `+` chain has a +// string literal containing `{` or `":`, which is how a hand-built JSON object +// looks and an ordinary display string ("Total: " + …) does not. A third +// argument of any value is accepted; checking the tag's spelling is left to the +// suggestion. +func (v *microflowValidator) checkLocaleNumberInJSON(label string, expr ast.Expression) { + if expr == nil { + return + } + reported := false + var walk func(e ast.Expression, inChain bool) + walk = func(e ast.Expression, inChain bool) { + switch n := e.(type) { + case *ast.SourceExpr: + walk(n.Expression, inChain) + case *ast.ParenExpr: + walk(n.Inner, false) + case *ast.BinaryExpr: + if n.Operator == "+" && !inChain && !reported { + var parts []ast.Expression + flattenConcat(n, &parts) + if call := localelessFormatDecimalInJSON(parts); call != nil { + reported = true + v.addViolation("MDL-JSONNUM01", linter.SeverityInfo, + fmt.Sprintf("%s builds JSON with formatDecimal(…) and no locale: it formats in the "+ + "user's language, so a Dutch user gets '12,50' and the JSON is invalid for them only", label), + "Use toString(round(x, 2)) — locale-independent, no exponent — or pass a hyphenated "+ + "locale: formatDecimal(x, '0.00', 'en-US'). An underscore tag ('en_US') is silently ignored.") + } + } + isPlus := n.Operator == "+" + walk(n.Left, isPlus) + walk(n.Right, isPlus) + case *ast.UnaryExpr: + walk(n.Operand, false) + case *ast.FunctionCallExpr: + for _, a := range n.Arguments { + walk(a, false) + } + case *ast.IfThenElseExpr: + walk(n.Condition, false) + walk(n.ThenExpr, false) + walk(n.ElseExpr, false) + } + } + walk(expr, false) +} + +// flattenConcat collects the operands of a `+` chain, looking through the +// SourceExpr wrapper but not through parentheses (a parenthesised `+` may be +// arithmetic). +func flattenConcat(e ast.Expression, out *[]ast.Expression) { + switch n := e.(type) { + case *ast.SourceExpr: + flattenConcat(n.Expression, out) + case *ast.BinaryExpr: + if n.Operator == "+" { + flattenConcat(n.Left, out) + flattenConcat(n.Right, out) + return + } + *out = append(*out, e) + default: + *out = append(*out, e) + } +} + +func localelessFormatDecimalInJSON(parts []ast.Expression) *ast.FunctionCallExpr { + looksJSON := false + var call *ast.FunctionCallExpr + for _, p := range parts { + for { + if s, ok := p.(*ast.SourceExpr); ok { + p = s.Expression + continue + } + break + } + switch n := p.(type) { + case *ast.LiteralExpr: + if s, ok := n.Value.(string); ok && n.Kind == ast.LiteralString && + (strings.Contains(s, "{") || strings.Contains(s, `":`)) { + looksJSON = true + } + case *ast.FunctionCallExpr: + if strings.EqualFold(n.Name, "formatDecimal") && len(n.Arguments) < 3 && call == nil { + call = n + } + } + } + if looksJSON { + return call + } + return nil +} diff --git a/mdl/executor/validate_json_locale_number_test.go b/mdl/executor/validate_json_locale_number_test.go new file mode 100644 index 0000000000..d2f060896e --- /dev/null +++ b/mdl/executor/validate_json_locale_number_test.go @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// jsonNumViolations parses real MDL and returns the MDL-JSONNUM01 hits, going +// through the visitor so the expression shape is the one users actually get. +func jsonNumViolations(t *testing.T, body string) int { + t.Helper() + prog := parseMDL(t, `create or replace microflow Shop.Build($Total: Decimal) returns String +begin + declare $Json String = ''; + `+body+` + return $Json; +end;`) + n := 0 + for _, stmt := range prog.Statements { + mf, ok := stmt.(*ast.CreateMicroflowStmt) + if !ok { + continue + } + for _, v := range ValidateMicroflow(mf) { + if v.RuleID == "MDL-JSONNUM01" { + n++ + } + } + } + return n +} + +// The skill's old idiom (ako/mxcli#982): JSON built by concatenation with a +// locale-less formatDecimal, which writes '12,50' for a Dutch user. +func TestJSONNum_LocalelessFormatDecimalInJSON(t *testing.T) { + for name, body := range map[string]string{ + "object key": `set $Json = $Json + ',"v":' + formatDecimal($Total, '0.00') + '}';`, + "brace only": `set $Json = '{' + formatDecimal($Total, '0.00') + '}';`, + "in declare": `declare $J String = '{"v":' + formatDecimal($Total, '0.00') + '}';`, + } { + if got := jsonNumViolations(t, body); got != 1 { + t.Errorf("%s: %d MDL-JSONNUM01, want 1", name, got) + } + } +} + +// Controls: the fixes, and the same call outside JSON, are not flagged. +func TestJSONNum_NotFlagged(t *testing.T) { + for name, body := range map[string]string{ + "toString(round)": `set $Json = $Json + ',"v":' + toString(round($Total, 2)) + '}';`, + "explicit locale": `set $Json = $Json + ',"v":' + formatDecimal($Total, '0.00', 'en-US') + '}';`, + "display string": `set $Json = 'Total: ' + formatDecimal($Total, '0.00');`, + "no concatenation": `set $Json = formatDecimal($Total, '0.00');`, + } { + if got := jsonNumViolations(t, body); got != 0 { + t.Errorf("%s: %d MDL-JSONNUM01, want 0", name, got) + } + } +} diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index b8c1d24932..3fdf5fd418 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -556,6 +556,9 @@ func (v *microflowValidator) checkStmtExprFunctions(s ast.MicroflowStatement) { // check but fail the build with CE0117. label describes where the expression // appears (e.g. "declare '$r'"). (findings #1) func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expression) { + // The one per-expression hook shared by microflows and nanoflows, so the + // JSON-number check rides along rather than keeping a third list of sites. + v.checkLocaleNumberInJSON(label, expr) src := microflowExprSource(expr) if src == "" { return From ba8406025ab04e9848ac8b7d0b031e9a86ebba8f Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:53:09 +0000 Subject: [PATCH 04/14] docs: changelog and findings for #982 Co-Authored-By: Claude Opus 5.5 --- .claude/skills/fix-issue/findings/cmd-mxcli.jsonl | 1 + .claude/skills/fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 3 +++ 3 files changed, 5 insertions(+) diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl index 76b151ce58..89be86dc8f 100644 --- a/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl +++ b/.claude/skills/fix-issue/findings/cmd-mxcli.jsonl @@ -161,3 +161,4 @@ {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch` (Mendix <= 11.13, rollup bundler): after a while every page change fails with `web client rebuild failed: web client watcher exited` (or `client bundle not served after apply: web client re-bundle: web client build timed out after 5m0s`) and nothing reaches the browser until `run --local` is restarted; after adding an entity the next changes fail with `ENOTDIR: not a directory, stat '.../web/pages/.js/package.json'`", "cause": "watchAndApply held a bare *WebClientWatcher: (1) once it exited nothing restarted it (only recoverMissingPages did), so WaitForRebuild failed every later change; (2) a failed incremental build (rollup's commonjs resolver hitting web/pages mid-rewrite by the serve build) left the watcher erroring and the change was dropped; (3) ensureClientServed's recovery ran a one-shot NODE_ENV=production BuildWebClient in the same web/ dir while the watcher was still running — two rollups on web/dist; (4) the 5m limit was hard-coded and a timeout printed nothing about why", "file": "cmd/mxcli/docker/webclient_supervisor.go, cmd/mxcli/docker/runlocal.go (watchAndApply, ensureClientServed), cmd/mxcli/docker/webclient.go (webClientTimeout, webClientBuildLogTail)", "fix": "bundlerSupervisor owns the bundler: EnsureAlive restarts an exited one (backoff 2s..60s after failed starts), AwaitRebuild retries a failed/aborted incremental rebuild once with a fresh bundler, Rebundle = stop+reap then start (never two). ensureClientServed takes the rebundle func; clientRebundler hands it the supervisor under --watch and the one-shot otherwise. --web-client-timeout / MXCLI_WEB_CLIENT_TIMEOUT; timeout errors append the last 30 lines of deployment/log/web-client-build.log. sessionNotice prints that a restart dropped sessions", "insight": "Killing the runner (`kill `) reproduces the dead-watcher state in seconds — no need to wait for it to die on its own. A fresh bundler is the universal recovery under --watch: its first build is a full bundle of the current source, so it covers missing pages, dangling chunks, a dist/ deleted by Gradle, and a transient incremental failure alike, without a second process on web/dist. Note: exec.Cmd.Wait called a second time concurrently with the reaper did block until exit on this Go version, so the old Stop was not the overlap — the one-shot in ensureClientServed was", "test": "cmd/mxcli/docker/webclient_supervisor_test.go (TestBundlerSupervisor_RestartsExitedBundler, _AwaitRebuildRecovers, _RebundleNeverOverlaps, TestEnsureClientServed_WatchModeRebundleIsExclusive, TestBuildWebClient_TimeoutShowsLogTail); live: 11.13 scratch app, 7 consecutive changes incl. a killed runner"} {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "`run --local --watch`: an `mxcli exec` (or save) made while a change is still building/applying is never built — no `Change detected` follows, the app keeps the previous model, and re-running the script writes nothing (byte-idempotent) so nothing re-triggers", "cause": "watchAndApply set `last = sourceMTime(...)` after every successful apply, under a comment claiming it kept mid-build edits; it did the opposite — the edit's mtime was folded into the baseline, so the next tick saw nothing newer", "file": "cmd/mxcli/docker/runlocal.go (watchAndApply)", "fix": "keep `last` at the settled mtime the build was taken from; the build writes nothing under the watched model/theme source (checked with find -newer during a live run), so this cannot self-trigger", "insight": "Found while reproducing #971 with a script that waited for the first output line of a change instead of its `applied` line — the next exec landed during a 2-minute restart-apply and vanished. Any test of a watch loop should include an edit made DURING a build, not only between builds", "test": "live only (11.13 scratch app, hsqldb): exec an entity add, exec a page change 15s into its build; fixed binary builds it as the next build, the binary with the refresh restored shows no further build after 45s"} {"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "ako/mxcli#970 item 2: `theme create --from design.css` with only a :root block (no dark block) wrote the design's light --mxt-ground/--mxt-ink/--mxt-brand into the base theme's dark mixin, whose other surfaces stayed dark; the first injected token also sat on the `@mixin … {` line, unindented", "cause": "Tokens.forVariant returned the base declarations for EITHER variant, and seedTokens applied it to the alt-palette mixin unconditionally; applyTokens matched `(?m)^(\\s*)name`, and \\s* at ^ swallows the preceding newline (and blank line), so the replacement lost its line break and indent", "file": "`cmd/mxcli/theme/create.go` (seedTokens, Create/CreateResult.UnseededVariant); `cmd/mxcli/theme/tokens.go` (applyTokens, Tokens.declares); `cmd/mxcli/cmd_theme.go` (note)", "fix": "seed the alt mixin only when the design declared a block for that variant; otherwise leave it byte-identical to the base and report UnseededVariant, which the CLI prints as a note; match the indent with [ \\t]* instead of \\s*", "insight": "A base palette is the default variant's palette, not 'both': a token set that does not say which variant it describes must not seed the other one. And under (?m), ^\\s* is not 'leading indentation' — it crosses lines; use [ \\t]*. The control for 'mixin untouched' is the same scaffold with no design at all, compared byte for byte", "test": "`cmd/mxcli/theme/create_variant_test.go` (all three bases, base-only vs variant block, indentation); `cmd/mxcli/cmd_theme_test.go` (TestThemeCreate_NotesTheVariantABaseOnlyDesignDidNotSeed)"} +{"area": "cmd/mxcli", "date": "2026-10-04", "symptom": "ako/mxcli#982 item 2: `run --local --admin-port 8091` printed `Query data: mxcli oql -p app.mpr` and that command failed `cannot connect to Mendix admin API at localhost:8090`; `mxcli log list` failed the same way", "cause": "oql resolved the admin port as flag > ADMIN_PORT env > .docker/.env > 8090 and log used flag defaults 8090/mxcli-local-dev; neither read the .mxcli/run-local.json handshake the loop publishes with its port and password (only `constant set --apply` did)", "file": "`cmd/mxcli/devloop_admin.go` (devLoopAdminOptions); `cmd/mxcli/docker_oql.go`; `cmd/mxcli/cmd_log.go` (logAdminOptions, logConnectionHint); `cmd/mxcli/docker/runlocal.go` (hint)", "fix": "one helper takes port/password from a LIVE handshake for any flag not given (explicit host means 'not this loop'), forces direct loopback HTTP; oql and log both use it; the log hint prints the resolved port", "insight": "A hint printed by the process that knows the port must be runnable without that knowledge: either print the flags or make the consumer read what the producer published. Reading the handshake fixes every consumer at once; the test is an httptest admin API on a random port with a fake run-local.json, controlled by the no-handshake and stale-pid cases", "test": "`cmd/mxcli/devloop_admin_test.go`"} diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 7723d68b23..35f1887a25 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -857,3 +857,4 @@ {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#968 / mendixlabs/mxcli#1263: `datepicker d (DateFormat: Time)` or `DateFormat: Custom, CustomDateFormat: '\u2026'` passes check and exec, mx check 0 errors, but every picker is stored FormattingInfo.DateFormat=Date; describe prints no format, so describe \u2192 exec silently turns a Studio Pro date-time picker into a date-only one. Same drop for a text box's DecimalPrecision/GroupDigits", "cause": "Three hops each dropped it: buildDatePickerV3/buildTextBoxV3 never read the properties, widget_write.go hard-coded newFormattingInfo() on DatePicker and TextBox, and describe never extracted FormattingInfo. Check stayed silent because validateStaticWidgetUnknownProps exempted the dynamic-text format keys (dateformat, customdateformat, \u2026) on EVERY widget type, not just dynamictext", "fix": "pages.DatePicker.FormattingInfo; executor input_formatting.go (inputFormattingInfo + inputFormattingProblems shared by builder and MDL-WIDGET18 check), writer formattingInfoToGen(x.FormattingInfo), describeInputFormatting, pagemutator setWidgetFormattingMut; per-widget allow-list pages.FormattingProperties. Measured: Custom with empty pattern = CE0493; Studio Pro stores CustomDateFormat beside DateFormat DateTime (TestApp WorkflowCommons), so only a pattern with NO DateFormat is refused \u2014 the param-format rule that refused it broke check on describe output", "file": "mdl/executor/input_formatting.go", "insight": "A key exempted from the unknown-property warning must be exempted per widget type: the dynamic-text format keys were skipped on every widget, which turned `DateFormat:` on a date picker (where nothing read it) into a silent drop. Before refusing a cross-field combination, scan Studio Pro-authored units for it \u2014 CustomDateFormat beside DateTime is stored by Studio Pro, and refusing it broke check on describe output.", "test": "mdl/executor/input_formatting_pedapp_test.go, input_formatting_test.go, mdl/backend/modelsdk/widget_formatting_write_test.go"} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 2: `alter page … { set Action = microflow M.X on btn }` with M.X created earlier in the same script failed check (\"microflow not found\") and exec refused the script; the same for nanoflow and show page targets", "cause": "validateAlterSetProperties dry-runs the SET against the stored document, and resolveMicroflow / resolveNanoflowByName / resolvePageRef only know the session cache (createdMicroflows, …) that executing fills — which check never does", "file": "`mdl/executor/validate_alter_set.go` (scriptDeclaresMissing)", "insight": "A dry run of a mutator in check must treat a NotFound for a name the script declares (scriptContext.microflows/nanoflows/pages/snippets) as satisfied, matching on the typed mdlerrors.NotFoundError Kind+Name through errors.As rather than the message. Do not register fake IDs in ctx.Cache instead: exec runs check on the same executor and would resolve to them. Control: an undeclared target still fails", "refs": ["#969"]} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 3: a list view / data grid / gallery with `datasource: $currentObject/M.Assoc` over a single-object association passed check and exec, then mxbuild failed CE8812 \"A grid association path must result in a list\"", "cause": "No rule modelled association multiplicity for list widgets", "file": "`mdl/executor/validate_assoc_list_source.go` (MDL-ASSOCDS01), hooked into attributeScopeValidator.walk", "insight": "Measured 8 shapes x 3 widgets on 11.13.0 and 11.14.0, identical: CE8812 for a Reference followed from its FROM entity (owner Default or Both) and for a Reference with owner Both from the TO entity (one-to-one); the reverse of a default Reference and every ReferenceSet build clean. Judge only those shapes with an exact context entity; skip specializations, self-associations and multi-hop paths. The attribute-scope walk already carries the data context, so hook there", "refs": ["#969"]} +{"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#982 item 1: a chart microflow built JSON with `',\"v\":' + formatDecimal($x, '0.00')` (as the Vega skill pack recommended); a Dutch user got `12,50` and 'Data is not valid JSON', while it worked for the author", "cause": "formatDecimal without a locale formats in the current user's language; an underscore tag ('nl_NL') is silently ignored, only hyphenated tags ('en-US') apply. toString(round(x, 2)) is locale-independent with no exponent (measured 11.13)", "file": "`.claude/skills/packs/mendix-vega-charts/SKILL.md`, `specs/README.md`; `mdl/executor/validate_json_locale_number.go` (MDL-JSONNUM01, hooked in checkExprFunctions)", "fix": "skill uses toString(round(x, 2)) and explains the locale trap; check emits info MDL-JSONNUM01 for a locale-less formatDecimal in a + chain whose string literals contain '{' or '\":'", "insight": "Locale-dependent output passes every test run by the author, whose language is the one that works; the measurement that settles it is the same call under a second locale. The lint heuristic keys on the JSON-looking literal in the same concatenation so display strings stay quiet", "test": "`mdl/executor/validate_json_locale_number_test.go` (positive shapes + controls: toString(round), explicit locale, display string, bare call)"} diff --git a/CHANGELOG.md b/CHANGELOG.md index f408007929..d1b972b27f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`mxcli oql -p` and `mxcli log` reach a `run --local` started with `--admin-port`** (ako/mxcli#982) — both read the admin port and password the loop records in `.mxcli/run-local.json` for any connection flag not given, so the `Query data: mxcli oql -p …` hint `run --local` prints works on a non-default port instead of failing "cannot connect … localhost:8090". `run --local` now also prints a `mxcli log list -p …` hint. Precedence: flags, then a live loop, then environment, `.docker/.env`, defaults; a handshake left by a stopped loop is ignored. +- **The Vega charts skill pack writes JSON numbers with `toString(round(x, 2))`** (ako/mxcli#982) — it recommended `formatDecimal(x, '0.00')`, which follows the user's language: a Dutch user got `12,50` and "Data is not valid JSON". Measured on 11.13: `toString(round(…))` is locale-independent and has no exponent; `formatDecimal(x, '0.00', 'en-US')` also works, but an underscore tag (`'en_US'`) is silently ignored. - **`run --local --watch` no longer loses an edit made while a change is being applied** — after each apply the loop moved its change baseline to the source's current mtime, so an `exec` or save that landed during the build (easily, while a structural change restarts the runtime) was never rebuilt: the app kept serving the previous model with nothing reported. The baseline now stays at the mtime the build was taken from, and the edit is built on the next poll. - **`run --local --watch` keeps its web client bundler alive and never runs two** (ako/mxcli#971) — a bundler that exited stayed dead, so every later page change failed with `web client watcher exited` and only restarting `run --local` recovered (reproduced on Mendix 11.13 by killing the runner: two changes in a row failed). It is now restarted on the next change, with backoff when it cannot start. An incremental rebuild that fails — measured after adding an entity: `ENOTDIR … web/pages/.js/package.json`, and the next change failed the same way — is retried once with a fresh bundler. A recovery re-bundle (a missing page, a dangling chunk, or `/dist/index.js` gone after a runtime restart) replaces the bundler instead of running a one-shot production bundle beside it in the same `web/` directory. The bundle-build limit is configurable (`--web-client-timeout`, or `MXCLI_WEB_CLIENT_TIMEOUT`; default 5m), and a timeout prints the tail of `deployment/log/web-client-build.log`. A change that restarts the runtime now says that browser sessions were dropped. - **`theme create --from` no longer paints a dark palette with a design's light colours** (ako/mxcli#970) — a design that declares only a base palette describes one variant, but its values were also written into the base theme's alternate-variant mixin: `--mxt-ground: #f7f9fb` and `--mxt-ink: #1b2733` landed in the dark mixin while its surfaces stayed dark, an unreadable mix. With no block for that variant the mixin is now left exactly as the base theme ships it, and `create` prints a note naming it and how to seed it (a `prefers-color-scheme: dark` block, or `light` for a dark-first base). Also fixed: the first rewritten token of a block landed on the `@mixin … {` line, unindented, and a rewritten token after a blank line swallowed the blank line. Verified with mxbuild 11.14.0's bundled sass on signal, console and ledger, with and without a variant block. @@ -146,6 +148,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **`check` notes a locale-less `formatDecimal` in hand-built JSON** (ako/mxcli#982) — **MDL-JSONNUM01** (info) for `formatDecimal(x, '0.00')` inside a `+` concatenation whose string literals contain `{` or `":`, which writes `12,50` for a Dutch user. Use `toString(round(x, 2))` or pass a hyphenated locale. A display string (`'Total: ' + formatDecimal(…)`) is not flagged. - **`mxcli fix hashes` verifies and repairs the MPR v2 ContentsHash index** (ako/mxcli#972) — every `Unit.ContentsHash` in the `.mpr` is compared with `base64(SHA-256)` of its `.mxunit`, and mismatches, missing files and orphan files are reported; the command exits 1 when any remain. `--repair` rewrites the mismatched hashes from the files in one transaction, under the same Studio Pro open-project guard as every write. Restoring a unit with `git checkout` leaves the index stale and `mx check` does not notice; measured on a TestApp copy, an entity added by `exec` and then reverted with a byte-level restore is reported as one `DomainModels$DomainModel` mismatch, and after `--repair` the verify is clean and `mx check` reports 0 errors. `exec` and `docker check` print a one-line warning when the index disagrees (about 0.2 s on 900 units). An MPR v1 project has no index, and the command says so. - **Warnings for git states that crash Studio Pro** (ako/mxcli#972) — Studio Pro 11.13 fails to open a project ("Unable to find 'system' property in 'system'") when its git branch has no upstream or git reports "detected dubious ownership". `docker check`, `run --local` and the new `mxcli diag -p app.mpr` project section warn with the remedy (`git push -u origin `; `git config --global --add safe.directory ` on the Studio Pro machine). Never fatal, and silent outside a git repository, on a detached HEAD, under `CI`, or with `MXCLI_NO_GIT_WARNINGS=1`. See the new "Working Outside Studio Pro" page. - **`create translations … without marketplace`, and an unscoped translations run warns when it writes into Marketplace modules** (ako/mxcli#970) — without `in `, `create [or modify|or replace] translations` reaches the whole project, Marketplace modules and their Atlas page templates and building blocks included; on TestApp `'Cancel' as 'Annuleren'` changed 35 Marketplace documents of 38. A module update replaces those modules, so the translations are lost at the next update and show up as unexpected diffs until then. The run now warns with the count per module and how many are templates or building blocks. `without marketplace` (also on `describe translations`, which emits it) keeps the run, and an `or replace` deletion, to the app's own modules and names the file's entries it left alone. The default is unchanged: what an existing script writes does not change underneath it (ADR-0011). From 4811f3271d80223fdfd7f5eaba7ca99cbc16d4d2 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 18:55:55 +0000 Subject: [PATCH 05/14] fix(navigation): keep a menu item action MDL cannot express on rewrite (#980) describe navigation printed a nanoflow / open-link / create-object menu item with no OnClick, and exec of that description stored Forms$NoAction: TestApp's 'Item 4' lost its nanoflow call at exit 0. The reader now keeps each item's stored action bytes; create or modify navigation and create or modify menu pair script items with stored ones by caption path and carry a stored action describe cannot print when the item states none, reporting each carried action. Describe flags such an action with a comment. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + mdl/backend/modelsdk/menu_write.go | 7 ++ .../modelsdk/navigation_keep_action_test.go | 35 +++++++ mdl/backend/modelsdk/navigation_read.go | 5 + mdl/backend/modelsdk/navigation_write.go | 9 ++ mdl/executor/cmd_menus.go | 35 ++++++- mdl/executor/cmd_menus_mock_test.go | 2 +- mdl/executor/cmd_navigation.go | 92 ++++++++++++++++++- .../cmd_navigation_keep_action_test.go | 90 ++++++++++++++++++ mdl/executor/menu_signout_test.go | 2 +- mdl/types/navigation.go | 16 +++- 12 files changed, 285 insertions(+), 10 deletions(-) create mode 100644 mdl/backend/modelsdk/navigation_keep_action_test.go create mode 100644 mdl/executor/cmd_navigation_keep_action_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 7723d68b23..7ca76a5bf2 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -857,3 +857,4 @@ {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#968 / mendixlabs/mxcli#1263: `datepicker d (DateFormat: Time)` or `DateFormat: Custom, CustomDateFormat: '\u2026'` passes check and exec, mx check 0 errors, but every picker is stored FormattingInfo.DateFormat=Date; describe prints no format, so describe \u2192 exec silently turns a Studio Pro date-time picker into a date-only one. Same drop for a text box's DecimalPrecision/GroupDigits", "cause": "Three hops each dropped it: buildDatePickerV3/buildTextBoxV3 never read the properties, widget_write.go hard-coded newFormattingInfo() on DatePicker and TextBox, and describe never extracted FormattingInfo. Check stayed silent because validateStaticWidgetUnknownProps exempted the dynamic-text format keys (dateformat, customdateformat, \u2026) on EVERY widget type, not just dynamictext", "fix": "pages.DatePicker.FormattingInfo; executor input_formatting.go (inputFormattingInfo + inputFormattingProblems shared by builder and MDL-WIDGET18 check), writer formattingInfoToGen(x.FormattingInfo), describeInputFormatting, pagemutator setWidgetFormattingMut; per-widget allow-list pages.FormattingProperties. Measured: Custom with empty pattern = CE0493; Studio Pro stores CustomDateFormat beside DateFormat DateTime (TestApp WorkflowCommons), so only a pattern with NO DateFormat is refused \u2014 the param-format rule that refused it broke check on describe output", "file": "mdl/executor/input_formatting.go", "insight": "A key exempted from the unknown-property warning must be exempted per widget type: the dynamic-text format keys were skipped on every widget, which turned `DateFormat:` on a date picker (where nothing read it) into a silent drop. Before refusing a cross-field combination, scan Studio Pro-authored units for it \u2014 CustomDateFormat beside DateTime is stored by Studio Pro, and refusing it broke check on describe output.", "test": "mdl/executor/input_formatting_pedapp_test.go, input_formatting_test.go, mdl/backend/modelsdk/widget_formatting_write_test.go"} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 2: `alter page … { set Action = microflow M.X on btn }` with M.X created earlier in the same script failed check (\"microflow not found\") and exec refused the script; the same for nanoflow and show page targets", "cause": "validateAlterSetProperties dry-runs the SET against the stored document, and resolveMicroflow / resolveNanoflowByName / resolvePageRef only know the session cache (createdMicroflows, …) that executing fills — which check never does", "file": "`mdl/executor/validate_alter_set.go` (scriptDeclaresMissing)", "insight": "A dry run of a mutator in check must treat a NotFound for a name the script declares (scriptContext.microflows/nanoflows/pages/snippets) as satisfied, matching on the typed mdlerrors.NotFoundError Kind+Name through errors.As rather than the message. Do not register fake IDs in ctx.Cache instead: exec runs check on the same executor and would resolve to them. Control: an undeclared target still fails", "refs": ["#969"]} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 3: a list view / data grid / gallery with `datasource: $currentObject/M.Assoc` over a single-object association passed check and exec, then mxbuild failed CE8812 \"A grid association path must result in a list\"", "cause": "No rule modelled association multiplicity for list widgets", "file": "`mdl/executor/validate_assoc_list_source.go` (MDL-ASSOCDS01), hooked into attributeScopeValidator.walk", "insight": "Measured 8 shapes x 3 widgets on 11.13.0 and 11.14.0, identical: CE8812 for a Reference followed from its FROM entity (owner Default or Both) and for a Reference with owner Both from the TO entity (one-to-one); the reverse of a default Reference and every ReferenceSet build clean. Judge only those shapes with an exact context entity; skip specializations, self-associations and multi-hop paths. The attribute-scope walk already carries the data context, so hook there", "refs": ["#969"]} +{"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#980: describe navigation printed a menu item whose action was a nanoflow call / open link / create object with no OnClick, and exec of that description stored Forms$NoAction — TestApp's 'Item 4' (Forms$CallNanoflowClientAction) lost its action at exit 0", "cause": "create or modify navigation / menu is a full menu rebuild from the spec; the reader kept only page/microflow/sign-out, so the spec had no way to say 'keep what is stored' and the writer fell through to NoAction", "file": "`mdl/executor/cmd_navigation.go` convertMenuItemDefs (pairs items by caption path, KeepAction), `mdl/executor/cmd_menus.go` menuItemsFromAST, writers `navigation_write.go` navMenuAction / `menu_write.go` menuActionToGen", "insight": "A full-rebuild writer needs the stored element at hand to carry what the language cannot spell — the same carry offline CompatibilityMode already had. Pair by caption path (first unused sibling with that caption); carry only when the script states no action so an explicit action still wins. The reader must keep the raw action bytes (element.Raw()), not a projection of it", "refs": ["#980"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index f408007929..de9145e544 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A navigation or menu rewrite keeps a menu item action it cannot express** (ako/mxcli#980, guard) — an item whose stored action describe could not print (a nanoflow call, open link, create object, …) was described with no action, and re-running that description stored `Forms$NoAction`, silently. The stored action is now carried when the script's item states none, exec says which it kept, and describe flags such an action with a comment. - **`run --local --watch` no longer loses an edit made while a change is being applied** — after each apply the loop moved its change baseline to the source's current mtime, so an `exec` or save that landed during the build (easily, while a structural change restarts the runtime) was never rebuilt: the app kept serving the previous model with nothing reported. The baseline now stays at the mtime the build was taken from, and the edit is built on the next poll. - **`run --local --watch` keeps its web client bundler alive and never runs two** (ako/mxcli#971) — a bundler that exited stayed dead, so every later page change failed with `web client watcher exited` and only restarting `run --local` recovered (reproduced on Mendix 11.13 by killing the runner: two changes in a row failed). It is now restarted on the next change, with backoff when it cannot start. An incremental rebuild that fails — measured after adding an entity: `ENOTDIR … web/pages/.js/package.json`, and the next change failed the same way — is retried once with a fresh bundler. A recovery re-bundle (a missing page, a dangling chunk, or `/dist/index.js` gone after a runtime restart) replaces the bundler instead of running a one-shot production bundle beside it in the same `web/` directory. The bundle-build limit is configurable (`--web-client-timeout`, or `MXCLI_WEB_CLIENT_TIMEOUT`; default 5m), and a timeout prints the tail of `deployment/log/web-client-build.log`. A change that restarts the runtime now says that browser sessions were dropped. - **`theme create --from` no longer paints a dark palette with a design's light colours** (ako/mxcli#970) — a design that declares only a base palette describes one variant, but its values were also written into the base theme's alternate-variant mixin: `--mxt-ground: #f7f9fb` and `--mxt-ink: #1b2733` landed in the dark mixin while its surfaces stayed dark, an unreadable mix. With no block for that variant the mixin is now left exactly as the base theme ships it, and `create` prints a note naming it and how to seed it (a `prefers-color-scheme: dark` block, or `light` for a dark-first base). Also fixed: the first rewritten token of a block landed on the `@mixin … {` line, unindented, and a rewritten token after a blank line swallowed the blank line. Verified with mxbuild 11.14.0's bundled sass on signal, console and ledger, with and without a variant block. diff --git a/mdl/backend/modelsdk/menu_write.go b/mdl/backend/modelsdk/menu_write.go index dde62538e1..43d440c9a9 100644 --- a/mdl/backend/modelsdk/menu_write.go +++ b/mdl/backend/modelsdk/menu_write.go @@ -176,6 +176,13 @@ func menuIconToGen(item *types.NavMenuItem) element.Element { // names; their storage names are what Mendix writes — PageClientAction is // Forms$FormAction, NoClientAction is Forms$NoAction. func menuActionToGen(item *types.NavMenuItem) element.Element { + // An action the script could not state, carried from storage verbatim + // (ako/mxcli#980); the executor sets StoredAction only for that case. + if len(item.StoredAction) > 0 { + if el, err := codec.NewDecoder(codec.DefaultRegistry).Decode(item.StoredAction); err == nil { + return el + } + } switch { case item.Page != "": a := genPages.NewPageClientAction() diff --git a/mdl/backend/modelsdk/navigation_keep_action_test.go b/mdl/backend/modelsdk/navigation_keep_action_test.go new file mode 100644 index 0000000000..a1ef7f22ab --- /dev/null +++ b/mdl/backend/modelsdk/navigation_keep_action_test.go @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson" + + "github.com/mendixlabs/mxcli/mdl/types" +) + +// ako/mxcli#980: a stored action the script could not state is written back +// verbatim. Without KeepAction the writer had only page / microflow / sign out +// to choose from and stored Forms$NoAction — a nanoflow menu item, TestApp's +// 'Item 4', came back dead. +func TestNavMenuAction_WritesAKeptActionVerbatim(t *testing.T) { + stored, err := bson.Marshal(bson.D{ + {Key: "$ID", Value: "keep-me"}, + {Key: "$Type", Value: "Forms$CallNanoflowClientAction"}, + {Key: "Nanoflow", Value: "MyFirstModule.Nanoflow"}, + }) + if err != nil { + t.Fatal(err) + } + got := navMenuAction(types.NavMenuItemSpec{Caption: "Item 4", KeepAction: stored}) + if navGetString(got, "$Type") != "Forms$CallNanoflowClientAction" || navGetString(got, "Nanoflow") != "MyFirstModule.Nanoflow" { + t.Fatalf("kept action not written verbatim: %v", got) + } + // CONTROL: the same item without a kept action is the writer's NoAction — + // the loss this guards against, so the case above is not passing by default. + if got := navMenuAction(types.NavMenuItemSpec{Caption: "Item 4"}); navGetString(got, "$Type") != "Forms$NoAction" { + t.Fatalf("control: an item with no action wrote %v", got) + } +} diff --git a/mdl/backend/modelsdk/navigation_read.go b/mdl/backend/modelsdk/navigation_read.go index b05e9dfc4c..b1d179c4ac 100644 --- a/mdl/backend/modelsdk/navigation_read.go +++ b/mdl/backend/modelsdk/navigation_read.go @@ -297,6 +297,11 @@ func resolveMenuAction(item *types.NavMenuItem, action element.Element) { if action == nil { return } + // The stored document itself, whatever its $Type: what describe renders + // and what a rewrite carries when MDL cannot express it (ako/mxcli#980). + if raw := action.Raw(); len(raw) > 0 { + item.StoredAction = append([]byte(nil), raw...) + } switch a := action.(type) { case *genPages.PageClientAction: item.ActionType = "PageAction" diff --git a/mdl/backend/modelsdk/navigation_write.go b/mdl/backend/modelsdk/navigation_write.go index 891b44dbe5..030f795563 100644 --- a/mdl/backend/modelsdk/navigation_write.go +++ b/mdl/backend/modelsdk/navigation_write.go @@ -356,6 +356,15 @@ func navCaptionBson(text string) bson.D { } func navMenuAction(mi types.NavMenuItemSpec) bson.D { + // An action the script could not state, carried from storage verbatim + // (ako/mxcli#980). Falling through to Forms$NoAction below is what deleted a + // nanoflow menu item's action on every describe -> exec. + if len(mi.KeepAction) > 0 { + var kept bson.D + if err := bson.Unmarshal(mi.KeepAction, &kept); err == nil { + return kept + } + } if mi.Page != "" { return bson.D{ {Key: "$ID", Value: navID()}, diff --git a/mdl/executor/cmd_menus.go b/mdl/executor/cmd_menus.go index 00fe00053b..4b493a95be 100644 --- a/mdl/executor/cmd_menus.go +++ b/mdl/executor/cmd_menus.go @@ -83,8 +83,13 @@ func execCreateMenu(ctx *ExecContext, s *ast.CreateMenuStmt) error { Name: s.Name.Name, ContainerID: containerID, Documentation: s.Documentation, - Items: menuItemsFromAST(s.Items), } + var storedItems []*types.NavMenuItem + if existing != nil { + storedItems = existing.Items + } + kept := map[string]string{} + md.Items = menuItemsFromAST(s.Items, storedItems, "", kept) // A rewrite that carried no doc comment keeps the stored one (#1018). if existing != nil { md.Documentation = carriedDocumentation(s.DocumentationSet, s.Documentation, existing.Documentation) @@ -106,6 +111,7 @@ func execCreateMenu(ctx *ExecContext, s *ast.CreateMenuStmt) error { return err } ctx.ReportMutation("Modified", "menu %s", s.Name.String()) + reportKeptMenuActions(ctx, kept) return nil } @@ -135,9 +141,23 @@ func execDropMenu(ctx *ExecContext, s *ast.DropMenuStmt) error { // menuItemsFromAST converts parsed menu items to the semantic model. The AST and // semantic shapes differ only in how the target is held (pointer vs string), so // this stays a direct mapping rather than acquiring behaviour. -func menuItemsFromAST(defs []ast.NavMenuItemDef) []*types.NavMenuItem { +// +// stored is the document's current items at the same place in the tree: an +// item the script gives no action whose stored counterpart (same caption) holds +// one MDL cannot express keeps that action verbatim rather than becoming +// Forms$NoAction (ako/mxcli#980), as convertMenuItemDefs does for navigation. +func menuItemsFromAST(defs []ast.NavMenuItemDef, stored []*types.NavMenuItem, path string, kept map[string]string) []*types.NavMenuItem { var out []*types.NavMenuItem + used := make([]bool, len(stored)) for _, d := range defs { + var match *types.NavMenuItem + for i, st := range stored { + if !used[i] && st.Caption == d.Caption { + used[i], match = true, st + break + } + } + itemPath := path + "'" + d.Caption + "'" item := &types.NavMenuItem{Caption: d.Caption, Icon: d.Icon} if d.Icon != "" { item.IconType = "Forms$IconCollectionIcon" @@ -156,7 +176,16 @@ func menuItemsFromAST(defs []ast.NavMenuItemDef) []*types.NavMenuItem { } else { item.ActionType = "NoAction" } - item.Items = menuItemsFromAST(d.Items) + var storedSubs []*types.NavMenuItem + if match != nil { + storedSubs = match.Items + if !menuItemStatesAction(d) && !menuActionExpressible(match) { + item.ActionType = match.ActionType + item.StoredAction = match.StoredAction + kept[itemPath] = match.ActionType + } + } + item.Items = menuItemsFromAST(d.Items, storedSubs, itemPath+" > ", kept) out = append(out, item) } return out diff --git a/mdl/executor/cmd_menus_mock_test.go b/mdl/executor/cmd_menus_mock_test.go index 500e9d57ee..09d6114f56 100644 --- a/mdl/executor/cmd_menus_mock_test.go +++ b/mdl/executor/cmd_menus_mock_test.go @@ -185,7 +185,7 @@ func TestMenuItemsFromAST_Nested(t *testing.T) { {Caption: "Mf", Microflow: &mf}, {Caption: "Plain"}, }, - }}) + }}, nil, "", map[string]string{}) if len(items) != 1 || len(items[0].Items) != 3 { t.Fatalf("expected 1 top item with 3 children, got %+v", items) diff --git a/mdl/executor/cmd_navigation.go b/mdl/executor/cmd_navigation.go index b0bd615d8e..e0f6cb96a0 100644 --- a/mdl/executor/cmd_navigation.go +++ b/mdl/executor/cmd_navigation.go @@ -5,6 +5,7 @@ package executor import ( "fmt" "io" + "sort" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -92,9 +93,15 @@ func execAlterNavigation(ctx *ExecContext, s *ast.AlterNavigationStmt) error { spec.NotFoundPage = s.NotFoundPage.String() } - for _, mi := range s.MenuItems { - spec.MenuItems = append(spec.MenuItems, convertMenuItemDef(mi)) + var stored []*types.NavMenuItem + for _, p := range nav.Profiles { + if strings.EqualFold(p.Name, s.ProfileName) { + stored = p.MenuItems + break + } } + kept := map[string]string{} + spec.MenuItems = convertMenuItemDefs(s.MenuItems, stored, "", kept) spec.ThrowSyncError = s.ThrowSyncError spec.HasSync = s.HasSyncBlock @@ -121,9 +128,83 @@ func execAlterNavigation(ctx *ExecContext, s *ast.AlterNavigationStmt) error { } else { fmt.Fprintf(ctx.Output, "Navigation profile '%s' updated.\n", s.ProfileName) } + reportKeptMenuActions(ctx, kept) return nil } +// reportKeptMenuActions says which stored actions a rewrite carried because the +// script could not state them, so a kept action is never a surprise. +func reportKeptMenuActions(ctx *ExecContext, kept map[string]string) { + paths := make([]string, 0, len(kept)) + for p := range kept { + paths = append(paths, p) + } + sort.Strings(paths) + for _, p := range paths { + fmt.Fprintf(ctx.Output, " kept the stored action of menu item %s (%s): MDL cannot express it, and the item states none\n", p, kept[p]) + } +} + +// convertMenuItemDefs converts a list of sibling menu items, pairing each with +// the stored item of the same caption at the same place in the tree. +// +// The pairing is what keeps a rewrite from deleting an action MDL cannot spell: +// when the script's item states no action and its stored counterpart holds one +// describe cannot print, the stored action is carried verbatim (KeepAction) +// instead of being replaced by Forms$NoAction. That replacement used to be +// silent — a renamed menu lost its nanoflow item's action at exit 0 +// (ako/mxcli#980). kept collects the carried items by caption path. +func convertMenuItemDefs(defs []ast.NavMenuItemDef, stored []*types.NavMenuItem, path string, kept map[string]string) []types.NavMenuItemSpec { + used := make([]bool, len(stored)) + var out []types.NavMenuItemSpec + for _, def := range defs { + var match *types.NavMenuItem + for i, st := range stored { + if !used[i] && st.Caption == def.Caption { + used[i], match = true, st + break + } + } + itemPath := path + "'" + def.Caption + "'" + spec := convertMenuItemDef(def) + var storedSubs []*types.NavMenuItem + if match != nil { + storedSubs = match.Items + if !menuItemStatesAction(def) && !menuActionExpressible(match) { + spec.KeepAction = match.StoredAction + kept[itemPath] = match.ActionType + } + } + spec.Items = convertMenuItemDefs(def.Items, storedSubs, itemPath+" > ", kept) + out = append(out, spec) + } + return out +} + +// menuItemStatesAction reports whether the script gave the item an action. +func menuItemStatesAction(def ast.NavMenuItemDef) bool { + return def.Page != nil || def.Microflow != nil || def.SignOut +} + +// menuActionExpressible reports whether describe prints the stored item's +// action as an OnClick that rebuilds it — false for an action it cannot spell, +// which a rewrite must carry rather than replace. No action at all, and +// Forms$NoAction, need no carrying. +func menuActionExpressible(item *types.NavMenuItem) bool { + if len(item.StoredAction) == 0 { + return true + } + switch item.ActionType { + case "", "NoAction", "SignOutAction": + return true + case "PageAction": + return item.Page != "" + case "MicroflowAction": + return item.Microflow != "" + } + return false +} + // convertMenuItemDef converts an AST NavMenuItemDef to a writer NavMenuItemSpec. func convertMenuItemDef(def ast.NavMenuItemDef) types.NavMenuItemSpec { spec := types.NavMenuItemSpec{ @@ -458,6 +539,13 @@ func printMenuMDL(w io.Writer, items []*types.NavMenuItem, depth int, reproducer if note := menuItemIconNote(item, reproducer); note != "" { fmt.Fprintf(w, "%s%s\n", indent, note) } + if !menuActionExpressible(item) { + // Said, not dropped: the item line above states no action, so + // re-running it keeps the stored one (convertMenuItemDefs). + fmt.Fprintf(w, "%s-- menu item '%s': its action (%s) has no MDL form; "+ + "%s keeps the stored action while the item states none\n", + indent, item.Caption, item.ActionType, reproducer) + } } } diff --git a/mdl/executor/cmd_navigation_keep_action_test.go b/mdl/executor/cmd_navigation_keep_action_test.go new file mode 100644 index 0000000000..d4d436c574 --- /dev/null +++ b/mdl/executor/cmd_navigation_keep_action_test.go @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" +) + +// ako/mxcli#980: a menu item whose stored action MDL cannot spell (a stored +// $Type outside what describe prints) was described without its action, and the +// rewrite of that description stored Forms$NoAction in its place — TestApp's +// 'Item 4', a nanoflow call, lost its action at exit 0. The stored action is +// now carried when the script's item states none, and describe says so. +func TestNavigationRewrite_KeepsAnActionMDLCannotExpress(t *testing.T) { + stored := []byte("stored-action-bytes") + var got types.NavigationProfileSpec + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{Profiles: []*types.NavigationProfile{{ + Name: "Responsive", Kind: "Responsive", + MenuItems: []*types.NavMenuItem{ + {Caption: "Reports", ActionType: "Forms$UnknownFutureClientAction", StoredAction: stored}, + {Caption: "Admin", ActionType: "NoAction", Items: []*types.NavMenuItem{ + {Caption: "Logs", ActionType: "Forms$UnknownFutureClientAction", StoredAction: stored}, + }}, + {Caption: "Home", ActionType: "Forms$UnknownFutureClientAction", StoredAction: stored}, + }, + }}}, nil + }, + UpdateNavigationProfileFunc: func(_ model.ID, _ string, spec types.NavigationProfileSpec) error { + got = spec + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + prog, errs := visitor.Build(`create or modify navigation Responsive { + menu item 'Reports' + menu 'Admin' { menu item 'Logs' } + menu item 'Home' ( OnClick: show page M.Home ) +};`) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + assertNoError(t, execAlterNavigation(ctx, prog.Statements[0].(*ast.AlterNavigationStmt))) + + if len(got.MenuItems) != 3 { + t.Fatalf("spec has %d items, want 3", len(got.MenuItems)) + } + if !bytes.Equal(got.MenuItems[0].KeepAction, stored) { + t.Errorf("'Reports' states no action: the stored one must be kept, got KeepAction=%q", got.MenuItems[0].KeepAction) + } + if len(got.MenuItems[1].Items) != 1 || !bytes.Equal(got.MenuItems[1].Items[0].KeepAction, stored) { + t.Errorf("sub-item 'Admin' > 'Logs' must be paired by caption path, got %+v", got.MenuItems[1].Items) + } + // CONTROL: an item the script gives an action is written from the script. + // A converter that kept every stored action would pass the two checks above. + if got.MenuItems[2].KeepAction != nil || got.MenuItems[2].Page != "M.Home" { + t.Errorf("'Home' states show page M.Home and must not keep the stored action: %+v", got.MenuItems[2]) + } + if !strings.Contains(buf.String(), "kept the stored action of menu item 'Reports'") { + t.Errorf("a kept action must be reported, got:\n%s", buf.String()) + } +} + +// Describe says an action is there that it cannot print, rather than printing +// the item as if it had none. +func TestDescribeNavigation_FlagsAnActionMDLCannotExpress(t *testing.T) { + var buf bytes.Buffer + printMenuMDL(&buf, []*types.NavMenuItem{ + {Caption: "Reports", ActionType: "Forms$UnknownFutureClientAction", StoredAction: []byte("x")}, + {Caption: "Plain", ActionType: "NoAction", StoredAction: []byte("x")}, + }, 1, "CREATE NAVIGATION") + out := buf.String() + if !strings.Contains(out, "menu item 'Reports': its action (Forms$UnknownFutureClientAction) has no MDL form") { + t.Errorf("unsupported action not flagged:\n%s", out) + } + // CONTROL: a plain item has nothing to flag. + if strings.Contains(out, "'Plain': its action") { + t.Errorf("a NoAction item was flagged:\n%s", out) + } +} diff --git a/mdl/executor/menu_signout_test.go b/mdl/executor/menu_signout_test.go index f8cd2480a5..f8b9c8db91 100644 --- a/mdl/executor/menu_signout_test.go +++ b/mdl/executor/menu_signout_test.go @@ -69,7 +69,7 @@ func TestMenuItemsFromAST_SignOutBecomesAnActionType(t *testing.T) { items := menuItemsFromAST([]ast.NavMenuItemDef{ {Caption: "Sign out", SignOut: true}, {Caption: "Plain"}, - }) + }, nil, "", map[string]string{}) if items[0].ActionType != "SignOutAction" { t.Errorf("ActionType = %q, want SignOutAction", items[0].ActionType) } diff --git a/mdl/types/navigation.go b/mdl/types/navigation.go index eccc0c438e..fc0fd8c871 100644 --- a/mdl/types/navigation.go +++ b/mdl/types/navigation.go @@ -70,8 +70,12 @@ type NavMenuItem struct { // identifies a glyph icon, since it carries no qualified name. Without it a // reader knows a glyph was there but not which one, so it can neither be // re-emitted by DESCRIBE nor carried through a rewrite. - IconCode int `json:"iconCode,omitempty"` - Items []*NavMenuItem `json:"items,omitempty"` + IconCode int `json:"iconCode,omitempty"` + // StoredAction is the item's client action exactly as stored (its raw + // BSON document), so an action MDL cannot spell is carried through a + // rewrite instead of being replaced by Forms$NoAction (ako/mxcli#980). + StoredAction []byte `json:"-"` + Items []*NavMenuItem `json:"items,omitempty"` } // HasIcon reports whether the item carries an icon of ANY of the three kinds. @@ -251,5 +255,11 @@ type NavMenuItemSpec struct { IconKind MenuIconKind // IconCode is the glyph's numeric Code, meaningful only for MenuIconGlyph. IconCode int - Items []NavMenuItemSpec + // KeepAction is a stored client action (raw BSON) the writer puts back + // verbatim instead of building one from Page/Microflow/SignOut. The executor + // sets it when the script's item states no action and the stored one is an + // action MDL cannot express — writing Forms$NoAction there would silently + // delete it (ako/mxcli#980). + KeepAction []byte + Items []NavMenuItemSpec } From 1e183e236c9fa6540b8f772d971a66fe9360d888 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 19:22:27 +0000 Subject: [PATCH 06/14] fix(check): predict view-entity select lists mxbuild or the database refuses MDL033 comparison as a select column (CE0174), MDL034 aggregate over a grouped column (CE0174), MDL035 plain column not grouped (CE0174), MDL036 non-aggregated expression not in the GROUP BY (passes mxbuild, PostgreSQL 42803 / HSQLDB 42574 at runtime) - errors in ValidateOQLSyntax. MDL037 literal aggregate argument (HSQLDB 42567, warning) and MDL038 bare integer/string literal column (info) in ValidateOQLPortability, which only check and the LSP run. Part of ako/mxcli#981. Co-Authored-By: Claude Opus 5.5 --- cmd/mxcli/lsp_diagnostics.go | 1 + mdl/executor/oql_type_inference.go | 4 + mdl/executor/oql_view_select_checks.go | 504 ++++++++++++++++++++ mdl/executor/oql_view_select_checks_test.go | 206 ++++++++ mdl/executor/validate_program.go | 1 + 5 files changed, 716 insertions(+) create mode 100644 mdl/executor/oql_view_select_checks.go create mode 100644 mdl/executor/oql_view_select_checks_test.go diff --git a/cmd/mxcli/lsp_diagnostics.go b/cmd/mxcli/lsp_diagnostics.go index 9521beccd2..2e0029fef9 100644 --- a/cmd/mxcli/lsp_diagnostics.go +++ b/cmd/mxcli/lsp_diagnostics.go @@ -362,6 +362,7 @@ func (s *mdlServer) runSemanticValidation(text string) []protocol.Diagnostic { if viewStmt.Query.RawQuery != "" { violations = append(violations, executor.ValidateOQLSyntax(viewStmt.Query.RawQuery)...) violations = append(violations, executor.ValidateOQLTypes(viewStmt.Query.RawQuery, viewStmt.Attributes)...) + violations = append(violations, executor.ValidateOQLPortability(viewStmt.Query.RawQuery)...) violations = append(violations, executor.ValidateViewAttributeDeclarations(viewStmt.Query.RawQuery, viewStmt.Attributes)...) } } diff --git a/mdl/executor/oql_type_inference.go b/mdl/executor/oql_type_inference.go index 7043f79b03..5845a9b81d 100644 --- a/mdl/executor/oql_type_inference.go +++ b/mdl/executor/oql_type_inference.go @@ -1381,6 +1381,10 @@ func ValidateOQLSyntax(oql string) []linter.Violation { }) } + // A comparison as a column, and the GROUP BY rules (MDL033–MDL036, + // ako/mxcli#981): select lists that are well typed and still fail. + violations = append(violations, validateOQLSelectExpressions(oql)...) + return violations } diff --git a/mdl/executor/oql_view_select_checks.go b/mdl/executor/oql_view_select_checks.go new file mode 100644 index 0000000000..3120f03d8b --- /dev/null +++ b/mdl/executor/oql_view_select_checks.go @@ -0,0 +1,504 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Select-list checks for view-entity OQL that the type rules cannot see +// (ako/mxcli#981). Each one predicts a failure that check used to pass: +// +// MDL033 a comparison as a select column mxbuild CE0174 +// MDL034 an aggregate over a grouped column mxbuild CE0174 +// MDL035 a plain column that is not grouped mxbuild CE0174 +// MDL036 an expression that is not grouped runtime (PostgreSQL 42803, HSQLDB 42574) +// MDL037 a literal as an aggregate argument runtime on HSQLDB (42567) +// MDL038 a bare integer/string literal column wrong data for consumers on HSQLDB +// +// MDL033–036 are errors and run inside ValidateOQLSyntax, so exec refuses them +// too. MDL037/038 are a warning and an info note about the database, not the +// model; they live in ValidateOQLPortability, which only check and the LSP +// call — exec turns every ValidateOQLSyntax finding into a refusal. +// +// Measured on mxbuild 11.13.0 and `run --local` on HSQLDB and PostgreSQL +// (ako/mxcli#981; the controls are in oql_view_select_checks_test.go). +package executor + +import ( + "fmt" + "regexp" + "sort" + "strings" + + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// oqlTopLevelMask marks the bytes of expr that sit at parenthesis depth 0, +// outside quotes and outside every CASE … END. An operator found at such a +// byte belongs to the expression itself, not to a function argument, a +// subquery or a WHEN condition. +func oqlTopLevelMask(expr string) []bool { + mask := make([]bool, len(expr)) + depth, caseDepth := 0, 0 + for i := 0; i < len(expr); i++ { + c := expr[i] + switch { + case c == '\'' || c == '"' || c == '`': + for i++; i < len(expr) && expr[i] != c; i++ { + } + continue + case c == '(': + depth++ + continue + case c == ')': + depth-- + continue + case isIdentChar(c) && (i == 0 || !isIdentChar(expr[i-1])): + j := i + for j < len(expr) && isIdentChar(expr[j]) { + j++ + } + if depth == 0 { + switch strings.ToUpper(expr[i:j]) { + case "CASE": + caseDepth++ + case "END": + if caseDepth > 0 { + caseDepth-- + } + } + } + i = j - 1 + continue + } + if depth == 0 && caseDepth == 0 { + mask[i] = true + } + } + return mask +} + +// topLevelComparison returns the comparison operator an expression is built +// on, or "" when it has none at the top level. +func topLevelComparison(expr string) string { + mask := oqlTopLevelMask(expr) + for i := 0; i < len(expr); i++ { + if !mask[i] { + continue + } + switch expr[i] { + case '=': + return "=" + case '!', '<', '>': + if i+1 < len(expr) && mask[i+1] && (expr[i+1] == '=' || (expr[i] == '<' && expr[i+1] == '>')) { + return expr[i : i+2] + } + if expr[i] != '!' { + return expr[i : i+1] + } + } + } + return "" +} + +// splitTopLevelPlus splits expr at every top-level '+'. One part means there +// is no top-level '+'. +func splitTopLevelPlus(expr string) []string { + mask := oqlTopLevelMask(expr) + var parts []string + last := 0 + for i := 0; i < len(expr); i++ { + if mask[i] && expr[i] == '+' { + parts = append(parts, strings.TrimSpace(expr[last:i])) + last = i + 1 + } + } + return append(parts, strings.TrimSpace(expr[last:])) +} + +// normalizeOQLExpr is the form two OQL expressions are compared in: case and +// whitespace folded and identifier quotes dropped outside string literals, so +// `datepart(YEAR, r."RaceDate")` and `DATEPART(year,r.RaceDate)` are equal. +func normalizeOQLExpr(expr string) string { + var b strings.Builder + for i := 0; i < len(expr); i++ { + c := expr[i] + switch { + case c == '\'': + j := i + 1 + for j < len(expr) && expr[j] != '\'' { + j++ + } + if j < len(expr) { + j++ + } + b.WriteString(expr[i:j]) + i = j - 1 + case isOQLSpace(c): + // Dropped, except between two words: `cast(m.ID as string)` must + // not read as a column `m.idasstring`. + j := i + for j < len(expr) && isOQLSpace(expr[j]) { + j++ + } + out := b.String() + if len(out) > 0 && j < len(expr) && isIdentChar(out[len(out)-1]) && isIdentChar(expr[j]) { + b.WriteByte(' ') + } + i = j - 1 + case c == '"' || c == '`': + // dropped + default: + b.WriteString(strings.ToLower(string(c))) + } + } + return b.String() +} + +var ( + oqlStringLiteralRe = regexp.MustCompile(`'[^']*'`) + // oqlColumnRefRe finds `alias.attr` column references (and the qualified + // segments of an association path) in a normalized expression. + oqlColumnRefRe = regexp.MustCompile(`[a-z_][a-z0-9_]*\.[a-z_][a-z0-9_]*`) + // oqlPlainColumnRe is an expression that is nothing but one column. + oqlPlainColumnRe = regexp.MustCompile(`^[a-z_][a-z0-9_]*\.[a-z_][a-z0-9_]*$`) + oqlBareIdentRe = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) + // oqlAggregateCallRe matches an aggregate call in a normalized expression. + oqlAggregateCallRe = regexp.MustCompile(`(^|[^a-z0-9_.])(count|sum|avg|min|max)\(`) + // oqlRawAggregateCallRe is oqlAggregateCallRe over query text as written. + oqlRawAggregateCallRe = regexp.MustCompile(`(?i)(^|[^A-Za-z0-9_.])(count|sum|avg|min|max)\s*\(`) +) + +// oqlColumnRefs lists the column references in a normalized expression, +// ignoring anything inside a string literal. +func oqlColumnRefs(n string) []string { + n = oqlStringLiteralRe.ReplaceAllString(n, "''") + var out []string + for _, loc := range oqlColumnRefRe.FindAllStringIndex(n, -1) { + if loc[0] > 0 && (isIdentChar(n[loc[0]-1]) || n[loc[0]-1] == '.') { + continue + } + out = append(out, n[loc[0]:loc[1]]) + } + return out +} + +// callArgument returns the text between the '(' at open and its matching ')'. +func callArgument(s string, open int) string { + depth := 0 + for j := open; j < len(s); j++ { + switch s[j] { + case '(': + depth++ + case ')': + depth-- + if depth == 0 { + return s[open+1 : j] + } + case '\'': + for j++; j < len(s) && s[j] != '\''; j++ { + } + } + } + return "" +} + +// oqlAggregateArgs returns the argument of every aggregate call in a +// normalized expression. +func oqlAggregateArgs(n string) []string { + var out []string + for _, loc := range oqlAggregateCallRe.FindAllStringIndex(n, -1) { + out = append(out, callArgument(n, loc[1]-1)) + } + return out +} + +// replaceOQLSubexpr replaces every occurrence of sub in n that stands on +// identifier boundaries, so `r.season` does not match inside `r.seasonend`. +func replaceOQLSubexpr(n, sub string) string { + if sub == "" { + return n + } + var b strings.Builder + for i := 0; i < len(n); { + if strings.HasPrefix(n[i:], sub) { + before := i == 0 || !(isIdentChar(n[i-1]) || n[i-1] == '.') + end := i + len(sub) + after := end >= len(n) || !isIdentChar(n[end]) + if before && after { + b.WriteString("#") + i = end + continue + } + } + b.WriteByte(n[i]) + i++ + } + return b.String() +} + +// groupByExpressions returns the top-level GROUP BY list of a query, in either +// clause order. ok is false when there is no GROUP BY, or when the query is a +// UNION, whose branches each carry their own list and select clause. +func groupByExpressions(oql string) ([]string, bool) { + upper := strings.ToUpper(oql) + if topLevelKeywordIndex(oql, upper, 0, "UNION") >= 0 { + return nil, false + } + at := topLevelKeywordIndex(oql, upper, 0, "GROUP BY") + if at < 0 { + return nil, false + } + start, _ := matchPhraseAt(oql, upper, at, "GROUP BY") + end := topLevelKeywordIndex(oql, upper, start, "HAVING", "ORDER BY", "LIMIT", "OFFSET", "SELECT") + if end < 0 { + end = len(oql) + } + var out []string + for _, e := range parseSelectColumns(oql[start:end]) { + if e = strings.TrimSpace(e); e != "" { + out = append(out, e) + } + } + return out, len(out) > 0 +} + +// selectColumnParts splits a select column into its expression and its alias +// (empty when it has none), dropping a leading DISTINCT. +func selectColumnParts(col string) (expr, alias string) { + col = strings.TrimSpace(col) + if m := oqlAliasSuffixRe.FindStringSubmatch(col); m != nil { + alias = unquoteOQLIdent(m[1]) + col = strings.TrimSuffix(col, m[0]) + } + col = strings.TrimSpace(col) + if len(col) > 9 && strings.EqualFold(col[:9], "distinct ") { + col = strings.TrimSpace(col[9:]) + } + return col, alias +} + +func columnName(alias string, i int) string { + if alias != "" { + return alias + } + return fmt.Sprintf("column %d", i+1) +} + +func viewOQLViolation(rule string, sev linter.Severity, msg, fix string) linter.Violation { + return linter.Violation{ + RuleID: rule, + Severity: sev, + Message: msg, + Location: linter.Location{DocumentType: "viewentity"}, + Suggestion: fix, + } +} + +// validateOQLSelectExpressions holds MDL033–MDL036: the select-list shapes +// mxbuild or the database refuses although every column is well typed. +func validateOQLSelectExpressions(oql string) []linter.Violation { + selectClause := extractSelectClause(oql) + if selectClause == "" { + return nil + } + columns := parseSelectColumns(selectClause) + out := comparisonColumns(columns) + if groupBy, ok := groupByExpressions(oql); ok { + out = append(out, groupByColumns(columns, groupBy)...) + } + return out +} + +// comparisonColumns is MDL033: `r.Season = r.CurrentSeason as IsCurrent` is +// CE0174 ("The '=' part is incomplete or incorrect. You could use here: +// FROM."), and `!=` likewise; the same comparison inside a CASE builds +// (measured, 11.13.0). +func comparisonColumns(columns []string) []linter.Violation { + var out []linter.Violation + for i, col := range columns { + expr, alias := selectColumnParts(col) + if strings.HasPrefix(strings.ToLower(strings.TrimLeft(expr, "( ")), "select") { + continue + } + op := topLevelComparison(expr) + if op == "" { + continue + } + name := columnName(alias, i) + out = append(out, viewOQLViolation("MDL033", linter.SeverityError, + fmt.Sprintf("select column %d (%s) is a comparison (%s) — a comparison is not a select "+ + "expression in OQL, and MxBuild rejects the view with CE0174 (\"The '%s' part is "+ + "incomplete or incorrect\")", i+1, name, op, op), + fmt.Sprintf("Wrap it in a CASE: `case when %s then true else false end as %s`", expr, name))) + } + return out +} + +// groupByColumns is MDL034–MDL036, the select columns of a grouped query. +func groupByColumns(columns, groupBy []string) []linter.Violation { + var out []linter.Violation + gbNorm := make([]string, 0, len(groupBy)) + gbRefs := map[string]bool{} + gbAliases := map[string]bool{} + for _, g := range groupBy { + n := normalizeOQLExpr(g) + gbNorm = append(gbNorm, n) + for _, r := range oqlColumnRefs(n) { + gbRefs[r] = true + } + if oqlBareIdentRe.MatchString(n) { + gbAliases[n] = true + } + } + // Longest first, so `datepart(year,r.racedate)` is replaced before a + // shorter `r.racedate` could break it up. + sort.Slice(gbNorm, func(i, j int) bool { return len(gbNorm[i]) > len(gbNorm[j]) }) + + for i, col := range columns { + expr, alias := selectColumnParts(col) + n := normalizeOQLExpr(expr) + if strings.Contains(n, "(select") || strings.HasPrefix(n, "select") { + continue // a subquery is its own scope + } + name := columnName(alias, i) + + if args := oqlAggregateArgs(n); len(args) > 0 { + // MDL034: an aggregate whose argument reads a column a GROUP BY + // expression also reads — count(r.Season) by r.Season, + // max(r.RaceDate) by datepart(YEAR, r.RaceDate), sum(r.Season) by + // r.Season + 1 — is CE0174 on 11.13.0. count(r.Name) by r.Season + // builds. + if r := firstGroupedRef(args, gbRefs); r != "" { + out = append(out, viewOQLViolation("MDL034", linter.SeverityError, + fmt.Sprintf("select column %d (%s) aggregates %s, which the GROUP BY also uses — "+ + "MxBuild rejects the view with CE0174", i+1, name, r), + "Aggregate a different column (count a non-null column that is not grouped), "+ + "or compute the value in a subquery")) + } + continue // aggregated: the grouping rules below do not apply + } + + // MDL035/036: every other column has to be one of the GROUP BY + // expressions, built from them, or a constant. + if gbAliases[strings.ToLower(alias)] { + continue // `group by Yr` groups the column aliased Yr (builds, 11.13.0) + } + rest := n + for _, g := range gbNorm { + rest = replaceOQLSubexpr(rest, g) + } + left := oqlColumnRefs(rest) + if len(left) == 0 { + continue + } + if oqlPlainColumnRe.MatchString(n) { + out = append(out, viewOQLViolation("MDL035", linter.SeverityError, + fmt.Sprintf("select column %d (%s) is %s, which is neither aggregated nor in the GROUP BY — "+ + "MxBuild rejects the view with CE0174 (\"Every expression in the SELECT clause must be "+ + "included in the GROUP BY clause\"); grouping by the ID does not exempt it", i+1, name, expr), + fmt.Sprintf("Add %s to the GROUP BY, or aggregate it (e.g. max(%s))", expr, expr))) + continue + } + out = append(out, viewOQLViolation("MDL036", linter.SeverityError, + fmt.Sprintf("select column %d (%s) is not aggregated and is not a GROUP BY expression "+ + "(it reads %s outside one) — MxBuild accepts the view, but the database refuses the query "+ + "whenever the view is read (PostgreSQL 42803, HSQLDB 42574)", i+1, name, strings.Join(left, ", ")), + fmt.Sprintf("Add `%s` to the GROUP BY, or aggregate it", expr))) + } + return out +} + +// firstGroupedRef returns the first column an aggregate argument reads that a +// GROUP BY expression also reads, or "". +func firstGroupedRef(args []string, gbRefs map[string]bool) string { + for _, a := range args { + for _, r := range oqlColumnRefs(a) { + if gbRefs[r] { + return r + } + } + } + return "" +} + +var ( + oqlIntLiteralRe = regexp.MustCompile(`^-?\d+$`) + oqlBoolLiteralRe = regexp.MustCompile(`^(?i:true|false)$`) +) + +// oqlUntypedLiteral reports whether expr is a literal Mendix sends to the +// database as an untyped `?` parameter: an integer, string or boolean. +// Decimal literals are sent with a cast, so they are not among them. +func oqlUntypedLiteral(expr string) (kind string, ok bool) { + expr = strings.TrimSpace(expr) + for len(expr) >= 2 && expr[0] == '(' && expr[len(expr)-1] == ')' { + expr = strings.TrimSpace(expr[1 : len(expr)-1]) + } + switch { + case oqlIntLiteralRe.MatchString(expr): + return "Integer", true + case oqlBoolLiteralRe.MatchString(expr): + return "Boolean", true + case len(expr) >= 2 && expr[0] == '\'' && expr[len(expr)-1] == '\'' && + !strings.Contains(strings.ReplaceAll(expr[1:len(expr)-1], "''", ""), "'"): + return "String", true + } + return "", false +} + +// ValidateOQLPortability reports the literals Mendix sends to the database as +// untyped parameters, which HSQLDB — Studio Pro's default database — cannot +// type (MDL037, MDL038). Neither concerns the model: mxbuild and PostgreSQL +// accept both, so they are a warning and a note, and exec does not run them. +func ValidateOQLPortability(oql string) []linter.Violation { + var out []linter.Violation + oql = stripOQLComments(oql) + + // MDL037: sum(1), count(1), max(0), count('x'), count(true) give HSQLDB + // 42567 "data type cast needed" when the view is read. Anywhere in the + // query, every branch. + seen := map[string]bool{} + for _, loc := range oqlRawAggregateCallRe.FindAllStringSubmatchIndex(oql, -1) { + arg := strings.TrimSpace(callArgument(oql, loc[1]-1)) + kind, ok := oqlUntypedLiteral(arg) + if !ok { + continue + } + fn := strings.ToLower(oql[loc[4]:loc[5]]) + call := fn + "(" + arg + ")" + if seen[call] { + continue + } + seen[call] = true + out = append(out, viewOQLViolation("MDL037", linter.SeverityWarning, + fmt.Sprintf("%s aggregates a bare %s literal — Mendix sends it to the database as an untyped "+ + "parameter, and HSQLDB (Studio Pro's default database) refuses the query with 42567 "+ + "\"data type cast needed\" when the view is read; PostgreSQL accepts it", call, strings.ToLower(kind)), + fmt.Sprintf("Aggregate a non-null column (`count(t.SomeColumn)`), or cast the literal: `%s(cast(%s as %s))`", + fn, arg, kind))) + } + + // MDL038: a bare integer or string literal as a view column reaches the + // database untyped. The view reads fine; a consumer that aggregates it + // fails as above, and on HSQLDB arithmetic on it concatenates instead + // (`v.One + 1` is 11 there, 2 on PostgreSQL). Boolean columns are not + // reported: nothing does arithmetic on them. + selectClause := extractSelectClause(oql) + if selectClause == "" { + return out + } + for i, col := range parseSelectColumns(selectClause) { + expr, alias := selectColumnParts(col) + kind, ok := oqlUntypedLiteral(expr) + if !ok || kind == "Boolean" { + continue + } + name := columnName(alias, i) + risk := "a consumer that aggregates it fails on HSQLDB, and arithmetic on it concatenates instead " + + "(`v." + name + " + 1` is 11 for a column holding 1, where PostgreSQL gives 2)" + if kind == "String" { + risk = "a consumer that aggregates it fails on HSQLDB, which cannot type the parameter" + } + out = append(out, viewOQLViolation("MDL038", linter.SeverityInfo, + fmt.Sprintf("select column %d (%s) is a bare %s literal, which Mendix sends to the database as an "+ + "untyped parameter: the view reads fine, but %s", i+1, name, strings.ToLower(kind), risk), + fmt.Sprintf("Give it a type: `cast(%s as %s) as %s`", expr, kind, name))) + } + return out +} diff --git a/mdl/executor/oql_view_select_checks_test.go b/mdl/executor/oql_view_select_checks_test.go new file mode 100644 index 0000000000..bb856b7df4 --- /dev/null +++ b/mdl/executor/oql_view_select_checks_test.go @@ -0,0 +1,206 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#981 — view-entity OQL that check passed and mxbuild or the +// runtime refused. Every failing query below is one measured on mxbuild +// 11.13.0 (and, for MDL036–038, `run --local` on HSQLDB and PostgreSQL); every +// control is a form measured to build and run. The failing forms come from +// the JTSBootLogboek project (H16, H18) over +// +// MyFirstModule.Race (Name: String(100), Season: Integer, +// CurrentSeason: Integer, RaceDate: DateTime, +// Points: Decimal, Nr: AutoNumber) +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/linter" +) + +func selectRuleViolations(oql string, rule string) []linter.Violation { + var out []linter.Violation + all := append(ValidateOQLSyntax(oql), ValidateOQLPortability(oql)...) + for _, v := range all { + if v.RuleID == rule { + out = append(out, v) + } + } + return out +} + +type oqlRuleCase struct { + name string + oql string + fires bool +} + +func runOQLRuleCases(t *testing.T, rule string, cases []oqlRuleCase) { + t.Helper() + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := selectRuleViolations(c.oql, rule) + if (len(got) > 0) != c.fires { + t.Fatalf("%s fired=%v, want %v\n oql: %s\n got: %v", rule, len(got) > 0, c.fires, c.oql, got) + } + }) + } +} + +// MDL033 — a comparison as a select column is CE0174 ("The '=' part is +// incomplete or incorrect. You could use here: FROM."); `!=` likewise. +func TestMDL033_ComparisonAsSelectColumn(t *testing.T) { + runOQLRuleCases(t, "MDL033", []oqlRuleCase{ + {"= as a column (CE0174)", + `select r.Season as Season, r.Season = r.CurrentSeason as IsCurrent from MyFirstModule.Race as r`, true}, + {"!= as a column (CE0174)", + `select r.Season != r.CurrentSeason as IsCurrent from MyFirstModule.Race as r`, true}, + // Controls, both build at 0 errors. + {"the same comparison inside CASE", + `select r.Season as Season, case when r.Season = r.CurrentSeason then true else false end as IsCurrent from MyFirstModule.Race as r`, false}, + {"a comparison in WHERE", + `select r.Name as Nm from MyFirstModule.Race as r where r.Season > 1 and r.Name != 'x'`, false}, + {"a comparison inside a function argument or subquery", + `select (select count(x.Name) from MyFirstModule.Race as x where x.Season = r.Season) as N from MyFirstModule.Race as r`, false}, + {"an operator inside a string literal", + `select 'a=b' as L from MyFirstModule.Race as r`, false}, + }) + v := selectRuleViolations(`select r.Season = r.CurrentSeason as IsCurrent from MyFirstModule.Race as r`, "MDL033")[0] + if !strings.Contains(v.Suggestion, "case when r.Season = r.CurrentSeason then true else false end as IsCurrent") { + t.Errorf("the suggestion should spell out the CASE form: %s", v.Suggestion) + } +} + +// MDL034 — an aggregate over a column the GROUP BY also uses is CE0174. +func TestMDL034_AggregateOverAGroupedColumn(t *testing.T) { + runOQLRuleCases(t, "MDL034", []oqlRuleCase{ + {"count(r.Season) by r.Season (CE0174)", + `select r.Season as Season, count(r.Season) as N from MyFirstModule.Race as r group by r.Season`, true}, + {"max(r.RaceDate) by datepart(YEAR, r.RaceDate) (CE0174)", + `select datepart(YEAR, r.RaceDate) as Yr, max(r.RaceDate) as LastDate from MyFirstModule.Race as r group by datepart(YEAR, r.RaceDate)`, true}, + {"sum(r.Season) by r.Season + 1 (CE0174)", + `select r.Season + 1 as S, sum(r.Season) as Total from MyFirstModule.Race as r group by r.Season + 1`, true}, + {"from-first clause order", + `from MyFirstModule.Race as r group by r.Season select r.Season as Season, count(r.Season) as N`, true}, + // Controls: build at 0 errors. + {"count(r.Name) by r.Season", + `select r.Season as Season, count(r.Name) as N from MyFirstModule.Race as r group by r.Season`, false}, + {"count(r.Points) by r.Season, r.Name", + `select r.Season as Season, r.Name as Nm, count(r.Points) as N from MyFirstModule.Race as r group by r.Season, r.Name`, false}, + {"no GROUP BY at all", + `select count(r.Season) as N from MyFirstModule.Race as r`, false}, + }) +} + +// MDL035 — a plain column that is neither aggregated nor grouped is CE0174 +// ("Every expression in the SELECT clause must be included in the GROUP BY +// clause"), whatever the GROUP BY holds — an expression, another column, or +// the ID. +func TestMDL035_PlainColumnNotGrouped(t *testing.T) { + runOQLRuleCases(t, "MDL035", []oqlRuleCase{ + {"r.Name next to group by datepart(...) (CE0174)", + `select datepart(YEAR, r.RaceDate) as Yr, r.Name as Nm from MyFirstModule.Race as r group by datepart(YEAR, r.RaceDate)`, true}, + {"r.Name next to group by r.Season (CE0174)", + `select r.Season as Season, r.Name as Nm from MyFirstModule.Race as r group by r.Season`, true}, + {"r.Name next to group by r.ID (CE0174, no functional dependency)", + `select r.Name as Nm, count(r.Season) as N from MyFirstModule.Race as r group by r.ID`, true}, + {"from-first clause order (CE0174)", + `from MyFirstModule.Race as r group by r.Season select r.Season as Season, r.Name as Nm`, true}, + // Controls: build at 0 errors. + {"every plain column grouped", + `select r.Season as Season, r.Name as Nm, count(r.Points) as N from MyFirstModule.Race as r group by r.Season, r.Name`, false}, + {"group by the select alias", + `select datepart(YEAR, r.RaceDate) as Yr, count(r.Name) as N from MyFirstModule.Race as r group by Yr`, false}, + {"a constant next to the group", + `select 'x' as L, r.Season as Season from MyFirstModule.Race as r group by r.Season`, false}, + {"quoting and case differ from the GROUP BY", + `select R."Season" as Season, count(r.Name) as N from MyFirstModule.Race as r GROUP BY r.Season`, false}, + {"a UNION is not judged", + `select r.Name as Nm from MyFirstModule.Race as r group by r.Season union all select r.Name as Nm from MyFirstModule.Race as r`, false}, + }) +} + +// MDL036 — a non-aggregated EXPRESSION that is not a GROUP BY expression +// passes mxbuild and fails when the view is read: PostgreSQL 42803, HSQLDB +// 42574. +func TestMDL036_ExpressionNotGrouped(t *testing.T) { + runOQLRuleCases(t, "MDL036", []oqlRuleCase{ + {"datepart(MONTH) next to group by datepart(YEAR) (42803 / 42574)", + `select datepart(YEAR, r.RaceDate) as Yr, datepart(MONTH, r.RaceDate) as Mo from MyFirstModule.Race as r group by datepart(YEAR, r.RaceDate)`, true}, + // Controls. + {"the select expression equals the GROUP BY expression", + `select datepart(YEAR, r.RaceDate) as Yr, count(r.Name) as N from MyFirstModule.Race as r group by datepart(YEAR, r.RaceDate)`, false}, + {"built from the GROUP BY expression (builds)", + `select datepart(YEAR, r.RaceDate) + 1 as Y1, count(r.Name) as N from MyFirstModule.Race as r group by datepart(YEAR, r.RaceDate)`, false}, + {"a cast of the grouped ID (keywords stay words)", + `from M.Reading as r join r/M.Reading_Meter/M.Meter as m group by m.ID select cast(m.ID as string) as MeterId, sum(r.Kwh) as TotalKwh`, false}, + {"a function of a grouped column", + `select datepart(YEAR, r.RaceDate) as Yr, count(r.Name) as N from MyFirstModule.Race as r group by r.RaceDate`, false}, + {"a constant expression", + `select 1 + 1 as Two, count(r.Name) as N from MyFirstModule.Race as r group by r.Season`, false}, + }) + // The plain-column case is MDL035, not both. + if got := selectRuleViolations(`select r.Season as Season, r.Name as Nm from MyFirstModule.Race as r group by r.Season`, "MDL036"); len(got) != 0 { + t.Errorf("a plain column is MDL035's; MDL036 fired too: %v", got) + } +} + +// MDL037 — a literal aggregate argument is sent untyped, and HSQLDB refuses +// it with 42567 "data type cast needed". +func TestMDL037_LiteralAggregateArgument(t *testing.T) { + runOQLRuleCases(t, "MDL037", []oqlRuleCase{ + {"sum(1)", `select r.Season as Season, sum(1) as N from MyFirstModule.Race as r group by r.Season`, true}, + {"count(1)", `select count(1) as N from MyFirstModule.Race as r`, true}, + {"max(0)", `select max(0) as N from MyFirstModule.Race as r`, true}, + {"count('x')", `select count('x') as N from MyFirstModule.Race as r`, true}, + {"count(true)", `select count(true) as N from MyFirstModule.Race as r`, true}, + {"in a UNION branch", `select count(r.Name) as N from MyFirstModule.Race as r union all select sum(1) as N from MyFirstModule.Race as r`, true}, + // Controls: run on HSQLDB. + {"count(a column)", `select count(r.Name) as N from MyFirstModule.Race as r`, false}, + {"sum(cast(1 as Integer))", `select sum(cast(1 as Integer)) as N from MyFirstModule.Race as r`, false}, + {"sum(case … then 1 else 0 end)", `select sum(case when r.Season > 0 then 1 else 0 end) as N from MyFirstModule.Race as r`, false}, + {"a decimal literal, which Mendix casts", `select sum(1.5) as N from MyFirstModule.Race as r`, false}, + }) + for _, v := range selectRuleViolations(`select count(1) as N from MyFirstModule.Race as r`, "MDL037") { + if v.Severity != linter.SeverityWarning { + t.Errorf("MDL037 severity = %v, want warning: PostgreSQL and mxbuild accept it", v.Severity) + } + if !strings.Contains(v.Suggestion, "cast(1 as Integer)") { + t.Errorf("suggestion should give the cast form: %s", v.Suggestion) + } + } + // It is not an exec refusal: exec turns every ValidateOQLSyntax finding + // into one, so the HSQLDB rules must stay out of it. + for _, v := range ValidateOQLSyntax(`select count(1) as N, 1 as One from MyFirstModule.Race as r`) { + if v.RuleID == "MDL037" || v.RuleID == "MDL038" { + t.Errorf("%s is reported by ValidateOQLSyntax, which exec refuses on", v.RuleID) + } + } +} + +// MDL038 — a bare integer or string literal column is sent untyped. The view +// reads, but `v.One + 1` is 11 on HSQLDB (2 on PostgreSQL), and a consumer that +// aggregates the column fails. +func TestMDL038_BareLiteralColumnIsANoteOnly(t *testing.T) { + runOQLRuleCases(t, "MDL038", []oqlRuleCase{ + {"1 as One", `select r.Name as Nm, 1 as One from MyFirstModule.Race as r`, true}, + {"'Schipper' as Rol", `select r.Name as Nm, 'Schipper' as Rol from MyFirstModule.Race as r`, true}, + // Controls. + {"cast('Schipper' as String)", `select r.Name as Nm, cast('Schipper' as String) as Rol from MyFirstModule.Race as r`, false}, + {"true as B (typed)", `select r.Name as Nm, true as B from MyFirstModule.Race as r`, false}, + {"0.0 as D (cast by Mendix)", `select r.Name as Nm, 0.0 as D from MyFirstModule.Race as r`, false}, + {"case … then 1 else 0 end", `select r.Name as Nm, case when r.Season > 0 then 1 else 0 end as X from MyFirstModule.Race as r`, false}, + }) + // The 'TOTAL' label the docs use must never be more than a note: it is + // how a summary row is written, and it runs. + for _, v := range append(ValidateOQLSyntax(`select 'TOTAL' as Label, sum(r.Points) as Amt from MyFirstModule.Race as r`), + ValidateOQLPortability(`select 'TOTAL' as Label, sum(r.Points) as Amt from MyFirstModule.Race as r`)...) { + if v.Severity > linter.SeverityInfo { + t.Errorf("'TOTAL' as Label drew %s %s: %s", v.Severity, v.RuleID, v.Message) + } + } + v := selectRuleViolations(`select 1 as One from MyFirstModule.Race as r`, "MDL038")[0] + if !strings.Contains(v.Message, "11") || !strings.Contains(v.Suggestion, "cast(1 as Integer) as One") { + t.Errorf("the note should name the HSQLDB result and the cast: %s / %s", v.Message, v.Suggestion) + } +} diff --git a/mdl/executor/validate_program.go b/mdl/executor/validate_program.go index 32c9695de0..82213720a0 100644 --- a/mdl/executor/validate_program.go +++ b/mdl/executor/validate_program.go @@ -157,6 +157,7 @@ func ValidateProgram(prog *ast.Program, projectPath string) []linter.Violation { if viewStmt.Query.RawQuery != "" { violations = append(violations, ValidateOQLSyntax(viewStmt.Query.RawQuery)...) violations = append(violations, ValidateOQLTypes(viewStmt.Query.RawQuery, viewStmt.Attributes)...) + violations = append(violations, ValidateOQLPortability(viewStmt.Query.RawQuery)...) violations = append(violations, ValidateViewAttributeDeclarations(viewStmt.Query.RawQuery, viewStmt.Attributes)...) } } From 25a787a0049ed4ae18afd18094e20ddb7c6b4e43 Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 19:22:34 +0000 Subject: [PATCH 07/14] fix(check): view column types for string concatenation and AutoNumber String concatenation (r.Name + ' x') is a derived String(200) in mxbuild; string / string(100) declarations are CE6770 and now MDL031. A view attribute declared autonumber over an AutoNumber column (CE6770) is refused under mdl 1 and warns MDL-V1-VIEWAUTONUMBER without the header (ADR-0011); check's reference tier now runs under the script's language version. Part of ako/mxcli#981. Co-Authored-By: Claude Opus 5.5 --- docs-site/src/language/versions.md | 3 +- mdl/executor/language_changes.go | 2 +- mdl/executor/oql_type_inference.go | 69 ++++++++++++-- mdl/executor/oql_view_column_types_test.go | 100 +++++++++++++++++++++ mdl/executor/validate.go | 8 +- 5 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 mdl/executor/oql_view_column_types_test.go diff --git a/docs-site/src/language/versions.md b/docs-site/src/language/versions.md index 0af0449210..f9048f0caf 100644 --- a/docs-site/src/language/versions.md +++ b/docs-site/src/language/versions.md @@ -226,7 +226,7 @@ refuse the spelling; until then it only warns. ### Changes of meaning (`MDL-V1-*`) -21 constructs mean something different under the `mdl 1;` header. Without the header each keeps the meaning in the second column and warns with its code. +22 constructs mean something different under the `mdl 1;` header. Without the header each keeps the meaning in the second column and warns with its code. | Code | Without the header (mdl 0) | Under `mdl 1;` | Decided at | Rewritten by `fmt --upgrade` | |---|---|---|---|---| @@ -250,6 +250,7 @@ refuse the spelling; until then it only warns. | `MDL-V1-SLASH` | a `/` after a statement is accepted as a terminator (SQL*Plus style) | an error: `;` is the only statement terminator | parse | yes: `fmt --upgrade --header` | | `MDL-V1-TEMPLATE` | a message template written as one string literal that spans lines is an expression: the template is `{1}` and the literal its parameter | the template text, as a literal on one line is; a line break in a template is written into the literal | parse | yes: `fmt --upgrade --header` | | `MDL-V1-TEMPLATEATTR` | a text-template parameter bound to a non-String attribute (`{1} = $Order.Total`) was stored as `toString($Order/Total)` by earlier mxcli releases | an attribute reference, rendered with the attribute's formatting, as Studio Pro stores it | exec | no: decided by exec against the project; `check -p` reports it | +| `MDL-V1-VIEWAUTONUMBER` | a view entity attribute declared `autonumber` over an AutoNumber column passes check, and mxbuild reports CE6770 "View Entity is out of sync with the OQL Query" | a check error that names the attribute and suggests `long`, the type the view gives the column | exec | no: decided by exec against the project; `check -p` reports it | | `MDL-V1-WHILE` | `while ` accepts a body without `begin` and an `end` without `while` | an error: a while loop is `while begin … end while;`, like `loop … begin … end loop;` | parse | yes: `fmt --upgrade --header` | ### Deprecated spellings (`MDL-DEPR*`) diff --git a/mdl/executor/language_changes.go b/mdl/executor/language_changes.go index 2e566bd435..3d07d500c0 100644 --- a/mdl/executor/language_changes.go +++ b/mdl/executor/language_changes.go @@ -12,6 +12,6 @@ import "github.com/mendixlabs/mxcli/mdl/langver" func LanguageChanges() []langver.Change { return []langver.Change{ actionSlotRefused, galleryClickRefused, flowRebuildRefused, boundaryDropAmbiguous, - remoteTypeChangeRefused, templateAttrBinding, + remoteTypeChangeRefused, templateAttrBinding, viewAutoNumberRefused, } } diff --git a/mdl/executor/oql_type_inference.go b/mdl/executor/oql_type_inference.go index 5845a9b81d..560b037f1a 100644 --- a/mdl/executor/oql_type_inference.go +++ b/mdl/executor/oql_type_inference.go @@ -9,6 +9,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/langver" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -259,6 +260,20 @@ func inferTypeStatic(expr string) ast.DataType { } } + // String concatenation: a top-level `+` with a string operand is a derived + // string, String(200) in mxbuild whatever the operands' lengths — + // `r.Name + ' x'` over a String(100) Name builds only when the view + // declares String(200); `string` and `string(100)` are CE6770 (measured, + // 11.13.0, ako/mxcli#981). Checked before the prefix rules below, which + // would otherwise read `cast(…) + ' x'` as the cast alone. + if parts := splitTopLevelPlus(expr); len(parts) > 1 { + for _, p := range parts { + if p != "" && inferTypeStatic(p).Kind == ast.TypeString { + return ast.DataType{Kind: ast.TypeString, Length: derivedStringLength} + } + } + } + // count(...) → Integer (Mendix OQL COUNT returns Integer) if strings.HasPrefix(upper, "COUNT(") { return ast.DataType{Kind: ast.TypeInteger} @@ -461,9 +476,23 @@ func passthroughLengthError(attrName string, declared, inferred ast.DataType, ex sourceEntity, sourceAttr, attrName, formatDataTypeForMDL(inferred)) } -// validateViewEntityTypes validates that declared attribute types match inferred OQL types. -func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) []string { - var errors []string +// viewAutoNumberRefused is the language change for ako/mxcli#981 item 4: a view +// attribute declared AutoNumber over an AutoNumber column. Mendix reports +// CE6770 "View Entity is out of sync with the OQL Query" for it (measured, mx +// check 11.13.0); the column is a Long in the view. check accepted it on +// purpose until a header could carry the rejection (ADR-0011). +var viewAutoNumberRefused = langver.Change{ + Code: "MDL-V1-VIEWAUTONUMBER", + Since: langver.V1, + Old: "a view entity attribute declared `autonumber` over an AutoNumber column passes check, " + + "and mxbuild reports CE6770 \"View Entity is out of sync with the OQL Query\"", + New: "a check error that names the attribute and suggests `long`, the type the view gives the column", +} + +// validateViewEntityTypes validates that declared attribute types match +// inferred OQL types. The warnings are the findings a headerless script keeps +// at their old meaning (viewAutoNumberRefused). +func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) (errors, warnings []string) { // First validate OQL syntax for common mistakes syntaxViolations := ValidateOQLSyntax(stmt.Query.RawQuery) @@ -507,6 +536,21 @@ func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) [ continue } + if attr.Type.Kind == ast.TypeAutoNumber && col.InferredType.Kind == ast.TypeAutoNumber { + msg := fmt.Sprintf( + "attribute '%s': declared as AutoNumber over the AutoNumber column '%s' — a view reads the "+ + "column as a Long, and mxbuild reports CE6770 \"View Entity is out of sync with the OQL Query\". "+ + "Fix: change to '%s: Long'", attr.Name, col.Expression, attr.Name) + if viewAutoNumberRefused.Applies(ctx.LanguageVersion) { + errors = append(errors, msg) + } else { + warnings = append(warnings, fmt.Sprintf("[%s] view entity %s: %s. %s", + viewAutoNumberRefused.Code, stmt.Name.String(), msg, + viewAutoNumberRefused.Warning(ctx.LanguageVersion))) + } + continue + } + // Compare types if !typesCompatible(attr.Type, col.InferredType) { errors = append(errors, fmt.Sprintf( @@ -520,7 +564,7 @@ func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) [ } } - return errors + return errors, warnings } // Mendix OQL has TWO clause orders and the MDL grammar accepts both @@ -771,6 +815,17 @@ func parseSelectColumns(selectClause string) []string { func inferTypeFromExpression(ctx *ExecContext, expr string, col *OQLColumnInfo, aliasMap map[string]string) ast.DataType { expr = strings.TrimSpace(expr) + // String concatenation is a derived String(200), whatever its operands — + // see inferTypeStatic. Here an operand can also be a string attribute. + if parts := splitTopLevelPlus(expr); len(parts) > 1 { + for _, p := range parts { + var scratch OQLColumnInfo + if p != "" && inferTypeFromExpression(ctx, p, &scratch, aliasMap).Kind == ast.TypeString { + return ast.DataType{Kind: ast.TypeString, Length: derivedStringLength} + } + } + } + // Check for aggregate functions if aggType := inferAggregateType(ctx, expr, col, aliasMap); aggType.Kind != ast.TypeUnknown { return aggType @@ -1076,8 +1131,10 @@ func typesCompatible(declared, inferred ast.DataType) bool { // OQL Query" for one declared AutoNumber (measured, mx check 11.13). Long // used to be refused here once the source entity existed — accepted on the // run that created it, refused on every run after (ako/mxcli#859, - // rehearsal V1). A declared AutoNumber stays accepted as before: refusing - // it is a new rejection, which ADR-0011 reserves for the mdl 1 header. + // rehearsal V1). A declared AutoNumber is accepted here: validateViewEntityTypes + // refuses it under `mdl 1;` and warns without the header, because refusing + // it is a new rejection, which ADR-0011 reserves for the header + // (viewAutoNumberRefused, ako/mxcli#981). if inferred.Kind == ast.TypeAutoNumber { if declared.Kind == ast.TypeAutoNumber { return true diff --git a/mdl/executor/oql_view_column_types_test.go b/mdl/executor/oql_view_column_types_test.go new file mode 100644 index 0000000000..9e10ddeef6 --- /dev/null +++ b/mdl/executor/oql_view_column_types_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 + +// ako/mxcli#981 items 4 and 5 — view-entity column types check accepted and +// mxbuild reports as CE6770 (measured, 11.13.0). +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/langver" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// Item 5 — `r.Name + ' x'` is String(200) in mxbuild whatever Name's length: +// declared String(200) builds, `string` and `string(100)` are CE6770. +func TestStringConcatenationIsADerivedString200(t *testing.T) { + const oql = `select r.Name + ' x' as S from MyFirstModule.Race as r` + for _, c := range []struct { + declared ast.DataType + refused bool + }{ + {ast.DataType{Kind: ast.TypeString}, true}, + {ast.DataType{Kind: ast.TypeString, Length: 100}, true}, + {ast.DataType{Kind: ast.TypeString, Length: 102}, true}, + {ast.DataType{Kind: ast.TypeString, Length: 200}, false}, + } { + vs := ValidateOQLTypes(oql, []ast.ViewAttribute{{Name: "S", Type: c.declared}}) + if (len(vs) > 0) != c.refused { + t.Errorf("declared %s: refused=%v, want %v (%v)", formatDataTypeForError(c.declared), len(vs) > 0, c.refused, vs) + } + } + // Control: a literal on the left builds as String(200) too (measured). + if vs := ValidateOQLTypes(`select 'Season ' + r.Name as S from MyFirstModule.Race as r`, + []ast.ViewAttribute{{Name: "S", Type: ast.DataType{Kind: ast.TypeString, Length: 200}}}); len(vs) > 0 { + t.Errorf("'Season ' + r.Name declared String(200) was refused: %v", vs) + } + // Numeric `+` is not a string. + if got := inferTypeStatic("r.Season + 1"); got.Kind == ast.TypeString { + t.Errorf("r.Season + 1 inferred as a string") + } +} + +// Item 4 — a view attribute declared AutoNumber over an AutoNumber column is +// CE6770 (measured, 11.13.0); Long builds. A new rejection, so per ADR-0011 it +// is an error under `mdl 1;` and a warning without the header. +func TestViewAutoNumberOverAutoNumberIsGatedOnTheHeader(t *testing.T) { + race := &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: "ent-race", TypeName: "DomainModels$Entity"}, + Name: "Race", + Persistable: true, + Attributes: []*domainmodel.Attribute{ + {BaseElement: model.BaseElement{ID: "a-nr"}, Name: "Nr", Type: &domainmodel.AutoNumberAttributeType{}}, + }, + } + view := func(kind ast.DataTypeKind) *ast.CreateViewEntityStmt { + return &ast.CreateViewEntityStmt{ + Name: ast.QualifiedName{Module: "ServiceCore", Name: "V5"}, + Attributes: []ast.ViewAttribute{{Name: "Nr", Type: ast.DataType{Kind: kind}}}, + Query: ast.OQLQuery{RawQuery: "select r.Nr as Nr from ServiceCore.Race as r"}, + } + } + for _, c := range []struct { + name string + lang langver.Version + declared ast.DataTypeKind + refused bool + warned bool + }{ + {"autonumber under mdl 1", langver.V1, ast.TypeAutoNumber, true, false}, + {"autonumber without the header", langver.V0, ast.TypeAutoNumber, false, true}, + // Control: Long builds at 0 errors, under either version. + {"long under mdl 1", langver.V1, ast.TypeLong, false, false}, + {"long without the header", langver.V0, ast.TypeLong, false, false}, + } { + t.Run(c.name, func(t *testing.T) { + ctx := dropCheckCtx(t, race) + ctx.LanguageVersion = c.lang + sc := newScriptContext() + err := validateWithContext(ctx, view(c.declared), sc) + if (err != nil) != c.refused { + t.Fatalf("refused=%v, want %v: %v", err != nil, c.refused, err) + } + if c.refused && !strings.Contains(err.Error(), "'Nr: Long'") { + t.Errorf("the refusal should suggest Long: %v", err) + } + warned := false + for _, w := range sc.warnings { + if strings.Contains(w, viewAutoNumberRefused.Code) { + warned = true + } + } + if warned != c.warned { + t.Errorf("warned=%v, want %v: %v", warned, c.warned, sc.warnings) + } + }) + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 728989e983..247875c29c 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -518,6 +518,7 @@ func pageDefinedAfter(prog *ast.Program, ref string, fromIdx int) bool { // ValidateProgram validates all statements in a program, skipping references // to objects that are defined within the script itself. func (e *Executor) ValidateProgram(prog *ast.Program) []error { + defer e.enterLanguage(prog.LanguageVersion)() return validateProgram(e.newExecContext(context.Background()), prog) } @@ -525,6 +526,9 @@ func (e *Executor) ValidateProgram(prog *ast.Program) []error { // block: unresolved references inside EXCLUDED documents, which Mendix does // not validate. Callers print them so that nothing the check relaxed is hidden. func (e *Executor) ValidateProgramWithWarnings(prog *ast.Program) ([]error, []string) { + // The script's header decides the gated rejections check predicts + // (viewAutoNumberRefused), as it does for exec. + defer e.enterLanguage(prog.LanguageVersion)() return validateProgramWithWarnings(e.newExecContext(context.Background()), prog) } @@ -867,7 +871,9 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext s.Name.String(), strings.Join(objErrors, "\n - ")) } // Validate OQL types match declared attribute types - if typeErrors := validateViewEntityTypes(ctx, s); len(typeErrors) > 0 { + typeErrors, typeWarnings := validateViewEntityTypes(ctx, s) + sc.warnings = append(sc.warnings, typeWarnings...) + if len(typeErrors) > 0 { return mdlerrors.NewValidationf("view entity '%s' has type mismatches:\n - %s", s.Name.String(), strings.Join(typeErrors, "\n - ")) } From 0953ed0799f16e8a55178fd786929de8cb33b0cc Mon Sep 17 00:00:00 2001 From: Ako Date: Sun, 4 Oct 2026 19:22:38 +0000 Subject: [PATCH 08/14] docs(skills): view-entity OQL rules from ako/mxcli#981; changelog and findings write-oql-queries: a view entity has no ID (count a non-null column), the GROUP BY / comparison / HSQLDB literal table, String(200) concatenation, AutoNumber -> Long. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 4 +++ .../skills/mendix/write-oql-queries/SKILL.md | 36 +++++++++++++++++-- CHANGELOG.md | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 7723d68b23..9f08db4526 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -857,3 +857,7 @@ {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#968 / mendixlabs/mxcli#1263: `datepicker d (DateFormat: Time)` or `DateFormat: Custom, CustomDateFormat: '\u2026'` passes check and exec, mx check 0 errors, but every picker is stored FormattingInfo.DateFormat=Date; describe prints no format, so describe \u2192 exec silently turns a Studio Pro date-time picker into a date-only one. Same drop for a text box's DecimalPrecision/GroupDigits", "cause": "Three hops each dropped it: buildDatePickerV3/buildTextBoxV3 never read the properties, widget_write.go hard-coded newFormattingInfo() on DatePicker and TextBox, and describe never extracted FormattingInfo. Check stayed silent because validateStaticWidgetUnknownProps exempted the dynamic-text format keys (dateformat, customdateformat, \u2026) on EVERY widget type, not just dynamictext", "fix": "pages.DatePicker.FormattingInfo; executor input_formatting.go (inputFormattingInfo + inputFormattingProblems shared by builder and MDL-WIDGET18 check), writer formattingInfoToGen(x.FormattingInfo), describeInputFormatting, pagemutator setWidgetFormattingMut; per-widget allow-list pages.FormattingProperties. Measured: Custom with empty pattern = CE0493; Studio Pro stores CustomDateFormat beside DateFormat DateTime (TestApp WorkflowCommons), so only a pattern with NO DateFormat is refused \u2014 the param-format rule that refused it broke check on describe output", "file": "mdl/executor/input_formatting.go", "insight": "A key exempted from the unknown-property warning must be exempted per widget type: the dynamic-text format keys were skipped on every widget, which turned `DateFormat:` on a date picker (where nothing read it) into a silent drop. Before refusing a cross-field combination, scan Studio Pro-authored units for it \u2014 CustomDateFormat beside DateTime is stored by Studio Pro, and refusing it broke check on describe output.", "test": "mdl/executor/input_formatting_pedapp_test.go, input_formatting_test.go, mdl/backend/modelsdk/widget_formatting_write_test.go"} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 2: `alter page … { set Action = microflow M.X on btn }` with M.X created earlier in the same script failed check (\"microflow not found\") and exec refused the script; the same for nanoflow and show page targets", "cause": "validateAlterSetProperties dry-runs the SET against the stored document, and resolveMicroflow / resolveNanoflowByName / resolvePageRef only know the session cache (createdMicroflows, …) that executing fills — which check never does", "file": "`mdl/executor/validate_alter_set.go` (scriptDeclaresMissing)", "insight": "A dry run of a mutator in check must treat a NotFound for a name the script declares (scriptContext.microflows/nanoflows/pages/snippets) as satisfied, matching on the typed mdlerrors.NotFoundError Kind+Name through errors.As rather than the message. Do not register fake IDs in ctx.Cache instead: exec runs check on the same executor and would resolve to them. Control: an undeclared target still fails", "refs": ["#969"]} {"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#969 item 3: a list view / data grid / gallery with `datasource: $currentObject/M.Assoc` over a single-object association passed check and exec, then mxbuild failed CE8812 \"A grid association path must result in a list\"", "cause": "No rule modelled association multiplicity for list widgets", "file": "`mdl/executor/validate_assoc_list_source.go` (MDL-ASSOCDS01), hooked into attributeScopeValidator.walk", "insight": "Measured 8 shapes x 3 widgets on 11.13.0 and 11.14.0, identical: CE8812 for a Reference followed from its FROM entity (owner Default or Both) and for a Reference with owner Both from the TO entity (one-to-one); the reverse of a default Reference and every ReferenceSet build clean. Judge only those shapes with an exact context entity; skip specializations, self-associations and multi-hop paths. The attribute-scope walk already carries the data context, so hook there", "refs": ["#969"]} +{"area": "mdl/executor", "date": "2026-10-04", "symptom": "ako/mxcli#981 items 1-3: view-entity OQL passed check and failed mxbuild CE0174 — `r.Season = r.CurrentSeason as IsCurrent` (\"The '=' part is incomplete or incorrect. You could use here: FROM.\"), `count(r.Season)` next to `group by r.Season` (also `max(r.D)` by `datepart(YEAR, r.D)`, `sum(r.S)` by `r.S + 1`), and a plain column that is not grouped (`r.Name` next to `group by datepart(...)`, `group by r.Season` or `group by r.ID`)", "cause": "ValidateOQLSyntax had no select-expression or GROUP BY rules at all", "file": "`mdl/executor/oql_view_select_checks.go` (MDL033/034/035), called from ValidateOQLSyntax", "insight": "Measure the controls along with the failures — they decide the predicate: the comparison inside CASE builds; count of a NON-grouped column builds; `group by