From 207c48c8f761238744fd838b1c959cc09b26fcca Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 05:49:35 +0000 Subject: [PATCH 1/2] fix(mdl): refuse `set` on an object variable instead of writing CE7247 `set $Cursor = $Next;` with both variables single objects passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable; ako/mxcli#949 routed the list case to Change list Replace and left objects on the primitive-only path. The builder (exec) and the check validator now refuse it and name the alternatives. The check validator typed every association retrieve as a list, so the reported shape (a Reference followed from its FROM entity) looked like a list to check; under --references it now takes the association shapes MDL-ASSOCDS01 already builds and types that retrieve as the object it is. Self-association retrieves stay lists (Mendix types them so, and `set` on them is a valid Replace). Measured on 11.14.0: faulted build -> CE7247; fixed build refuses in check and exec; the self-association and recursive sub-microflow forms in the bug-test -> 0 errors. Fixes mendixlabs/mxcli#1323 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PMEw8qsiJuVoaaGJawicfo --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../reference/data-operations.md | 6 + CHANGELOG.md | 1 + .../bug-tests/1323-set-object-variable.mdl | 48 ++++++ mdl/executor/cmd_microflows_builder.go | 5 + .../cmd_microflows_builder_actions.go | 37 +++++ mdl/executor/cmd_microflows_builder_graph.go | 1 + .../cmd_microflows_builder_validate.go | 39 ++++- mdl/executor/set_object_variable_test.go | 137 ++++++++++++++++++ mdl/executor/validate.go | 4 +- 10 files changed, 270 insertions(+), 9 deletions(-) create mode 100644 mdl-examples/bug-tests/1323-set-object-variable.mdl create mode 100644 mdl/executor/set_object_variable_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 78100e936..3b710bf04 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -868,3 +868,4 @@ {"area": "mdl/executor", "date": "2026-10-05", "symptom": "On Mendix 11.15, `TestMxCheck_DoctypeScripts/40-message-definition-examples.mdl` fails with CE0270 \"No root element could be found in the schema\" at MsgTest.IMM_Order. On a converted or Studio Pro 11.15 project, `list message definition collections` finds nothing, and `describe import mapping` prints no source", "cause": "Mendix 11.15 replaced `MessageDefinitions$MessageDefinitionCollection` with one `MessageDefinitions$MessageDefinition2` document per definition. `mx convert` turns the collection into a `Projects$Folder` with the same unit ID and name, and the ExposedEntity tree is byte-identical apart from $IDs. mxcli read and wrote only collections, so on 11.15 there was nothing to resolve a mapping against", "file": "`mdl/backend/modelsdk/messagedefinition_document.go` (read/write, reusing the collection's exposedNodeFromGen/messageNodeToGen), `mdl/executor/cmd_messagedefinition_documents.go` (create/describe/drop/list + version refusals), `mdl/executor/mapping_messagedefinition.go` (`findMessageDefinition(ctx, ref)` returns the stored reference), `cmd_messagedefinitions.go` (alter target resolution, association-drop guard), grammar `createMessageDefinitionStatement` / `DROP|DESCRIBE MESSAGE DEFINITION` / `LIST MESSAGE DEFINITIONS`", "insight": "**`mx convert` is the oracle for a storage change.** Converting a known 11.14 project gives a Studio Pro-authored 11.15 reference for the new shape and for how references move, so no guessing is needed. **Under the revert, 11.15 fails earlier than CE0270.** A two-part name written into the old `MessageDefinition` key makes mxbuild refuse to load the project (`StorageLoadException: ... is not a valid OldMessageDefinitionIdentifier`). The old key is typed as a three-part identifier even though 11.15 no longer reads it. The doctype script uses `-- @version: ..11.14` / `11.15+` sections, and the 11.15 mapping needs a different name from the older section's, because a plain `check` sees both sections and reports MDL-DUPDEF", "refs": ["ako/mxcli#987"]} {"area": "mdl/executor", "date": "2026-10-06", "symptom": "v0.25.0: `describe microflow` 4-8x slower than v0.24.0 on a flow arranged by hand in Studio Pro, and the cost grows faster than the flow (60 if/else blocks: 4.7 s -> 35.6 s); flows laid out by mxcli barely change", "cause": "The canonical describe's layout derivation (#748) re-renders the description every round, and a hand-laid flow takes gatedLayoutRounds+2 = 8 rounds. Each render re-ran the stored flow's split/merge analysis (findSplitMergePoints, labelCrossedMerges) and the body warnings (postDominators via microflowgraph.Analyze) - superlinear, and independent of which annotations the round keeps", "file": "`mdl/executor/cmd_microflows_derived_layout.go` (flowAnalysisMemo, installed by useDerivedFlowLayout, rebuildOnly view for the rounds), `mdl/executor/cmd_microflows_show.go` (findSplitMergePoints / labels / microflowBodyWarnings go through it)", "insight": "**Profile before believing the issue's theory.** The report blamed the round count, but rounds were already capped at 8; the rebuild itself was cheap. The CPU profile showed the cost was the *render* inside each round - the stored flow's graph analysis, redone per round. Memoising it per describe (keyed on the stored collection pointer) and leaving the `-- WARNING` comment lines out of rounds (the rebuild never reads them) gave 14.2 s -> 1.7 s at 60 blocks, with the printed description byte-identical to the unfixed build. A wall-clock assertion would be flaky; the test counts split/merge analyses per describe (18 over 8 rounds before, 2 after), with the round count as its control", "refs": ["mendixlabs/mxcli#1301"]} {"area":"mdl-executor","date":"2026-10-05","symptom":"A view entity selecting a **non-localized** DateTime column (Studio Pro's \"Localize\" unticked — the normal choice for a calendar date) passes `mxcli check --references` and exec, then `mx check` fails CE6770 \"View Entity is out of sync with the OQL Query.\" The view attribute was always written LocalizeDate = true, and MDL has no spelling for the flag, so describe -> exec re-broke it on every run","cause":"execCreateViewEntity built every view attribute with convertDataType, whose DateTime is Mendix's default LocalizeDate = true; nothing looked at the attribute the column reads","file":"`mdl/executor/oql_view_localize_date.go` (viewDateTimeLocalize), `mdl/executor/cmd_entities.go` (execCreateViewEntity); test `mdl/executor/view_entity_localize_date_test.go`; bug-test `mdl-examples/bug-tests/1297-view-entity-non-localized-datetime.mdl`","insight":"**Derive, don't add syntax**: on a view entity the column's localization is a property of the query, so the fix reads it from the source attribute and describe -> exec round-trips (`Unchanged`) with nothing new to spell — same reasoning as the view association (the column is the declaration). **Measure the shapes before choosing the scope**: one project, one view per shape, mxbuild 11.12.5 — pass-through, `s/Attr`, MIN, MAX and CASE over a non-localized source are ALL CE6770 when written localized and 0 errors when patched to false, so a pass-through-only rule (the obvious one, mirroring the string-length rule) would have fixed the report and left MAX(date) broken. Sources that disagree (coalesce of a localized and a non-localized column) are unmeasured and keep the default. **Repro needs a Studio Pro-authored flag**: mxcli writes every persistent DateTime localized, so the bug is invisible from MDL alone — the reporter's pymongo patch flipping Sale.SaleDate is the cheapest stand-in. Control: stubbing the assignment fails 5 of 6 cases with `LocalizeDate = true`; pre-fix binary on the same project gives 5 × CE6770, fixed gives 0","refs":["mendixlabs/mxcli#1297"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $Cursor = $Next;` where both are OBJECT variables (Reference retrieves followed from the FROM entity) passes `check --references` and `exec` (\"Created microflow\"), is written as a Microflows$ChangeVariableAction, and mx check reports [CE7247] \"Variable 'Cursor' does not have a primitive type.\" Natural to write when walking a parent chain in a `while` loop", "cause": "`set` on a plain variable had two outcomes: Change list Replace for a known list (ako/mxcli#949) and Change variable for everything else, so an object fell into the primitive-only action. Mendix has NO action that reassigns an object variable. Check could not have caught it even with a guard: the check-time validator (validateFlowBody) typed every association retrieve as `List of `, because it had no association multiplicity, so the forward-Reference object read as a list", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (`objectVariableType`, `refuseSetOnObject`), called from `cmd_microflows_builder_graph.go` (exec) and `cmd_microflows_builder_validate.go` (check); `validate.go` passes `checkAssociationShapes(ctx, sc)` into `validateFlowBody` so `forwardReferenceTarget` types a forward Reference retrieve as an object", "insight": "A refusal is only as good as the variable typing under it, and check and exec type variables differently: exec asks the backend for the association, check guessed `List of`. A guard added to the check validator alone passes the reported repro because the object it should refuse is, to check, a list. Write the check-path test against the REPORTED shape (association retrieve), not an object parameter, or it goes green while the issue stays open. Reuse `checkAssociationShapes` (project + script associations, built for MDL-ASSOCDS01) rather than a new lookup. Type only the unambiguous shape (Reference followed from FROM, not a self-association): Mendix types a self-association retrieve as a list, and that `set` is a valid Change list Replace. Measured on 11.14.0: faulted build exec -> CE7247; the self-association and recursive sub-microflow forms -> 0 errors. Plain `check` with no -p does not run validateFlowBody at all, so a `.fail.mdl` cannot pin this", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#949"], "ce": ["CE7247"]} diff --git a/.claude/skills/mendix/write-microflows/reference/data-operations.md b/.claude/skills/mendix/write-microflows/reference/data-operations.md index e7f319897..7870d40ab 100644 --- a/.claude/skills/mendix/write-microflows/reference/data-operations.md +++ b/.claude/skills/mendix/write-microflows/reference/data-operations.md @@ -134,6 +134,12 @@ primitive type"). mxcli knows a variable is a list when it is a list parameter, a `create list`, a list retrieve or a list operation's result. Both work in microflows and nanoflows. +`set` on an **object** variable is refused (`check --references` and `exec`): +Mendix has no action that reassigns an object variable, and a Change variable on +one is the same CE7247. To walk a chain (`$Cursor = $Next` in a `while` loop), +write a sub-microflow that **returns** the next object and recurse; to change the +object itself, use `change $Obj (…)`. + ### One statement per activity Every list operation and aggregate is **one Studio Pro activity**, and it is diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a7e5387..3fb6393f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **`set $Obj = …` on an object variable is refused** (mendixlabs/mxcli#1323) — with both variables single objects (e.g. a Reference retrieved from its FROM entity), `set $Cursor = $Next;` passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable, so `check --references` and `exec` now refuse it and name the alternatives (a sub-microflow that returns the next object, `change $Obj (…)`). `check --references` now types a Reference retrieve from its FROM entity as the object it is. `set` on a list variable stays a Change list Replace (ako/mxcli#949). - **`run --local --watch` starts on Mendix 10.24 and 11.6** — it waited the full web-client timeout (5 minutes) for a bundle that had finished in seconds, then failed with `web client watcher timed out`. The rollup runner those versions ship reports its status as `{"code":"SUCCESS"}` rather than the modern-web-bundler protocol mxcli was reading; both are now understood, including that runner's error reports. - **`run --local --watch` no longer loses a change made while the app boots** — the watch loop took its baseline after the boot, so a model written during the ~15 s boot was never built and the app kept serving the model from before it. The baseline is now the source time the boot build was made from, so that change is built on the first tick. diff --git a/mdl-examples/bug-tests/1323-set-object-variable.mdl b/mdl-examples/bug-tests/1323-set-object-variable.mdl new file mode 100644 index 000000000..5d5413928 --- /dev/null +++ b/mdl-examples/bug-tests/1323-set-object-variable.mdl @@ -0,0 +1,48 @@ +mdl 1; +-- ============================================================================ +-- Upstream #1323: `set $Obj = $Other` on an OBJECT variable is refused +-- ============================================================================ +-- +-- Reported (v0.24.0 and v0.25.0, Mendix 11.14.0): with $Cursor and $Next both +-- single G46.Group objects (a Reference followed from its FROM entity), +-- +-- set $Cursor = $Next; +-- +-- passed `check --references` and `exec`, and was written as a +-- Microflows$ChangeVariableAction. mx check then answered +-- [CE7247] "Variable 'Cursor' does not have a primitive type." +-- Mendix has no action that reassigns an object variable: Change variable +-- takes only a primitive, and Change list Replace (ako/mxcli#949) only a list. +-- `check --references` and `exec` now refuse it, naming the alternatives. +-- (Plain `check` without -p does not run the flow-body validator, so the +-- refusal is pinned by mdl/executor/set_object_variable_test.go, not here.) +-- +-- This file holds the forms that DO build — each measured on 11.14.0 with +-- mx check reporting 0 errors. +-- ============================================================================ + +create module F1323; +create persistent entity F1323.Node ("Name": String(50)); +create association F1323.Node_Parent from F1323.Node to F1323.Node type Reference; + +-- 1. The workaround for a chain walk: recursion in a sub-microflow that +-- RETURNS the next object, instead of reassigning a cursor variable. +create microflow F1323.GET_Root ($Node: F1323.Node) returns F1323.Node as $Root +begin + retrieve $Parent from $Node/F1323.Node_Parent; + if $Parent = empty then + return $Node; + else + $Root = call microflow F1323.GET_Root(Node = $Parent); + return $Root; + end if; +end; + +-- 2. A self-association retrieve is typed as a LIST by Mendix, so `set` on it +-- is a Change list Replace (ako/mxcli#949) and stays accepted. +create microflow F1323.ReplaceList ($Start: F1323.Node, $Other: F1323.Node) +begin + retrieve $Cursor from $Start/F1323.Node_Parent; + retrieve $Next from $Other/F1323.Node_Parent; + set $Cursor = $Next; +end; diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index dc96d1f1c..d273f4212 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -23,6 +23,11 @@ type flowBuilder struct { // validator scoped names per branch and counted every call output, so it // was wrong both ways. Rules keep it — MDL063 does not run on them. duplicateNamesOwnedElsewhere bool + // checkAssocShapes is check --references' view of the project's and the + // script's associations, so the validator types a forward Reference + // retrieve as the object it yields (validateFlowBody). nil on the exec path, + // which resolves associations through the backend instead. + checkAssocShapes map[string]assocShape objects []microflows.MicroflowObject flows []*microflows.SequenceFlow diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index c7466830c..5fe9c8f83 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -2117,6 +2117,43 @@ func (fb *flowBuilder) isListVariable(name string) bool { return strings.HasPrefix(fb.varTypes[strings.TrimPrefix(name, "$")], "List of ") } +// objectVariableType returns the entity of a variable this flow knows to hold +// a single OBJECT (a parameter, a retrieve's object range or forward Reference, +// a create object, a cast, a head/find), and false for a primitive, a list, a +// member path or a variable of unknown type. +func (fb *flowBuilder) objectVariableType(name string) (string, bool) { + name = strings.TrimPrefix(name, "$") + if strings.Contains(name, "/") { + return "", false + } + if _, primitive := fb.declaredVars[name]; primitive { + return "", false + } + t := fb.varTypes[name] + if t == "" || strings.HasPrefix(t, "List of ") || !strings.Contains(t, ".") { + return "", false + } + return t, true +} + +// refuseSetOnObject reports `set $Obj = …` on an object variable. Mendix has no +// action that reassigns one: Change variable takes only a primitive, and +// mxbuild answers CE7247 "Variable 'X' does not have a primitive type" +// (mendixlabs/mxcli#1323). A list has Change list Replace (ako/mxcli#949); an +// object has nothing, so the statement is refused rather than written. +func (fb *flowBuilder) refuseSetOnObject(target string) bool { + entity, ok := fb.objectVariableType(target) + if !ok { + return false + } + name := strings.TrimPrefix(target, "$") + fb.addError("cannot set object variable '$%s' (%s): Mendix has no action that reassigns an object variable — "+ + "Change variable takes only a primitive, and mxbuild rejects it with CE7247 \"Variable '%s' does not have a primitive type\". "+ + "Return the new object from a sub-microflow instead (recursion for a chain walk), retrieve it into a new variable, "+ + "or change the object's members with `change $%s (…)`", name, entity, name, name) + return true +} + // addChangeListAction appends a Change list activity of the given operation. // eh is the statement's `on error` clause, nil for the statements that take // none; the caller finishes a custom handler. diff --git a/mdl/executor/cmd_microflows_builder_graph.go b/mdl/executor/cmd_microflows_builder_graph.go index b83567a12..90dbaf9bc 100644 --- a/mdl/executor/cmd_microflows_builder_graph.go +++ b/mdl/executor/cmd_microflows_builder_graph.go @@ -656,6 +656,7 @@ func (fb *flowBuilder) addStatement(stmt ast.MicroflowStatement) model.ID { if fb.isListVariable(s.Target) { return fb.addReplaceListAction(s) } + fb.refuseSetOnObject(s.Target) return fb.addChangeVariableAction(s) case *ast.ReturnStmt: return fb.addEndEventWithReturn(s) diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index fc7d7d9d8..aefc5c497 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -5,6 +5,7 @@ package executor import ( "fmt" + "strings" "github.com/mendixlabs/mxcli/mdl/ast" ) @@ -12,19 +13,21 @@ import ( // ValidateMicroflowBody validates the microflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateMicroflowBody(s *ast.CreateMicroflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true) + return validateFlowBody(s.Parameters, s.Body, true, nil) } // ValidateNanoflowBody validates the nanoflow body for semantic errors without building objects. // This is used by the check command to validate scripts without executing them. func ValidateNanoflowBody(s *ast.CreateNanoflowStmt) []string { - return validateFlowBody(s.Parameters, s.Body, true) + return validateFlowBody(s.Parameters, s.Body, true, nil) } // validateFlowBody validates parameters and body statements for semantic errors. // duplicatesOwnedElsewhere leaves duplicate variable names to MDL063 — see -// flowBuilder.duplicateNamesOwnedElsewhere. -func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool) []string { +// flowBuilder.duplicateNamesOwnedElsewhere. assocs, when check --references has +// a project, types an association retrieve as the object or list it yields; +// nil leaves every association retrieve a list, as without a project. +func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement, duplicatesOwnedElsewhere bool, assocs map[string]assocShape) []string { varTypes := make(map[string]string) declaredVars := make(map[string]string) @@ -56,6 +59,7 @@ func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement declaredVars: declaredVars, errors: []string{}, duplicateNamesOwnedElsewhere: duplicatesOwnedElsewhere, + checkAssocShapes: assocs, } fb.validateStatements(body) @@ -101,6 +105,8 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { fb.addErrorWithExample( fmt.Sprintf("variable '%s' is not declared", s.Target), errorExampleDeclareVariable(s.Target)) + } else if !strings.Contains(s.Target, "/") { + fb.refuseSetOnObject(s.Target) } case *ast.IfStmt: @@ -308,8 +314,11 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { // Register retrieved variable if s.Variable != "" && s.Source.Module != "" { if s.StartVariable != "" { - // Association retrieve always returns a list - fb.varTypes[s.Variable] = "List of " + s.Source.Module + "." + s.Source.Name + if to, ok := fb.forwardReferenceTarget(s); ok { + fb.varTypes[s.Variable] = to + } else { + fb.varTypes[s.Variable] = "List of " + s.Source.Module + "." + s.Source.Name + } } else if s.First { fb.varTypes[s.Variable] = s.Source.Module + "." + s.Source.Name } else { @@ -397,5 +406,21 @@ func (fb *flowBuilder) validateOutputVariable(varName, statement string) { // counterpart of ValidateMicroflowBody. What a rule may not *contain* is a // separate question, answered by validateRule. func ValidateRuleBody(s *ast.CreateRuleStmt) []string { - return validateFlowBody(s.Parameters, s.Body, false) + return validateFlowBody(s.Parameters, s.Body, false, nil) +} + +// forwardReferenceTarget reports the entity a retrieve over association yields +// when that is one object: a Reference followed from its FROM entity, the same +// reading the builder makes. A self-association, a reverse traversal, a +// ReferenceSet or an association check cannot resolve stays a list, so the +// rules keyed on it never see an object that is not one. +func (fb *flowBuilder) forwardReferenceTarget(s *ast.RetrieveStmt) (string, bool) { + shape, ok := fb.checkAssocShapes[strings.ToLower(s.Source.Module+"."+s.Source.Name)] + if !ok || shape.refSet || strings.EqualFold(shape.from, shape.to) { + return "", false + } + if !strings.EqualFold(fb.varTypes[s.StartVariable], shape.from) { + return "", false + } + return shape.to, true } diff --git a/mdl/executor/set_object_variable_test.go b/mdl/executor/set_object_variable_test.go new file mode 100644 index 000000000..8d20f051e --- /dev/null +++ b/mdl/executor/set_object_variable_test.go @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// The repro from mendixlabs/mxcli#1323, verbatim: two Reference retrieves +// followed from the association's FROM entity, so both $Cursor and $Next are +// single G46.Group objects, then `set $Cursor = $Next`. It passed +// `check --references` and `exec`, and mx check answered +// [CE7247] "Variable 'Cursor' does not have a primitive type." +const setObjectRepro = `create module "G46"; +create persistent entity "G46"."Group" ("Name": String(50)); +create persistent entity "G46"."Node" ("Name": String(50)); +create association "G46"."Node_Group" from "G46"."Node" to "G46"."Group" type Reference; +create microflow "G46"."GET_OtherGroup" ($A: "G46"."Node", $B: "G46"."Node") returns "G46"."Group" as $Cursor +begin + retrieve $Cursor from $A/G46.Node_Group; + retrieve $Next from $B/G46.Node_Group; + set $Cursor = $Next; + return $Cursor; +end; +/ +` + +// checkScriptMicroflows runs check --references' per-statement validation over +// every microflow in src and returns the joined errors. +func checkScriptMicroflows(t *testing.T, src string) string { + t.Helper() + ctx := assocShapeCtx(t) + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + sc := newScriptContext() + sc.collectDefinitions(prog) + var out []string + for _, st := range prog.Statements { + if _, ok := st.(*ast.CreateMicroflowStmt); !ok { + continue + } + if err := validateWithContext(ctx, st, sc); err != nil { + out = append(out, err.Error()) + } + } + return strings.Join(out, "\n") +} + +// check --references refuses the #1323 repro, naming the CE7247 it prevents. +func TestCheckRefusesSetOnAssociationRetrievedObject(t *testing.T) { + got := checkScriptMicroflows(t, setObjectRepro) + if !strings.Contains(got, "CE7247") || !strings.Contains(got, "$Cursor") { + t.Fatalf("check accepted `set` on object variable $Cursor; got %q", got) + } +} + +// Controls for the check path: the shapes where the retrieve yields a LIST +// keep passing, because `set` on a list is a Change list Replace (ako/mxcli#949). +func TestCheckAcceptsSetOnAssociationRetrievedList(t *testing.T) { + for name, mf := range map[string]string{ + // Reference followed from its TO entity: the reverse is a list. + "reverse Reference": `create microflow C88.M ($X: C88.B, $Y: C88.B) +begin + retrieve $L from $X/C88.R_def; + retrieve $M from $Y/C88.R_def; + set $L = $M; +end; +/ +`, + // ReferenceSet from the FROM entity: a list. + "ReferenceSet": `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + retrieve $L from $X/C88.RS_def; + retrieve $M from $Y/C88.RS_def; + set $L = $M; +end; +/ +`, + } { + t.Run(name, func(t *testing.T) { + if got := checkScriptMicroflows(t, mf); got != "" { + t.Errorf("check refused `set` on a list: %s", got) + } + }) + } +} + +// The same refusal for a stored association, from the project rather than the +// script, and for object variables check knows without one (a parameter). +func TestCheckRefusesSetOnObjectVariable(t *testing.T) { + for name, mf := range map[string]string{ + "stored Reference forward": `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + retrieve $O from $X/C88.R_def; + retrieve $P from $Y/C88.R_def; + set $O = $P; +end; +/ +`, + "object parameter": `create microflow C88.M ($X: C88.A, $Y: C88.A) +begin + set $X = $Y; +end; +/ +`, + } { + t.Run(name, func(t *testing.T) { + if got := checkScriptMicroflows(t, mf); !strings.Contains(got, "CE7247") { + t.Errorf("check accepted `set` on an object variable; got %q", got) + } + }) + } +} + +// exec: the builder refuses `set` on an object variable rather than writing a +// Change variable action mxbuild rejects with CE7247. +func TestSetOnObjectVariableRefusedByBuilder(t *testing.T) { + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + varTypes: map[string]string{"Cursor": "G46.Group", "Next": "G46.Group"}, + declaredVars: map[string]string{}, + } + fb.buildFlowGraph([]ast.MicroflowStatement{ + &ast.MfSetStmt{Target: "Cursor", Value: &ast.VariableExpr{Name: "Next"}}, + }, nil) + if got := strings.Join(fb.errors, "\n"); !strings.Contains(got, "CE7247") || !strings.Contains(got, "$Cursor") { + t.Fatalf("builder accepted `set` on object variable $Cursor; errors = %q", got) + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 747bf4cd8..8466b9ec3 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -694,7 +694,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := ValidateMicroflowBody(s) + validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside microflow body (pages, microflows, java actions, entities) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if len(refErrors) > 0 && s.Excluded { @@ -737,7 +737,7 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext // Reported together with the reference errors below rather than // instead of them: a body error used to hide a call's unknown // parameter, which mxbuild reports as well (CE1613, #953). - validationErrors := ValidateNanoflowBody(s) + validationErrors := validateFlowBody(s.Parameters, s.Body, true, checkAssociationShapes(ctx, sc)) // Validate references inside nanoflow body (an excluded nanoflow's are warnings) refErrors := validateFlowBodyReferences(ctx, s.Body, sc) if len(refErrors) > 0 && s.Excluded { From 032909943166175b73fd6500261fecf897d07d7f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 7 Oct 2026 09:37:03 +0000 Subject: [PATCH 2/2] feat(check): MDL-SET01 refuses `set` on an object variable without a project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain `mxcli check` runs only the MDL0xx rule set, never validateFlowBody, so the #1323 refusal reached `check --references` and `exec` but not a check without -p (and check-mdl could not pin it with a .fail.mdl). MDL-SET01 judges the object producers visible in the text: entity parameters, create, `retrieve … first`, loop iterators over a known list, head, cast. Association retrieves need the project and stay with validateFlowBody, which now reports only those, so --references prints the refusal once. Microflows and nanoflows. Measured on 11.14.0 with the faulted build: every producer gives CE7247; a parameter is worded "Parameter 'A' cannot be changed.", which the message now quotes. Part of mendixlabs/mxcli#1323 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PMEw8qsiJuVoaaGJawicfo --- .../fix-issue/findings/mdl-executor.jsonl | 1 + .../reference/data-operations.md | 2 +- CHANGELOG.md | 2 +- cmd/mxcli/syntax/features_microflow.go | 5 +- docs-site/src/appendixes/error-messages.md | 23 ++++ .../1323-set-object-variable.fail.mdl | 27 ++++ .../bug-tests/1323-set-object-variable.mdl | 5 +- mdl/executor/cmd_microflows_builder.go | 4 + .../cmd_microflows_builder_validate.go | 7 +- mdl/executor/set_object_variable_test.go | 122 ++++++++++++++++-- mdl/executor/validate_microflow.go | 1 + mdl/executor/validate_microflow_set_object.go | 104 +++++++++++++++ mdl/executor/validate_nanoflow.go | 2 + 13 files changed, 286 insertions(+), 19 deletions(-) create mode 100644 mdl-examples/bug-tests/1323-set-object-variable.fail.mdl create mode 100644 mdl/executor/validate_microflow_set_object.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index 3b710bf04..e4554f95c 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -869,3 +869,4 @@ {"area": "mdl/executor", "date": "2026-10-06", "symptom": "v0.25.0: `describe microflow` 4-8x slower than v0.24.0 on a flow arranged by hand in Studio Pro, and the cost grows faster than the flow (60 if/else blocks: 4.7 s -> 35.6 s); flows laid out by mxcli barely change", "cause": "The canonical describe's layout derivation (#748) re-renders the description every round, and a hand-laid flow takes gatedLayoutRounds+2 = 8 rounds. Each render re-ran the stored flow's split/merge analysis (findSplitMergePoints, labelCrossedMerges) and the body warnings (postDominators via microflowgraph.Analyze) - superlinear, and independent of which annotations the round keeps", "file": "`mdl/executor/cmd_microflows_derived_layout.go` (flowAnalysisMemo, installed by useDerivedFlowLayout, rebuildOnly view for the rounds), `mdl/executor/cmd_microflows_show.go` (findSplitMergePoints / labels / microflowBodyWarnings go through it)", "insight": "**Profile before believing the issue's theory.** The report blamed the round count, but rounds were already capped at 8; the rebuild itself was cheap. The CPU profile showed the cost was the *render* inside each round - the stored flow's graph analysis, redone per round. Memoising it per describe (keyed on the stored collection pointer) and leaving the `-- WARNING` comment lines out of rounds (the rebuild never reads them) gave 14.2 s -> 1.7 s at 60 blocks, with the printed description byte-identical to the unfixed build. A wall-clock assertion would be flaky; the test counts split/merge analyses per describe (18 over 8 rounds before, 2 after), with the round count as its control", "refs": ["mendixlabs/mxcli#1301"]} {"area":"mdl-executor","date":"2026-10-05","symptom":"A view entity selecting a **non-localized** DateTime column (Studio Pro's \"Localize\" unticked — the normal choice for a calendar date) passes `mxcli check --references` and exec, then `mx check` fails CE6770 \"View Entity is out of sync with the OQL Query.\" The view attribute was always written LocalizeDate = true, and MDL has no spelling for the flag, so describe -> exec re-broke it on every run","cause":"execCreateViewEntity built every view attribute with convertDataType, whose DateTime is Mendix's default LocalizeDate = true; nothing looked at the attribute the column reads","file":"`mdl/executor/oql_view_localize_date.go` (viewDateTimeLocalize), `mdl/executor/cmd_entities.go` (execCreateViewEntity); test `mdl/executor/view_entity_localize_date_test.go`; bug-test `mdl-examples/bug-tests/1297-view-entity-non-localized-datetime.mdl`","insight":"**Derive, don't add syntax**: on a view entity the column's localization is a property of the query, so the fix reads it from the source attribute and describe -> exec round-trips (`Unchanged`) with nothing new to spell — same reasoning as the view association (the column is the declaration). **Measure the shapes before choosing the scope**: one project, one view per shape, mxbuild 11.12.5 — pass-through, `s/Attr`, MIN, MAX and CASE over a non-localized source are ALL CE6770 when written localized and 0 errors when patched to false, so a pass-through-only rule (the obvious one, mirroring the string-length rule) would have fixed the report and left MAX(date) broken. Sources that disagree (coalesce of a localized and a non-localized column) are unmeasured and keep the default. **Repro needs a Studio Pro-authored flag**: mxcli writes every persistent DateTime localized, so the bug is invisible from MDL alone — the reporter's pymongo patch flipping Sale.SaleDate is the cheapest stand-in. Control: stubbing the assignment fails 5 of 6 cases with `LocalizeDate = true`; pre-fix binary on the same project gives 5 × CE6770, fixed gives 0","refs":["mendixlabs/mxcli#1297"]} {"area": "mdl/executor", "date": "2026-10-07", "symptom": "`set $Cursor = $Next;` where both are OBJECT variables (Reference retrieves followed from the FROM entity) passes `check --references` and `exec` (\"Created microflow\"), is written as a Microflows$ChangeVariableAction, and mx check reports [CE7247] \"Variable 'Cursor' does not have a primitive type.\" Natural to write when walking a parent chain in a `while` loop", "cause": "`set` on a plain variable had two outcomes: Change list Replace for a known list (ako/mxcli#949) and Change variable for everything else, so an object fell into the primitive-only action. Mendix has NO action that reassigns an object variable. Check could not have caught it even with a guard: the check-time validator (validateFlowBody) typed every association retrieve as `List of `, because it had no association multiplicity, so the forward-Reference object read as a list", "file": "`mdl/executor/cmd_microflows_builder_actions.go` (`objectVariableType`, `refuseSetOnObject`), called from `cmd_microflows_builder_graph.go` (exec) and `cmd_microflows_builder_validate.go` (check); `validate.go` passes `checkAssociationShapes(ctx, sc)` into `validateFlowBody` so `forwardReferenceTarget` types a forward Reference retrieve as an object", "insight": "A refusal is only as good as the variable typing under it, and check and exec type variables differently: exec asks the backend for the association, check guessed `List of`. A guard added to the check validator alone passes the reported repro because the object it should refuse is, to check, a list. Write the check-path test against the REPORTED shape (association retrieve), not an object parameter, or it goes green while the issue stays open. Reuse `checkAssociationShapes` (project + script associations, built for MDL-ASSOCDS01) rather than a new lookup. Type only the unambiguous shape (Reference followed from FROM, not a self-association): Mendix types a self-association retrieve as a list, and that `set` is a valid Change list Replace. Measured on 11.14.0: faulted build exec -> CE7247; the self-association and recursive sub-microflow forms -> 0 errors. Plain `check` with no -p does not run validateFlowBody at all, so a `.fail.mdl` cannot pin this", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#949"], "ce": ["CE7247"]} +{"area": "mdl/executor", "date": "2026-10-07", "symptom": "Plain `mxcli check` (no -p) passed `set $X = $Y;` on an OBJECT variable (entity parameter, create, retrieve first, loop iterator, head) even after check --references and exec refused it; the build then failed with CE7247. The bug-test could not be a .fail.mdl because check-mdl runs plain check", "cause": "Plain check runs only the MDL0xx rule set (ValidateProgram -> ValidateMicroflow/ValidateNanoflow); validateFlowBody, where the first #1323 refusal lived, runs only under --references (validate.go) and in exec. A refusal added to one validator is invisible to the other path", "file": "`mdl/executor/validate_microflow_set_object.go` (`checkSetOnObjectVariable`, MDL-SET01), wired from `validate_microflow.go` (validate) and `validate_nanoflow.go`; `cmd_microflows_builder_validate.go` now reports only `assocObjectVars`", "insight": "Know which validator each entry point runs before adding a refusal: plain check = ValidateMicroflow; --references = that PLUS validateFlowBody; exec = the builder (plus execEnforcedMicroflowRules). Putting the rule in both check validators made --references print it twice; split by what each can see — the rule takes every object visible in the text, validateFlowBody only the association-typed ones that need the project. Measure every producer a rule judges, with the faulted build: all five plus a nanoflow gave CE7247 on 11.14.0, but a PARAMETER gives different wording, \"Parameter 'A' cannot be changed.\" — and that wording also fires for a PRIMITIVE parameter (`set $N = 1` on `$N: Integer`), a separate unrefused gap the object-only rule does not cover. Leave `find` out of the object producers: it is also the String function", "refs": ["mendixlabs/mxcli#1323", "ako/mxcli#1011"], "ce": ["CE7247"], "rules": ["MDL-SET01"]} diff --git a/.claude/skills/mendix/write-microflows/reference/data-operations.md b/.claude/skills/mendix/write-microflows/reference/data-operations.md index 7870d40ab..89394c7f7 100644 --- a/.claude/skills/mendix/write-microflows/reference/data-operations.md +++ b/.claude/skills/mendix/write-microflows/reference/data-operations.md @@ -134,7 +134,7 @@ primitive type"). mxcli knows a variable is a list when it is a list parameter, a `create list`, a list retrieve or a list operation's result. Both work in microflows and nanoflows. -`set` on an **object** variable is refused (`check --references` and `exec`): +`set` on an **object** variable is refused (MDL-SET01 in `check`; `check --references` and `exec` also catch an object from an association retrieve): Mendix has no action that reassigns an object variable, and a Change variable on one is the same CE7247. To walk a chain (`$Cursor = $Next` in a `while` loop), write a sub-microflow that **returns** the next object and recurse; to change the diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fb6393f1..efead3c56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **`set $Obj = …` on an object variable is refused** (mendixlabs/mxcli#1323) — with both variables single objects (e.g. a Reference retrieved from its FROM entity), `set $Cursor = $Next;` passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable, so `check --references` and `exec` now refuse it and name the alternatives (a sub-microflow that returns the next object, `change $Obj (…)`). `check --references` now types a Reference retrieve from its FROM entity as the object it is. `set` on a list variable stays a Change list Replace (ako/mxcli#949). +- **`set $Obj = …` on an object variable is refused** (mendixlabs/mxcli#1323) — with both variables single objects (e.g. a Reference retrieved from its FROM entity), `set $Cursor = $Next;` passed `check --references` and `exec` and was written as a Change variable action, which mxbuild refuses with CE7247 "Variable 'Cursor' does not have a primitive type". Mendix has no action that reassigns an object variable, so `check` (MDL-SET01, for the objects it can see without a project), `check --references` and `exec` now refuse it and name the alternatives (a sub-microflow that returns the next object, `change $Obj (…)`). `check --references` now types a Reference retrieve from its FROM entity as the object it is. `set` on a list variable stays a Change list Replace (ako/mxcli#949). - **`run --local --watch` starts on Mendix 10.24 and 11.6** — it waited the full web-client timeout (5 minutes) for a bundle that had finished in seconds, then failed with `web client watcher timed out`. The rollup runner those versions ship reports its status as `{"code":"SUCCESS"}` rather than the modern-web-bundler protocol mxcli was reading; both are now understood, including that runner's error reports. - **`run --local --watch` no longer loses a change made while the app boots** — the watch loop took its baseline after the boot, so a model written during the ~15 s boot was never built and the app kept serving the model from before it. The baseline is now the source time the boot build was made from, so that change is built on the first tick. diff --git a/cmd/mxcli/syntax/features_microflow.go b/cmd/mxcli/syntax/features_microflow.go index 2af7960b7..c5fd8406a 100644 --- a/cmd/mxcli/syntax/features_microflow.go +++ b/cmd/mxcli/syntax/features_microflow.go @@ -134,7 +134,10 @@ func init() { "DECLARE $Var Type = expression;\n" + "$Var = expression; -- assign; SET is optional\n" + "$Var/Attribute = expression;\n" + - "SET $Var = expression; -- same statement, explicit form", + "SET $Var = expression; -- same statement, explicit form\n" + + "-- $Var must be a primitive or a list (a list is a Change list Replace).\n" + + "-- An OBJECT variable cannot be reassigned (CE7247, MDL-SET01): return the\n" + + "-- new object from a sub-microflow, or `change $Obj (…)` its members.", Example: "DECLARE $Count Integer = 0;\nDECLARE $Name String;\nset $Count = $Count + 1;\nset $Name = 'Hello';\nSET $Order/Status = 'Pending';", SeeAlso: []string{"microflow.object-operations"}, }) diff --git a/docs-site/src/appendixes/error-messages.md b/docs-site/src/appendixes/error-messages.md index fce524759..cfe706547 100644 --- a/docs-site/src/appendixes/error-messages.md +++ b/docs-site/src/appendixes/error-messages.md @@ -276,6 +276,29 @@ $n = count $Approved; The same applies to both operands of `union`/`intersect`/`subtract`. +### MDL-SET01: `set` on an object variable + +``` +cannot set object variable '$Cursor' (G46.Group): Mendix has no action that reassigns an +object variable — Change variable takes only a primitive, and mxbuild rejects it with +CE7247 "Variable 'Cursor' does not have a primitive type". [MDL-SET01] +``` + +**Cause:** `set $Var = …` is a Change variable activity, which takes only a primitive +variable; on a list it is a Change list Replace. Mendix has no activity that reassigns an +object variable, so the statement used to be written as a Change variable and fail the build +with CE7247 (mendixlabs/mxcli#1323). It comes up naturally when walking a parent chain with +`set $Cursor = $Next` in a `while` loop. On a parameter mxbuild words the same code +`"Parameter 'X' cannot be changed."`. + +Plain `check` reports it for the objects it can see without a project — entity parameters, +`create`, `retrieve … first`, loop iterators, casts, `head`. Whether an association retrieve +is an object or a list depends on the association, so that case is reported by +`check --references` and `exec`. + +**Solution:** Return the new object from a sub-microflow (recursion for a chain walk), +retrieve it into a new variable, or change the object's members with `change $Obj (…)`. + ### MDL-EMAIL02 / 03: A `send email` setting Studio Pro would not allow ``` diff --git a/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl b/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl new file mode 100644 index 000000000..9bac0011e --- /dev/null +++ b/mdl-examples/bug-tests/1323-set-object-variable.fail.mdl @@ -0,0 +1,27 @@ +mdl 1; +-- ============================================================================ +-- Upstream #1323 (the refusal): `set` on an OBJECT variable is MDL-SET01 +-- ============================================================================ +-- +-- Mendix has no action that reassigns an object variable. Written as a Change +-- variable action, this is CE7247 "Variable 'Cursor' does not have a primitive +-- type." at build time (measured on 11.14.0). Plain `mxcli check` must refuse +-- it. The reported script used association retrieves, whose object-or-list +-- typing needs the project (check --references / exec refuse that form); here +-- the cursor is a `retrieve … first`, which plain check can see is an object. +-- (On a parameter mxbuild words it "Parameter 'X' cannot be changed.", still +-- CE7247.) See the companion 1323-set-object-variable.mdl for the forms that +-- build. +-- +-- This file is expected to FAIL `mxcli check`. +-- ============================================================================ + +create module F1323F; +create persistent entity F1323F.Node ("Name": String(50)); + +create microflow F1323F.Reassign ($Next: F1323F.Node) returns F1323F.Node +begin + retrieve $Cursor from F1323F.Node first; + set $Cursor = $Next; + return $Cursor; +end; diff --git a/mdl-examples/bug-tests/1323-set-object-variable.mdl b/mdl-examples/bug-tests/1323-set-object-variable.mdl index 5d5413928..160cb1f53 100644 --- a/mdl-examples/bug-tests/1323-set-object-variable.mdl +++ b/mdl-examples/bug-tests/1323-set-object-variable.mdl @@ -13,9 +13,8 @@ mdl 1; -- [CE7247] "Variable 'Cursor' does not have a primitive type." -- Mendix has no action that reassigns an object variable: Change variable -- takes only a primitive, and Change list Replace (ako/mxcli#949) only a list. --- `check --references` and `exec` now refuse it, naming the alternatives. --- (Plain `check` without -p does not run the flow-body validator, so the --- refusal is pinned by mdl/executor/set_object_variable_test.go, not here.) +-- `check` (MDL-SET01), `check --references` and `exec` now refuse it, naming +-- the alternatives; 1323-set-object-variable.fail.mdl is the refusal. -- -- This file holds the forms that DO build — each measured on 11.14.0 with -- mx check reporting 0 errors. diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index d273f4212..bf6e62e1f 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -28,6 +28,10 @@ type flowBuilder struct { // retrieve as the object it yields (validateFlowBody). nil on the exec path, // which resolves associations through the backend instead. checkAssocShapes map[string]assocShape + // assocObjectVars are the variables checkAssocShapes typed as one object, + // the only objects whose `set` the validator reports itself (MDL-SET01 + // reports the rest without a project). + assocObjectVars map[string]bool objects []microflows.MicroflowObject flows []*microflows.SequenceFlow diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index aefc5c497..fa3408236 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -60,6 +60,7 @@ func validateFlowBody(params []ast.MicroflowParam, body []ast.MicroflowStatement errors: []string{}, duplicateNamesOwnedElsewhere: duplicatesOwnedElsewhere, checkAssocShapes: assocs, + assocObjectVars: map[string]bool{}, } fb.validateStatements(body) @@ -105,7 +106,10 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { fb.addErrorWithExample( fmt.Sprintf("variable '%s' is not declared", s.Target), errorExampleDeclareVariable(s.Target)) - } else if !strings.Contains(s.Target, "/") { + } else if fb.assocObjectVars[strings.TrimPrefix(s.Target, "$")] { + // Only the association-retrieved object: every other object + // producer is visible without a project, and MDL-SET01 reports + // those once (checkSetOnObjectVariable). fb.refuseSetOnObject(s.Target) } @@ -316,6 +320,7 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { if s.StartVariable != "" { if to, ok := fb.forwardReferenceTarget(s); ok { fb.varTypes[s.Variable] = to + fb.assocObjectVars[s.Variable] = true } else { fb.varTypes[s.Variable] = "List of " + s.Source.Module + "." + s.Source.Name } diff --git a/mdl/executor/set_object_variable_test.go b/mdl/executor/set_object_variable_test.go index 8d20f051e..9f2191855 100644 --- a/mdl/executor/set_object_variable_test.go +++ b/mdl/executor/set_object_variable_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" ) @@ -92,29 +93,36 @@ end; } // The same refusal for a stored association, from the project rather than the -// script, and for object variables check knows without one (a parameter). +// script. func TestCheckRefusesSetOnObjectVariable(t *testing.T) { - for name, mf := range map[string]string{ - "stored Reference forward": `create microflow C88.M ($X: C88.A, $Y: C88.A) + mf := `create microflow C88.M ($X: C88.A, $Y: C88.A) begin retrieve $O from $X/C88.R_def; retrieve $P from $Y/C88.R_def; set $O = $P; end; / -`, - "object parameter": `create microflow C88.M ($X: C88.A, $Y: C88.A) +` + if got := checkScriptMicroflows(t, mf); !strings.Contains(got, "CE7247") { + t.Errorf("check accepted `set` on an object variable; got %q", got) + } +} + +// An object plain check can see (a parameter) is MDL-SET01's to report; the +// reference validator, which check --references runs as well, stays quiet on +// it so the author sees the refusal once. +func TestCheckReferencesLeavesVisibleObjectsToMDLSET01(t *testing.T) { + mf := `create microflow C88.M ($X: C88.A, $Y: C88.A) begin set $X = $Y; end; / -`, - } { - t.Run(name, func(t *testing.T) { - if got := checkScriptMicroflows(t, mf); !strings.Contains(got, "CE7247") { - t.Errorf("check accepted `set` on an object variable; got %q", got) - } - }) +` + if got := checkScriptMicroflows(t, mf); got != "" { + t.Errorf("reference validator reported what MDL-SET01 owns: %q", got) + } + if hits := setObjectRuleHits(t, mf); len(hits) != 1 { + t.Errorf("want one %s, got %q", setObjectRule, hits) } } @@ -135,3 +143,93 @@ func TestSetOnObjectVariableRefusedByBuilder(t *testing.T) { t.Fatalf("builder accepted `set` on object variable $Cursor; errors = %q", got) } } + +// setObjectRuleHits parses src and returns the MDL-SET01 messages plain check +// (ValidateMicroflow / ValidateNanoflow, no project) reports for it. +func setObjectRuleHits(t *testing.T, src string) []string { + t.Helper() + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + var hits []string + for _, st := range prog.Statements { + var vs []linter.Violation + switch s := st.(type) { + case *ast.CreateMicroflowStmt: + vs = ValidateMicroflow(s) + case *ast.CreateNanoflowStmt: + vs = ValidateNanoflow(s) + } + for _, v := range vs { + if v.RuleID == setObjectRule { + hits = append(hits, v.Message) + } + } + } + return hits +} + +// Plain check, with no project, refuses `set` on a variable it can see is an +// object without one: the object producers that need no association lookup. +func TestPlainCheckRefusesSetOnObjectVariable(t *testing.T) { + for name, body := range map[string]string{ + "parameter": `set $A = $B;`, + "create object": `$N = create M.E (Name = 'x'); + set $N = $B;`, + "retrieve first": `retrieve $F from M.E first; + set $F = $B;`, + "loop iterator": `retrieve $L from M.E; + loop $It in $L begin + set $It = $B; + end loop;`, + "head": `retrieve $L from M.E; + $H = head($L); + set $H = $B;`, + } { + t.Run(name, func(t *testing.T) { + src := "create microflow M.MF ($A: M.E, $B: M.E)\nbegin\n " + body + "\nend;\n" + hits := setObjectRuleHits(t, src) + if len(hits) != 1 || !strings.Contains(hits[0], "CE7247") { + t.Errorf("want one %s naming CE7247, got %q", setObjectRule, hits) + } + }) + } + nf := "create nanoflow M.NF ($A: M.E, $B: M.E)\nbegin\n set $A = $B;\nend;\n" + if hits := setObjectRuleHits(t, nf); len(hits) != 1 { + t.Errorf("nanoflow: want one %s, got %q", setObjectRule, hits) + } +} + +// Controls: what plain check cannot or must not call an object stays accepted +// — a primitive, a list (Change list Replace, ako/mxcli#949), a member path, +// and an association retrieve, whose cardinality needs the project. +func TestPlainCheckAcceptsSetOnNonObject(t *testing.T) { + for name, body := range map[string]string{ + "primitive": `declare $N Integer = 0; + set $N = 1;`, + "list parameter": `set $Ls = $Ls2;`, + "list retrieve": `retrieve $L from M.E; + set $L = $Ls;`, + "member path": `set $A/Name = 'x';`, + "association retrieve": `retrieve $C from $A/M.E_Other; + retrieve $D from $B/M.E_Other; + set $C = $D;`, + } { + t.Run(name, func(t *testing.T) { + src := "create microflow M.MF ($A: M.E, $B: M.E, $Ls: list of M.E, $Ls2: list of M.E)\nbegin\n " + body + "\nend;\n" + if hits := setObjectRuleHits(t, src); len(hits) != 0 { + t.Errorf("refused a set that is not on a known object: %q", hits) + } + }) + } +} + +// mxbuild words CE7247 differently for a parameter (measured on 11.14.0), and +// the message quotes what the author will see at build time. +func TestPlainCheckQuotesParameterRejection(t *testing.T) { + hits := setObjectRuleHits(t, "create microflow M.MF ($A: M.E, $B: M.E)\nbegin\n set $A = $B;\nend;\n") + if len(hits) != 1 || !strings.Contains(hits[0], `"Parameter 'A' cannot be changed"`) { + t.Errorf("want the parameter wording of CE7247, got %q", hits) + } +} diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 1b07758ff..9f2b3a7b1 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -120,6 +120,7 @@ func (v *microflowValidator) addViolation(ruleID string, severity linter.Severit func (v *microflowValidator) validate(body []ast.MicroflowStatement) { v.checkListOperationIterator(body) v.checkRetrieveLimitOneAsList(body) + v.checkSetOnObjectVariable(v.params, body) v.checkListOperationSource(body) v.checkMergeJoinLabels(body) v.checkAnnotationLabels(body) diff --git a/mdl/executor/validate_microflow_set_object.go b/mdl/executor/validate_microflow_set_object.go new file mode 100644 index 000000000..208d04bc3 --- /dev/null +++ b/mdl/executor/validate_microflow_set_object.go @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// setObjectRule is the rule ID for "set on an object variable". +const setObjectRule = "MDL-SET01" + +// checkSetOnObjectVariable flags `set $X = …` where $X is a single OBJECT. +// Mendix has no action that reassigns an object variable: Change variable +// takes only a primitive, and mxbuild answers CE7247 "Variable 'X' does not +// have a primitive type" (mendixlabs/mxcli#1323, measured on 11.14.0). +// +// This is plain check's half, which runs without a project, so it judges only +// the object producers it can see in the text: an entity parameter, a create +// object, a database `retrieve … first`, a loop iterator, a cast, a head. An +// association retrieve is an object or a list by the association's type and +// direction, which needs the project — check --references and exec decide that +// one (flowBuilder.refuseSetOnObject), and this rule leaves it alone rather +// than guess. +func (v *microflowValidator) checkSetOnObjectVariable(params []ast.MicroflowParam, body []ast.MicroflowStatement) { + // objects maps each variable currently known to hold one object to its entity. + objects := map[string]string{} + // isParam marks the object parameters: mxbuild words their CE7247 + // differently ("Parameter 'A' cannot be changed."), measured on 11.14.0. + isParam := map[string]bool{} + for _, p := range params { + if p.Type.EntityRef != nil && p.Type.Kind != ast.TypeListOf { + objects[p.Name] = p.Type.EntityRef.String() + isParam[p.Name] = true + } + } + lists := map[string]string{} + for _, p := range params { + if p.Type.EntityRef != nil && p.Type.Kind == ast.TypeListOf { + lists[p.Name] = p.Type.EntityRef.String() + } + } + + forEachMicroflowStatement(body, func(s ast.MicroflowStatement) { + if set, ok := s.(*ast.MfSetStmt); ok && !strings.Contains(set.Target, "/") { + name := strings.TrimPrefix(set.Target, "$") + if entity, ok := objects[name]; ok { + rejection := fmt.Sprintf("CE7247 \"Variable '%s' does not have a primitive type\"", name) + if isParam[name] { + rejection = fmt.Sprintf("CE7247 \"Parameter '%s' cannot be changed\"", name) + } + v.addViolation(setObjectRule, linter.SeverityError, + fmt.Sprintf("cannot set object variable '$%s' (%s): Mendix has no action that reassigns an "+ + "object variable — Change variable takes only a primitive, and mxbuild rejects it with "+ + "%s.", name, entity, rejection), + fmt.Sprintf("Return the new object from a sub-microflow instead (recursion for a chain walk), "+ + "retrieve it into a new variable, or change the object's members with `change $%s (…)`.", name)) + } + } + + // Rebinding first: a name a statement produces is whatever that + // statement makes it, and nothing it was before. + for _, p := range statementProducedVars(s) { + delete(objects, p.name) + delete(lists, p.name) + delete(isParam, p.name) + } + switch st := s.(type) { + case *ast.CreateObjectStmt: + if st.Variable != "" && st.EntityType.Module != "" { + objects[st.Variable] = st.EntityType.String() + } + case *ast.RetrieveStmt: + if st.Variable != "" && st.StartVariable == "" && st.Source.Module != "" { + if st.First { + objects[st.Variable] = st.Source.String() + } else { + lists[st.Variable] = st.Source.String() + } + } + case *ast.CreateListStmt: + if st.Variable != "" && st.EntityType.Module != "" { + lists[st.Variable] = st.EntityType.String() + } + case *ast.LoopStmt: + if entity, ok := lists[st.ListVariable]; ok && st.LoopVariable != "" { + objects[st.LoopVariable] = entity + } + case *ast.ListOperationStmt: + // head is list-only; find is also the String function, so it is left out. + if entity, ok := lists[st.InputVariable]; ok && st.OutputVariable != "" { + switch st.Operation { + case ast.ListOpHead: + objects[st.OutputVariable] = entity + case ast.ListOpFilter, ast.ListOpSort, ast.ListOpTail: + lists[st.OutputVariable] = entity + } + } + } + }) +} diff --git a/mdl/executor/validate_nanoflow.go b/mdl/executor/validate_nanoflow.go index bace87a20..5729126a9 100644 --- a/mdl/executor/validate_nanoflow.go +++ b/mdl/executor/validate_nanoflow.go @@ -49,6 +49,8 @@ func validateNanoflowWith(stmt *ast.CreateNanoflowStmt, voids *voidCodeActions) v.seedPrimitiveKinds(stmt.Parameters, stmt.Body) v.checkDuplicateVariableNames(v.params, stmt.Body) v.checkVoidCallOutputUse(v.params, stmt.Body) + // MDL-SET01: a nanoflow's Change variable is as primitive-only (CE7247). + v.checkSetOnObjectVariable(v.params, stmt.Body) // The nanoflow restrictions exec's build refuses (validateNanoflow): an // action a nanoflow cannot hold, an error-handling clause its activity // rejects (CE6035), a Binary return. They ran only inside exec, so `check`