diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index c2eb5cd01..786e7f5b9 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -59,6 +59,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | `SHOW ACCESS ON PAGE` / Page section of `SHOW SECURITY MATRIX` reports "no roles" for a restricted page on the **modelsdk** engine (legacy is correct) — under-reports page access, a security-audit hazard | `pageFromGen` never read the page's allowed module roles into `Page.AllowedRoles`, so it defaulted to empty. (The gen `Page` decode is correct — it has the storage-name override `allowedRoles`↔`AllowedModuleRoles`.) The microflow/nanoflow equivalent was fixed separately | `mdl/backend/modelsdk/page.go` (`pageFromGen`) | Populate `out.AllowedRoles` from the gen `AllowedRolesQualifiedNames()` (mirrors `microflowFromGen`). Fixes both `SHOW ACCESS ON PAGE` and the matrix Page section. Issue #722 | | `mxcli report` (and `lint`) hangs for many minutes on a large project — appears to deadlock after "Catalog ready", 130-byte banner-only output, process pinned at ~140% CPU (not blocked → not a lock deadlock) | O(N²) BSON re-decode: six lint rules loop `for mf := range ctx.Microflows()` and call `reader.GetMicroflow(mf.ID)` per microflow, but the modelsdk backend's `GetMicroflow` re-lists and re-decodes EVERY microflow unit on each call. N calls × N decodes → millions of full BSON parses. Diagnose with `kill -QUIT ` (GOTRACEBACK=all) — the running-goroutine stack names the stuck rule + `ListUnitsWithContainer` | `mdl/linter/context.go` (`FullMicroflow`, `LintReader`) + the six rules (`validation_feedback`, `conv_loop_commit`, `conv_split_caption`, `conv_error_handling`×2, `mpr008_overlapping_activities`) | Add `LintContext.FullMicroflow(id)` — loads ALL microflows once via `ListMicroflows()` and serves lookups from a per-run cache (safe: lint is read-only). Swap every `reader.GetMicroflow(mf.ID)` in a loop to `ctx.FullMicroflow(mf.ID)`. Turns O(N²) into O(N); report went from >40min hang to ~5s on 3259 microflows. Issue #720 | | `DESCRIBE MICROFLOW` (mdl/json) times out at 300s on a high-McCabe flow, but `--format mermaid` renders in ~1s — extraction is fast, the serializer hangs | Exponential path enumeration: `duplicateOutputVariableWarnings` (run during `formatMicroflowActivities`) walked EVERY execution path, cloning the visited map at each branch (`cloneIDBoolMap`), to find output vars assigned twice on one path — O(2^branches). ~20 sequential `if/end if` diamonds already took ~10s. Diagnose with `DBGPROF=… ` CPU profile / add a pprof block to `describeMicroflow`; top-cum names the offender (not the describe traversal, which is linear) | `mdl/executor/cmd_microflows_show.go` (`duplicateOutputVariableWarnings`) | Replace the all-paths walk with **reachability**: a name is a duplicate iff two of its assignments are path-ordered (one reaches the other); exclusive-branch assignments never reach each other. Memoized `reachableFrom` is O(V·E). Loop bodies inherit names assigned by activities that reach the loop node. Went 9.5s→0.1s at 20 diamonds; 120 diamonds completes instantly. Issue #710 | +| Microflow `set $x = ` passes `mxcli check` but `mx check` reports a generic CE0117: (#17) `/` used as division (`$Dec / 2`), (#18) a decimal literal loses its fraction (`2.0` → `2`, breaks Decimal typing), or (#19) a small decimal becomes scientific notation (`0.000001` → `1e-06`, which Mendix rejects) | (#18/#19) the value is re-serialized from the AST when `shouldPreserveExpressionSource` returns false and the AST serializer is lossy for decimals (`%v` on a float64 drops `.0` and uses sci-notation). (#17) **MDL division is `div`, not `/`** — `/` is the member-access separator, so `/`-as-division is a user error that mxcli silently wrote instead of flagging. **Do NOT preserve source on `/`** — that source-freezes every legit association path (`$Order/Assoc/Name` → regressed `TestAssociationNavParsing`) | `mdl/visitor/visitor_microflow_statements.go` (`shouldPreserveExpressionSource`); `mdl/executor/validate_microflow.go` (`checkDivisionSlash`/`exprHasSlashDivision`, MDL045) | (#18/#19) preserve raw source only when the expression contains a **decimal literal** (`.` adjacent to a digit) — same philosophy as the XPath-where path. (#17) add **MDL045**: walk the expression tree for a `BinaryExpr` with operator `/` (`$Dec / 2`, `(...) / $x`) and reject with "use `div`". The variable/variable form `$Dec / $Dec2` parses as a member-access path (the `$` on the RHS is stripped), so the visitor narrowly preserves source when a `/` is immediately followed by `$` (a real association path never has `$` after `/`) and MDL045 flags it by scanning the preserved source for a `/ $` **outside string literals** (`exprHasSlashDollarDivision` / `sourceHasSlashDollarDivision`). **Round-2 gap (ledger #17 re-test): the `$a / $b` division must be caught even when EMBEDDED in a larger expression** — `$a / $b + 1`, `round($a / $b)`, `$a / $b * 100` — where it degrades to a member-path `AttributePathExpr` nested under a `BinaryExpr`/`FunctionCallExpr`, so the structural `/`-BinaryExpr walk (which only sees literal division) misses it. The earlier `exprIsSlashDollarDivision` matched only when the SourceExpr **directly** wrapped an AttributePathExpr, so embedded division slipped through silently. Fix: walk the tree for any `SourceExpr` and scan its raw source for `/ $` with a string-literal-aware scanner (skips `'path/$x'`). [verification round: the `$a / $b` form must be caught by bare `check`, not deferred to `--references`.] **Lesson: `/` is overloaded (path separator vs the division a user *wanted*) — a blanket source-preserve on `/` corrupts the common case to rescue the rare misuse; reject the misuse explicitly instead. And an operator misuse that "degrades to a path" can hide anywhere in an expression tree, not just at its root — detect it from source, walking every sub-expression.** Tests: `TestShouldPreserveExpressionSource_Decimals`, `TestValidateMicroflow_SlashDivision` (incl. embedded cases + string-literal guard); repros `mdl-examples/bug-tests/ledger-17-19-expression-serialization.mdl` (pass) + `ledger-17-slash-division.fail.mdl` (MDL045, incl. embedded). Ledger findings #17–19 (round 2) | | `DESCRIBE` of an entity/module emits `grant ... (read (Module.Entity.Attr))` that fails to re-parse with `mismatched input '.'` — breaks the DESCRIBE roundtrip | Member emitter used the fully-qualified BY_NAME reference; grant grammar accepts a bare `IDENTIFIER` only | `mdl/executor/cmd_entities_access.go` → `resolveEntityMemberAccess` | Strip `memberName` to the last `.`-segment before appending (bare names have no dot, so it's a no-op for them). Issue #633 | | `exec` of a pluggable widget from a bundled multi-widget `.mpk` fails "no definition for widget … (run 'mxcli widget init')" even after init (e.g. only AreaChart of Charts.mpk registers) | `ParseMPK`/`getWidgetIDFromMPK` read only `WidgetFiles[0]`; a bundled `.mpk` (Charts) holds many widgets so only the first is registered/augmented | `sdk/widgets/mpk/mpk.go` (`ParseMPKAll`/`ParseMPKWidget`, `FindMPK`) + def-gen loops in `mdl/executor/widget_defs.go` + `mdl/catalog/builder_widget_definitions.go` + `cmd/mxcli/cmd_widget.go` | Parse every widgetFile (`ParseMPKAll`); register all ids in `FindMPK`; augment the specific id (`ParseMPKWidget`); bump `WidgetDefGeneratorVersion` so existing projects regenerate. Issue #679. (Per-series datasource + chart CE0463 are separate, still open.) | | `buttonstyle: ` passes `mxcli check` but the button renders btn-default in Studio Pro (silent at build) — typically a mis-cased value (`Primary`) or one Mendix doesn't have (`secondary`, `link`) | Visitor stored the style verbatim and the executor cast it straight to `pages.ButtonStyle`; only an empty value got a default, so any unknown string was written as-is and MxBuild degraded it | `sdk/pages/pages_widgets_action.go` (`CanonicalButtonStyle`) + `mdl/executor/cmd_pages_builder_v3_widgets.go` (button builder) + `mdl/executor/validate_widgets.go` (`validateStaticWidget`) | Normalize case-insensitively against the metamodel `PagesButtonStyle` set (Default/Primary/Success/Warning/Danger/Info/Inverse); reject unknown values as MDL-WIDGET02 at check time and in the executor. Issue #672 | @@ -158,6 +159,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | A **PieChart/HeatMap** (charts that bind data at the WIDGET level, no series object-list) fails MxBuild **after `mx update-widgets`** with **CE0642** "Value attribute is required" and (PieChart) **CE4899** "Series name is required" — even though `mxcli check` + `exec` are clean. The datasource persists; the value attribute + series name don't. Only surfaces once CE0463 version-drift is cleared by update-widgets | These widgets expose several attribute-typed top-level properties (`seriesValueAttribute`, `seriesSortAttribute`) plus a required `seriesName` texttemplate. The engine's `resolveMapping` `"Attribute"` case read a single generic `w.GetAttribute()` — ambiguous across multiple attribute props and blind to the friendly MDL names — so `ValueAttribute:` never reached `seriesValueAttribute`; and `GenerateDefJSON` **skips all top-level texttemplate props**, so `seriesName` had no mapping. Item 1b | `mdl/executor/widget_defs.go` (`propertyAliases`, `GenerateDefJSON` attribute-alias + gated texttemplate case) + `mdl/executor/widget_engine.go` (`PropertyMapping.MdlAliases`, `namedPropValue`, `resolveMapping` `"Attribute"`/`"TextTemplate"`, version bump) | Add `PropertyMapping.MdlAliases` (top-level analog of `ItemPropertyMapping.MdlAliases`) + a `propertyAliases` map (piechart/heatmap `seriesValueAttribute`←`ValueAttribute`, piechart `seriesName`←`SeriesName`). `resolveMapping` `"Attribute"` now reads via `namedPropValue` (property key + aliases) and resolves against the widget datasource entity (the DataSource mapping is ordered first), falling back to `w.GetAttribute()` for single-attribute widgets. Emit a top-level texttemplate mapping **only** when an alias is registered (keeps the broad skip). Bump `WidgetDefGeneratorVersion`; the example must supply the required `SeriesName:`. mxbuild-verified: whole chart file = 0 errors after update-widgets. **Diagnose "required property" CE0642/CE4899 by running `mx update-widgets` first (clears CE0463), then check which schema key is missing from the BSON.** Tests: `TestResolveMapping_NamedAttribute`, `TestGenerateDefJSON_PieChartNamedProperties`. Widget-level DESCRIBE of `seriesName`/datasource is a separate gap. Item 1b | | A view entity whose attribute is named after a Mendix OQL keyword — most often a date-part word like `Quarter`/`Month`/`Year` — passes `mxcli check`/`exec` but fails **MxBuild CE0174** "The 'Quarter' part is incomplete or incorrect. You could use here: … OPEN_QUOTE, or IDENTIFIER". A `Region`/`Period` column is fine | OQL reads the bare word as the keyword, not the attribute. mxcli **cannot escape it**: the OQL grammar has no quoted-identifier form (`s."Quarter"` is a parse error — `qualifiedName`/`atomicExpression` accept only `IDENTIFIER`/`keyword`, not `QUOTED_IDENTIFIER`), and adding one is high-risk (`qualifiedName` is shared by every name in the language). The OQL is a verbatim passthrough (`RawQuery` via `extractOriginalText`), so mxcli isn't mangling it — the name itself is unusable | `mdl/executor/oql_type_inference.go` (`ValidateOQLSyntax`, `oqlReservedWords`, **MDL032**) | **Don't try to fix by quoting** (grammar blast radius) — surface it at check time instead. MDL032 (warning) scans the view OQL for a reserved word in identifier position (after `.` or `as`) and tells the user to rename. Fix for the user: rename the attribute **and its source reference** (`s.Quarter as Quarter` → `s.Period as Period`) — both sides trip CE0174. Reserved set is the date-part words (not exhaustive; SQL words like SELECT are never plausible attribute names). Tests: `TestValidateOQLSyntax_ReservedWord`. (Making quoted OQL identifiers actually work is a deferred grammar change.) | | A view entity with a **derived string column** — `cast(x as string)`, a string-returning `CASE`, or a string expression — passes `mxcli check`/`exec` but fails **MxBuild CE6770** "View Entity is out of sync with the OQL Query" whenever the declared attribute length is anything other than **200** (e.g. `string(30)`, `string(50)`, unlimited). The OQL itself runs fine, so it looks like a serialization gap but is a **platform rule**: Mendix normalizes a derived string column to the default length String(200); a plain pass-through column (`c.Name as Name`) instead inherits its source length | Two parts. (1) The rule: only *derived* string columns are forced to 200 — a bare attribute ref infers `TypeUnknown` and is skipped, so any concretely-typed String the static inferrer sees is derived → 200. (2) The checker was **dead**: `inferTypeStatic`/`inferTypeFromExpression`/`inferCaseType` uppercased the expr (`upper := ToUpper(...)`) then compared against **lowercase** literals (`HasPrefix(upper, "cast(")`, `"count("`, `"case"`, `== "true"` …), so every prefix check failed and only `DATEPART(` (uppercase literal) ever matched — CAST/CASE/COUNT/SUM/AVG/MIN/MAX/LENGTH inference all returned Unknown. Sibling of Bug 9b's `extractSelectClause` case bug | `mdl/executor/oql_type_inference.go` (`inferTypeStatic` CAST/CASE + case-fixed prefixes, `castTargetType`, `derivedStringLength=200`, `ValidateOQLTypes` string-length normalize, `typesStrictlyCompatible` length compare, `inferCaseType`, `inferTypeFromExpression`) — **MDL031** | Infer `cast(expr AS string)` and string CASE as `String(200)`; in `ValidateOQLTypes` normalize any inferred `TypeString` length to 200 (derived); `typesStrictlyCompatible` compares String **length** (not just kind), so `string(30)` vs `String(200)` is flagged with `Fix: change to 'X: String(200)'`. **Fix the case-comparison bug** (lowercase→UPPERCASE literals) so the checker actually runs. **Guard against false positives**: `SUM(` over an un-inferable inner (a bare attribute ref → Unknown) returns **Unknown**, not a guessed Decimal — otherwise `sum(s.Units)` declared `integer` false-fires (caught on `34-chart-widget-examples.mdl`). mxbuild-confirmed on 11.6.6: `string(30)`→CE6770, `string(200)`→0 errors. Tests: `TestValidateOQLTypesDerivedString`, `TestValidateOQLTypesNoFalsePositive`; bug-test `mdl-examples/bug-tests/view-entity-derived-string-length.mdl` | +| A view entity with a **pass-through string column** — a bare source-attribute reference like `select c.Name as CategoryName` — declared with a length **different from the source attribute** (most often an unbounded `string` against a `string(100)` source) passes `mxcli check` but fails **MxBuild CE6770** "View Entity out of sync". This is the counterpart of the *derived*-column rule (which forces 200): a pass-through column inherits the source length **exactly** | The references-mode validator (`validateViewEntityTypes`) compared with `typesCompatible`, whose String rule only guards **truncation** (`declared.Length >= inferred.Length`), so an unbounded `string` (Length 0) or a wider `string(200)` against a `string(100)` source slipped through. Only *derived* columns (via `ValidateOQLTypes`/`typesStrictlyCompatible`, syntax mode) were length-exact | `mdl/executor/oql_type_inference.go` (`passthroughStringLengthMismatch`, called in `validateViewEntityTypes` before the generic `typesCompatible`) | For a pass-through column (`col.SourceAttr != ""`, set only for a bare `alias.attr` ref — aggregates use a throwaway col so it stays empty) require **exact** String length match; emit the source entity/attr + inherited length in the message. **References-mode only** (needs the domain model to resolve the source attribute's length), so the repro is not a `.fail.mdl` (the syntax-only `make check-mdl` harness can't see it). Test `TestPassthroughStringLengthMismatch`; example `mdl-examples/bug-tests/ledger-36-view-passthrough-length.mdl`; mxbuild-confirmed on 11.12.1 (`string`→CE6770, `string(100)`→clean). Ledger finding #36 | | `DESCRIBE` of a view entity on the **default (modelsdk)** engine omits the `as (… OQL …)` clause entirely (legacy renders it) — the describe→exec round-trip silently drops the query, leaving a queryless view. Reproduces on any Mendix **11.x** project | On Mendix 11.0+ the OQL is stored **only** in the separate `DomainModels$ViewEntitySourceDocument`; the inline `OqlViewEntitySource.Oql` field was removed (see the writer's `if !pv.IsAtLeast(11,0)` gate in `serializeOqlViewEntitySource`). The modelsdk read (`entityFromGen`: `out.OqlQuery = src.Oql()`) only read the now-empty inline field and never followed the `SourceDocumentRef`. Legacy was correct — `loadViewEntityOqlQueries` in `sdk/mpr/reader_documents.go` already joins the source docs | `mdl/backend/modelsdk/domainmodel.go` (`populateViewEntityOql`, wired into `ListDomainModels` + `GetDomainModel`) | After building the domain models, list `ViewEntitySourceDocument` units (`mprread.ListUnitsWithContainer[*genDm.ViewEntitySourceDocument]`), map `moduleName.docName → Oql()`, and fill `OqlQuery` for any view entity with an empty `OqlQuery` + a `SourceDocumentRef` (mirrors the legacy join; guarded by a cheap pre-check so non-view projects pay nothing). When a view-entity read differs between engines on 11.x, suspect a field the platform moved out-of-line into a source document. Test: `TestGetDomainModel_ViewEntityOqlFromSourceDocument` (round-trips via both read paths) | | An `actionbutton`/`linkbutton` `icon:` property is silently dropped on write (flagged `MDL-WIDGET07` "property `icon` … will be silently dropped") — no way to give a button an icon, so the button renders without one. `linkbutton` (Link render mode) itself already worked | The button builder read Caption/Action/ButtonStyle but never the `icon` property, and the `Forms$ActionButton` codec default nulls the `Icon` field. Nothing built the Mendix icon element | `sdk/pages/pages_widgets_action.go` (`Icon` struct + `IconTypeIconCollection`) + `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildButtonV3` reads `icon`) + `mdl/backend/modelsdk/widget_write.go` (`iconToGen`, wired into the `ActionButton` case) + describe (`cmd_pages_describe_parse.go` `extractIconRef`, `cmd_pages_describe_output.go` emit) + `validate_widgets.go` (`staticWidgetKnownProps` += `Icon`) | Build the modern **icon-collection** icon: `icon: 'Atlas_Core.Atlas_Filled.pencil'` → `Forms$IconCollectionIcon{Image: QN}` (verified against a Studio-Pro button — storage `$Type` is `Forms$`, not `Pages$`; no typed gen struct, so build it via `newElem`/`addStr`). `g.SetIcon(...)` overrides the null-Icon default. DESCRIBE reconstructs `Icon:` from the element's `Image`; add `Icon` to the known-props list so MDL-WIDGET07 stops warning. A non-existent icon name is (correctly) rejected by MxBuild with **CE1613** "selected custom icon … no longer exists". Tests: `TestBuildButtonV3_Icon`, `TestOutputWidgetMDLV3_ButtonIcon`; bug-test `mdl-examples/bug-tests/602-button-icon.mdl`. Issue #602 | | `DESCRIBE WORKFLOW` on the **default (modelsdk)** engine renders some activities as non-round-trippable comments — `-- [Workflows$StartWorkflowActivity]`, `-- [Workflows$JumpToActivity]`, `-- [Workflows$WaitForTimerActivity]`, `-- [Workflows$WaitForNotificationActivity]` — so `describe → exec` drops those activities and you can't learn their syntax from an existing workflow. User tasks, decisions, splits, call-microflow/workflow round-trip fine (legacy round-trips all of them) | The describe **formatter** (`formatWorkflowActivities`) already has typed cases for jump/wait/start/end, but the modelsdk **reader** never produced them: these activities have no `genWf` struct (written via generic `newElem`), so `workflowActivityFromGen`'s type switch fell through to `GenericWorkflowActivity`, which the formatter renders as `-- [$Type]`. Legacy's `parser_workflow.go` had dedicated cases, so legacy was correct — modelsdk-only gap | `mdl/backend/modelsdk/workflow_read.go` (`workflowActivityFromGen` default → new `workflowSimpleActivityFromGen`) | Recognise the untyped simple activities by `el.TypeName()` and rebuild the semantic struct, reading fields from raw BSON via `genWf.RawFieldString` (`TargetActivity` for jump, `Delay` for the timer) — mirrors the legacy parser. Start/end become typed so the formatter **skips** them (implicit in MDL); jump/wait become executable statements. When an engine emits `-- [$Type]` in describe, the *formatter* usually already handles the type — the gap is the *reader* decoding it to `GenericWorkflowActivity`. Test: `TestWorkflowSimpleActivities_ReconstructedTyped`; round-trip validated describe→drop→exec→`docker check` = 0 workflow errors. Bug 11b (11a = the missing `write-workflows.md` authoring skill) | @@ -188,6 +190,24 @@ to the symptom table below, so the next similar issue costs fewer reads. | `call javascript action` / `call java action` with a wrong-cased or misspelled PARAMETER name passes `check --references` (the action itself resolves) but writes a dangling reference that fails the build with CE1613 "The selected … parameter … no longer exists". E.g. `NanoflowCommons.OpenURL(url = …)` when the parameter is `Url` | The reference checker resolved only the action NAME (`buildJava*ActionQualifiedNames` returns names, discarding params); the call's `CallArgument` names were never compared to the action's declared parameters | `mdl/executor/validate.go` (`flowRefCollector` → `codeActionCallRef`, `validateCodeActionParams`) | Carry the call's `argNames` on the collector; after the name-exists check, `ReadJavaScriptActionByName`/`ReadJavaActionByName` → `.Parameters[].Name`, diff case-sensitively; casing-only mismatch → did-you-mean. Skip `System.*` (runtime-provided) and degrade to no-error when the backend reports no params. References-mode only (needs `-p`). Tests `TestValidateCodeActionParams`. RSS-reader follow-up finding | | `DESCRIBE PAGE` omits a textbox's `placeholder`/`onchange` even though they are written correctly (present in the .mxunit, live app works) — so a DESCRIBE round-trip is not a reliable way to confirm the write landed | The describe read path (`parseRawWidget`) only read `LabelTemplate` + `AttributeRef` for a textbox; `PlaceholderTemplate` and `OnChangeAction` were never read back into `rawWidget` | `mdl/executor/cmd_pages_describe_parse.go` (`extractPlaceholderText`), `cmd_pages_describe.go` (`rawWidget` fields), `cmd_pages_describe_output.go` (`renderClientActionMDL`/`extractOnChangeAction` + TextBox emit) | Add `Placeholder`/`OnChange` to `rawWidget`; read `PlaceholderTemplate` via `extractTextFromTemplate` (same as label) and `OnChangeAction` via a key-agnostic `renderClientActionMDL` (refactored out of `extractButtonAction`, since OnChangeAction is the same client-action type under a different key); emit `Placeholder:`/`OnChange:` in the TextBox case. Single describe path serves both engines (reads raw BSON via `GetRawUnit`). Test `TestParseRawWidget_TextBoxPlaceholderAndOnChange`. RSS-reader follow-up (verification note on #9) | | `DESCRIBE PAGE` of a `dynamictext` bound to a NON-STRING attribute (Integer/DateTime/…) emits `ContentParams: [{1} = toString($currentObject/Attr)]`; re-applying that output fails the build with CE1613 "attribute '…toString($currentObject/Attr)' no longer exists" — the rendered expression is treated as an attribute NAME. A string binding round-trips fine (bare attribute name) | The write side converts a non-String attribute binding to `toString($currentObject/Attr)` (`resolveTemplateAttributePathFull`), stored as a ClientTemplateParameter `Expression`; the describe reader emitted the Expression verbatim instead of reversing the transform | `mdl/executor/cmd_pages_describe_output.go` (`unwrapToStringAttrParam` in `extractClientTemplateParameters`) | When a ContentParam Expression is exactly `toString($currentObject/)` or `toString($param/)` (the auto-generated forms), emit the bare `` / `$param.attr`; the write side re-derives the toString on the next apply, so it round-trips. Any other expression (extra text, hand-written toString) is left untouched. Tests `TestUnwrapToStringAttrParam`, `TestParseRawWidget_DynamicTextNonStringAttribute`. RSS-reader follow-up finding | +| A `contentparams`/`captionparams` value bound to a client EXPRESSION (`[{1} = formatDateTime($obj/LastImport, 'd MMM yyyy')]`) passes `mxcli check` but Studio Pro rejects the page with CE1613 "attribute … no longer exists" — the whole expression is stored as a bogus attribute name. A plain attribute path (`$obj/Attr`) or quoted literal (`'text'`) works | A template-parameter slot is a **data binding**, not an expression: `buildClientTemplateParams` stores any unquoted value as an attribute path (`resolveTemplateAttributePathFull`). No check inspected the value for expression syntax | `mdl/executor/validate_widgets.go` (`validateTemplateParamExpressions`, called from `validateStaticWidget`) | New **MDL-WIDGET14**: for each `ContentParams`/`CaptionParams` value, skip quoted string literals, then regex `templateParamExprRe` (`Ident(` function call or an arithmetic/comparison operator) flags an expression; an attribute path never matches. Fix for the user: precompute the value onto the bound entity (calculated attribute) and bind that attribute. Test `TestValidateTemplateParamExpressions`; bug-test `mdl-examples/bug-tests/contentparam-expression-rejected.fail.mdl`. Ledger finding #26 | +| A microflow expression using `dateTime(2026, $Month, $Day)` (a variable/computed arg) passes `mxcli check` but fails the build with CE0117 — Mendix builds `dateTime()`/`dateTimeUTC()` from hardcoded numeric constants only | No check inspected date-construction arguments; the function name resolves so the unknown-function check (MDL044) is silent | `mdl/executor/validate_microflow.go` (`checkDateTimeLiterals`/`exprHasNonLiteralDateTime`, MDL046) | New **MDL046**: walk the expression for a `FunctionCallExpr` named `dateTime`/`dateTimeUTC` with any non-`LiteralExpr` argument. Fix for the user: step off a literal anchor date — `addDays(addMonths(dateTime(2026,1,1), $Month-1), $Day-1)` (addDays/addMonths take variables). Test `TestValidateMicroflow_DateTimeLiterals`; repro `mdl-examples/bug-tests/ledger-21-datetime-literals.fail.mdl`. Ledger finding #21 | +| A retrieve constraint `where [Ledger.Transaction_Category = empty]` (an ASSOCIATION `= empty`) passes `mxcli check` but fails the build with CE0161 "Error(s) in XPath constraint" — XPath `= empty` tests attribute nullability, not association nullability | No check inspected constraint strings for an association compared to `empty`; a bare attribute `= empty` IS valid, so a blanket ban would false-positive | `mdl/executor/validate_microflow.go` (`checkXPathAssociationEmpty`/`xpathAssocEmptyRe`, MDL047) | New **MDL047**: on a `RetrieveStmt.Where` (via `expressionToXPath`), regex a **module-qualified** name (one dot) directly `= empty`, with a leading boundary class that excludes `/` (so `Assoc/Attr = empty` — a valid attribute-over-association test — is NOT flagged) and `.`/word (so a 3-part enum literal tail isn't grabbed). Fix for the user: `[not(Assoc/Module.Target)]`. **Also covers page/widget datasource where-clauses** (`validateDatasourceXPathAssociationEmpty` on `DataSourceV3.Where`, shared regex via `xpathAssociationEmptyMatches`) — the ledger project hit it there first. Tests `TestValidateMicroflow_XPathAssociationEmpty`, `TestValidateDatasourceXPathAssociationEmpty`; repros `ledger-25-xpath-assoc-empty.fail.mdl` (retrieve) + `ledger-25-page-datasource-assoc-empty.fail.mdl` (datagrid). Ledger finding #25 | +| Two sibling `dynamictext` widgets in a container render as `€ 310Last import: 7/24/2026` — concatenated with no separator, regardless of each one's RenderMode | A Mendix DynamicText is always an inline ``; RenderMode does not make it block-level. Not a build error — a silent layout surprise, so a non-fatal advisory fits | `mdl/executor/validate_widgets.go` (`validateConsecutiveDynamicText`, called from `validateWidgetTreeIn`) | New **MDL-WIDGET15** (info): scan each sibling list; emit once when a run of ≥2 adjacent **inline** dynamictexts occurs. **Only H1–H6 are block-level** (`headingRenderModeRe`) — `Text`/unset AND `Paragraph` both render as an inline `` (verified on Mendix 11.12.1 + Atlas), so `inlineDynamicText` excludes only headings. A heading+subtitle pair is NOT flagged; a Paragraph+Paragraph pair IS (it fuses). Fix for the user: merge into one dynamictext, wrap each in a container, or use a **heading** RenderMode — NOT Paragraph. Info severity so it never fails the build. Test `TestValidateConsecutiveDynamicText`; repro `mdl-examples/bug-tests/ledger-27-consecutive-dynamictext.mdl`. **Lesson: verify Mendix render behavior empirically — `Paragraph` sounds block-level but isn't; don't infer `display` from a name.** Ledger findings #27/#29 | +| `create association … from/to ` passes `mxcli check --references` and `mxcli exec` creates it, but `mx check` rejects it with **CE6771** "It is not possible to create associations to/from View Entities." Both directions are invalid | Associations to/from a view entity are statically impossible, but the reference checker only validated that the endpoint modules exist — it never inspected whether an endpoint was a view entity | `mdl/executor/validate.go` (CreateAssociationStmt case) + `mdl/executor/cmd_entities.go` (`isViewEntity`) | Resolve both endpoints via `findEntity`; if either `isViewEntity` (Source `DomainModels$OqlViewEntitySource`, or OqlQuery/SourceDocumentRef set), reject with CE6771 and point at a non-persistent entity carrying a real reference. Same-script endpoints are skipped (validated on their own statement). **References-mode only** (needs the domain model). Tests `TestIsViewEntity`; example `mdl-examples/bug-tests/ledger-41-view-entity-association.mdl`. Verified on Mendix 11.12.1. Ledger finding #41 | +| A `create or modify entity` that omits (drops) an INDEXED attribute leaves the entity's index orphaned — its column references a GUID that no longer exists. `mxcli` prints only the attribute-drop warning; `mx check` then **CRASHES loading the project** with `System.AggregateException` "The given key '' was not present in the dictionary" (DESCRIBE shows a dangling `index ()`) | Two parts. (1) The executor rebuilt the entity from the statement (no index clause → empty `entity.Indexes`), so the existing index wasn't reconciled against the surviving attributes. (2) The write path: `entityToGen` produced an **empty, untouched** Indexes PartList, which the codec treats as "clean" and passes the raw (orphaned) index through — an empty typed list does NOT override raw bytes for an existing element | `mdl/executor/cmd_entities.go` (`reconcileDroppedIndexes`, before `UpdateEntity`) + `mdl/backend/modelsdk/domainmodel_alter.go` (`UpdateEntity` empty-index dirtying) | (1) When the statement lists no indexes, carry existing indexes forward, pruning columns for dropped attributes (attr IDs survive by name via #13) and dropping empty indexes — a **partial** drop (`index (A,B)` → drop B → `index (A)`) works via this non-empty list. (2) For the **all-removed** case, force the empty Indexes list dirty (`AddIndexes(NewIndex())` + `RemoveIndexes(0)` — `PartList.Remove` calls `markDirty`) so the codec re-emits it as empty, clearing the raw orphan. **Codec insight: an untouched empty PartList is `clean` → raw passthrough; only a *dirtied* list (even if empty) overrides raw for an existing element.** Tests `TestReconcileDroppedIndexes`; example `mdl-examples/bug-tests/ledger-39-drop-indexed-attribute.mdl`. **Verified end-to-end: `mx check` → 0 errors on Mendix 11.12.1** (previously crashed). Ledger finding #39 | +| `retrieve … where [id = $Var]` (constraining on the object id) passes `mxcli check` but fails the build with **CE0161** "Error(s) in XPath constraint" — whether `$Var` is String or Long. Mendix XPath has no id operator reachable from a microflow expression | No check inspected retrieve constraints for an id comparison; `id` is a reserved member so it resolves loosely | `mdl/executor/validate_microflow.go` (`checkXPathIdConstraint`/`xpathIdConstraintRe`, MDL048) | New **MDL048**: on `RetrieveStmt.Where`, regex a bare `id` (word-boundaried, case-insensitive) before a comparison, **capturing the operand**, and flag only when the operand is a VALUE — a `$`-var whose kind is a primitive (in `varKinds`; objects aren't) or a string/number literal. **Comparing `id` to an OBJECT variable (`[id != $obj]`, the valid "exclude self" pattern) is NOT flagged** — verified valid on mx check; an over-broad `\bid\b\s*[=…]` regex false-positived `16-xpath-examples.mdl`'s exclude-self example. Fix for the user: a marketplace GUID action (GetObjectByGuid), **or** expose the id as a String on a view entity (`cast(id as string) as ObjectId`) and constrain on that String column. Test `TestValidateMicroflow_XPathIdConstraint`; repro `mdl-examples/bug-tests/ledger-42-retrieve-by-id.fail.mdl`. **Lesson: `[id = $x]` splits on the operand type — value → CE0161, object → valid; a checker that ignores the operand mislabels the valid case.** Ledger finding #42 | +| A call argument bound to an association-object path (`call M.Consume(B = $Edit/M.Edit_Budget)` — the object reached over an association) passes `mxcli check` but fails the build with **CE0117** "Error(s) in expression." An attribute value over the same association (`$Edit/M.Edit_Budget/Name`) is fine | Mendix does not treat an association path as a value — it must be materialized (`retrieve`) first. No check inspected call arguments for an association-object path | `mdl/executor/validate_microflow.go` (`checkAssociationObjectArgs`/`exprIsAssociationObjectPath`, MDL049; wired for CallMicroflowStmt + CallNanoflowStmt) | New **MDL049**: an argument whose value is an `AttributePathExpr` whose FINAL segment is module-qualified (a `.` → an association, yielding an object) is flagged; a final bare segment (an attribute) is not. Fix for the user: `retrieve $x from $Edit/M.Edit_Budget;` then pass `$x`. Test `TestValidateMicroflow_AssociationObjectArg`; repro `mdl-examples/bug-tests/ledger-44-assoc-path-as-value.fail.mdl`. Ledger findings #43/#44 | +| ~~MDL050 "format function + association navigation"~~ **REMOVED — it was a false positive.** MDL050 flagged `formatDateTime($obj/Mod.Assoc/Date, …)` and `formatDecimal(…) + $obj/Mod.Assoc/Attr` as CE0117. Re-verification against `mx check` on 11.12.1 (once the real #48 root cause — the dropped association target-entity step — was fixed) showed the premise was wrong on BOTH cases | The earlier "format-function + association" correlation was an artifact of TWO unrelated bugs, neither of which is about format functions: (1) **association navigation dropped its target-entity step** → CE0117 for ANY `$obj/Assoc/Attr` in an expression (see the "#48 root cause" row above), which happened to include the formatDateTime example; (2) **`formatDecimal($x, 2)` fails CE0117 on a PLAIN local decimal** with no association at all — a `formatDecimal`-signature bug, not an association issue. With (1) fixed, `formatDateTime($obj/Assoc/Date, …)` builds clean → MDL050 rejected valid code | `mdl/executor/validate_microflow.go` (removed `checkFormatWithAssociation`/`exprHasRenderFunc`/`exprHasAssociationNav`/`renderFuncsIncompatibleWithAssoc`) | Deleted the check, its call sites, its test (`TestValidateMicroflow_FormatWithAssociation`), and its bug-test. **Lesson (reinforced): even after "reproducing the failing neighbours", a rule can still be mis-premised if the neighbours share a DIFFERENT hidden bug — here the missing entity step. Verify a check is still valid after every related write-path fix; a correlation-based rule is fragile.** The remaining real issue — `formatDecimal($x, precision)` failing CE0117 regardless of associations — is a **separate open finding** (likely a wrong function signature/arity; investigate against `mx` and fix in the executor or flag via the function checker). Ledger finding #48 (root cause corrected; MDL050 retired) | +| **Attribute-over-association navigation in a microflow EXPRESSION** (`$T/Mod.Assoc/Attr` in a Change Variable / if-condition / return) passes `mxcli check` but `mx check` fails with **CE0117** "Error(s) in expression". Trips hardest when the module or entity name ends in a digit (`L48.`, `Account2.`) or when a decimal literal co-occurs (`$T/Mod.Assoc/Bal + 2.0`). Studio Pro supports this pattern | Mendix expression syntax needs the **target-entity step** in the path (`$T/Mod.Assoc/Mod.Entity/Attr`); the builder inserts it via `resolveAssociationPaths`→`resolvePathSegments`. Two bugs skipped that insertion: (1) `shouldPreserveExpressionSource` treated a `.` **preceded by a digit** as a decimal literal, so a name segment ending in a digit froze the whole expression's source and bypassed resolution (source is written verbatim); (2) a **legitimately** source-frozen expression (real decimal / whitespace) also bypassed resolution, so association nav + decimal in one expression broke even for non-digit modules | `mdl/visitor/visitor_microflow_statements.go` (`shouldPreserveExpressionSource` + `dotIsQualifiedNameSeparator`/`isIdentByte`); `mdl/executor/cmd_microflows_builder.go` (`resolveAssociationPaths` SourceExpr arm + `collectAttributePaths`) | (1) A `.` is a decimal point only when its adjacent digit run is a standalone number — NOT when the preceding token contains a letter/underscore (a qualified-name segment). `dotIsQualifiedNameSeparator` walks back over ident chars; a letter/`_` means name, not decimal. (2) For a source-preserved `SourceExpr`, still rewrite association paths **inside the raw source**: for each `AttributePathExpr`, `expressionToString(orig)` vs `expressionToString(resolve(orig))`; if they differ, `strings.ReplaceAll` in the source. Keeps decimal/whitespace fidelity AND inserts the entity step. Tests: `TestShouldPreserveExpressionSource_Decimals` (digit-module cases); example `mdl-examples/bug-tests/ledger-48-assoc-nav-digit-module.mdl`. **Verified: `mx check` → 0 errors on 11.12.1** for plain nav, if-then-else enum compare, and decimal-mixed. **Lesson: a heuristic that keys on a single character (`.`→decimal) collides with other grammar (qualified names); and "preserve source" and "resolve semantics" are in tension — a frozen source still needs its association paths fixed.** Ledger finding #48 (root cause) | +| An action property on a pluggable widget — e.g. `onClick: microflow …` on a DataGrid2 — passes `mxcli check --references` AND `mx check`, builds and runs, but is **silently discarded**: `DESCRIBE PAGE` omits it and the model round-trips minus the action, with no error or warning | The .def.json **generator** never emitted an `action` operation for `type="action"` properties (across 42 generated defs, no action operation existed at all). The writer reads only `propertyMappings` from the def, so with no action mapping it had nothing to write. The `.mpk` correctly declares the action slot; the gap is purely in generation | `mdl/executor/widget_defs.go` (`GenerateDefJSON` `case "action"` + `actionSourceForKey`); `mdl/executor/widget_engine.go` (`resolveMapping` `case "OnChange"`; `WidgetDefGeneratorVersion` bump 12→13) | Emit an `action` `PropertyMapping` for the action slots MDL can author: `onClick`→source `OnClick` (reads the widget's `Action` property, i.e. the `onClick:`/`Action:` alias) and `onChange`→source `OnChange`. The engine's **existing** `applyOperation "action"` → `builder.SetAction` path (nil-guarded) then serializes the `ClientAction` with its parameter mapping. Non-authorable action slots (DataGrid2 `onSelectionChange`/`onConfigurationChange`) return `""` from `actionSourceForKey` → no mapping (no MDL surface yet). Bump the generator version so existing projects regenerate. Tests `TestGenerateDefJSON_ActionMapping`; example `mdl-examples/bug-tests/ledger-67-pluggable-widget-action.mdl`. **Verified end-to-end: `mx check` → 0 errors on 11.12.1**, and the DataGrid2 persists the onClick microflow action with its `Thing: $currentObject` mapping (the raw page unit carries `L67.OnClick` + `L67.OnClick.Thing`). Ledger finding #67 (write path) | +| **General guard for the #67 class:** a real widget property that the generator doesn't map to a write path could be silently dropped (or, via an alias, slip through) with no diagnostic — the same failure mode as #67 for any unmapped property type (`expression`, `icon`, a future type, an action slot with no MDL surface) | The `WidgetDefinition.KnownProperties` field + the `MDL-WIDGET06` "recognized but not persisted" path already existed, but the **generator never populated `KnownProperties`**, so the warning never fired: an unmapped property either false-errored as MDL-WIDGET01 "no such property" or (via an alias like `Action`) passed silently | `mdl/executor/widget_defs.go` (`GenerateDefJSON` — populate `KnownProperties`); `WidgetDefGeneratorVersion` bump 13→14 | Compute `KnownProperties` purely from the two artifacts mxcli already has — **every `.mpk`-declared property key with no mapping in the generated def** (subtract PropertyMappings + aliases + ChildSlots + ObjectLists + mode mappings from the full key set). No per-widget knowledge. The existing `knownUnmappedProperties`/MDL-WIDGET06 check then WARNS "recognized but not persisted — the value will be dropped." **Three-way discrimination verified on DataGrid2 (11.12.1): a mapped property (`onClick`) is silent, a real-but-unmapped one (`rowClass`) warns MDL-WIDGET06, a truly-unknown one (`totallyBogusProp`) still errors MDL-WIDGET01.** Test `TestGenerateDefJSON_KnownPropertiesUnmapped`. **This is the tester's suggested general check — it catches the whole class without guessing which widgets support what; the `action` mapping remains the specific fix for onClick.** Ledger finding #67 (general guard) | +| **DESCRIBE read gap (follow-up to #67):** after the write fix, `DESCRIBE PAGE` still omitted a DataGrid2's `onClick` action — a describe round-trip silently lost it | The datagrid2 describe parse/output paths read datasource + columns + paging but never read the widget-level action; the param-aware `renderClientActionMDL` reader (used for button/onchange) was never called for a pluggable widget's action | `mdl/executor/cmd_pages_describe_pluggable.go` (`customWidgetPropertyActionMap`), `cmd_pages_describe.go` (`rawWidget.OnClick`), `cmd_pages_describe_parse.go` (datagrid2 branch), `cmd_pages_describe_output.go` (datagrid2 emit) | New `customWidgetPropertyActionMap` returns the raw `Forms$*ClientAction` map for a CustomWidget property (NoAction → nil); the read is wired in **two** describe paths — the `datagrid2` branch AND the **generic `pluggablewidget '…'` branch** (`!isKnownCustomWidgetType`, where **CustomChart — the finding's actual widget — is described**). Each renders it via `renderClientActionMDL` (param-aware) into `rawWidget.OnClick`; the output emits `onClick: ` (the generic branch's guard also fires on `w.OnClick != ""` so an action-only widget still gets its `pluggablewidget` header). **Verified end-to-end on BOTH DataGrid2 and CustomChart: describe → re-exec → `mx check` 0 errors, round-trip preserves the full `onClick: microflow …(param: $currentObject)` including the parameter mapping** (CustomChart quotes the param name, `"Data":`, which re-parses fine). Test `TestCustomWidgetPropertyActionMap`. Ledger finding #67 (DESCRIBE read gap closed for datagrid2 + generic pluggable/CustomChart) | +| Two loops in the same microflow reusing the same iterator name (`loop $R in … end loop; loop $R in … end loop`) pass `mxcli check` but `mx check` fails with **CE0111** "Duplicate variable name 'R'." at Loop | A Mendix loop iterator is scoped to the **whole microflow**, not to its loop — so the second loop re-creates an existing variable. No check tracked loop iterator names across a microflow | `mdl/executor/validate_microflow.go` (`checkDuplicateLoopVariables`, MDL052) | New **MDL052**: walk the microflow body (recursing through if/case/while/loop bodies) collecting `LoopStmt.LoopVariable` names; flag any name used by a second loop. Catches sequential AND nested reuse (a nested loop reusing an outer iterator is also CE0111). Distinct iterators, a single loop, and the same name across DIFFERENT microflows are fine. Fix for the user: give each loop a distinct iterator (`$R`, `$C`, `$M`). Test `TestValidateMicroflow_DuplicateLoopVariable`; repro `mdl-examples/bug-tests/ledger-64-duplicate-loop-variable.fail.mdl`. Ledger finding #64 | +| A `break` nested inside an `if`/`case` within a loop passes `mxcli check` but serializes a **dangling sequence-flow reference** — `mx check` then **CRASHES loading the project** with an unhandled `System.AggregateException` ("key … not present in the dictionary"), an unrecoverable failure. A break placed **directly** in the loop body serializes fine | The flow builder (`addBreakEvent`) creates the Break event but the sequence flow connecting it from inside a conditional dangles — a write-path (flow-graph serialization) bug, still open. `break` directly in the loop body wires correctly | `mdl/executor/validate_microflow.go` (`loopBodyHasConditionalBreak`/`stmtsContainBreak`, MDL051) — **interim check**, pending the serialization fix | New **MDL051**: on a `LoopStmt`, scan its body for a `break` inside an if/case/inheritance-split (not descending into nested loops — a nested loop traps its own break); a direct-child break is not flagged. Since the pattern currently produces a *crash*, a check-time rejection is a strict improvement. Fix for the user: a guard variable (`declare $Done Boolean = false; … if not($Done) then … set $Done = true`) — **verified clean on mx check**. Test `TestValidateMicroflow_ConditionalBreak`; repro `mdl-examples/bug-tests/ledger-52-break-in-conditional.fail.mdl`. Verified on 11.12.1: conditional break → KeyNotFound crash, direct break + guard-var workaround → 0 errors. **The real fix is the break-in-conditional flow serialization (write path) — MDL051 is the interim guard.** Ledger finding #52 | +| `set $Match = contains($Hay, $Needle)` over two **String** values was serialized as a **List operation** activity 'Contains', so string `contains(haystack, needle)` was unusable: a pre-declared Boolean `$Match` collided (**CE0111** "Duplicate variable name"), and a String input to a list op is otherwise rejected (CE0023/CE0097). `contains` in an IF condition worked (stayed an expression) | `contains` is overloaded — a LIST operation `contains(list, object)` **and** a STRING function `contains(haystack, needle)` — but the visitor unconditionally rewrote `set $x = contains(...)` into a `ListOperationStmt`, **dropping literal arguments** (`extractVariableName` returns "" for a literal). A list operation CREATES its output variable; the string function assigns to a pre-declared one | `mdl/visitor/visitor_microflow_statements.go` (`buildSetStatement` CONTAINS case + `isPlainVariableArg`) + `mdl/executor/cmd_microflows_builder_actions.go` (`addListOperationAction`) + `mdl/executor/cmd_microflows_builder_validate.go` (ListOperationStmt case) | **Write-path fix, two parts.** (1) Visitor: a literal/computed argument means the string function unambiguously → fall through to `MfSetStmt` (a Change Variable value expression); both-plain-variables stays a `ListOperationStmt` (ambiguous, kind unknown at parse). (2) Flow builder: when `declaredVars[InputVariable] == "String"`, emit a **Change Variable** action carrying `contains($In, $Second)` instead of a `ContainsOperation`; the validate pre-pass mirrors this (validate the target IS declared, don't flag CE0111 duplicate). Genuine list form (list var + object var) unchanged. Tests `TestContainsOverloadParsing` (visitor), `TestBuildContains_StringVsList` (builder); example `mdl-examples/bug-tests/ledger-53-string-contains.mdl`. **Verified: `mx check` → 0 errors on 11.12.1** for literal-arg, two-String-var, and genuine-list forms. Ledger finding #53 | +| `set $At = find($Raw, '"id":"')` — the STRING function `find(haystack, needle)` (substring index) — was serialized as a List operation activity 'Find by expression', so a pre-declared `$At` collided (**CE0111** "Duplicate variable name 'At'"). String `find()` was unusable. Same overload trap as #53 (contains) | `find` is overloaded — the LIST operation `find(list, condition)` (filter by a boolean condition) and the STRING function `find(haystack, needle)` → index — but the visitor unconditionally rewrote `set $x = find(...)` into a `ListOperationStmt` (list find), and a list op CREATES its output variable | `mdl/visitor/visitor_microflow_statements.go` (`buildSetStatement` FIND case + `isStringLiteralArg`) + `mdl/executor/cmd_microflows_builder_actions.go` (`addListOperationAction`) + `mdl/executor/cmd_microflows_builder_validate.go` (ListOperationStmt case) | **Write-path fix, mirrors #53.** (1) Visitor: a **string-literal** second argument (you never filter a list by a bare string literal) means the string function → fall through to `MfSetStmt`; a boolean condition or both-plain-variables stays a `ListOperationStmt`. (2) Flow builder + validate pre-pass: when `declaredVars[InputVariable] == "String"`, emit a **Change Variable** carrying `find($in, )` (arg1 is stored in the stmt's `Condition`) instead of a list FindOperation, and validate the target as a pre-declared SET (no CE0111). The contains and find String-input handling now share one branch. Tests `TestFindOverloadParsing` (visitor), `TestBuildContains_StringVsList` find cases (builder); example `mdl-examples/bug-tests/ledger-63-string-find.mdl`. **Verified: `mx check` → 0 errors on 11.12.1** for literal-arg, two-String-var, and genuine-list-condition forms. Ledger finding #63 | +| A datagrid **column** with an explicit empty caption (`Caption: ''`) passes `mxcli check` but `mx check` rejects the page with **CE0463** "The definition of this widget has changed" on the Data grid 2 (error points at the widget version, not the caption). Omitting the caption, or a non-empty string, both build clean | The pluggable widget engine's column-header fallback treated a **present-but-empty** header property as "has header" and skipped the attribute-name default — so `Caption: ''` emitted an empty header (which Studio Pro rejects) while an **absent** caption got the fallback. The keyword datagrid path already handled it (`if caption == "" { caption = col.Attribute }` in `datagrid_column.go`) | `mdl/executor/widget_engine.go` (`applyColumnHeaderFallback`) | **Write-path fix.** Detect an empty header (a `texttemplate` op with empty `TextTemplate` and no `Parameters`) and treat it like an absent one: fill it **in place** with the bound attribute's leaf name (not appended — that would duplicate the `header` prop). A header WITH params (`Caption: '{1}'`) is left untouched. Result: `Caption: ''` now behaves like omitting it. **Round 2 (custom-content columns):** the first fix left a column with **no bound attribute** (an action/custom-content column) untouched — it had nothing to derive a header from, so an empty OR absent caption still tripped CE0463 (a custom-content column requires a non-empty header). Fixed by falling back to the **column's own name** when there's no attribute, and **gating the whole fallback on the item template having a `header` slot** (`mapping.ItemProperties`) so header-less object-list items (chart series, accordion groups) are never given a spurious header. `applyColumnHeaderFallback(spec, columnName, hasHeaderSlot)`. Test `TestApplyColumnHeaderFallback` (cases 1–8); examples `ledger-54-empty-column-caption.mdl` + custom-content verified via exec. **Verified: `mx check` → 0 errors on 11.12.1 for attribute columns AND custom-content columns (empty + absent caption)** (previously CE0463). Ledger finding #54 (custom-content columns) | | Follow-up to round 2 (#25 re-test): the subscriber is registered but `runtime.log` **still** holds only the 4 JVM-banner lines — a probe microflow's `log info/warning/error` and a forced runtime exception produce **zero** lines. Isolation test: boot+subscriber → 0 probe lines; then call `start_logging` → 6 lines | A standalone runtime boots with logging **not started**, so a registered subscriber sits **inert** — nothing is delivered until the logging subsystem is activated. `create_log_subscriber` alone is necessary but not sufficient | `cmd/mxcli/docker/runtime_controller.go` (`configureRuntimeLogging` — renamed from `attachFileLogSubscriber`; now also calls `start_logging`) | After `create_log_subscriber`, call `CallM2EE(c.opts, "start_logging", nil)` in the same step (order: create subscriber → start_logging). Treat an "already started" response as success (`start` re-runs on the DB-update retry / restart paths on a still-running JVM). Still best-effort (warn to Stdout, never fail an up runtime). Test `TestStart_StartLoggingAlreadyStartedIsSuccess`; `TestStart_AttachesLogSubscriber` now asserts the `[start, create_log_subscriber, start_logging]` sequence. **Diagnosis pattern**: when a registered sink receives nothing, check whether the subsystem that feeds it is even *running* — registration ≠ activation. Findings #25 (round 3) | --- diff --git a/.claude/skills/mendix/alter-page.md b/.claude/skills/mendix/alter-page.md index 4865f46c5..b26f36b81 100644 --- a/.claude/skills/mendix/alter-page.md +++ b/.claude/skills/mendix/alter-page.md @@ -321,6 +321,34 @@ alter page MyModule.Customer_Edit { | SET on non-existent widget | Widget names are case-sensitive; check with DESCRIBE | | Missing semicolons between operations | Each operation inside `{ }` ends with `;` | +## Limitations — prefer binding at page creation (ledger finding #45) + +`ALTER PAGE` is best for *content* edits (add/remove/retitle widgets). Three things +it cannot do; when you hit them, define the referenced microflows **before** the +page and bind the buttons at creation time instead of rewiring afterwards: + +1. **`SET` cannot rewire a button's action.** `set` accepts a fixed property list + (`caption`, `class`, `visible`, …) — `action` is not on it, so + `set Action = microflow … on btnSave` is a parse error. Set the button's action + when the button is created (or `REPLACE` the button subtree). + +2. **`REPLACE` cannot reuse a widget name that lives inside the subtree being + replaced.** The replacement is *built* (registering its widget names) before the + old subtree is removed, so reusing e.g. `btnSave` collides with the still-present + old `btnSave` ("duplicate widget name 'btnSave'"). Give the replacement widgets + fresh names, or rebuild the whole page with `create or replace page`. + +3. **A footer is not addressable by its author-given name.** A `footer myName { … }` + is a *marker*: its children are hoisted into the data view's footer and the + footer itself is serialized as `footer1`, so `drop widget myName` (and even + `drop widget footer1`) report "not found". To change footer contents, edit the + children by their own names, or `create or replace page`. + +**Recommended pattern**: put save/reset microflows in a file that runs *before* the +page definition, and bind the popup/footer buttons to them at creation. The +apparent "page needs microflow, microflow needs page" cycle usually exists only +between *different* microflows, not within the page itself. + ## Validation Checklist 1. **Get widget names first**: Run `describe page Module.PageName` to see all widget names diff --git a/.claude/skills/mendix/run-local.md b/.claude/skills/mendix/run-local.md index b685543cd..4b9bd7d5b 100644 --- a/.claude/skills/mendix/run-local.md +++ b/.claude/skills/mendix/run-local.md @@ -86,6 +86,21 @@ does nothing until you restart `run --local` (or re-run with `--watch`). `mxbuil changes, so **never** `rm -rf theme-cache/ .mendix-cache/ deployment/` — clearing caches is a red herring. +A **theme compile error fails the build step, before the runtime starts** — the +error text is in the serve `/build` output (printed by `run --local`), not a +runtime problem. A common Dart Sass trap: `rgba()` needs a real color, so a +comma-list variable is rejected — + +```scss +// WRONG: Dart Sass reports "$color: 168, 50, 30 is not a color" +$cf-over: 168, 50, 30; +background-color: rgba($cf-over, 0.1); + +// RIGHT: make the variable a color, then adjust alpha +$cf-over: rgb(168, 50, 30); +background-color: rgba($cf-over, 0.1); +``` + ## "My edit didn't show up" — stale process, not stale cache `run --local` refuses to boot when its ports (8080/8090/6543) are already answering, diff --git a/.claude/skills/mendix/theme-styling.md b/.claude/skills/mendix/theme-styling.md index e7cbb290f..1b57c69c2 100644 --- a/.claude/skills/mendix/theme-styling.md +++ b/.claude/skills/mendix/theme-styling.md @@ -90,6 +90,31 @@ container ctn (style: 'color: red;') { This also applies to `alter styling` and `alter page set style` — never target a DYNAMICTEXT widget with Style. +### DataGrid2 Renders ARIA `
`s, Not a `` — and `Size` Is a Flex Weight + +Two surprises when styling a **DataGrid2** matrix/pivot (ledger finding #46): + +1. **It is not a `
`.** DataGrid2 emits `role="grid"` / `role="row"` / + `role="gridcell"` **`
`s**, so `th, td { … }` selectors match nothing. A + `td { white-space: nowrap }` intended to keep `€ 5,200` on one line does not + apply, and amounts wrap. Target the ARIA roles instead: + + ```scss + .ledger-matrix [role='gridcell'] { white-space: nowrap; } + ``` + + Playwright/tests see the same DOM: assert on `[role="row"]`, not `tr`. + +2. **`Size` is a flex weight, not pixels.** On a column, `ColumnWidth: manual, + Size: 132` does **not** set a 132px width — it divides available width by the + weights across all columns. To give a wide matrix room, set a min-width on the + grid and let it scroll: + + ```scss + .ledger-matrix [role='grid'] { min-width: 1320px; } + .ledger-matrix { overflow-x: auto; } + ``` + ### Design Property Keys Are Case-Sensitive Keys must match the `name` field in `design-properties.json` exactly: diff --git a/.claude/skills/mendix/write-microflows.md b/.claude/skills/mendix/write-microflows.md index 3e60908bf..5b7185192 100644 --- a/.claude/skills/mendix/write-microflows.md +++ b/.claude/skills/mendix/write-microflows.md @@ -571,6 +571,8 @@ commit $Product with events refresh; **Best Practice**: Use `with events` when you want before/after commit event handlers to execute. Use `refresh` when the committed object is displayed in the client and you want the UI to update immediately. +> **Re-sorting a database-datasource grid needs `refresh`.** A plain `commit $Obj;` updates the committed attribute *values* in the grid, but a grid backed by a **database** datasource does **not** re-run its sort — so after changing a sort key (e.g. a reorder that rewrites a `SequenceNumber`), the row stays in its old position until you `commit $Obj refresh;`. The `refresh` re-queries the datasource, which re-applies the sort. (Ledger #57.) + > **Binding a microflow to an entity event is MDL — you do NOT need to map it manually in Studio Pro.** After writing a handler microflow (e.g. a `BeforeCommit` validation), wire it directly: > ```mdl > alter entity Sales.Order @@ -590,6 +592,21 @@ add head($SourceItems) to $Items; Use expression-valued `add` only when the expression returns an object compatible with the target list element type. +### `contains` is overloaded — string vs list + +`contains(a, b)` is both a **string** function (`contains(haystack, needle)` → substring test) and a **list** operation (`contains(list, object)` → membership test). mxcli picks the right serialization automatically: + +```mdl +-- STRING contains — assign to a PRE-DECLARED Boolean (a Change Variable action) +declare $HasAt Boolean = false; +set $HasAt = contains($Email, '@'); + +-- LIST contains — do NOT pre-declare the output (the list op creates it) +set $Found = contains($Items, $Item); +``` + +The distinction: a **literal or computed** second argument is always the string function. When both arguments are plain variables, the input variable's declared type decides — a **String** input becomes the string function (Change Variable, so declare the Boolean first), anything else stays a list operation (which creates its own output variable, so leave it undeclared). Getting the declare wrong is what triggers `CE0111 "Duplicate variable name"`. + ## Database Operations ### RETRIEVE Statement @@ -734,7 +751,11 @@ $Result = $A * $B; -- Multiplication $Result = $A div $B; -- Division (use 'div', not '/') ``` -**Important**: Use `div` for division, NOT `/`. +**Important**: Use `div` for division, NOT `/`. In a Mendix expression `/` is the +member/association separator (`$obj/Attr`), so `$A / $B` is not division — +`mxcli check` rejects it as **MDL045** (it would fail the build with CE0117). +Integer/decimal division always yields a Decimal; wrap it in `round()`/`trunc()` +for an Integer result (else **MDL041**). ### Comparison @@ -762,6 +783,21 @@ if $IsActive and $IsValid and $HasStock then end if; ``` +### Date construction + +`dateTime(...)` / `dateTimeUTC(...)` build a date from **literal numeric +constants only** — a variable or computed argument fails the build with CE0117 +(`mxcli check` flags it as **MDL046**). To build a date from variables, step off +a literal anchor with `addDays()` / `addMonths()` (which *do* take variables): + +```mdl +-- WRONG: variable args to dateTime() (CE0117 / MDL046) +set $D = dateTime(2026, $Month, $Day); + +-- RIGHT: anchor on a literal, then step with addMonths/addDays +set $D = addDays(addMonths(dateTime(2026, 1, 1), $Month - 1), $Day - 1); +``` + ## Logging ```mdl @@ -987,6 +1023,29 @@ retrieve $Items from Module.Entity where Active = true; **Note**: `returns type as $Var` in the microflow signature does NOT create an activity variable — it only names the return value. So `$Var = call java action ...` after `returns as $Var` is fine (one creation). +**Variables are scoped to the branch that creates them.** A variable first created +inside an `if`/`else` arm (including by `$Var = call ...`) is not visible outside +that arm. To use one value after a conditional, `declare` it *before* the +conditional and assign in every branch: + +```mdl +-- WRONG: $GTotalText is created only in the `then` arm → not declared in `else` +if $HasVariance then + $GTotalText = call microflow Module.FMT_Variance($v); -- created here only +else + set $GTotalText = 'n/a'; -- error: not declared +end if; + +-- CORRECT: declare before, then set in each branch (call into a temp, then set) +declare $GTotalText string = ''; +if $HasVariance then + $Tmp = call microflow Module.FMT_Variance($v); + set $GTotalText = $Tmp; +else + set $GTotalText = 'n/a'; +end if; +``` + **Fallback chains reuse the same name = CE0111.** Because each `$Var = call microflow …` (or retrieve/create) is a *fresh* variable creation, the natural "try A, else try B" shape is invalid — even when the retry is inside an `if`: ```mdl diff --git a/.claude/skills/mendix/write-oql-queries.md b/.claude/skills/mendix/write-oql-queries.md index 39c878524..87a900d0c 100644 --- a/.claude/skills/mendix/write-oql-queries.md +++ b/.claude/skills/mendix/write-oql-queries.md @@ -46,34 +46,65 @@ create view entity Finance.CashFlowProjection ( ); ``` -**RULE 2: NEVER use ORDER BY or LIMIT in VIEW entity OQL** +**RULE 2: `ORDER BY` requires a `LIMIT` — prefer letting the consuming query sort** -The UI component or microflow using the view will handle sorting and pagination: +`ORDER BY` **alone** is rejected (`mxcli check` → **MDL030**; `mx check` → CE0174). +`ORDER BY` **with** a `LIMIT` is valid and builds clean — that is exactly how you +express a top-N view. As a default, prefer *no* `ORDER BY`/`LIMIT` so the UI +component or microflow can sort and paginate the same view differently; reach for +`ORDER BY … LIMIT` only when the view is intrinsically a top-N. ```sql --- ❌ WRONG - Hardcoded sorting and limits +-- ❌ WRONG - ORDER BY without LIMIT (MDL030 / CE0174) create view entity Finance.TopCustomers (...) as ( select c.Name as CustomerName, sum(o.Amount) as TotalSpent from Finance.Customer as c inner join Finance.Order_Customer/Finance.Order as o GROUP by c.Name - ORDER by TotalSpent desc -- Remove this - limit 100 -- Remove this + ORDER by TotalSpent desc -- needs a LIMIT ); --- ✅ CORRECT - No ORDER BY or LIMIT +-- ✅ CORRECT (preferred) - let the consuming page/microflow sort +create view entity Finance.CustomerTotals (...) as ( + select c.Name as CustomerName, sum(o.Amount) as TotalSpent + from Finance.Customer as c + inner join Finance.Order_Customer/Finance.Order as o + GROUP by c.Name +); + +-- ✅ ALSO VALID - an intrinsic top-N view (ORDER BY paired with LIMIT) create view entity Finance.TopCustomers (...) as ( select c.Name as CustomerName, sum(o.Amount) as TotalSpent from Finance.Customer as c inner join Finance.Order_Customer/Finance.Order as o GROUP by c.Name - -- Let the UI component handle sorting and limits + ORDER by TotalSpent desc + LIMIT 100 ); ``` **Why these rules matter:** - **Explicit aliases**: Required for proper OQL-to-entity attribute mapping in Mendix -- **No ORDER BY/LIMIT**: Provides flexibility - different pages/microflows can sort and paginate the same view differently +- **ORDER BY/LIMIT**: Omitting both keeps the view flexible (each page/microflow sorts and paginates as it needs); when you *do* sort, `ORDER BY` must be paired with `LIMIT` (MDL030) + +**`UNION` / `UNION ALL` are supported** in view-entity OQL and round-trip cleanly — +use them to combine multiple row kinds in one view (e.g. category rows plus group +subtotals). Column count and types must line up across branches; `ORDER BY` (with +its `LIMIT`) applies to the whole unioned result, not a single branch. + +```sql +create or modify view entity Ledger.CategoryAndSubtotals ( + Label: string(100), Amount: decimal +) as ( + select c.Name as Label, sum(t.Amount) as Amount + from Ledger.Category as c + left join Ledger.Transaction_Category/Ledger.Transaction as t + group by c.Name + union all + select 'TOTAL' as Label, sum(t.Amount) as Amount + from Ledger.Transaction as t +); +``` ### 1. Aggregate Functions (MUST BE LOWERCASE) ```sql @@ -431,7 +462,7 @@ create view entity Shop.ProductCurrentPrice ( **Key points:** - Use `pr/Shop.Price_Product = p.ID` (association path with `.ID`) - Never use bare alias: `pr/Shop.Price_Product = p` will fail -- ORDER BY and LIMIT are valid inside correlated subqueries (just not at the view level) +- ORDER BY and LIMIT are valid inside correlated subqueries; at the view level, ORDER BY is allowed only when paired with LIMIT (MDL030) ### Pattern 10: JOIN with ON Clause (Non-Association) ```sql @@ -509,8 +540,8 @@ mxcli check view.mdl -p app.mpr --references This catches type mismatches (e.g., declaring `long` for a `count()` column that returns `integer`), missing module references, and OQL syntax errors — before they become MxBuild errors like CE6770 ("View Entity is out of sync with the OQL Query"). ### Step 9: Final Check -- Remove any ORDER BY, LIMIT, or OFFSET clauses -- These should be handled by the UI component or microflow +- Prefer no ORDER BY/LIMIT/OFFSET — let the UI component or microflow sort and paginate +- If the view is intrinsically a top-N, ORDER BY is allowed **only when paired with a LIMIT** (MDL030 / CE0174) ## Common Mistakes to Avoid @@ -583,19 +614,25 @@ where pr/Shop.Price_Product = p where pr/Shop.Price_Product = p.ID ``` -### ❌ Mistake 8: Using ORDER BY or LIMIT in VIEW +### ❌ Mistake 8: ORDER BY without a LIMIT in a VIEW ```sql --- WRONG - Hardcoded in view +-- WRONG - ORDER BY alone (MDL030 / CE0174) create view entity Finance.TopItems (...) as ( select ... - ORDER by Amount desc - limit 100 + ORDER by Amount desc -- needs a LIMIT ); --- CORRECT - Let UI handle it +-- CORRECT (preferred) - let the UI sort/paginate +create view entity Finance.ItemTotals (...) as ( + select ... + -- no ORDER BY / LIMIT +); + +-- ALSO VALID - an intrinsic top-N view create view entity Finance.TopItems (...) as ( select ... - -- No ORDER BY or LIMIT + ORDER by Amount desc + LIMIT 100 ); ``` @@ -640,7 +677,7 @@ create view entity Shop.MonthlyRevenue ( 3. ✅ Proper COUNT: `count(o.OrderId)` not `count(*)` 4. ✅ Comma syntax for DATEPART: `datepart(YEAR, o.OrderDate)` 5. ✅ GROUP BY matches SELECT non-aggregated expressions -6. ✅ No ORDER BY or LIMIT (UI will handle sorting) +6. ✅ ORDER BY omitted (UI sorts) — or, for a top-N view, paired with a LIMIT (MDL030) ## Testing OQL Queries @@ -707,7 +744,7 @@ When writing OQL queries for VIEW entities, always verify: - [ ] **CRITICAL**: Entity has @Position annotation (e.g., @Position(300, 500)) - [ ] **CRITICAL**: All SELECT columns have explicit AS aliases matching entity attributes -- [ ] **CRITICAL**: No ORDER BY, LIMIT, or OFFSET clauses (let UI handle sorting) +- [ ] ORDER BY omitted so the UI sorts — or, for a top-N view, ORDER BY paired with a LIMIT (MDL030 rejects ORDER BY without LIMIT) - [ ] Aggregate functions are lowercase (`sum`, `avg`, `count`, `max`, `min`) - [ ] Using `count(entity.ID)` not `count(*)` - [ ] DATEPART uses comma syntax: `datepart(YEAR, field)` diff --git a/.claude/skills/mendix/xpath-constraints.md b/.claude/skills/mendix/xpath-constraints.md index 9b1ebe8b4..0db67be7e 100644 --- a/.claude/skills/mendix/xpath-constraints.md +++ b/.claude/skills/mendix/xpath-constraints.md @@ -97,6 +97,12 @@ where [not(Module.Order_Customer/Module.Customer)] **Rule**: Always use the fully qualified association name (`Module.AssociationName`). +> **`= empty` does not work on associations (CE0161 / MDL047).** `= empty` tests +> *attribute* nullability only. To test whether an object *has no* associated +> object, use negated existence: `[not(Module.Order_Customer/Module.Customer)]` — +> **not** `[Module.Order_Customer = empty]`. `mxcli check` flags the association +> `= empty` form as **MDL047** before the build does. + ### Variable Paths ```mdl diff --git a/docs/05-mdl-specification/01-language-reference.md b/docs/05-mdl-specification/01-language-reference.md index 4f94a3ded..5cbfb97f9 100644 --- a/docs/05-mdl-specification/01-language-reference.md +++ b/docs/05-mdl-specification/01-language-reference.md @@ -42,7 +42,7 @@ boolean, both, business, by, call, cancel, caption, cascade, catalog, change, CHILD, close, column, combobox, commit, connect, configuration, connector, constant, constraint, container, create, CRUD, datagrid, dataview, date, datetime, declare, default, delete, -delete_behavior, DELETE_BUT_KEEP_REFERENCES, DELETE_CASCADE, demo, +delete_behavior, DELETE_BUT_KEEP_REFERENCES, CASCADE, demo, depth, desc, DESCENDING, describe, DIFF, disconnect, drop, else, empty, end, entity, enumeration, error, event, events, execute, exec, EXIT, export, extends, external, false, folder, footer, @@ -585,16 +585,17 @@ create association - `Parent` - Parent owns the association - `Child` - Child owns the association -**Delete Behavior:** +**Delete Behavior:** one of `DELETE_BUT_KEEP_REFERENCES`, `CASCADE`, +`DELETE_IF_NO_REFERENCES`, `DELETE_AND_REFERENCES`, `PREVENT` - `DELETE_BUT_KEEP_REFERENCES` - Delete object but keep references -- `DELETE_CASCADE` - Delete associated objects +- `CASCADE` - Delete associated objects too (there is **no** `DELETE_CASCADE` keyword) **Example:** ```sql -/** Links orders to customers */ +/** Links orders to customers (Order owns the FK → from Order to Customer) */ create association Sales.Order_Customer - from Sales.Customer - to Sales.Order + from Sales.Order + to Sales.Customer type reference owner default delete_behavior DELETE_BUT_KEEP_REFERENCES; diff --git a/docs/05-mdl-specification/03-domain-model.md b/docs/05-mdl-specification/03-domain-model.md index 7cf4606a0..700df4c08 100644 --- a/docs/05-mdl-specification/03-domain-model.md +++ b/docs/05-mdl-specification/03-domain-model.md @@ -227,11 +227,18 @@ Associations define relationships between entities. ### Association Syntax +**Direction (important):** an association goes **`from` the entity that owns the +foreign key `to` the entity it references**. In a many-to-one, that means +`from to ` — e.g. each `Order` knows its +`Customer`, so `from Order to Customer`. This is the opposite of what many people +expect; getting it backwards passes `mxcli check` but fails the Mendix build with +`CE0854 "not reachable from entity"`. + ```sql [/** */] create association . - from - to + from -- owns the reference (the "many" side) + to -- is referenced (the "one" side) type [owner ] [delete_behavior ] @@ -243,8 +250,8 @@ create association . | Property | MDL Clause | Description | |----------|------------|-------------| | Name | `Module.Name` | Association identifier | -| Parent | `from entity` | Parent (owner/many) side of relationship | -| Child | `to entity` | Child (referenced/one) side of relationship | +| From | `from entity` | The entity that **owns the foreign key** — the "many" side of a many-to-one | +| To | `to entity` | The **referenced** entity — the "one" side | | Type | `type reference/ReferenceSet` | Cardinality type | | Owner | `owner` | Which side can modify | | Delete Behavior | `delete_behavior` | What happens on delete | @@ -262,16 +269,19 @@ create association . | Behavior | MDL Keyword | Description | |----------|-------------|-------------| -| Delete but keep references | `DELETE_BUT_KEEP_REFERENCES` | Delete object, nullify references | -| Delete cascade | `DELETE_CASCADE` | Delete associated objects too | +| Keep references | `DELETE_BUT_KEEP_REFERENCES` | Delete object, nullify references | +| Delete referenced too | `CASCADE` | Delete the associated objects too (there is **no** `DELETE_CASCADE`) | +| Only if unreferenced | `DELETE_IF_NO_REFERENCES` | Delete only when nothing references it | +| Delete both | `DELETE_AND_REFERENCES` | Delete this object and the ones it references | +| Prevent | `PREVENT` | Block the delete while a reference exists | ### Examples ```sql -/** Order belongs to Customer (many-to-one) */ +/** Order belongs to Customer (many-to-one): Order owns the FK → from Order to Customer */ create association Sales.Order_Customer - from Sales.Customer - to Sales.Order + from Sales.Order + to Sales.Customer type reference owner default delete_behavior DELETE_BUT_KEEP_REFERENCES; @@ -285,12 +295,12 @@ create association Sales.Order_Product owner both; / -/** Invoice must be deleted with Order */ +/** Invoice must be deleted with its Order */ create association Sales.Order_Invoice from Sales.Order to Sales.Invoice type reference - delete_behavior DELETE_CASCADE; + delete_behavior CASCADE; / ``` @@ -480,10 +490,10 @@ index (OrderNumber) index (OrderDate desc); / --- Create association +-- Create association (Order owns the FK to Customer → from Order to Customer) create association Sales.Order_Customer - from Sales.Customer - to Sales.Order + from Sales.Order + to Sales.Customer type reference owner default delete_behavior DELETE_BUT_KEEP_REFERENCES; diff --git a/docs/11-proposals/PROPOSAL_check_mxbuild_gap_heuristics.md b/docs/11-proposals/PROPOSAL_check_mxbuild_gap_heuristics.md index d6024fa4d..6137ec998 100644 --- a/docs/11-proposals/PROPOSAL_check_mxbuild_gap_heuristics.md +++ b/docs/11-proposals/PROPOSAL_check_mxbuild_gap_heuristics.md @@ -1,16 +1,61 @@ --- title: check heuristics for constructs MxBuild rejects -status: draft +status: in-progress date: 2026-07-02 --- # Proposal: `check` heuristics for constructs MxBuild rejects -**Status:** Draft -**Date:** 2026-07-02 +**Status:** In progress — Wave 1 shipped; ongoing as a standing programme (see +[§ Implementation status](#implementation-status)) +**Date:** 2026-07-02 (updated 2026-07-29) **Relates to:** `PROPOSAL_expression_type_checking.md` (shares the `ModelResolver` resolution core — see [§ Relation to expression type checking](#relation-to-expression-type-checking)) +## Implementation status + +This proposal has become a **standing programme**: every time a real project +reports a construct that passes `mxcli check` but `mx check`/MxBuild rejects, a +static heuristic is added to close the gap (verified against the bundled `mx` +where feasible). The originally-cataloged six cases: + +| Case | Rule | Status | +|------|------|--------| +| 2 — user task without a page (CE1834) | `MDL-WF01` | ✅ shipped (`validate_workflow.go`) | +| 4 — single outcome with nested activities (CE1876) | `MDL-WF02` | ✅ shipped | +| 1 — free-text decision outcome | `MDL-WF03` | ✅ shipped | +| 5 — control-bar button `$currentObject` (CE1571) | `MDL-BUTTON01` | ✅ shipped (`validate_page_button_context.go`) | +| 3 — user-task page context ≠ `System.WorkflowUserTask` (CE7412) | — | ⬜ **deferred** — needs `--references` + page-context resolution, and is FP-prone (WorkflowUserTask specializations, snippet/microflow-sourced dataviews). No current test app exercises workflows, so the CE7412 trigger and the FP boundary can't be verified against `mx` yet. Pick up when a workflow app enters the test rotation. | +| 6 — dynamictext contentparams/content on enum/date (CE0117) | — | 🟡 **partial** — the non-String→`toString()` write path landed (RSS-reader follow-up); the standalone `--references` warn + a focused fail-open repro remain | + +**Ledger-driven catalog expansion (2026-07).** The `ako/mxcli-ledger` project has +since surfaced ~10 more cases of exactly this class, each implemented on the same +pattern (rule + `.fail.mdl` repro + `fix-issue.md` CE→rule row): + +| Rule | Construct → MxBuild error | Verdict | +|------|---------------------------|---------| +| `MDL045` | `/` used as division → CE0117 (`/` is member access; use `div`) | syntax-only | +| `MDL046` | `dateTime()`/`dateTimeUTC()` with a non-literal arg → CE0117 | syntax-only | +| `MDL047` | association `= empty` in a retrieve **or** widget datasource → CE0161 | syntax-only | +| `MDL048` | `[id = ]` retrieve (vs the valid `[id != $obj]`) → CE0161 | syntax-only (operand-typed) | +| `MDL049` | association-object path passed as a call argument → CE0117 | syntax-only | +| `MDL-WIDGET13` | association traversal in a widget expression prop → CE0117 | syntax-only | +| `MDL-WIDGET14` | a client expression in `contentparams`/`captionparams` → CE1613 | syntax-only | +| `MDL-WIDGET15` | adjacent inline (Text/Paragraph) dynamictexts fuse | info (layout) | +| `MDL031` (pass-through) | view pass-through string column length ≠ source → CE6770 | `--references` | +| (assoc validate) | `create association` to/from a view entity → CE6771 | `--references` | + +Two more ledger cases in this class were closed by **fixing the write path** rather +than adding a check — the MDL is structurally valid, so `check` can't see it; the +model was being *written* wrong: navigation-list item names (CE7247/CE0495) and an +orphaned index left by `create or modify` dropping an indexed attribute (which +*crashed* `mx check`). These belong to the same "check ↔ build parity" mission but +are writer fixes, not heuristics. + +**Standing policy:** when a new missing check is reported, implement it here (or as +a write-path fix when the construct is valid MDL). The two remaining originally- +cataloged gaps are **case 3 (CE7412)** and the standalone **case 6 warn**. + ## Problem Statement `mxcli check … --references` reports "Check passed" for several constructs that diff --git a/mdl-examples/bug-tests/contentparam-expression-rejected.fail.mdl b/mdl-examples/bug-tests/contentparam-expression-rejected.fail.mdl new file mode 100644 index 000000000..3c447a02e --- /dev/null +++ b/mdl-examples/bug-tests/contentparam-expression-rejected.fail.mdl @@ -0,0 +1,38 @@ +-- ============================================================================ +-- contentparams / captionparams reject a client expression (ledger finding #26) +-- ============================================================================ +-- +-- Symptom (before fix): +-- A template parameter bound to a function-call expression — +-- dynamictext dImport (content: 'Last import: {1}', +-- contentparams: [{1} = formatDateTime($currentObject/LastImport, 'd MMM yyyy')]) +-- passed `mxcli check` but Studio Pro rejected the page with CE1613 +-- "attribute no longer exists": the unquoted value is stored as an ATTRIBUTE +-- PATH (a data binding), so the whole expression became a bogus attribute name. +-- +-- After fix: +-- `mxcli check` rejects it with MDL-WIDGET14 and tells the user to precompute +-- the value onto the bound entity (e.g. a calculated attribute) and bind that +-- attribute instead. A quoted string literal ([{1} = 'literal']) is still fine, +-- and a plain attribute path ([{1} = $obj/Attr]) is still fine. +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/contentparam-expression-rejected.fail.mdl +-- ============================================================================ + +create entity MyFirstModule.Report ( + LastImport: DateTime +); + +create or replace page MyFirstModule.ReportPage +( + Title: 'Report', + Layout: Atlas_Core.Atlas_Default, + Params: { $Report: MyFirstModule.Report } +) +{ + dataview dv (datasource: $Report) { + -- Expression in a data-binding slot → MDL-WIDGET14 (CE1613 in Studio Pro). + dynamictext dImport (content: 'Last import: {1}', contentparams: [{1} = formatDateTime($currentObject/LastImport, 'd MMM yyyy')]) + } +} diff --git a/mdl-examples/bug-tests/ledger-17-19-expression-serialization.mdl b/mdl-examples/bug-tests/ledger-17-19-expression-serialization.mdl new file mode 100644 index 000000000..5887ee612 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-17-19-expression-serialization.mdl @@ -0,0 +1,37 @@ +-- ============================================================================ +-- Ledger findings #17–19: microflow expression serialization corruption +-- ============================================================================ +-- +-- Symptom (before fix): a `set $x = ` whose value was re-serialized from +-- the AST (rather than kept as source) silently corrupted the stored expression. +-- `mxcli check` passed; `mx check` reported only a generic CE0117: +-- #17 set $B = $Dec / 2; `/` is NOT Mendix division (that is `div`); +-- `/` navigates associations, so this is invalid +-- #18 set $B = $Dec div 2.0; stored as $Dec div 2 (2.0 → 2, breaks typing) +-- #19 set $B = $Dec * 0.000001; stored as ... * 1e-06 (scientific notation) +-- +-- After fix: +-- #17 `/`-as-division is rejected at check time (MDL045) with a clear message +-- to use `div` — instead of silently writing a broken expression. +-- #18/#19 shouldPreserveExpressionSource() keeps the raw source for any +-- expression containing a decimal literal, so `2.0` and `0.000001` +-- survive verbatim (no truncation, no scientific notation). +-- +-- Note: MDL division is `div`, not `/`. This example uses `div` (the correct +-- form); the `/`-misuse case lives in ledger-17-slash-division.fail.mdl. +-- +-- Usage (round-trip: exec then describe, compare the set lines): +-- mxcli exec mdl-examples/bug-tests/ledger-17-19-expression-serialization.mdl -p app.mpr +-- mxcli -p app.mpr -c "describe microflow MyModule.LedgerExprBug" +-- ~/.mxcli/mxbuild/*/modeler/mx check app.mpr # must be clean (no CE0117) + +create or modify microflow MyModule.LedgerExprBug ( + $Dec: decimal, + $Dec2: decimal, + $I: integer +) +begin + set $Ratio = $Dec div $Dec2; -- #17: MDL division is `div`, not `/` + set $Half = $Dec div 2.0; -- #18: 2.0 must stay 2.0 (not 2) + set $Tiny = $Dec * $I * $I * 0.000001; -- #19: no scientific notation (not 1e-06) +end diff --git a/mdl-examples/bug-tests/ledger-17-slash-division.fail.mdl b/mdl-examples/bug-tests/ledger-17-slash-division.fail.mdl new file mode 100644 index 000000000..0cd1c9573 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-17-slash-division.fail.mdl @@ -0,0 +1,34 @@ +-- ============================================================================ +-- Ledger finding #17: `/` used as a division operator (MDL045) +-- ============================================================================ +-- +-- Symptom (before fix): `set $R = $Dec / 2;` passed `mxcli check` but `mx check` +-- reported a generic CE0117 "Error(s) in expression". In a Mendix expression `/` +-- is only the member/association separator (`$obj/Attr`); arithmetic division is +-- `div`. mxcli silently wrote the invalid `/` expression. +-- +-- After fix: MDL045 rejects `/`-as-division at check time and tells the user to +-- use `div`. Both forms are caught: `/ literal` and `/ (expr)` (a BinaryExpr with +-- operator `/`), and the variable/variable form `$Dec / $Dec2` (which parses as a +-- member-access path — the visitor preserves its `/ $` source so MDL045 sees it). +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-17-slash-division.fail.mdl +-- ============================================================================ + +create microflow MyModule.SlashDivision ( + $Dec: decimal, + $Dec2: decimal +) +returns decimal +begin + set $Bad = $Dec / 2; -- MDL045: use `div` + set $AlsoBad = ($Dec + 1) / $Dec2; -- MDL045: use `div` + set $VarVar = $Dec / $Dec2; -- MDL045: variable/variable form (finding #17) + -- Embedded division-by-variable — the `$a / $b` part degrades to a member + -- path nested in a larger expression, but is still caught (finding #17 round 2): + set $Embed1 = $Dec / $Dec2 + 1; -- MDL045: embedded in an addition + set $Embed2 = round($Dec / $Dec2); -- MDL045: embedded in a function call + set $Embed3 = $Dec / $Dec2 * 100; -- MDL045: embedded in a multiplication + return $Bad; +end diff --git a/mdl-examples/bug-tests/ledger-21-datetime-literals.fail.mdl b/mdl-examples/bug-tests/ledger-21-datetime-literals.fail.mdl new file mode 100644 index 000000000..d60d90435 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-21-datetime-literals.fail.mdl @@ -0,0 +1,26 @@ +-- ============================================================================ +-- Ledger finding #21: dateTime()/dateTimeUTC() accept only literal constants +-- ============================================================================ +-- +-- Symptom (before fix): `dateTime(2026, $Month, $Day)` passed `mxcli check` but +-- failed the build with CE0117 "Error(s) in expression" — Mendix builds these +-- date functions from hardcoded numeric constants only; a variable or computed +-- argument is rejected. +-- +-- After fix: MDL046 rejects a non-literal dateTime()/dateTimeUTC() argument at +-- check time and points to the addDays()/addMonths() workaround (those DO take +-- variables — step off a literal anchor date). +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-21-datetime-literals.fail.mdl +-- ============================================================================ + +create microflow MyModule.DateTimeLiterals ( + $Month: integer, + $Day: integer +) +returns dateTime +begin + set $Bad = dateTime(2026, $Month, $Day); -- MDL046: variable args + return $Bad; +end diff --git a/mdl-examples/bug-tests/ledger-25-page-datasource-assoc-empty.fail.mdl b/mdl-examples/bug-tests/ledger-25-page-datasource-assoc-empty.fail.mdl new file mode 100644 index 000000000..63e1c1f29 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-25-page-datasource-assoc-empty.fail.mdl @@ -0,0 +1,33 @@ +-- ============================================================================ +-- Ledger finding #25 (verification round): association `= empty` in a PAGE +-- datasource where-clause +-- ============================================================================ +-- +-- Symptom (before fix): MDL047 only inspected microflow `retrieve` constraints, +-- so an association `= empty` in a widget datasource where-clause slipped through +-- `mxcli check` and failed the build with CE0161 "Error(s) in XPath constraint". +-- The ledger project originally hit this in a page-datasource context. +-- +-- After fix: MDL047 also inspects `DataSourceV3.Where` on every widget. The +-- nullability test is the negation form `[not(Assoc/Target)]`. +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-25-page-datasource-assoc-empty.fail.mdl +-- ============================================================================ + +create entity Ledger.Category ( Name: String ); +create entity Ledger.Transaction ( Description: String ); +create association Ledger.Transaction_Category + from Ledger.Transaction to Ledger.Category; + +create or replace page Ledger.UncategorizedPage +( + Title: 'Uncategorized', + Layout: Atlas_Core.Atlas_Default +) +{ + datagrid dgProbe ( + datasource: database from Ledger.Transaction + where [Ledger.Transaction_Category = empty] -- MDL047 (CE0161 in Studio Pro) + ) +} diff --git a/mdl-examples/bug-tests/ledger-25-xpath-assoc-empty.fail.mdl b/mdl-examples/bug-tests/ledger-25-xpath-assoc-empty.fail.mdl new file mode 100644 index 000000000..867b8de00 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-25-xpath-assoc-empty.fail.mdl @@ -0,0 +1,28 @@ +-- ============================================================================ +-- Ledger finding #25: XPath has no `= empty` for associations +-- ============================================================================ +-- +-- Symptom (before fix): `where [Ledger.Transaction_Category = empty]` passed +-- `mxcli check` but failed the build with CE0161 "Error(s) in XPath constraint" +-- — `= empty` tests attribute nullability, not association nullability. +-- +-- After fix: MDL047 rejects `[Module.Association = empty]` at check time and +-- points to the negation form `[not(Assoc/Target)]`. A bare attribute +-- (`[Description = empty]`) and an attribute-over-association +-- (`[Assoc/Attr = empty]`) stay valid and are not flagged. +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-25-xpath-assoc-empty.fail.mdl +-- ============================================================================ + +create entity Ledger.Category ( Name: String ); +create entity Ledger.Transaction ( Description: String ); +create association Ledger.Transaction_Category + from Ledger.Transaction to Ledger.Category; + +create microflow MyModule.UncategorizedTransactions () +returns list of Ledger.Transaction +begin + retrieve $t from Ledger.Transaction where [Ledger.Transaction_Category = empty]; -- MDL047 + return $t; +end diff --git a/mdl-examples/bug-tests/ledger-27-consecutive-dynamictext.mdl b/mdl-examples/bug-tests/ledger-27-consecutive-dynamictext.mdl new file mode 100644 index 000000000..8b913e94b --- /dev/null +++ b/mdl-examples/bug-tests/ledger-27-consecutive-dynamictext.mdl @@ -0,0 +1,50 @@ +-- ============================================================================ +-- Ledger finding #27: consecutive dynamictext widgets render inline +-- ============================================================================ +-- +-- Symptom: two sibling dynamictexts inside a container produced +-- "This month: € 310Last import: 7/24/2026" (no separator) — a DynamicText +-- renders as an inline regardless of RenderMode, so adjacent ones +-- concatenate. +-- +-- After fix: MDL-WIDGET15 (info, non-fatal) warns about the layout surprise and +-- suggests merging into one dynamictext with two content params, wrapping each in +-- its own container, or using a heading RenderMode (H1-H6, which is block-level). +-- +-- Finding #29 (verification round): only H1-H6 are block-level. `Paragraph` mode +-- ALSO renders inline () on Mendix 11.12.1 + Atlas, so two Paragraph +-- widgets fuse too and are (correctly) still flagged. A heading + subtitle pair +-- is NOT flagged. This example shows all three: fused Text pair (flagged), fused +-- Paragraph pair (flagged), heading + subtitle (not flagged). +-- +-- Usage (check passes; emits MDL-WIDGET15 info lines for the two fused pairs): +-- mxcli check mdl-examples/bug-tests/ledger-27-consecutive-dynamictext.mdl +-- ============================================================================ + +create entity MyModule.Summary ( Amount: Decimal, LastImport: DateTime ); + +create or replace page MyModule.SummaryPage +( + Title: 'Summary', + Layout: Atlas_Core.Atlas_Default, + Params: { $Summary: MyModule.Summary } +) +{ + dataview dv (datasource: $Summary) { + -- Text-mode pair fuses → flagged + container box { + dynamictext dMonth (content: 'This month: {1}', contentparams: [{1} = $currentObject/Amount]) + dynamictext dImport (content: 'Last import: {1}', contentparams: [{1} = $currentObject/LastImport]) + } + -- Paragraph-mode pair ALSO fuses (renders inline) → flagged (#29) + container paras { + dynamictext pMonth (content: 'This month: {1}', RenderMode: Paragraph, contentparams: [{1} = $currentObject/Amount]) + dynamictext pImport (content: 'Last import: {1}', RenderMode: Paragraph, contentparams: [{1} = $currentObject/LastImport]) + } + -- Heading (block-level) + subtitle does NOT fuse → not flagged + container heads { + dynamictext hTitle (content: 'Summary {1}', RenderMode: H2, contentparams: [{1} = $currentObject/Amount]) + dynamictext hSub (content: 'as of {1}', contentparams: [{1} = $currentObject/LastImport]) + } + } +} diff --git a/mdl-examples/bug-tests/ledger-36-view-passthrough-length.mdl b/mdl-examples/bug-tests/ledger-36-view-passthrough-length.mdl new file mode 100644 index 000000000..63e25a884 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-36-view-passthrough-length.mdl @@ -0,0 +1,32 @@ +-- ============================================================================ +-- Ledger finding #36: view-entity pass-through string column length must match +-- the source attribute exactly +-- ============================================================================ +-- +-- Trap: a PASS-THROUGH column (a bare source-attribute reference like `c.Name`) +-- inherits the source attribute's length verbatim. Declaring the view column with +-- a DIFFERENT length — including an unbounded `string` against a `string(100)` +-- source — passes plain `mxcli check` but fails the build with CE6770 +-- "View Entity out of sync with OQL Query". +-- +-- -- WRONG: unbounded string against a string(100) source (CE6770) +-- create or modify view entity Ledger.VCategory ( CategoryName: string ) as ( +-- select c.Name as CategoryName from Ledger.Category as c +-- ); +-- +-- After fix: `mxcli check ... --references` resolves the source attribute and +-- flags the mismatch pre-build (extends the view-entity length check — which +-- previously covered only DERIVED columns like cast/case — to pass-through +-- columns). The correct form below matches the source length exactly. +-- +-- Usage (the mismatch is caught in references mode, against a project): +-- mxcli check mdl-examples/bug-tests/ledger-36-view-passthrough-length.mdl -p app.mpr --references +-- ============================================================================ + +create entity Ledger.Category ( Name: string(100) ); + +-- CORRECT: declared length matches the source attribute (string(100)). +create or modify view entity Ledger.VCategory ( CategoryName: string(100) ) as ( + select c.Name as CategoryName + from Ledger.Category as c +); diff --git a/mdl-examples/bug-tests/ledger-39-drop-indexed-attribute.mdl b/mdl-examples/bug-tests/ledger-39-drop-indexed-attribute.mdl new file mode 100644 index 000000000..85251d351 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-39-drop-indexed-attribute.mdl @@ -0,0 +1,34 @@ +-- ============================================================================ +-- Ledger finding #39: dropping an indexed attribute left an orphaned index +-- ============================================================================ +-- +-- Symptom (before fix): a `create or modify entity` that omits (drops) an indexed +-- attribute left the entity's index behind, orphaned — its column referenced a +-- GUID that no longer existed. `mxcli` reported only the attribute-drop warning; +-- `mx check` then CRASHED loading the project with an unhandled +-- System.AggregateException ("The given key '' was not present in the +-- dictionary"), because the index pointed at the removed attribute. +-- +-- After fix: create-or-modify carries existing indexes forward, pruning any +-- column that references a dropped attribute and removing indexes left empty (and +-- the write path clears the index list authoritatively, so the last index is +-- removed rather than left as an orphaned `index ()`). Verified end-to-end: +-- `mx check` reports 0 errors on Mendix 11.12.1. +-- +-- Usage (run the two steps against a project, then mx check): +-- mxcli exec step1 (create Transaction with index (MonthKey)) +-- mxcli exec step2 (this file — drops MonthKey) +-- mx check app.mpr # must not crash +-- ============================================================================ + +-- Step 1 (run first): create the indexed attribute. +create or modify entity Ledger.Transaction ( + Amount: decimal, + MonthKey: string(7) +) +index (MonthKey); + +-- Step 2: drop MonthKey — its index is pruned, not left orphaned. +create or modify entity Ledger.Transaction ( + Amount: decimal +); diff --git a/mdl-examples/bug-tests/ledger-41-view-entity-association.mdl b/mdl-examples/bug-tests/ledger-41-view-entity-association.mdl new file mode 100644 index 000000000..3cfcd9fe2 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-41-view-entity-association.mdl @@ -0,0 +1,25 @@ +-- ============================================================================ +-- Ledger finding #41: associations to/from a view entity are impossible (CE6771) +-- ============================================================================ +-- +-- Symptom (before fix): `create association ... from/to ` passed +-- `mxcli check --references` and `mxcli exec` created it silently, but `mx check` +-- rejected it with CE6771 "It is not possible to create associations to/from +-- View Entities." Both directions are invalid. +-- +-- After fix: `mxcli check --references` rejects it, naming the view entity. Use a +-- non-persistent entity carrying a real reference to the target instead. +-- +-- Usage (the association is rejected in references mode, against a project that +-- already has the view entity): +-- mxcli exec 'create entity Ledger.Category (Name: string(100));' -p app.mpr +-- mxcli exec 'create or modify view entity Ledger.VCat (CatName: string(100)) +-- as (select c.Name as CatName from Ledger.Category as c);' -p app.mpr +-- mxcli check mdl-examples/bug-tests/ledger-41-view-entity-association.mdl -p app.mpr --references +-- ============================================================================ + +-- Rejected: Ledger.VCat is a view entity (CE6771). The reverse direction +-- (from Ledger.Category to Ledger.VCat) is rejected the same way. +create association Ledger.VCat_Category + from Ledger.VCat + to Ledger.Category; diff --git a/mdl-examples/bug-tests/ledger-42-retrieve-by-id.fail.mdl b/mdl-examples/bug-tests/ledger-42-retrieve-by-id.fail.mdl new file mode 100644 index 000000000..7163bc3d3 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-42-retrieve-by-id.fail.mdl @@ -0,0 +1,26 @@ +-- ============================================================================ +-- Ledger finding #42: retrieving by object id is not expressible in XPath +-- ============================================================================ +-- +-- Symptom (before fix): `retrieve $B from ... where [id = $Var]` passed +-- `mxcli check` but failed the build with CE0161 "Error(s) in XPath constraint" +-- (whether $Var is String or Long). Mendix XPath has no id operator reachable +-- from a microflow expression. +-- +-- After fix: MDL048 rejects it at check time and points at the two real options: +-- 1. a marketplace GUID action (NanoflowCommons GetObjectByGuid / CommunityCommons) +-- 2. expose the id as a String on a VIEW ENTITY — `cast(id as string) as ObjectId` +-- in the OQL — and constrain on that String column instead +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-42-retrieve-by-id.fail.mdl +-- ============================================================================ + +create microflow MyModule.FindById ( + $Id: string +) +returns list of MyModule.Budget +begin + retrieve $B from MyModule.Budget where [id = $Id]; -- MDL048 (CE0161) + return $B; +end diff --git a/mdl-examples/bug-tests/ledger-44-assoc-path-as-value.fail.mdl b/mdl-examples/bug-tests/ledger-44-assoc-path-as-value.fail.mdl new file mode 100644 index 000000000..d912137ce --- /dev/null +++ b/mdl-examples/bug-tests/ledger-44-assoc-path-as-value.fail.mdl @@ -0,0 +1,29 @@ +-- ============================================================================ +-- Ledger finding #43/#44: an association path is not a value +-- ============================================================================ +-- +-- Symptom (before fix): passing an association-object path as a call argument — +-- $R = call microflow MyModule.Consume(B = $Edit/MyModule.Edit_Budget) +-- passed `mxcli check` but failed the build with CE0117 "Error(s) in expression." +-- Mendix does not treat an association path (an object reached over an +-- association) as a value; it must be materialized first. +-- +-- After fix: MDL049 rejects it at check time. Fix for the user: retrieve the +-- object into a variable first, then pass the variable: +-- retrieve $B from $Edit/MyModule.Edit_Budget; +-- $R = call microflow MyModule.Consume(B = $B); +-- An attribute value over an association ($Edit/MyModule.Edit_Budget/Name) is a +-- legal value and is NOT flagged. +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-44-assoc-path-as-value.fail.mdl +-- ============================================================================ + +create microflow MyModule.PassAssoc ( + $Edit: MyModule.Edit +) +returns boolean +begin + $R = call microflow MyModule.Consume(B = $Edit/MyModule.Edit_Budget); -- MDL049 (CE0117) + return $R; +end diff --git a/mdl-examples/bug-tests/ledger-48-assoc-nav-digit-module.mdl b/mdl-examples/bug-tests/ledger-48-assoc-nav-digit-module.mdl new file mode 100644 index 000000000..69a4b5502 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-48-assoc-nav-digit-module.mdl @@ -0,0 +1,74 @@ +-- ============================================================================ +-- Ledger finding #48 (root cause): association navigation in a microflow +-- expression dropped its target-entity step → CE0117 +-- ============================================================================ +-- +-- Symptom (before fix): a microflow expression navigating an association to read +-- an attribute (`$T/Mod.Assoc/Attr` in a Change Variable / if-condition) passed +-- `mxcli check` but `mx check` reported CE0117 "Error(s) in expression". Mendix +-- expression syntax needs the target-entity step in the path +-- (`$T/Mod.Assoc/Mod.Entity/Attr`); mxcli's builder inserts it via +-- resolveAssociationPaths. +-- +-- Root cause: shouldPreserveExpressionSource treated a `.` preceded by a digit as +-- a DECIMAL literal. A module or entity whose name ends in a digit (`L48.`, +-- `Account2.` — very common) therefore froze the whole expression's source, which +-- bypassed the target-entity insertion and wrote `$T/L48.Assoc/Attr` (no entity). +-- A second path: a real decimal literal co-occurring with an association nav +-- (`$T/L48.Assoc/Bal + 2.0`) froze the source for a legitimate reason and skipped +-- resolution too. +-- +-- After fix: (1) a `.` inside a qualified name (segment contains a letter/_) is no +-- longer mistaken for a decimal point; (2) association paths are rewritten inside +-- a preserved source, so decimal + association-nav in one expression also resolves. +-- Verified: `mx check` → 0 errors on Mendix 11.12.1. + +create module L48; + +create enumeration L48.GroupType ( Income, Expense ); + +create entity L48.Account ( + Name: string(100), + Bal: decimal +); + +create entity L48.CategoryGroup ( + GroupType: enum L48.GroupType +); + +create entity L48.Transaction ( + Amount: decimal +); +create association L48.Transaction_Account from L48.Transaction to L48.Account; + +create entity L48.Category ( + Name: string(100) +); +create association L48.Category_CategoryGroup from L48.Category to L48.CategoryGroup; + +-- Plain attribute-over-association navigation (digit-ending module) +create microflow L48.ReadName ($T: L48.Transaction) +returns String +begin + declare $S String = ''; + set $S = $T/L48.Transaction_Account/Name; + return $S; +end; + +-- Association navigation co-occurring with a real decimal literal +create microflow L48.Mix ($T: L48.Transaction) +returns Decimal +begin + declare $D Decimal = 0; + set $D = $T/L48.Transaction_Account/Bal + 2.0; + return $D; +end; + +-- Enum-over-association compared to an enum literal inside an if-then-else +create microflow L48.Fav ($Cat: L48.Category, $Actual: Decimal, $Budget: Decimal) +returns Decimal +begin + declare $F Decimal = 0; + set $F = if $Cat/L48.Category_CategoryGroup/GroupType = L48.GroupType.Income then $Actual - $Budget else $Budget - $Actual; + return $F; +end; diff --git a/mdl-examples/bug-tests/ledger-52-break-in-conditional.fail.mdl b/mdl-examples/bug-tests/ledger-52-break-in-conditional.fail.mdl new file mode 100644 index 000000000..94e1265cd --- /dev/null +++ b/mdl-examples/bug-tests/ledger-52-break-in-conditional.fail.mdl @@ -0,0 +1,39 @@ +-- ============================================================================ +-- Ledger finding #52: `break` inside a conditional in a loop → unloadable model +-- ============================================================================ +-- +-- Symptom (before fix): a `break` nested inside an `if`/`case` within a loop +-- passed `mxcli check --references` but serialized a DANGLING sequence-flow +-- reference. `mx check` then CRASHED loading the project with an unhandled +-- System.AggregateException ("The given key '' was not present in the +-- dictionary") — an unrecoverable project-load failure, not a normal error. +-- +-- A `break` placed DIRECTLY in the loop body serializes fine; only the useful +-- conditional form (`if then break`) is affected. +-- +-- After fix: MDL051 rejects the conditional-break form at check time (a diagnostic +-- beats a crash) until the flow serialization is fixed. Workaround — a guard +-- variable (verified clean on mx check): +-- declare $Done Boolean = false; +-- loop $R in $Rules begin +-- if not($Done) then +-- if then set $Done = true; end if; +-- end if; +-- end loop +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-52-break-in-conditional.fail.mdl +-- ============================================================================ + +create microflow MyModule.FirstActive ( + $Rules: list of MyModule.Rule +) +returns boolean +begin + loop $R in $Rules begin + if $R/Active then + break; -- MDL051: crashes mx check (unloadable model) + end if; + end loop + return true; +end diff --git a/mdl-examples/bug-tests/ledger-53-string-contains.mdl b/mdl-examples/bug-tests/ledger-53-string-contains.mdl new file mode 100644 index 000000000..b6b0775d3 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-53-string-contains.mdl @@ -0,0 +1,63 @@ +-- ============================================================================ +-- Ledger finding #53: string `contains()` serialized as a LIST operation +-- ============================================================================ +-- +-- Symptom (before fix): `set $Match = contains($Hay, $Needle)` on two String +-- values was always serialized as a List operation activity 'Contains'. Since a +-- list operation CREATES its output variable, a pre-declared Boolean `$Match` +-- collided (`[CE0111] "Duplicate variable name 'Match'."`), and a String input +-- to a list operation is otherwise rejected (CE0023/CE0097). Net effect: the +-- string `contains(haystack, needle)` function was unavailable from MDL. +-- +-- Root cause: the visitor unconditionally rewrote `set $x = contains(...)` into +-- a ListOperationStmt, dropping literal arguments entirely. +-- +-- After fix (two parts): +-- 1. Visitor — a literal/computed argument means the string function; it stays +-- a value expression (Change Variable action), not a list operation. +-- 2. Flow builder — when the input variable is a declared String, the ambiguous +-- both-variables form also becomes a Change Variable carrying `contains(...)`. +-- The genuine list form (list variable + object variable) is unchanged. +-- +-- Verified: `mx check` reports 0 errors on a fresh Mendix 11.12.1 project. + +create module Ledger53; + +create entity Ledger53.Item ( + Name: String +); + +-- (A) string contains with a literal second argument +create microflow Ledger53.HasAt ( + $Email: String +) +returns Boolean +begin + declare $Match Boolean = false; + set $Match = contains($Email, '@'); + return $Match; +end; + +-- (B) string contains with two String variables (ambiguous at parse time) +create microflow Ledger53.StrContains ( + $Hay: String, + $Needle: String +) +returns Boolean +begin + declare $Match Boolean = false; + set $Match = contains($Hay, $Needle); + return $Match; +end; + +-- (C) genuine LIST contains: list variable + object variable. +-- A list operation creates its output variable, so it is NOT pre-declared. +create microflow Ledger53.ListContains ( + $Items: list of Ledger53.Item, + $One: Ledger53.Item +) +returns Boolean +begin + set $Found = contains($Items, $One); + return $Found; +end; diff --git a/mdl-examples/bug-tests/ledger-54-custom-content-column.mdl b/mdl-examples/bug-tests/ledger-54-custom-content-column.mdl new file mode 100644 index 000000000..20804b8f3 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-54-custom-content-column.mdl @@ -0,0 +1,55 @@ +-- ============================================================================ +-- Ledger finding #54 (round 2): custom-content column with empty/absent caption +-- ============================================================================ +-- +-- Symptom (after the first #54 fix): the attribute-bound column fallback worked, +-- but a CUSTOM-CONTENT column (a column with child widgets / an actions button +-- and NO bound attribute) with an empty `Caption: ''` — or no caption at all — +-- still failed `mx check` with CE0463. A custom-content column requires a +-- non-empty header, and the first fix had no attribute name to fall back to. +-- +-- After fix: the header falls back to the COLUMN'S OWN NAME when there is no +-- bound attribute. The fallback is gated on the item template having a `header` +-- slot, so header-less object-list items (chart series, accordion groups) are +-- never given a spurious header. +-- +-- Verified: `mx check` → 0 errors on Mendix 11.12.1 for both the empty-caption +-- and no-caption custom-content columns (previously CE0463). +-- +-- NOTE: requires the project's generated widget defs (`mxcli widget init`), so +-- this is exercised via exec/mx-check, not just syntax check. + +create module L54c; + +create entity L54c.Thing ( + Name: string(100) +); + +create microflow L54c.RowAction ( + $Thing: L54c.Thing +) +returns Boolean +begin + return true; +end; + +create or replace page L54c.Grid ( + Title: 'Grid', + Layout: Atlas_Core.Atlas_Default +) { + pluggablewidget 'com.mendix.widget.web.datagrid.Datagrid' dg ( + DataSource: database from L54c.Thing + ) { + column colName (Attribute: Name, Caption: 'Name') + -- Custom-content column, empty caption → header falls back to the column name + column colActionsEmpty (Caption: '') { + actionbutton btnA (Caption: 'Go', Action: microflow L54c.RowAction(Thing: $currentObject)) + } + -- Custom-content column, no caption → header falls back to the column name + column colActionsBare { + actionbutton btnB (Caption: 'Go2', Action: microflow L54c.RowAction(Thing: $currentObject)) + } + } +} + +grant view on page L54c.Grid to L54c.User; diff --git a/mdl-examples/bug-tests/ledger-54-empty-column-caption.mdl b/mdl-examples/bug-tests/ledger-54-empty-column-caption.mdl new file mode 100644 index 000000000..d927be79a --- /dev/null +++ b/mdl-examples/bug-tests/ledger-54-empty-column-caption.mdl @@ -0,0 +1,42 @@ +-- ============================================================================ +-- Ledger finding #54: empty datagrid column caption → CE0463 +-- ============================================================================ +-- +-- Symptom (before fix): a datagrid column with an explicit empty caption +-- (`Caption: ''`) passed `mxcli check` but `mx check` rejected the page with +-- CE0463 "The definition of this widget has changed" on the Data grid 2 — the +-- error pointed at the widget version, not the caption field. Omitting the +-- caption entirely, or giving it a non-empty string, both built cleanly. +-- +-- Root cause: the pluggable widget engine's column-header fallback +-- (applyColumnHeaderFallback) treated a PRESENT-but-EMPTY header property as +-- "has header" and skipped the attribute-name default — so an explicit +-- `Caption: ''` emitted an empty header that Studio Pro rejects, while an +-- ABSENT caption got the fallback. The keyword datagrid path already handled +-- this (`if caption == "" { caption = col.Attribute }`). +-- +-- After fix: an empty column caption is treated like an absent one — the header +-- falls back to the bound attribute's leaf name (here "Amount"), matching the +-- omitted-caption behaviour. Verified: `mx check` → 0 errors on Mendix 11.12.1. + +create module L54; + +create entity L54.Thing ( + Name: string(100), + Amount: integer +); + +create or replace page L54.Overview ( + Title: 'Overview', + Layout: Atlas_Core.Atlas_Default +) { + pluggablewidget 'com.mendix.widget.web.datagrid.Datagrid' dg ( + DataSource: database from L54.Thing + ) { + column colName (Attribute: Name, Caption: 'Name') + -- Empty caption: falls back to the attribute name ("Amount") as the header + column colAmount (Attribute: Amount, Caption: '') + } +} + +grant view on page L54.Overview to L54.User; diff --git a/mdl-examples/bug-tests/ledger-63-string-find.mdl b/mdl-examples/bug-tests/ledger-63-string-find.mdl new file mode 100644 index 000000000..5ad4e6dcb --- /dev/null +++ b/mdl-examples/bug-tests/ledger-63-string-find.mdl @@ -0,0 +1,62 @@ +-- ============================================================================ +-- Ledger finding #63: string `find()` serialized as a LIST operation +-- ============================================================================ +-- +-- Symptom (before fix): `set $At = find($Raw, '"id":"')` — the string function +-- find(haystack, needle) returning an index — was serialized as a List operation +-- activity 'Find by expression'. Since a list operation CREATES its output +-- variable, a pre-declared `$At` collided: `mx check` reported +-- [CE0111] "Duplicate variable name 'At'.". Net effect: string find was +-- unusable from MDL. +-- +-- Root cause: the visitor unconditionally rewrote `set $x = find(...)` into a +-- ListOperationStmt (list find), dropping the string-function interpretation. +-- +-- After fix (mirrors #53 contains): +-- 1. Visitor — a string-literal second argument means the string function; it +-- stays a value expression (Change Variable), not a list operation. +-- 2. Flow builder — when the input variable is a declared String, the ambiguous +-- both-variables form also becomes a Change Variable carrying `find(...)`. +-- The genuine list form find(list, condition) is unchanged. +-- +-- Verified: `mx check` → 0 errors on Mendix 11.12.1. + +create module L63; + +create entity L63.Item ( + Key: string(50) +); + +-- (A) string find with a string-literal needle +create microflow L63.FindAt ( + $Raw: String +) +returns Integer +begin + declare $At Integer = 0; + set $At = find($Raw, '"id":"'); + return $At; +end; + +-- (B) string find with two String variables +create microflow L63.FindVar ( + $Hay: String, + $Needle: String +) +returns Integer +begin + declare $At Integer = 0; + set $At = find($Hay, $Needle); + return $At; +end; + +-- (C) genuine LIST find: list + boolean condition (output not pre-declared) +create microflow L63.ListFind ( + $Items: list of L63.Item, + $K: String +) +returns L63.Item +begin + set $Found = find($Items, Key = $K); + return $Found; +end; diff --git a/mdl-examples/bug-tests/ledger-64-duplicate-loop-variable.fail.mdl b/mdl-examples/bug-tests/ledger-64-duplicate-loop-variable.fail.mdl new file mode 100644 index 000000000..339265f47 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-64-duplicate-loop-variable.fail.mdl @@ -0,0 +1,38 @@ +-- ============================================================================ +-- Ledger finding #64: a loop variable is scoped to the microflow, not the loop +-- ============================================================================ +-- +-- Symptom (before fix): two loops in the same microflow reusing the same iterator +-- name (`loop $R in … end loop; loop $R in … end loop`) passed `mxcli check` but +-- `mx check` failed with [CE0111] "Duplicate variable name 'R'." at Loop. A +-- Mendix loop variable is scoped to the WHOLE microflow, so the second loop +-- re-creates an existing variable. +-- +-- After fix: MDL052 rejects a reused loop iterator at check time and tells the +-- user to give each loop a distinct name. A nested loop reusing an outer +-- iterator is caught too; distinct iterators (and the same name across different +-- microflows) are fine. +-- +-- Usage (expected to FAIL check): +-- mxcli check mdl-examples/bug-tests/ledger-64-duplicate-loop-variable.fail.mdl +-- ============================================================================ + +create module L64; + +create entity L64.Row ( + Name: string(50) +); + +create microflow L64.TwoLoops ( + $Rows: list of L64.Row +) +returns Boolean +begin + loop $R in $Rows begin + set $x = $R/Name; -- first use of $R + end loop; + loop $R in $Rows begin -- MDL052: '$R' reused (CE0111) + set $y = $R/Name; + end loop; + return true; +end diff --git a/mdl-examples/bug-tests/ledger-67-customchart-action.mdl b/mdl-examples/bug-tests/ledger-67-customchart-action.mdl new file mode 100644 index 000000000..8f8360fc0 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-67-customchart-action.mdl @@ -0,0 +1,61 @@ +-- ============================================================================ +-- Ledger finding #67 (as originally reported): a CustomChart onClick action was +-- silently dropped on write, and omitted by DESCRIBE +-- ============================================================================ +-- +-- The finding was reported on the Charts CustomChart widget: +-- pluggablewidget 'com.mendix.widget.web.customchart.CustomChart' … ( +-- onClick: microflow Ledger.ACT_SunburstClick(Context: $currentObject) ) +-- The action passed mxcli check --references and mx check, built and ran, but was +-- silently discarded — DESCRIBE omitted it and the model round-tripped without it. +-- +-- Root cause + fix: see ledger-67-pluggable-widget-action.mdl (DataGrid2). The +-- .def.json generator now emits an `action` mapping for `type="action"` +-- properties (onClick → the widget Action / OnClick source), so the engine +-- serializes the client action; and DESCRIBE reads it back through the generic +-- `pluggablewidget '…'` path (where CustomChart is described), rendering the full +-- action with its parameter mapping. +-- +-- Verified end-to-end on Mendix 11.12.1: mx check → 0 errors; the CustomChart +-- persists the onClick microflow action with its `Data: $currentObject` mapping; +-- and describe → re-exec round-trips the onClick. +-- +-- NOTE: requires the project's generated widget defs (`mxcli widget init`), so +-- this is exercised via exec/mx-check, not just syntax check. + +create module CCm; + +create entity CCm.Data ( + ChartData: string(2000), + ChartLayout: string(2000) +); + +create microflow CCm.OnPointClick ( + $Data: CCm.Data +) +returns Boolean +begin + return true; +end; + +create microflow CCm.ChartSource () +returns CCm.Data +begin + $New = create CCm.Data; + return $New; +end; + +create or replace page CCm.Chart ( + Title: 'Chart', + Layout: Atlas_Core.Atlas_Default +) { + dataview dv (datasource: microflow CCm.ChartSource) { + pluggablewidget 'com.mendix.widget.web.customchart.CustomChart' chart ( + dataAttribute: ChartData, + layoutAttribute: ChartLayout, + onClick: microflow CCm.OnPointClick(Data: $currentObject) + ) + } +} + +grant view on page CCm.Chart to CCm.User; diff --git a/mdl-examples/bug-tests/ledger-67-pluggable-widget-action.mdl b/mdl-examples/bug-tests/ledger-67-pluggable-widget-action.mdl new file mode 100644 index 000000000..faa5298d0 --- /dev/null +++ b/mdl-examples/bug-tests/ledger-67-pluggable-widget-action.mdl @@ -0,0 +1,54 @@ +-- ============================================================================ +-- Ledger finding #67: a pluggable widget's action property was silently dropped +-- ============================================================================ +-- +-- Symptom (before fix): supplying an action property on a pluggable widget — +-- e.g. `onClick: microflow …` on a DataGrid2 — passed `mxcli check --references`, +-- passed `mx check`, and built/ran, but the action was silently discarded: +-- DESCRIBE PAGE omitted it and the model round-tripped minus the action, with no +-- error or warning. +-- +-- Root cause: the .def.json GENERATOR never emitted an `action` operation for +-- `type="action"` properties (across 42 widget defs, no action operation existed +-- at all). The writer reads only propertyMappings from the def, so with no action +-- mapping it had nothing to write. The .mpk correctly declares the action slot. +-- +-- After fix: the generator emits an `action` PropertyMapping for the action slots +-- MDL can author (`onClick` → the widget's Action property; filter `onChange` → +-- OnChange). The engine's existing applyOperation "action" → SetAction path then +-- serializes the client action (with its parameter mapping). WidgetDefGeneratorVersion +-- bumped so existing projects regenerate their defs. +-- +-- Verified: `mx check` → 0 errors on Mendix 11.12.1, and the DataGrid2 widget +-- persists the onClick microflow action (with `Thing: $currentObject` mapping). +-- +-- NOTE: requires the project's generated widget defs (`mxcli widget init`), so +-- this is exercised via exec/mx-check, not just syntax check. + +create module L67; + +create entity L67.Thing ( + Name: string(100) +); + +create microflow L67.OnRowClick ( + $Thing: L67.Thing +) +returns Boolean +begin + return true; +end; + +create or replace page L67.Grid ( + Title: 'Grid', + Layout: Atlas_Core.Atlas_Default +) { + pluggablewidget 'com.mendix.widget.web.datagrid.Datagrid' dg ( + DataSource: database from L67.Thing, + onClick: microflow L67.OnRowClick(Thing: $currentObject) + ) { + column c1 (Attribute: Name, Caption: 'Name') + } +} + +grant view on page L67.Grid to L67.User; diff --git a/mdl/backend/modelsdk/domainmodel_alter.go b/mdl/backend/modelsdk/domainmodel_alter.go index ebbdc8b20..1c8795d9a 100644 --- a/mdl/backend/modelsdk/domainmodel_alter.go +++ b/mdl/backend/modelsdk/domainmodel_alter.go @@ -132,6 +132,19 @@ func (b *Backend) UpdateEntity(domainModelID model.ID, entity *domainmodel.Entit ge.SetRaw(raw) } + // When the update removes ALL indexes but the stored entity had some, the fresh + // (empty) Indexes list on ge is "clean" (untouched), so the codec passes the + // raw indexes through unchanged — a dropped indexed attribute would then leave + // an orphaned index pointing at a GUID that no longer exists, which crashes + // `mx check` with an unhandled AggregateException (ledger #39). Touch the list + // (append + remove) to mark it dirty so the codec re-emits it as empty, + // clearing the raw. A non-empty new index list is already dirty (entityToGen + // appended to it), so this is only needed for the all-removed case. + if len(entity.Indexes) == 0 && len(orig.IndexesItems()) > 0 { + ge.AddIndexes(genDm.NewIndex()) + ge.RemoveIndexes(0) + } + // Rebuild the list in place: drop all, re-add in original order swapping the // target. Re-added existing elements stay clean (only the list is dirtied), // so the codec re-emits them byte-faithfully; only ge is built fresh. diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index a8779ecaa..8da808cea 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -651,6 +651,12 @@ func navListItemToGen(item *pages.NavigationListItem) (element.Element, error) { g.SetID(element.ID(item.ID)) } assignID(g) + // The item MUST carry its name, else Studio Pro rejects the project with + // CE7247 "name cannot be empty" (and CE0495 "duplicate name ''" when there is + // more than one item). The gen NavigationListItem type has no typed Name + // setter, so write it as a raw property (like the legacy writer's Name key). + // (ledger finding #24) + addStr(&g.Base, "Name", item.Name) g.SetAppearance(newAppearance("", "", "", nil)) act, err := clientActionToGen(item.Action) if err != nil { @@ -660,7 +666,13 @@ func navListItemToGen(item *pages.NavigationListItem) (element.Element, error) { widgets := item.Widgets if len(widgets) == 0 && item.Caption != nil { - widgets = []pages.Widget{&pages.DynamicText{Content: &pages.ClientTemplate{Template: item.Caption}}} + // The caption becomes a DynamicText — which itself needs a name, or it + // hits the same empty-name errors. Mirror the legacy writer's `text_`. + widgets = []pages.Widget{&pages.DynamicText{ + BaseWidget: pages.BaseWidget{Name: "text_" + item.Name}, + Content: &pages.ClientTemplate{Template: item.Caption}, + RenderMode: pages.TextRenderModeText, + }} } for _, w := range widgets { wg, err := widgetToGen(w) diff --git a/mdl/backend/modelsdk/widget_write_navlist_test.go b/mdl/backend/modelsdk/widget_write_navlist_test.go new file mode 100644 index 000000000..d75e8acb2 --- /dev/null +++ b/mdl/backend/modelsdk/widget_write_navlist_test.go @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "bytes" + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/modelsdk/codec" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// TestNavListItemToGen_WritesNames guards ledger finding #24: the modelsdk +// writer must emit the navigation item's Name and give the caption's generated +// DynamicText a name — otherwise Studio Pro rejects the project with CE7247 +// "name cannot be empty" / CE0495 "duplicate name ''". +func TestNavListItemToGen_WritesNames(t *testing.T) { + item := &pages.NavigationListItem{ + Name: "itemTransactions", + Caption: &model.Text{ + Translations: map[string]string{"en_US": "Transactions"}, + }, + } + el, err := navListItemToGen(item) + if err != nil { + t.Fatalf("navListItemToGen: %v", err) + } + raw, err := (&codec.Encoder{}).Encode(el) + if err != nil { + t.Fatalf("encode: %v", err) + } + if !bytes.Contains(raw, []byte("itemTransactions")) { + t.Error("encoded item is missing its Name (CE7247/CE0495 in Studio Pro)") + } + if !bytes.Contains(raw, []byte("text_itemTransactions")) { + t.Error("caption DynamicText is missing its name (empty-name error in Studio Pro)") + } +} diff --git a/mdl/executor/cmd_entities.go b/mdl/executor/cmd_entities.go index 90141cd10..480640858 100644 --- a/mdl/executor/cmd_entities.go +++ b/mdl/executor/cmd_entities.go @@ -350,6 +350,12 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { fmt.Fprintf(ctx.Output, " To add attributes without disturbing the rest, use: alter entity %s add attribute : ;\n", s.Name) } + // Carry forward (and prune) indexes so a dropped indexed attribute doesn't + // leave an orphaned index that crashes `mx check` (finding #39). + if droppedIdx := reconcileDroppedIndexes(entity, existingEntity); droppedIdx > 0 { + fmt.Fprintf(ctx.Output, + " Dropped %d index(es) that referenced removed attribute(s).\n", droppedIdx) + } // Update existing entity entity.ID = existingEntity.ID if err := ctx.Backend.UpdateEntity(dm.ID, entity); err != nil { @@ -374,6 +380,64 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { return nil } +// reconcileDroppedIndexes fixes ledger finding #39: when CREATE OR MODIFY drops +// an indexed attribute, the entity's index for it is left orphaned (its column +// references a GUID that no longer exists), which crashes `mx check` with an +// unhandled AggregateException. When the statement lists NO index clause of its +// own (so `entity.Indexes` is empty), carry the existing entity's indexes forward +// but prune any column referencing an attribute the new entity no longer has, and +// drop indexes left with no columns. A statement that DOES list indexes replaces +// wholesale (unchanged). Attribute IDs survive by name across CREATE OR MODIFY +// (findings #13), so a surviving attribute keeps the ID its index references. +// Returns the number of indexes dropped, for the drop warning. +func reconcileDroppedIndexes(entity, existing *domainmodel.Entity) int { + if entity == nil || existing == nil || len(entity.Indexes) > 0 { + return 0 + } + valid := make(map[model.ID]bool, len(entity.Attributes)) + for _, a := range entity.Attributes { + valid[a.ID] = true + } + var kept []*domainmodel.Index + dropped := 0 + for _, idx := range existing.Indexes { + var attrs []*domainmodel.IndexAttribute + for _, ia := range idx.Attributes { + if valid[ia.AttributeID] { + attrs = append(attrs, ia) + } + } + var ids []model.ID + for _, id := range idx.AttributeIDs { + if valid[id] { + ids = append(ids, id) + } + } + if len(attrs) > 0 || len(ids) > 0 { + idx.Attributes = attrs + idx.AttributeIDs = ids + kept = append(kept, idx) + } else { + dropped++ + } + } + entity.Indexes = kept + return dropped +} + +// isViewEntity reports whether an entity is an OQL view entity. View entities +// cannot participate in associations (CE6771) and behave differently from +// persistent entities in several checks. The canonical marker is the +// DomainModels$OqlViewEntitySource source type; OqlQuery / SourceDocumentRef are +// belt-and-suspenders fallbacks for read paths that populate one but not the other. +func isViewEntity(e *domainmodel.Entity) bool { + if e == nil { + return false + } + return e.Source == "DomainModels$OqlViewEntitySource" || + e.OqlQuery != "" || e.SourceDocumentRef != "" +} + // droppedEntityMembers reports the members present on existing but absent from // replacement — i.e. what a CREATE OR MODIFY replace would delete. Named // attributes are compared case-insensitively; the four audit system fields are diff --git a/mdl/executor/cmd_entities_viewentity_test.go b/mdl/executor/cmd_entities_viewentity_test.go new file mode 100644 index 000000000..bdea782ab --- /dev/null +++ b/mdl/executor/cmd_entities_viewentity_test.go @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// TestReconcileDroppedIndexes guards ledger finding #39: a CREATE OR MODIFY that +// drops an indexed attribute must not leave an orphaned index (which crashes +// `mx check`). When the statement lists no indexes, existing indexes are carried +// forward with columns for dropped attributes pruned and empty indexes removed. +func TestReconcileDroppedIndexes(t *testing.T) { + idA, idB, idC := model.ID("a"), model.ID("b"), model.ID("c") + existing := func() *domainmodel.Entity { + return &domainmodel.Entity{ + Attributes: []*domainmodel.Attribute{{BaseElement: model.BaseElement{ID: idA}, Name: "A"}, {BaseElement: model.BaseElement{ID: idB}, Name: "B"}, {BaseElement: model.BaseElement{ID: idC}, Name: "C"}}, + Indexes: []*domainmodel.Index{ + {AttributeIDs: []model.ID{idA, idB}}, + {AttributeIDs: []model.ID{idC}}, + }, + } + } + + t.Run("drop C: its single-column index is removed, composite kept+pruned", func(t *testing.T) { + newEnt := &domainmodel.Entity{Attributes: []*domainmodel.Attribute{{BaseElement: model.BaseElement{ID: idA}, Name: "A"}, {BaseElement: model.BaseElement{ID: idB}, Name: "B"}}} + dropped := reconcileDroppedIndexes(newEnt, existing()) + if dropped != 1 { + t.Errorf("dropped = %d, want 1", dropped) + } + if len(newEnt.Indexes) != 1 || len(newEnt.Indexes[0].AttributeIDs) != 2 { + t.Fatalf("expected 1 kept index over [A,B], got %+v", newEnt.Indexes) + } + }) + + t.Run("drop B: composite index pruned to [A]", func(t *testing.T) { + newEnt := &domainmodel.Entity{Attributes: []*domainmodel.Attribute{{BaseElement: model.BaseElement{ID: idA}, Name: "A"}, {BaseElement: model.BaseElement{ID: idC}, Name: "C"}}} + reconcileDroppedIndexes(newEnt, existing()) + // index over [A,B] → [A]; index over [C] kept. + if len(newEnt.Indexes) != 2 { + t.Fatalf("expected 2 indexes, got %d", len(newEnt.Indexes)) + } + if len(newEnt.Indexes[0].AttributeIDs) != 1 || newEnt.Indexes[0].AttributeIDs[0] != idA { + t.Errorf("first index should be pruned to [A], got %v", newEnt.Indexes[0].AttributeIDs) + } + }) + + t.Run("statement provides its own indexes: no carry-forward", func(t *testing.T) { + newEnt := &domainmodel.Entity{ + Attributes: []*domainmodel.Attribute{{BaseElement: model.BaseElement{ID: idA}, Name: "A"}}, + Indexes: []*domainmodel.Index{{AttributeIDs: []model.ID{idA}}}, + } + if dropped := reconcileDroppedIndexes(newEnt, existing()); dropped != 0 { + t.Errorf("dropped = %d, want 0 (statement indexes replace wholesale)", dropped) + } + if len(newEnt.Indexes) != 1 { + t.Errorf("statement index list must be left intact, got %d", len(newEnt.Indexes)) + } + }) +} + +// TestIsViewEntity guards ledger finding #41: view entities cannot participate in +// associations (CE6771), so the checker must recognize one. The canonical marker +// is the OqlViewEntitySource source type; OqlQuery / SourceDocumentRef are +// fallbacks for read paths that populate only one. +func TestIsViewEntity(t *testing.T) { + cases := []struct { + name string + ent *domainmodel.Entity + want bool + }{ + {"nil", nil, false}, + {"plain persistent entity", &domainmodel.Entity{Name: "Customer"}, false}, + {"view via Source", &domainmodel.Entity{Name: "VCat", Source: "DomainModels$OqlViewEntitySource"}, true}, + {"view via OqlQuery", &domainmodel.Entity{Name: "VCat", OqlQuery: "select 1"}, true}, + {"view via SourceDocumentRef", &domainmodel.Entity{Name: "VCat", SourceDocumentRef: "Mod.VCatVe"}, true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := isViewEntity(c.ent); got != c.want { + t.Errorf("isViewEntity = %v, want %v", got, c.want) + } + }) + } +} diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index eb4fbcc11..49ec0188c 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -472,11 +472,24 @@ func (fb *flowBuilder) resolveAssociationPaths(expr ast.Expression) ast.Expressi } case *ast.SourceExpr: if e.Source != "" { - // Non-empty Source is the exact expression text to write back. - // Rebuilding it here would defeat the whitespace-preservation - // purpose of SourceExpr, so keep the parsed tree only for callers - // that need semantic inspection. - return e + // Non-empty Source is the exact expression text to write back (kept + // for decimal/whitespace fidelity, #17-19). But an association + // navigation inside that source still needs its target-entity step + // inserted (`$T/Mod.Assoc/attr` → `$T/Mod.Assoc/Mod.Entity/attr`) or + // Mendix rejects the whole expression with CE0117 — e.g. a decimal + // literal co-occurring with an association nav (`$T/Mod.Assoc/Bal + 2.0`) + // froze the source and skipped resolution (ledger #48). Rewrite each + // association path within the source while preserving everything else. + resolvedInner := fb.resolveAssociationPaths(e.Expression) + fixed := e.Source + for _, orig := range collectAttributePaths(e.Expression) { + o := expressionToString(orig) + r := expressionToString(fb.resolveAssociationPaths(orig)) + if o != r { + fixed = strings.ReplaceAll(fixed, o, r) + } + } + return &ast.SourceExpr{Expression: resolvedInner, Source: fixed} } return fb.resolveAssociationPaths(e.Expression) default: @@ -522,6 +535,38 @@ func (fb *flowBuilder) resolvePathSegments(path []string) []string { return resolved } +// collectAttributePaths returns every AttributePathExpr in an expression tree, +// used to rewrite association navigations inside a source-preserved expression. +func collectAttributePaths(expr ast.Expression) []*ast.AttributePathExpr { + var out []*ast.AttributePathExpr + var walk func(ast.Expression) + walk = func(e ast.Expression) { + switch v := e.(type) { + case *ast.AttributePathExpr: + out = append(out, v) + case *ast.BinaryExpr: + walk(v.Left) + walk(v.Right) + case *ast.UnaryExpr: + walk(v.Operand) + case *ast.ParenExpr: + walk(v.Inner) + case *ast.FunctionCallExpr: + for _, a := range v.Arguments { + walk(a) + } + case *ast.IfThenElseExpr: + walk(v.Condition) + walk(v.ThenExpr) + walk(v.ElseExpr) + case *ast.SourceExpr: + walk(v.Expression) + } + } + walk(expr) + return out +} + // buildSplitCondition constructs the right SplitCondition variant for an IF // statement. When the condition is a qualified call into a rule, it emits a // RuleSplitCondition (nested RuleCall with ParameterMappings). Everything else diff --git a/mdl/executor/cmd_microflows_builder_actions.go b/mdl/executor/cmd_microflows_builder_actions.go index 26d7dd438..a2e5348e0 100644 --- a/mdl/executor/cmd_microflows_builder_actions.go +++ b/mdl/executor/cmd_microflows_builder_actions.go @@ -1066,6 +1066,42 @@ func entityQualifiedNameFromAttribute(attrPath string) string { // addListOperationAction creates list operations like HEAD, TAIL, FIND, etc. func (fb *flowBuilder) addListOperationAction(s *ast.ListOperationStmt) model.ID { + // `contains` and `find` are overloaded: contains(list, object) / find(list, + // condition) are list operations, but contains(haystack, needle) and + // find(haystack, needle) over strings are String functions. When the input is + // a declared String variable, Mendix requires a Change Variable action + // carrying the string expression — a List operation activity on strings fails + // the build (CE0023/CE0097/CE0111). Ledger findings #53 (contains) and #63 (find). + if fb.declaredVars != nil && fb.declaredVars[s.InputVariable] == "String" { + switch s.Operation { + case ast.ListOpContains: + return fb.addChangeVariableAction(&ast.MfSetStmt{ + Target: s.OutputVariable, + Value: &ast.FunctionCallExpr{ + Name: "contains", + Arguments: []ast.Expression{ + &ast.VariableExpr{Name: s.InputVariable}, + &ast.VariableExpr{Name: s.SecondVariable}, + }, + }, + }) + case ast.ListOpFind: + // The string find's second argument is carried as Condition (the + // visitor stores arg1 there); rebuild `find($in, )`. + second := s.Condition + if second == nil { + second = &ast.VariableExpr{Name: s.SecondVariable} + } + return fb.addChangeVariableAction(&ast.MfSetStmt{ + Target: s.OutputVariable, + Value: &ast.FunctionCallExpr{ + Name: "find", + Arguments: []ast.Expression{&ast.VariableExpr{Name: s.InputVariable}, second}, + }, + }) + } + } + var operation microflows.ListOperation switch s.Operation { diff --git a/mdl/executor/cmd_microflows_builder_contains_test.go b/mdl/executor/cmd_microflows_builder_contains_test.go new file mode 100644 index 000000000..f44a711f3 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_contains_test.go @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// containsActionKind builds a flow graph for a single `contains` list-operation +// statement and reports which activity the builder emitted for it. +func containsActionKind(t *testing.T, declaredVars, varTypes map[string]string, s *ast.ListOperationStmt) (isChangeVar, isListOp bool) { + t.Helper() + fb := &flowBuilder{ + posX: 100, + posY: 100, + spacing: HorizontalSpacing, + declaredVars: declaredVars, + varTypes: varTypes, + measurer: &layoutMeasurer{varTypes: varTypes}, + } + oc := fb.buildFlowGraph([]ast.MicroflowStatement{s}, &ast.MicroflowReturnType{Type: ast.DataType{Kind: ast.TypeBoolean}}) + if errs := fb.GetErrors(); len(errs) > 0 { + t.Fatalf("unexpected build errors: %v", errs) + } + for _, obj := range oc.Objects { + act, ok := obj.(*microflows.ActionActivity) + if !ok { + continue + } + switch act.Action.(type) { + case *microflows.ChangeVariableAction: + isChangeVar = true + case *microflows.ListOperationAction: + isListOp = true + } + } + return isChangeVar, isListOp +} + +// TestBuildContains_StringVsList covers ledger finding #53 at the flow-builder +// layer. When both `contains` arguments are plain variables the visitor cannot +// tell a string contains from a list contains, so it emits a ListOperationStmt. +// The builder disambiguates: a String-typed input becomes a Change Variable +// action carrying the `contains(...)` expression (a List operation activity on +// strings fails the Mendix build with CE0023/CE0097/CE0111), while a list-typed +// input stays a List operation activity. +func TestBuildContains_StringVsList(t *testing.T) { + t.Run("string input becomes change variable", func(t *testing.T) { + isChangeVar, isListOp := containsActionKind(t, + map[string]string{"Hay": "String", "Needle": "String", "Match": "Boolean"}, + map[string]string{}, + &ast.ListOperationStmt{ + Operation: ast.ListOpContains, + InputVariable: "Hay", + SecondVariable: "Needle", + OutputVariable: "Match", + }) + if !isChangeVar { + t.Error("string contains must emit a Change Variable action") + } + if isListOp { + t.Error("string contains must NOT emit a List operation activity") + } + }) + + t.Run("list input stays a list operation", func(t *testing.T) { + isChangeVar, isListOp := containsActionKind(t, + map[string]string{}, + map[string]string{"Items": "List of M.Item", "One": "M.Item"}, + &ast.ListOperationStmt{ + Operation: ast.ListOpContains, + InputVariable: "Items", + SecondVariable: "One", + OutputVariable: "Found", + }) + if isChangeVar { + t.Error("list contains must NOT emit a Change Variable action") + } + if !isListOp { + t.Error("list contains must emit a List operation activity") + } + }) + + // Ledger #63: the same String-input disambiguation applies to `find`. + t.Run("string-input find becomes change variable", func(t *testing.T) { + isChangeVar, isListOp := containsActionKind(t, + map[string]string{"Hay": "String", "Needle": "String", "At": "Integer"}, + map[string]string{}, + &ast.ListOperationStmt{ + Operation: ast.ListOpFind, + InputVariable: "Hay", + SecondVariable: "Needle", + OutputVariable: "At", + }) + if !isChangeVar { + t.Error("string find must emit a Change Variable action") + } + if isListOp { + t.Error("string find must NOT emit a List operation activity") + } + }) + + t.Run("list-input find stays a list operation", func(t *testing.T) { + isChangeVar, isListOp := containsActionKind(t, + map[string]string{}, + map[string]string{"Items": "List of M.Item"}, + &ast.ListOperationStmt{ + Operation: ast.ListOpFind, + InputVariable: "Items", + Condition: &ast.BinaryExpr{Left: &ast.AttributePathExpr{Path: []string{"Key"}}, Operator: "=", Right: &ast.VariableExpr{Name: "K"}}, + OutputVariable: "Found", + }) + if isChangeVar { + t.Error("list find must NOT emit a Change Variable action") + } + if !isListOp { + t.Error("list find must emit a List operation activity") + } + }) +} diff --git a/mdl/executor/cmd_microflows_builder_validate.go b/mdl/executor/cmd_microflows_builder_validate.go index a9e96e799..b45fd9b63 100644 --- a/mdl/executor/cmd_microflows_builder_validate.go +++ b/mdl/executor/cmd_microflows_builder_validate.go @@ -315,6 +315,19 @@ func (fb *flowBuilder) validateStatement(stmt ast.MicroflowStatement) { } case *ast.ListOperationStmt: + // String contains(haystack, needle) / find(haystack, needle) are value + // expressions assigned to a pre-declared variable, not list operations that + // create a new variable (ledger #53/#63). Validate like a SET: the target + // must already be declared, and must not be flagged as a duplicate. + if (s.Operation == ast.ListOpContains || s.Operation == ast.ListOpFind) && + fb.declaredVars[s.InputVariable] == "String" { + if !fb.isVariableDeclared(s.OutputVariable) { + fb.addErrorWithExample( + fmt.Sprintf("variable '%s' is not declared", s.OutputVariable), + errorExampleDeclareVariable(s.OutputVariable)) + } + break + } fb.validateOutputVariable(s.OutputVariable, "list operation") if s.OutputVariable != "" { fb.declaredVars[s.OutputVariable] = "Unknown" diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index f24281281..6e5f57e10 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -593,6 +593,7 @@ type rawWidget struct { LabelPosition string // "Left", "Top", etc. Placeholder string // Placeholder hint text (from PlaceholderTemplate) OnChange string // MDL rendering of the OnChangeAction client action + OnClick string // MDL rendering of a pluggable widget's onClick action (e.g. DataGrid2) // Filter widget properties FilterAttributes []string // Attributes to filter on FilterExpression string // Default filter expression (contains, startsWith, etc.) diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 91ddd04cd..a194e5660 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -473,6 +473,10 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if w.Selection != "" { props = append(props, fmt.Sprintf("Selection: %s", w.Selection)) } + // onClick action (ledger #67) + if w.OnClick != "" { + props = append(props, fmt.Sprintf("onClick: %s", w.OnClick)) + } // Add paging properties if non-default props = appendDataGridPagingProps(props, w) props = appendAppearanceProps(props, w) @@ -605,9 +609,9 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { props = appendConditionalProps(props, w) props = appendAppearanceProps(props, w) formatWidgetProps(ctx.Output, prefix, header, props, "\n") - } else if (len(w.ExplicitProperties) > 0 || len(w.ObjectLists) > 0) && w.WidgetID != "" { - // Generic pluggable widget with explicit properties and/or object-list - // child blocks (chart series/lines/scaleColors). + } else if (len(w.ExplicitProperties) > 0 || len(w.ObjectLists) > 0 || w.OnClick != "") && w.WidgetID != "" { + // Generic pluggable widget with explicit properties, object-list child + // blocks (chart series/lines/scaleColors), and/or an onClick action. header := fmt.Sprintf("pluggablewidget '%s' %s", w.WidgetID, mdlIdent(w.Name)) props := []string{} if w.Caption != "" { @@ -616,6 +620,10 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { for _, ep := range w.ExplicitProperties { props = append(props, fmt.Sprintf("%s: %s", ep.Key, ep.Value)) } + // onClick action (ledger #67 — reported on CustomChart) + if w.OnClick != "" { + props = append(props, fmt.Sprintf("onClick: %s", w.OnClick)) + } props = appendAppearanceProps(props, w) if len(w.ObjectLists) == 0 { formatWidgetProps(ctx.Output, prefix, header, props, "\n") diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index c1b9d241f..9fabb10b2 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -318,6 +318,9 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s } widget.DataGridColumns = extractDataGrid2Columns(ctx, w, widget.EntityContext) widget.ControlBar = extractDataGrid2ControlBar(ctx, w) + // onClick action (ledger #67): read the client action back with full + // parameter mappings so a describe round-trip re-emits the onClick. + widget.OnClick = renderClientActionMDL(ctx, customWidgetPropertyActionMap(ctx, w, "onClick")) } // For Gallery, extract datasource, content widgets, filter widgets, and selection mode if widget.RenderMode == "gallery" { @@ -348,6 +351,10 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s if !isKnownCustomWidgetType(widget.RenderMode) { widget.ExplicitProperties = extractExplicitProperties(ctx, w) widget.ObjectLists = extractObjectLists(ctx, w) + // onClick action (ledger #67 — reported on CustomChart): read the client + // action back with full parameter mappings so a describe round-trip + // re-emits it (the finding's original widget goes through this path). + widget.OnClick = renderClientActionMDL(ctx, customWidgetPropertyActionMap(ctx, w, "onClick")) } return []rawWidget{widget} diff --git a/mdl/executor/cmd_pages_describe_pluggable.go b/mdl/executor/cmd_pages_describe_pluggable.go index fc155e110..edc5d3486 100644 --- a/mdl/executor/cmd_pages_describe_pluggable.go +++ b/mdl/executor/cmd_pages_describe_pluggable.go @@ -1251,6 +1251,45 @@ func extractCustomWidgetPropertyTextTemplate(ctx *ExecContext, w map[string]any, return "" } +// customWidgetPropertyActionMap returns the raw Forms$*ClientAction map stored on +// a CustomWidget property (e.g. DataGrid2 `onClick`), or nil when unset or a +// NoAction. Unlike extractCustomWidgetPropertyAction (which formats a one-line +// summary), this returns the action map so callers can render it with full +// parameter mappings via renderClientActionMDL — a faithful describe round-trip. +func customWidgetPropertyActionMap(ctx *ExecContext, w map[string]any, propertyKey string) map[string]any { + obj, ok := w["Object"].(map[string]any) + if !ok { + return nil + } + // DataGrid2/Gallery keep PropertyTypes at the ObjectType level, so use the + // fallback form (same as the widget-slot readers). + propTypeKeyMap := buildPropertyTypeKeyMap(w, true) + for _, prop := range getBsonArrayElements(obj["Properties"]) { + propMap, ok := prop.(map[string]any) + if !ok { + continue + } + if propTypeKeyMap[extractBinaryID(propMap["TypePointer"])] != propertyKey { + continue + } + value, ok := propMap["Value"].(map[string]any) + if !ok { + continue + } + action, ok := value["Action"].(map[string]any) + if !ok || action == nil { + return nil + } + // A default/empty action serializes as NoAction — treat it as unset so the + // describe output stays clean. + if t := extractString(action["$Type"]); t == "Forms$NoAction" || t == "Pages$NoAction" { + return nil + } + return action + } + return nil +} + // extractCustomWidgetPropertyAction extracts an action description from a CustomWidget property. // Returns a formatted string like "CALL_MICROFLOW Module.Flow" or "SHOW_PAGE Module.Page". func extractCustomWidgetPropertyAction(ctx *ExecContext, w map[string]any, propertyKey string) string { diff --git a/mdl/executor/cmd_pages_describe_pluggable_test.go b/mdl/executor/cmd_pages_describe_pluggable_test.go index 7656ca4ea..66574fd85 100644 --- a/mdl/executor/cmd_pages_describe_pluggable_test.go +++ b/mdl/executor/cmd_pages_describe_pluggable_test.go @@ -11,6 +11,7 @@ package executor import ( + "context" "testing" ) @@ -158,3 +159,57 @@ func TestExtractCustomWidgetPropertyAssociation_DoesNotReturnCaptionAttribute(t t.Errorf("extractCustomWidgetPropertyAssociation returned CaptionAttribute value %q; this is the original bug (Issue #21)", got) } } + +// buildDataGridOnClickWidget builds a mock DataGrid2 CustomWidget map with an +// onClick property carrying the given action ($Type). Mirrors the BSON the +// pluggable engine writes (property resolved via TypePointer → PropertyKey). +func buildDataGridOnClickWidget(actionType, microflow string) map[string]any { + const idOnClick = "type-id-onclick" + widgetType := map[string]any{ + "WidgetId": "com.mendix.widget.web.datagrid.Datagrid", + "ObjectType": map[string]any{ + "PropertyTypes": []any{ + map[string]any{"$ID": idOnClick, "PropertyKey": "onClick"}, + }, + }, + } + action := map[string]any{"$Type": actionType} + if microflow != "" { + action["MicroflowSettings"] = map[string]any{ + "$Type": "Forms$MicroflowSettings", + "Microflow": microflow, + } + } + return map[string]any{ + "Type": widgetType, + "Object": map[string]any{ + "Properties": []any{ + map[string]any{"TypePointer": idOnClick, "Value": map[string]any{"Action": action}}, + }, + }, + } +} + +// TestCustomWidgetPropertyActionMap covers the ledger #67 DESCRIBE read gap: a +// pluggable widget's onClick action is read back so a describe round-trip +// re-emits it. A NoAction (the default) reads as unset. +func TestCustomWidgetPropertyActionMap(t *testing.T) { + ex := &Executor{} + ctx := ex.newExecContext(context.Background()) + + // Microflow action → returned, with the microflow reachable for rendering. + w := buildDataGridOnClickWidget("Forms$MicroflowClientAction", "L67.OnRowClick") + got := customWidgetPropertyActionMap(ctx, w, "onClick") + if got == nil { + t.Fatal("onClick microflow action not read back — DESCRIBE would drop it (#67)") + } + if extractString(got["$Type"]) != "Forms$MicroflowClientAction" { + t.Errorf("action $Type = %q, want Forms$MicroflowClientAction", extractString(got["$Type"])) + } + + // A default NoAction reads as unset (nil) so DESCRIBE stays clean. + wNo := buildDataGridOnClickWidget("Forms$NoAction", "") + if customWidgetPropertyActionMap(ctx, wNo, "onClick") != nil { + t.Error("NoAction should read as unset (nil)") + } +} diff --git a/mdl/executor/oql_type_inference.go b/mdl/executor/oql_type_inference.go index 92183406e..07e353250 100644 --- a/mdl/executor/oql_type_inference.go +++ b/mdl/executor/oql_type_inference.go @@ -330,6 +330,20 @@ func inferCaseType(expr string) ast.DataType { return ast.DataType{Kind: ast.TypeUnknown} } +// passthroughStringLengthMismatch reports whether a PASS-THROUGH view-entity +// column (a bare source-attribute reference, so sourceAttr is set) declares a +// string length different from the source attribute's inherited length. Mendix's +// view-entity sync requires an exact match — a mismatch (including declaring an +// unbounded `string`, Length 0, against a `string(100)` source) fails the build +// with CE6770. This is stricter than typesCompatible's >= truncation guard, so +// pass-through strings are checked separately. sourceAttr is empty for aggregates +// and derived expressions, which this correctly ignores. (ledger finding #36) +func passthroughStringLengthMismatch(declared, inferred ast.DataType, sourceAttr string) bool { + return sourceAttr != "" && + declared.Kind == ast.TypeString && inferred.Kind == ast.TypeString && + declared.Length != inferred.Length +} + // validateViewEntityTypes validates that declared attribute types match inferred OQL types. func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) []string { var errors []string @@ -361,6 +375,28 @@ func validateViewEntityTypes(ctx *ExecContext, stmt *ast.CreateViewEntityStmt) [ continue } + // A PASS-THROUGH string column (a bare source-attribute reference, e.g. + // `c.Name`) inherits the source attribute's length verbatim, and Mendix's + // view-entity sync requires the declared length to match it EXACTLY — + // declaring `string` (unbounded) or a different `string(N)` against a + // `string(100)` source fails the build with CE6770 "View Entity out of sync". + // This is stricter than typesCompatible's >= rule (which only guards + // truncation), so pass-through strings are validated here. col.SourceAttr is + // set only for direct attribute references, never for aggregates/derived + // expressions. (ledger finding #36) + if passthroughStringLengthMismatch(attr.Type, col.InferredType, col.SourceAttr) { + errors = append(errors, fmt.Sprintf( + "attribute '%s': declared as %s but pass-through column '%s' inherits length %d from source attribute %s.%s — Mendix requires an exact length match (CE6770 \"View Entity out of sync\"). Fix: change to '%s: %s'", + attr.Name, + formatDataTypeForError(attr.Type), + col.Expression, + col.InferredType.Length, + col.SourceEntity, col.SourceAttr, + attr.Name, + formatDataTypeForMDL(col.InferredType))) + continue + } + // Compare types if !typesCompatible(attr.Type, col.InferredType) { errors = append(errors, fmt.Sprintf( diff --git a/mdl/executor/oql_type_inference_test.go b/mdl/executor/oql_type_inference_test.go index 9918ba833..eb0491ca9 100644 --- a/mdl/executor/oql_type_inference_test.go +++ b/mdl/executor/oql_type_inference_test.go @@ -14,6 +14,34 @@ import ( // returned "" for every real query. That made `check --references` both // false-positive ("could not parse select clause from OQL query") and silently // skip all OQL type validation. Bug 9b. +// TestPassthroughStringLengthMismatch guards ledger finding #36: a pass-through +// view-entity string column must declare the SAME length as its source +// attribute, or Mendix fails the build with CE6770 "View Entity out of sync". +func TestPassthroughStringLengthMismatch(t *testing.T) { + str := func(n int) ast.DataType { return ast.DataType{Kind: ast.TypeString, Length: n} } + cases := []struct { + name string + declared ast.DataType + inferred ast.DataType + sourceAttr string + want bool + }{ + {"unbounded vs string(100) source", str(0), str(100), "Name", true}, + {"string(50) vs string(100) source", str(50), str(100), "Name", true}, + {"string(200) vs string(100) source", str(200), str(100), "Name", true}, + {"exact match", str(100), str(100), "Name", false}, + {"not a pass-through (aggregate/derived)", str(0), str(200), "", false}, + {"non-string pass-through ignored", ast.DataType{Kind: ast.TypeInteger}, ast.DataType{Kind: ast.TypeInteger}, "Age", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := passthroughStringLengthMismatch(c.declared, c.inferred, c.sourceAttr); got != c.want { + t.Errorf("passthroughStringLengthMismatch = %v, want %v", got, c.want) + } + }) + } +} + func TestExtractSelectClause(t *testing.T) { cases := []struct { name string diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 484ca431b..999a2b363 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -329,6 +329,21 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFoundMsg("module", s.Child.Module, "child entity module not found: "+s.Child.Module) } } + // Mendix forbids associations to OR from a view entity (CE6771) — statically + // impossible, so catch it here instead of letting it reach mxbuild. Both + // endpoints are checked (either direction is rejected). Endpoints created in + // the same script are skipped (a view entity created here is validated on its + // own statement). (ledger finding #41) + for _, ep := range []ast.QualifiedName{s.Parent, s.Child} { + if ep.Module == "" || sc.entities[ep.String()] { + continue + } + if ent, err := findEntity(ctx, ep.Module, ep.Name); err == nil && isViewEntity(ent) { + return mdlerrors.NewValidationf( + "cannot create association %s: %s is a view entity — Mendix does not allow associations to or from view entities (CE6771). Use a non-persistent entity with a real reference to the target instead.", + s.Name.String(), ep.String()) + } + } case *ast.CreateImageCollectionStmt: if s.Name.Module != "" && !sc.modules[s.Name.Module] { if _, err := findModule(ctx, s.Name.Module); err != nil { diff --git a/mdl/executor/validate_microflow.go b/mdl/executor/validate_microflow.go index 5dcbee53e..a4a52b1f6 100644 --- a/mdl/executor/validate_microflow.go +++ b/mdl/executor/validate_microflow.go @@ -4,6 +4,7 @@ package executor import ( "fmt" + "regexp" "strings" "github.com/mendixlabs/mxcli/mdl/ast" @@ -83,6 +84,54 @@ func (v *microflowValidator) validate(body []ast.MicroflowStatement) { // Check 3: variable scope — detect variables declared inside branches but used after v.checkBranchScoping(body) + + // Duplicate loop iterator names — a Mendix loop variable is scoped to the whole + // microflow, so reusing a name across loops is CE0111 at build time. + v.checkDuplicateLoopVariables(body) +} + +// checkDuplicateLoopVariables flags a loop iterator name used by more than one +// loop in the same microflow. A Mendix loop variable is scoped to the WHOLE +// microflow (not to its loop), so two `loop $R in …` — even sequential ones, or a +// nested loop reusing an outer name — build as CE0111 "Duplicate variable name". +// (ledger finding #64). Fix for the user: give each loop a distinct iterator. +func (v *microflowValidator) checkDuplicateLoopVariables(body []ast.MicroflowStatement) { + seen := map[string]bool{} + var walk func([]ast.MicroflowStatement) + walk = func(stmts []ast.MicroflowStatement) { + for _, s := range stmts { + switch st := s.(type) { + case *ast.LoopStmt: + if name := st.LoopVariable; name != "" { + if seen[name] { + v.addViolation("MDL052", linter.SeverityError, + fmt.Sprintf("loop iterator '$%s' is reused by another loop in this microflow; "+ + "a Mendix loop variable is scoped to the whole microflow, so this builds as "+ + "CE0111 \"Duplicate variable name\"", name), + fmt.Sprintf("Give each loop a distinct iterator name (e.g. rename one '$%s' to '$%s2')", name, name)) + } + seen[name] = true + } + walk(st.Body) + case *ast.IfStmt: + walk(st.ThenBody) + walk(st.ElseBody) + case *ast.WhileStmt: + walk(st.Body) + case *ast.EnumSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + case *ast.InheritanceSplitStmt: + for _, c := range st.Cases { + walk(c.Body) + } + walk(st.ElseBody) + } + } + } + walk(body) } // walkBody recursively walks microflow body statements looking for per-statement issues. @@ -99,8 +148,12 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { case *ast.ReturnStmt: v.checkReturn(stmt) v.checkExprFunctions("return", stmt.Value) + v.checkDivisionSlash("return", stmt.Value) + v.checkDateTimeLiterals("return", stmt.Value) case *ast.IfStmt: v.checkExprFunctions("if condition", stmt.Condition) + v.checkDivisionSlash("if condition", stmt.Condition) + v.checkDateTimeLiterals("if condition", stmt.Condition) v.walkBody(stmt.ThenBody) v.walkBody(stmt.ElseBody) case *ast.EnumSplitStmt: @@ -187,6 +240,8 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { } } v.checkExprFunctions(fmt.Sprintf("declare '$%s'", stmt.Variable), stmt.InitialValue) + v.checkDivisionSlash(fmt.Sprintf("declare '$%s'", stmt.Variable), stmt.InitialValue) + v.checkDateTimeLiterals(fmt.Sprintf("declare '$%s'", stmt.Variable), stmt.InitialValue) case *ast.MfSetStmt: // SET on a plain variable target (not $var/Member = …, which is a // member change). Flag a Decimal value assigned to an Integer/Long var. @@ -196,9 +251,20 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { } } v.checkExprFunctions(fmt.Sprintf("set '%s'", stmt.Target), stmt.Value) + v.checkDivisionSlash(fmt.Sprintf("set '%s'", stmt.Target), stmt.Value) + v.checkDateTimeLiterals(fmt.Sprintf("set '%s'", stmt.Target), stmt.Value) case *ast.RetrieveStmt: // RETRIEVE populates a list variable — remove from empty tracking delete(v.emptyListVars, stmt.Variable) + if stmt.Where != nil { + xp := expressionToXPath(stmt.Where) + v.checkXPathAssociationEmpty(stmt.Variable, xp) + v.checkXPathIdConstraint(stmt.Variable, xp) + } + case *ast.CallMicroflowStmt: + v.checkAssociationObjectArgs("microflow "+stmt.MicroflowName.String(), stmt.Arguments) + case *ast.CallNanoflowStmt: + v.checkAssociationObjectArgs("nanoflow "+stmt.NanoflowName.String(), stmt.Arguments) case *ast.LoopStmt: // Check: @caption on a loop is silently dropped — Mendix for-loops // have no caption (Microflows$LoopedActivity has no Caption @@ -211,12 +277,19 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { "Use @annotation to attach a note to the loop instead.", "Replace @caption with @annotation to label the loop") } - // Check: nested loop anti-pattern + // Check: nested loop anti-pattern. This is a heuristic — a nested loop is + // only wasteful when the inner loop is a key LOOKUP (find one matching + // item). Intentional aggregation that must visit every element (group × + // category × month totals) is O(N*M) by nature and correct as written, so + // the message flags the lookup case without asserting the loop is wrong. if v.loopDepth > 0 { v.addViolation("MDL001", linter.SeverityWarning, - "nested loop detected (loop inside a loop). "+ - "Use FIND($List, ) for in-memory list matching instead of an inner loop (O(N) vs O(N^2)).", - "Replace the inner loop with $Match = FIND($List, key = $item/key) (a plain retrieve ... where cannot filter a list variable)") + "nested loop detected (loop inside a loop). If the inner loop is a "+ + "key LOOKUP (finding one matching item), replace it with "+ + "FIND($List, ) for an in-memory match (O(N) vs O(N^2)). "+ + "If it is intentional aggregation that must visit every element "+ + "(e.g. totals over group x category x month), this is correct — ignore this hint.", + "For a lookup: $Match = FIND($List, key = $item/key). For genuine aggregation over all elements, no change is needed (a plain retrieve ... where cannot filter a list variable).") } // Check: loop over empty declared list if v.emptyListVars[stmt.ListVariable] { @@ -226,6 +299,21 @@ func (v *microflowValidator) walkBody(body []ast.MicroflowStatement) { stmt.ListVariable), "Pass the list as a microflow parameter instead of creating an empty variable") } + // Check: a `break` nested inside a conditional within a loop currently + // serializes a dangling sequence-flow reference, producing an UNLOADABLE + // .mpr — `mx check` crashes with an unhandled AggregateException + // ("key … not present in the dictionary") rather than an error. A break + // that is a direct child of the loop body serializes fine, but the useful + // form (`if then break`) is the broken one. Reject it with the + // guard-variable workaround until the flow serialization is fixed. (#52) + if loopBodyHasConditionalBreak(stmt.Body) { + v.addViolation("MDL051", linter.SeverityError, + "a `break` inside an if/case within a loop currently produces an unloadable model — "+ + "`mx check` crashes with an unhandled exception (a dangling sequence-flow reference), "+ + "not a normal error. (A break placed directly in the loop body serializes fine.)", + "Until the serialization is fixed, use a guard variable: "+ + "`declare $Done Boolean = false;` then `loop … if not($Done) then … set $Done = true; end if; end loop`.") + } v.loopDepth++ v.walkBody(stmt.Body) v.loopDepth-- @@ -310,6 +398,344 @@ func (v *microflowValidator) checkExprFunctions(label string, expr ast.Expressio } } +// checkDivisionSlash flags `/` used as an arithmetic division operator, which +// Mendix rejects with CE0117 — in a Mendix expression `/` is only the +// member/association separator (`$obj/Attr`); integer/decimal division is `div`. +// `$Dec / 2` parses to a BinaryExpr whose operator is literally `/`; the walk +// finds it wherever it appears (nested in functions, if-then-else, etc.). +// (The `$a / $b` form degrades to a member-access path and is caught by +// `check --references` as an unresolvable attribute, so it is not re-flagged here.) +// (ledger finding #17) +func (v *microflowValidator) checkDivisionSlash(label string, expr ast.Expression) { + // Two forms of `/`-as-division: a `BinaryExpr` with operator `/` (right operand + // is a literal or parenthesized expr, e.g. `$Dec / 2`), and the variable/ + // variable form (`$Dec / $Dec2`) which parses as a member-access path — the + // visitor preserves its raw source precisely because the `$` on the right + // marks it as division, not navigation. The `$a / $b` form is detected from + // the preserved source, so it is caught even when EMBEDDED in a larger + // expression (`$a / $b + 1`, `round($a / $b)`) where the division degrades to + // a member-path AttributePathExpr nested under a BinaryExpr/FunctionCallExpr. + if exprHasSlashDivision(expr) || exprHasSlashDollarDivision(expr) { + v.addViolation("MDL045", linter.SeverityError, + fmt.Sprintf("%s uses '/' as a division operator, which Mendix rejects "+ + "(CE0117 \"Error(s) in expression\") — '/' navigates associations, it does not divide", label), + "Use 'div' for division: `$a div $b` (integer/decimal division is always Decimal — wrap in round()/trunc() for an integer result).") + } +} + +// exprHasSlashDollarDivision reports the `$a / $b` division-misuse form: a +// source-preserved expression whose raw source uses `/` (optionally spaced) +// immediately before a `$` variable, OUTSIDE any string literal. A real +// member/association path never writes `/$` — a path segment is a bare or +// qualified name — so this is an unambiguous division misuse. Scanning the +// preserved source (rather than requiring the SourceExpr to wrap an +// AttributePathExpr directly) catches the division even when it is nested in a +// larger expression: `$a / $b + 1`, `round($a / $b)`, `$a / $b * 100`. The +// string-literal-aware scan avoids a false positive on a `/$` inside a quoted +// literal (e.g. `'path/$x'`). +func exprHasSlashDollarDivision(expr ast.Expression) bool { + switch e := expr.(type) { + case *ast.SourceExpr: + if sourceHasSlashDollarDivision(e.Source) { + return true + } + return exprHasSlashDollarDivision(e.Expression) + case *ast.BinaryExpr: + return exprHasSlashDollarDivision(e.Left) || exprHasSlashDollarDivision(e.Right) + case *ast.UnaryExpr: + return exprHasSlashDollarDivision(e.Operand) + case *ast.ParenExpr: + return exprHasSlashDollarDivision(e.Inner) + case *ast.FunctionCallExpr: + for _, arg := range e.Arguments { + if exprHasSlashDollarDivision(arg) { + return true + } + } + case *ast.IfThenElseExpr: + return exprHasSlashDollarDivision(e.Condition) || + exprHasSlashDollarDivision(e.ThenExpr) || + exprHasSlashDollarDivision(e.ElseExpr) + } + return false +} + +// sourceHasSlashDollarDivision scans a raw expression source for a `/` used as +// division with a variable divisor (`… / $x …`) outside any single-quoted +// string literal. Doubled ” inside a string is a Mendix-escaped quote. +func sourceHasSlashDollarDivision(source string) bool { + inStr := false + for i := 0; i < len(source); i++ { + c := source[i] + if c == '\'' { + if inStr && i+1 < len(source) && source[i+1] == '\'' { + i++ // skip the escaped quote pair + continue + } + inStr = !inStr + continue + } + if inStr || c != '/' { + continue + } + j := i + 1 + for j < len(source) && (source[j] == ' ' || source[j] == '\t') { + j++ + } + if j < len(source) && source[j] == '$' { + return true + } + } + return false +} + +// exprHasSlashDivision reports whether the expression tree contains a BinaryExpr +// whose operator is a literal `/` (an arithmetic-division misuse). +func exprHasSlashDivision(expr ast.Expression) bool { + switch e := expr.(type) { + case *ast.BinaryExpr: + if strings.TrimSpace(e.Operator) == "/" { + return true + } + return exprHasSlashDivision(e.Left) || exprHasSlashDivision(e.Right) + case *ast.UnaryExpr: + return exprHasSlashDivision(e.Operand) + case *ast.ParenExpr: + return exprHasSlashDivision(e.Inner) + case *ast.FunctionCallExpr: + for _, arg := range e.Arguments { + if exprHasSlashDivision(arg) { + return true + } + } + case *ast.IfThenElseExpr: + return exprHasSlashDivision(e.Condition) || exprHasSlashDivision(e.ThenExpr) || exprHasSlashDivision(e.ElseExpr) + case *ast.SourceExpr: + return exprHasSlashDivision(e.Expression) + } + return false +} + +// xpathAssocEmptyRe matches a module-qualified association compared directly to +// `empty` in an XPath constraint (`Ledger.Transaction_Category = empty`). The +// leading boundary class excludes a `/` (so an attribute-over-association path +// like `Assoc/Ledger.Category = empty` is NOT matched — that is a valid +// attribute nullability test) and a `.`/word char (so it captures the whole +// qualified name, not the tail of a 3-part enum literal). +var xpathAssocEmptyRe = regexp.MustCompile(`(^|[^\w./])([A-Za-z_]\w*\.[A-Za-z_]\w*)\s*=\s*empty\b`) + +// xpathAssociationEmptyMatches returns the module-qualified association names an +// XPath constraint compares directly to `empty` (`Ledger.Transaction_Category = +// empty`). Shared by the microflow-retrieve check (MDL047) and the page/widget +// datasource check. Empty result → nothing to flag. +func xpathAssociationEmptyMatches(xpath string) []string { + var out []string + for _, m := range xpathAssocEmptyRe.FindAllStringSubmatch(xpath, -1) { + out = append(out, m[2]) + } + return out +} + +// checkXPathAssociationEmpty flags `[Module.Association = empty]` in a retrieve +// constraint. Mendix XPath has no `= empty` test for an association — it fails +// the build with CE0161; the nullability test is `not(Module.Association/Module.Target)`. +// A bare attribute (`Name = empty`) is valid and is not module-qualified, so it +// never matches. (ledger finding #25) +func (v *microflowValidator) checkXPathAssociationEmpty(variable, xpath string) { + for _, assoc := range xpathAssociationEmptyMatches(xpath) { + v.addViolation("MDL047", linter.SeverityError, + fmt.Sprintf("retrieve '$%s' constraint tests association `%s = empty`, which Mendix XPath does not support "+ + "(CE0161 \"Error(s) in XPath constraint\") — `= empty` works on attributes, not associations", variable, assoc), + fmt.Sprintf("Test for the absence of the associated object with negation: `[not(%s/)]`.", assoc)) + } +} + +// xpathIdConstraintRe matches a constraint comparing the object id against a VALUE +// (`id = $strVar`, `id = '123'`, `id != 5`). It captures the right-hand operand. +// Comparing `id` against an OBJECT variable (`[id != $ExistingOrder]` — the valid +// "exclude self" pattern) is intentionally NOT matched here: the operand filter in +// checkXPathIdConstraint requires a primitive value, and an object variable is not +// one. `id` is a Mendix reserved member, so a bare `id` in identifier position is +// always the system id. +var xpathIdConstraintRe = regexp.MustCompile(`(?:^|[^\w./])(?i:id)\s*(?:=|!=|<|>)\s*(\$\w+|'[^']*'|-?\d+)`) + +// checkXPathIdConstraint flags a retrieve that constrains the object id against a +// stored id VALUE (`where [id = $Id]` with `$Id` a String/Long, or a literal). +// Mendix XPath has no id operator reachable from a microflow expression, so this +// fails the build with CE0161. Comparing `id` to an OBJECT variable is a valid +// identity test and is left alone (its operand is not a primitive). (ledger #42) +func (v *microflowValidator) checkXPathIdConstraint(variable, xpath string) { + for _, m := range xpathIdConstraintRe.FindAllStringSubmatch(xpath, -1) { + operand := m[1] + if strings.HasPrefix(operand, "$") { + // A $-variable operand: flag only when it is a primitive VALUE (a stored + // id — String/Long/Integer). An object variable is not in varKinds, so an + // identity comparison (`id != $obj`) is correctly not flagged. + if _, isPrimitive := v.varKinds[operand[1:]]; !isPrimitive { + continue + } + } + v.addViolation("MDL048", linter.SeverityError, + fmt.Sprintf("retrieve '$%s' constrains the object id against a value (`[id = %s]`), which Mendix XPath does not support "+ + "(CE0161 \"Error(s) in XPath constraint\") — there is no id operator reachable from a microflow expression", variable, operand), + "Retrieve by GUID with a marketplace action (NanoflowCommons GetObjectByGuid / CommunityCommons), "+ + "or expose the id as a String on a view entity (`cast(id as string) as ObjectId`) and constrain on that String column. "+ + "(Comparing id against an OBJECT variable — `[id != $obj]` — IS valid and is not flagged.)") + return + } +} + +// checkAssociationObjectArgs flags a call argument bound to an association-object +// path (`$obj/Module.Assoc`, which yields the associated OBJECT). Mendix rejects +// an association path used as a value with CE0117 — it must be materialized first +// (`retrieve $x from $obj/Module.Assoc;`). An attribute value over an association +// (`$obj/Module.Assoc/Attr`) is a legal value and is NOT flagged. (ledger #43/#44) +func (v *microflowValidator) checkAssociationObjectArgs(callee string, args []ast.CallArgument) { + for _, a := range args { + if exprIsAssociationObjectPath(a.Value) { + v.addViolation("MDL049", linter.SeverityError, + fmt.Sprintf("call %s: argument '%s' passes an association path (an object reached over an association), "+ + "which Mendix rejects as a value (CE0117 \"Error(s) in expression\")", callee, a.Name), + fmt.Sprintf("Materialize the object first, then pass the variable: `retrieve $%s from %s;` then `%s = $%s`.", + a.Name, microflowExprSource(a.Value), a.Name, a.Name)) + } + } +} + +// loopBodyHasConditionalBreak reports whether a `break` appears inside a +// conditional (if / case / inheritance split) directly within this loop body — the +// pattern that serializes a dangling reference and crashes `mx check` (#52). A +// break that is a *direct* statement of the loop body is not flagged (it +// serializes fine). Nested loops are not descended into: a break there belongs to +// that loop and is validated when it is walked. +func loopBodyHasConditionalBreak(stmts []ast.MicroflowStatement) bool { + for _, s := range stmts { + switch n := s.(type) { + case *ast.IfStmt: + if stmtsContainBreak(n.ThenBody) || stmtsContainBreak(n.ElseBody) { + return true + } + case *ast.EnumSplitStmt: + for _, c := range n.Cases { + if stmtsContainBreak(c.Body) { + return true + } + } + if stmtsContainBreak(n.ElseBody) { + return true + } + case *ast.InheritanceSplitStmt: + for _, c := range n.Cases { + if stmtsContainBreak(c.Body) { + return true + } + } + if stmtsContainBreak(n.ElseBody) { + return true + } + } + } + return false +} + +// stmtsContainBreak reports whether a `break` belonging to the enclosing loop +// appears anywhere in these statements. Descends into conditionals but NOT into +// nested loops (a nested loop traps its own break). +func stmtsContainBreak(stmts []ast.MicroflowStatement) bool { + for _, s := range stmts { + switch n := s.(type) { + case *ast.BreakStmt: + return true + case *ast.IfStmt: + if stmtsContainBreak(n.ThenBody) || stmtsContainBreak(n.ElseBody) { + return true + } + case *ast.EnumSplitStmt: + for _, c := range n.Cases { + if stmtsContainBreak(c.Body) { + return true + } + } + if stmtsContainBreak(n.ElseBody) { + return true + } + case *ast.InheritanceSplitStmt: + for _, c := range n.Cases { + if stmtsContainBreak(c.Body) { + return true + } + } + if stmtsContainBreak(n.ElseBody) { + return true + } + } + } + return false +} + +// exprIsAssociationObjectPath reports whether an expression is an attribute path +// whose FINAL segment is a module-qualified association (`$obj/Module.Assoc`) — +// i.e. it resolves to an associated OBJECT, not an attribute value. A final bare +// segment (`$obj/Module.Assoc/Attr` → `Attr`) is an attribute and returns false. +func exprIsAssociationObjectPath(expr ast.Expression) bool { + if se, ok := expr.(*ast.SourceExpr); ok { + expr = se.Expression + } + ap, ok := expr.(*ast.AttributePathExpr) + if !ok || len(ap.Path) == 0 { + return false + } + return strings.Contains(ap.Path[len(ap.Path)-1], ".") +} + +// dateTimeLiteralFuncs are the date-construction functions whose arguments +// Mendix requires to be literal numeric constants — a variable or computed +// argument fails the build with CE0117. +var dateTimeLiteralFuncs = map[string]bool{"datetime": true, "datetimeutc": true} + +// checkDateTimeLiterals flags a dateTime()/dateTimeUTC() call with a non-literal +// argument. Mendix builds these from hardcoded numeric constants only; a +// variable or expression (`dateTime(2026, $Month, $Day)`) is CE0117. (ledger #21) +func (v *microflowValidator) checkDateTimeLiterals(label string, expr ast.Expression) { + if exprHasNonLiteralDateTime(expr) { + v.addViolation("MDL046", linter.SeverityError, + fmt.Sprintf("%s calls dateTime()/dateTimeUTC() with a non-literal argument, which Mendix rejects "+ + "(CE0117 \"Error(s) in expression\") — these functions accept only hardcoded numeric constants", label), + "Step off a literal anchor date instead: `addDays(addMonths(dateTime(2026,1,1), $Month - 1), $Day - 1)` (addDays/addMonths take variables).") + } +} + +// exprHasNonLiteralDateTime reports whether the tree contains a dateTime/ +// dateTimeUTC call any of whose arguments is not a plain numeric literal. +func exprHasNonLiteralDateTime(expr ast.Expression) bool { + switch e := expr.(type) { + case *ast.FunctionCallExpr: + if dateTimeLiteralFuncs[strings.ToLower(e.Name)] { + for _, arg := range e.Arguments { + if _, ok := arg.(*ast.LiteralExpr); !ok { + return true + } + } + } + for _, arg := range e.Arguments { + if exprHasNonLiteralDateTime(arg) { + return true + } + } + case *ast.BinaryExpr: + return exprHasNonLiteralDateTime(e.Left) || exprHasNonLiteralDateTime(e.Right) + case *ast.UnaryExpr: + return exprHasNonLiteralDateTime(e.Operand) + case *ast.ParenExpr: + return exprHasNonLiteralDateTime(e.Inner) + case *ast.IfThenElseExpr: + return exprHasNonLiteralDateTime(e.Condition) || exprHasNonLiteralDateTime(e.ThenExpr) || exprHasNonLiteralDateTime(e.ElseExpr) + case *ast.SourceExpr: + return exprHasNonLiteralDateTime(e.Expression) + } + return false +} + // microflowExprSource returns the Mendix source text of a microflow value // expression: the preserved raw source when available, otherwise the structured // expression rendered back to a string. Returns "" when nothing is available. diff --git a/mdl/executor/validate_microflow_div_test.go b/mdl/executor/validate_microflow_div_test.go index 5226bc05a..adda920e5 100644 --- a/mdl/executor/validate_microflow_div_test.go +++ b/mdl/executor/validate_microflow_div_test.go @@ -55,6 +55,60 @@ func TestValidateMicroflow_DivIntoInteger(t *testing.T) { } } +// TestValidateMicroflow_SlashDivision covers MDL045 (ledger finding #17): `/` +// used as an arithmetic division operator is CE0117 in Mendix — `/` navigates +// associations, division is `div`. The `$a / literal` and `(...) / $x` forms +// parse to a BinaryExpr with operator `/` and must be flagged; a legitimate +// member/association path (`$obj/Attr`) and correct `div` must not be. +func TestValidateMicroflow_SlashDivision(t *testing.T) { + cases := []struct { + name string + params string + body string + wantMDL bool + }{ + {"slash divide by literal", "$Dec: Decimal", "set $R = $Dec / 2;", true}, + {"slash divide by variable", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec / $D2;", true}, + {"slash divide parenthesized", "$Dec: Decimal, $D2: Decimal", "set $R = ($Dec + 1) / $D2;", true}, + {"slash divide by variable spaced", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec/$D2;", true}, + {"slash inside function arg", "$Dec: Decimal", "set $R = round($Dec / 3);", true}, + {"slash in return", "$Dec: Decimal", "return $Dec / 4;", true}, + // Division-by-variable EMBEDDED in a larger expression (ledger #17 round 2): + // `$a / $b` degrades to a member-path AttributePathExpr nested under a + // BinaryExpr/FunctionCallExpr, so the structural `/`-BinaryExpr walk misses + // it; the source scan catches it. + {"embedded div then add", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec / $D2 + 1;", true}, + {"embedded add then div", "$Dec: Decimal, $D2: Decimal", "set $R = 1 + $Dec / $D2;", true}, + {"embedded div in function", "$Dec: Decimal, $D2: Decimal", "set $R = round($Dec / $D2);", true}, + {"embedded div then mul", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec / $D2 * 100;", true}, + {"embedded div in return", "$Dec: Decimal, $D2: Decimal", "return $Dec / $D2 + 1;", true}, + {"div is fine", "$Dec: Decimal, $D2: Decimal", "set $R = $Dec div $D2;", false}, + {"member path is fine", "$O: M.Order", "set $R = $O/M.Order_Cust/Name;", false}, + {"spaced member path is fine", "$O: M.Order", "set $R = $O / M.Order_Cust / Name;", false}, + // A `/$` sequence inside a string literal is NOT a division misuse. + {"slash-dollar inside string literal is fine", "$x: String", "set $R = 'path/$var here';", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F (" + tc.params + ")\nreturns String\nbegin\n " + tc.body + "\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL045" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL045 fired=%v, want %v (body: %q)", got, tc.wantMDL, tc.body) + } + }) + } +} + // TestValidateMicroflow_DivMessage checks the diagnostic names the target and // the div cause, and suggests the fix. func TestValidateMicroflow_DivMessage(t *testing.T) { diff --git a/mdl/executor/validate_microflow_hints_test.go b/mdl/executor/validate_microflow_hints_test.go new file mode 100644 index 000000000..6640c0725 --- /dev/null +++ b/mdl/executor/validate_microflow_hints_test.go @@ -0,0 +1,260 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// TestValidateMicroflow_DateTimeLiterals covers MDL046 (ledger finding #21): +// dateTime()/dateTimeUTC() accept only literal numeric constants — a variable or +// computed argument fails the build with CE0117. +func TestValidateMicroflow_DateTimeLiterals(t *testing.T) { + cases := []struct { + name string + params string + body string + wantMDL bool + }{ + {"variable arg", "$Month: Integer, $Day: Integer", "set $C = dateTime(2026, $Month, $Day);", true}, + {"computed arg", "$Y: Integer", "set $C = dateTime($Y + 1, 1, 1);", true}, + {"utc variable arg", "$Month: Integer", "set $C = dateTimeUTC(2026, $Month, 1);", true}, + {"all literals", "", "set $C = dateTime(2026, 1, 1);", false}, + {"literals with time", "", "set $C = dateTime(2026, 12, 31, 23, 59, 59);", false}, + {"no datetime call", "$X: Integer", "set $C = $X + 1;", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F (" + tc.params + ")\nreturns DateTime\nbegin\n " + tc.body + "\n return $C;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL046" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL046 fired=%v, want %v (body: %q)", got, tc.wantMDL, tc.body) + } + }) + } +} + +// TestValidateMicroflow_XPathAssociationEmpty covers MDL047 (ledger finding #25): +// `[Module.Association = empty]` is CE0161 — XPath has no `= empty` for +// associations; the nullability test is `not(Assoc/Target)`. A bare attribute +// (`Name = empty`) and an attribute-over-association (`Assoc/Attr = empty`) are +// valid and must not be flagged. +func TestValidateMicroflow_XPathAssociationEmpty(t *testing.T) { + cases := []struct { + name string + where string + wantMDL bool + }{ + {"association = empty", "[Ledger.Transaction_Category = empty]", true}, + {"negation form is fine", "[not(Ledger.Transaction_Category/Ledger.Category)]", false}, + {"bare attribute = empty is fine", "[Description = empty]", false}, + {"attribute over association is fine", "[Ledger.Transaction_Category/Ledger.Name = empty]", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ()\nreturns list of Ledger.Transaction\nbegin\n retrieve $t from Ledger.Transaction where " + tc.where + ";\n return $t;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL047" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL047 fired=%v, want %v (where: %q)", got, tc.wantMDL, tc.where) + } + }) + } +} + +// TestValidateMicroflow_XPathIdConstraint covers MDL048 (ledger #42): a retrieve +// that constrains on the object id (`[id = $x]`) fails the build with CE0161. +func TestValidateMicroflow_XPathIdConstraint(t *testing.T) { + cases := []struct { + name string + where string + wantMDL bool + }{ + {"id equals a String value var", "[id = $Id]", true}, + {"ID case-insensitive against value var", "[ID = $Id]", true}, + {"id equals a string literal", "[id = '123']", true}, + {"id in boolean clause", "[Active = true and id = $Id]", true}, + // Comparing id to an OBJECT variable is the valid "exclude self" pattern. + {"id not-equals an object var is fine", "[id != $This]", false}, + {"attribute containing id is fine", "[Valid = true]", false}, + {"paidstatus is fine", "[PaidStatus = $x]", false}, + {"plain attribute is fine", "[Name = $n]", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($Id: String, $x: String, $n: String, $This: M.B)\nreturns list of M.B\nbegin\n retrieve $B from M.B where " + tc.where + ";\n return $B;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL048" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL048 fired=%v, want %v (where: %q)", got, tc.wantMDL, tc.where) + } + }) + } +} + +// TestValidateMicroflow_AssociationObjectArg covers MDL049 (ledger #43/#44): a +// call argument bound to an association-object path (`$obj/Module.Assoc`) fails +// the build with CE0117 — it must be materialized first. +func TestValidateMicroflow_AssociationObjectArg(t *testing.T) { + cases := []struct { + name string + arg string + wantMDL bool + }{ + {"association object path", "B = $E/M.Edit_Budget", true}, + {"attribute over association is fine", "Name = $E/M.Edit_Budget/Label", false}, + {"plain attribute is fine", "Name = $E/Note", false}, + {"variable is fine", "B = $E", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($E: M.Edit)\nreturns boolean\nbegin\n $R = call microflow M.Consume(" + tc.arg + ");\n return $R;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL049" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL049 fired=%v, want %v (arg: %q)", got, tc.wantMDL, tc.arg) + } + }) + } +} + +// TestValidateMicroflow_ConditionalBreak covers MDL051 (ledger #52): a `break` +// nested inside a conditional within a loop serializes a dangling reference that +// crashes `mx check` (unloadable model). A break directly in the loop body, or no +// break, is fine. +func TestValidateMicroflow_ConditionalBreak(t *testing.T) { + cases := []struct { + name string + body string + wantMDL bool + }{ + {"break inside if in loop", "loop $R in $L begin if $R/Active then break; end if; end loop", true}, + {"break inside nested if in loop", "loop $R in $L begin if $R/Active then if $R/Active then break; end if; end if; end loop", true}, + {"break directly in loop is fine", "loop $R in $L begin break; end loop", false}, + {"no break is fine", "loop $R in $L begin if $R/Active then set $x = 1; end if; end loop", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($L: list of M.R)\nreturns boolean\nbegin\n " + tc.body + "\n return true;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL051" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL051 fired=%v, want %v (body: %q)", got, tc.wantMDL, tc.body) + } + }) + } +} + +// TestValidateDatasourceXPathAssociationEmpty covers the page/widget-datasource +// arm of MDL047 (ledger #25 verification round): the original check only saw +// microflow retrieves, so `datagrid (datasource: database ... where [Assoc = +// empty])` slipped through. +func TestValidateDatasourceXPathAssociationEmpty(t *testing.T) { + dg := func(where string) *ast.WidgetV3 { + return &ast.WidgetV3{Type: "datagrid", Name: "dg", Properties: map[string]any{ + "DataSource": &ast.DataSourceV3{Type: "database", Reference: "Ledger.Transaction", Where: where}, + }} + } + cases := []struct { + name string + widget *ast.WidgetV3 + want bool + }{ + {"association = empty", dg("[Ledger.Transaction_Category = empty]"), true}, + {"negation is fine", dg("[not(Ledger.Transaction_Category/Ledger.Category)]"), false}, + {"attribute = empty is fine", dg("[Description = empty]"), false}, + {"no where clause", dg(""), false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := len(validateDatasourceXPathAssociationEmpty(c.widget, "page X")) > 0 + if got != c.want { + t.Errorf("MDL047 present = %v, want %v", got, c.want) + } + }) + } +} + +// TestValidateMicroflow_DuplicateLoopVariable covers MDL052 (ledger #64): a loop +// iterator is scoped to the whole microflow, so reusing a name across loops +// builds as CE0111. Distinct names — and a single loop — are fine. +func TestValidateMicroflow_DuplicateLoopVariable(t *testing.T) { + cases := []struct { + name string + body string + wantMDL bool + }{ + {"two loops same iterator", "loop $R in $L begin set $x = 1; end loop loop $R in $L begin set $y = 1; end loop", true}, + {"nested loop reuses outer iterator", "loop $R in $L begin loop $R in $L begin set $x = 1; end loop end loop", true}, + {"distinct iterators are fine", "loop $R in $L begin set $x = 1; end loop loop $C in $L begin set $y = 1; end loop", false}, + {"single loop is fine", "loop $R in $L begin set $x = 1; end loop", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($L: list of M.R)\nreturns Boolean\nbegin\n " + tc.body + "\n return true;\nend;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var got bool + for _, vi := range ValidateMicroflow(mf) { + if vi.RuleID == "MDL052" { + got = true + } + } + if got != tc.wantMDL { + t.Errorf("MDL052 fired=%v, want %v (body: %q)", got, tc.wantMDL, tc.body) + } + }) + } +} diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 868da3d9a..aa3afca31 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -119,6 +119,7 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc out = append(out, validatePluggableWidgetProperties(w, registry, locationPrefix)...) out = append(out, validateWidgetVisibility(w, registry, locationPrefix)...) out = append(out, validateStaticWidget(w, locationPrefix)...) + out = append(out, validateDatasourceXPathAssociationEmpty(w, locationPrefix)...) // Unknown-property warning applies only to built-in widgets; pluggable // widgets get the stricter def.json check (MDL-WIDGET01) above, and // object-list items are validated by the object-list engine. @@ -133,9 +134,80 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc out = append(out, validateWidgetTreeIn(w.Children, registry, locationPrefix, objectListMappingSet(def))...) } } + out = append(out, validateConsecutiveDynamicText(widgets, locationPrefix)...) return out } +// validateDatasourceXPathAssociationEmpty flags `[Module.Association = empty]` in +// a widget's database datasource `where` clause — the page-datasource counterpart +// of the microflow-retrieve MDL047 check. Mendix XPath has no `= empty` for an +// association (CE0161); the nullability test is `not(Assoc/Target)`. (ledger #25, +// verification round: MDL047 originally covered only microflow retrieves.) +func validateDatasourceXPathAssociationEmpty(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + ds := w.GetDataSource() + if ds == nil || ds.Where == "" { + return nil + } + var out []linter.Violation + for _, assoc := range xpathAssociationEmptyMatches(ds.Where) { + out = append(out, linter.Violation{ + RuleID: "MDL047", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: widget `%s` datasource constraint tests association `%s = empty`, which Mendix XPath does not support (CE0161 \"Error(s) in XPath constraint\") — `= empty` works on attributes, not associations", + locationPrefix, w.Name, assoc), + Suggestion: fmt.Sprintf("Test for the absence of the associated object with negation: `[not(%s/)]`.", assoc), + }) + } + return out +} + +// headingRenderModeRe matches the block-level dynamictext render modes H1–H6. +var headingRenderModeRe = regexp.MustCompile(`(?i)^h[1-6]$`) + +// inlineDynamicText reports whether a widget is a dynamictext that renders +// INLINE — i.e. a ``. Only the heading modes H1–H6 are genuinely +// block-level (`display: block`); `Text`/unset AND `Paragraph` both render as an +// inline `` (verified on Mendix 11.12.1 + Atlas), so adjacent ones fuse. +// A heading followed by a subtitle therefore does NOT concatenate and must not +// be flagged. (ledger #27; #29 corrected the earlier mis-classification of +// Paragraph as block-level.) +func inlineDynamicText(w *ast.WidgetV3) bool { + if w == nil || !strings.EqualFold(w.Type, "dynamictext") { + return false + } + return !headingRenderModeRe.MatchString(w.GetRenderMode()) +} + +// validateConsecutiveDynamicText emits an advisory (MDL-WIDGET15) when two or +// more INLINE dynamictext widgets are direct siblings: Mendix renders a Text- or +// Paragraph-mode DynamicText inline (a ``), so adjacent ones concatenate +// with no separator (`€ 310` + `7/24/2026` → `€ 3107/24/2026`). Only a heading +// render mode (H1–H6) is block-level and breaks the run. Info severity — it does +// not fail the build, it warns the author about a layout surprise. (ledger #27/#29) +func validateConsecutiveDynamicText(siblings []*ast.WidgetV3, locationPrefix string) []linter.Violation { + run := 0 + for _, w := range siblings { + if inlineDynamicText(w) { + run++ + } else { + run = 0 + } + // Emit once, on the second inline dynamictext of a run, so a group of N + // only warns once. + if run == 2 { + return []linter.Violation{{ + RuleID: "MDL-WIDGET15", + Severity: linter.SeverityInfo, + Message: fmt.Sprintf( + "%s: adjacent inline dynamictext widgets (RenderMode Text or Paragraph, both ) render with no separator, so their text concatenates. Merge them into one dynamictext with multiple content params, wrap each in its own container, or use a heading RenderMode (H1–H6, which is block-level). Note: Paragraph does NOT fix this — it also renders inline.", + locationPrefix), + }} + } + } + return nil +} + // validateWidgetVisibility warns (MDL-WIDGET10) when a property the user set on a // pluggable widget is hidden under that widget's current configuration — the // widget's editorConfig.js suppresses it, so Studio Pro ignores the written value. @@ -566,6 +638,13 @@ func validateStaticWidget(w *ast.WidgetV3, locationPrefix string) []linter.Viola // association legitimately. (findings #4) out = append(out, validateWidgetExpressionAssociations(w, locationPrefix)...) + // A contentparams/captionparams value is a DATA BINDING (an attribute path), + // not a client-side expression: mxcli stores an unquoted value as an attribute + // name, so a function call like `formatDateTime($obj/Date, 'd MMM')` is written + // as a bogus attribute and Studio Pro rejects the page with CE1613 "attribute + // no longer exists". Catch it at check time. (ledger finding #26) + out = append(out, validateTemplateParamExpressions(w, locationPrefix)...) + return out } @@ -604,6 +683,49 @@ func validateWidgetExpressionAssociations(w *ast.WidgetV3, locationPrefix string return out } +// templateParamExprRe detects a client-side expression where an attribute-path +// data binding is expected. A binding is a path of identifier segments joined by +// `/` or `.` (optionally `$`-prefixed): `$obj/Date`, `Order_Customer/Name`. An +// expression carries a function call `foo(` or an arithmetic/comparison operator, +// none of which can appear in an attribute path. +var templateParamExprRe = regexp.MustCompile(`[A-Za-z_][A-Za-z0-9_]*\s*\(|[+\-*<>=!]`) + +// validateTemplateParamExpressions flags a client expression supplied to a +// contentparams/captionparams slot (MDL-WIDGET14). Those slots are DATA BINDINGS: +// an unquoted value is stored as an attribute path, so an expression like +// `formatDateTime($obj/Date, 'd MMM')` is written as a bogus attribute name and +// Studio Pro rejects the page with CE1613. A quoted value is a legal string +// literal and is left alone. +func validateTemplateParamExpressions(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + var out []linter.Violation + check := func(slot string, params []ast.ParamAssignmentV3) { + for _, p := range params { + val, ok := p.Value.(string) + if !ok || val == "" { + continue + } + // A quoted string literal is a valid contentparams value. + if strings.HasPrefix(val, "'") || strings.HasPrefix(val, "\"") { + continue + } + if !templateParamExprRe.MatchString(val) { + continue + } + out = append(out, linter.Violation{ + RuleID: "MDL-WIDGET14", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: widget `%s` %s value `%s` is an expression, but a template parameter is a data binding — it accepts an attribute path (`$obj/Attr`) or a quoted string literal, not an expression (CE1613 in Studio Pro). Precompute it onto the bound entity (e.g. a calculated attribute) and bind that attribute instead.", + locationPrefix, w.Name, slot, val, + ), + }) + } + } + check("contentparams", w.GetContentParams()) + check("captionparams", w.GetCaptionParams()) + return out +} + var templatePlaceholderRe = regexp.MustCompile(`\{(\d+)\}`) // validateDynamicTextPlaceholders flags a dynamictext whose Content template has diff --git a/mdl/executor/validate_widgets_test.go b/mdl/executor/validate_widgets_test.go index a7d8a8f1b..8c49b3990 100644 --- a/mdl/executor/validate_widgets_test.go +++ b/mdl/executor/validate_widgets_test.go @@ -195,6 +195,83 @@ func TestValidateWidgetExpressionAssociations(t *testing.T) { } } +// TestValidateTemplateParamExpressions — MDL-WIDGET14 flags a client expression +// supplied to a contentparams/captionparams slot (a data binding). An attribute +// path or quoted string literal is fine. (ledger finding #26) +func TestValidateTemplateParamExpressions(t *testing.T) { + cp := func(vals ...any) *ast.WidgetV3 { + params := make([]ast.ParamAssignmentV3, len(vals)) + for i, v := range vals { + params[i] = ast.ParamAssignmentV3{Index: i + 1, Value: v} + } + return &ast.WidgetV3{Type: "dynamictext", Name: "d", Properties: map[string]any{"ContentParams": params}} + } + cases := []struct { + name string + widget *ast.WidgetV3 + want bool // expect an MDL-WIDGET14 violation + }{ + {"function call → rejected", cp("formatDateTime($currentObject/LastImport,'d MMM yyyy')"), true}, + {"arithmetic expression → rejected", cp("$currentObject/Qty * $currentObject/Price"), true}, + {"attribute path → ok", cp("$currentObject/Name"), false}, + {"association-navigated attribute path → ok", cp("MyMod.A_B/Name"), false}, + {"quoted string literal → ok", cp("'literal text'"), false}, + {"quoted string with parens → ok", cp("'formatDateTime(x)'"), false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := false + for _, v := range validateStaticWidget(c.widget, "page X") { + if v.RuleID == "MDL-WIDGET14" { + got = true + } + } + if got != c.want { + t.Errorf("MDL-WIDGET14 present = %v, want %v", got, c.want) + } + }) + } +} + +// TestValidateConsecutiveDynamicText — MDL-WIDGET15 (info) flags two or more +// adjacent dynamictext siblings, which Mendix renders inline (concatenated with +// no separator) regardless of RenderMode. (ledger finding #27) +func TestValidateConsecutiveDynamicText(t *testing.T) { + dt := func(name string) *ast.WidgetV3 { return &ast.WidgetV3{Type: "dynamictext", Name: name} } + dtRM := func(name, rm string) *ast.WidgetV3 { + return &ast.WidgetV3{Type: "dynamictext", Name: name, Properties: map[string]any{"RenderMode": rm}} + } + tb := func(name string) *ast.WidgetV3 { return &ast.WidgetV3{Type: "textbox", Name: name} } + cases := []struct { + name string + siblings []*ast.WidgetV3 + want bool + }{ + {"two adjacent dynamictexts", []*ast.WidgetV3{dt("a"), dt("b")}, true}, + {"three adjacent (warns once)", []*ast.WidgetV3{dt("a"), dt("b"), dt("c")}, true}, + {"explicit Text render mode", []*ast.WidgetV3{dtRM("a", "Text"), dtRM("b", "Text")}, true}, + {"separated by another widget", []*ast.WidgetV3{dt("a"), tb("x"), dt("b")}, false}, + {"single dynamictext", []*ast.WidgetV3{dt("a")}, false}, + {"no dynamictext", []*ast.WidgetV3{tb("x"), tb("y")}, false}, + // Only headings (H1–H6) are block-level. Paragraph renders inline () + // and fuses, so it IS flagged (#29 corrected treating it as block-level). + {"two paragraphs fuse", []*ast.WidgetV3{dtRM("p1", "Paragraph"), dtRM("p2", "Paragraph")}, true}, + {"paragraph then text fuse", []*ast.WidgetV3{dtRM("p", "Paragraph"), dt("t")}, true}, + // Headings render block-level, so a heading + subtitle does not concatenate. + {"heading then subtitle", []*ast.WidgetV3{dtRM("h", "H2"), dt("sub")}, false}, + {"two headings", []*ast.WidgetV3{dtRM("h1", "H2"), dtRM("h2", "H3")}, false}, + {"heading breaks a run of inlines", []*ast.WidgetV3{dt("a"), dtRM("h", "H2"), dt("b")}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := len(validateConsecutiveDynamicText(c.siblings, "page X")) > 0 + if got != c.want { + t.Errorf("MDL-WIDGET15 present = %v, want %v", got, c.want) + } + }) + } +} + // TestValidateObjectListItemEnums — MDL-WIDGET08 flags an object-list item's // enumeration sub-property whose value isn't a declared member key (e.g. a Maps // marker LocationType outside {address, latlng}). Studio Pro silently defaults diff --git a/mdl/executor/widget_defs.go b/mdl/executor/widget_defs.go index 8ea2e5afe..97749b18b 100644 --- a/mdl/executor/widget_defs.go +++ b/mdl/executor/widget_defs.go @@ -311,6 +311,23 @@ func GenerateDefJSON(mpkDef *mpk.WidgetDefinition, mdlName string) *WidgetDefini Default: p.DefaultValue, Description: p.Description, }) + case "action": + // Action-typed properties (e.g. DataGrid2 `onClick`, filter `onChange`) + // were silently skipped — no `action` operation was ever emitted, so the + // writer had no mapping and dropped the action with no error or warning + // (ledger #67). Emit an action mapping so the engine's applyOperation + // "action" writes the client action. Only the slots MDL can currently + // author (onClick → the widget's Action property, onChange → OnChange) + // are wired; other action slots have no MDL surface yet, so emitting a + // mapping for them would resolve to nothing. + if src := actionSourceForKey(p.Key); src != "" { + def.PropertyMappings = append(def.PropertyMappings, PropertyMapping{ + PropertyKey: p.Key, + Source: src, + Operation: "action", + Description: p.Description, + }) + } case "boolean", "integer", "decimal", "string", "enumeration": m := PropertyMapping{ PropertyKey: p.Key, @@ -325,6 +342,52 @@ func GenerateDefJSON(mpkDef *mpk.WidgetDefinition, mdlName string) *WidgetDefini } def.PropertyMappings = append(def.PropertyMappings, assocMappings...) + // KnownProperties: every property the widget's definition (.mpk) declares that + // has NO mapping in the generated def — i.e. a real property mxcli does not + // persist. Computed purely from the two artifacts mxcli already has (the .mpk + // property list and the generated mappings), with no per-widget knowledge. The + // checker uses this to WARN "recognized but not persisted; the value will be + // dropped" instead of silently discarding it or falsely rejecting it as an + // unknown property. This is the general guard for the class behind ledger #67 + // (a type the generator doesn't map — e.g. `expression`, `icon`, or an action + // slot with no MDL surface); the specific fix for that finding is the `action` + // mapping above. + mapped := make(map[string]bool) + markMapped := func(key string) { + if key != "" { + mapped[strings.ToLower(key)] = true + } + } + for _, m := range def.PropertyMappings { + markMapped(m.PropertyKey) + for _, a := range m.MdlAliases { + markMapped(a) + } + } + for _, cs := range def.ChildSlots { + markMapped(cs.PropertyKey) + } + for _, ol := range def.ObjectLists { + markMapped(ol.PropertyKey) + } + for _, m := range def.Modes { + for _, pm := range m.PropertyMappings { + markMapped(pm.PropertyKey) + for _, a := range pm.MdlAliases { + markMapped(a) + } + } + for _, cs := range m.ChildSlots { + markMapped(cs.PropertyKey) + } + } + for _, p := range mpkDef.Properties { + if p.Key == "" || mapped[strings.ToLower(p.Key)] { + continue + } + def.KnownProperties = append(def.KnownProperties, p.Key) + } + def.PropertyVisibility = widgetVisibilityRules[mpkDef.ID] return def @@ -343,6 +406,7 @@ func GenerateDefJSON(mpkDef *mpk.WidgetDefinition, mdlName string) *WidgetDefini // "expression"===e.type && hidePropertiesIn(["videoUrl","posterUrl"]) // Timeline (editorConfig.js): // e.customVisualization ? hidePropertiesIn(["title","description","icon","timeIndication",...]) : ... +// // mergeVisibilityRules returns the extracted rules plus any hand-authored fallback // rules whose PropertyKey the extractor did not cover. Extracted rules are // version-specific (lifted from the installed .mpk's editorConfig.js) and win on @@ -495,6 +559,21 @@ var propertyAliases = map[string]map[string][]string{ // `groups`) into an ObjectListMapping. The MDL keyword is the singular form of // the property key (groups → GROUP, basicItems → ITEM, series → SERIES, // markers → MARKER). +// actionSourceForKey maps an action-typed property key to the BuildContext +// resolution source the engine understands. Only the action slots MDL can author +// today are wired: `onClick` → the widget's Action property, `onChange` → +// OnChange. Other action slots (e.g. DataGrid2 `onSelectionChange`) have no MDL +// surface yet, so they return "" and no mapping is emitted. +func actionSourceForKey(key string) string { + switch strings.ToLower(key) { + case "onclick": + return "OnClick" + case "onchange": + return "OnChange" + } + return "" +} + func makeObjectListMapping(widgetID string, p mpk.PropertyDef) ObjectListMapping { mapping := ObjectListMapping{ PropertyKey: p.Key, diff --git a/mdl/executor/widget_defs_test.go b/mdl/executor/widget_defs_test.go index f9e977f71..55cb28949 100644 --- a/mdl/executor/widget_defs_test.go +++ b/mdl/executor/widget_defs_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/sdk/pages" "github.com/mendixlabs/mxcli/sdk/widgets/mpk" ) @@ -139,11 +140,12 @@ func TestGenerateDefJSON_SkipsComplexTypes(t *testing.T) { def := GenerateDefJSON(mpkDef, "COMPLEX") // textTemplate now always yields a mapping (so captions like Badge `value` - // are authorable); the other complex types (action/expression/icon/non-list - // object) are still skipped. So exactly one mapping — the texttemplate — is - // expected. + // are authorable); expression/icon/non-list object are skipped. An `action` + // property only maps when its key is MDL-authorable (onClick/onChange) — here + // the key is `myAction`, so it is skipped too. So exactly one mapping — the + // texttemplate — is expected. if len(def.PropertyMappings) != 1 { - t.Fatalf("PropertyMappings count = %d, want 1 (only the texttemplate maps; action/expression/icon/object skipped)", len(def.PropertyMappings)) + t.Fatalf("PropertyMappings count = %d, want 1 (only the texttemplate maps; unnamed action/expression/icon/object skipped)", len(def.PropertyMappings)) } tt := def.PropertyMappings[0] if tt.PropertyKey != "myTemplate" || tt.Operation != "texttemplate" { @@ -178,6 +180,66 @@ func TestGenerateDefJSON_TextTemplatesAlwaysMapped(t *testing.T) { } } +// TestGenerateDefJSON_ActionMapping covers ledger #67: an `onClick` action +// property must yield an `action` PropertyMapping (source OnClick) so the writer +// serializes the client action instead of silently dropping it. A non-authorable +// action key (e.g. DataGrid2 `onSelectionChange`) has no MDL surface yet, so no +// mapping is emitted for it. +func TestGenerateDefJSON_ActionMapping(t *testing.T) { + mpkDef := &mpk.WidgetDefinition{ + ID: "com.mendix.widget.web.datagrid.Datagrid", + Name: "Datagrid", + Properties: []mpk.PropertyDef{ + {Key: "onClick", Type: "action"}, + {Key: "onSelectionChange", Type: "action"}, + }, + } + def := GenerateDefJSON(mpkDef, "DATAGRID") + onClick := findMapping(def.PropertyMappings, "onClick") + if onClick == nil { + t.Fatal("onClick (action) has no mapping — the action would be silently dropped (#67)") + } + if onClick.Operation != "action" || onClick.Source != "OnClick" { + t.Errorf("onClick mapping = {op=%s src=%s}, want {action OnClick}", onClick.Operation, onClick.Source) + } + if m := findMapping(def.PropertyMappings, "onSelectionChange"); m != nil { + t.Errorf("onSelectionChange has no MDL surface yet; expected no mapping, got {op=%s src=%s}", m.Operation, m.Source) + } +} + +// TestGenerateDefJSON_KnownPropertiesUnmapped covers the general #67-class guard: +// every declared property with NO mapping in the generated def is recorded in +// KnownProperties, so the checker warns "recognized but not persisted" rather +// than silently dropping it or falsely rejecting it as unknown. A mapped property +// must NOT appear there. +func TestGenerateDefJSON_KnownPropertiesUnmapped(t *testing.T) { + mpkDef := &mpk.WidgetDefinition{ + ID: "com.mendix.widget.web.datagrid.Datagrid", + Name: "Datagrid", + Properties: []mpk.PropertyDef{ + {Key: "onClick", Type: "action"}, // mapped (action → OnClick) + {Key: "pageSize", Type: "integer"}, // mapped (primitive) + {Key: "onSelectionChange", Type: "action"}, // unmapped action slot + {Key: "rowClass", Type: "expression"}, // unmapped (expression not handled) + }, + } + def := GenerateDefJSON(mpkDef, "DATAGRID") + known := make(map[string]bool) + for _, k := range def.KnownProperties { + known[k] = true + } + for _, want := range []string{"onSelectionChange", "rowClass"} { + if !known[want] { + t.Errorf("KnownProperties missing unmapped property %q — the checker can't warn it will be dropped", want) + } + } + for _, notWant := range []string{"onClick", "pageSize"} { + if known[notWant] { + t.Errorf("KnownProperties should not list mapped property %q", notWant) + } + } +} + func TestGenerateDefJSON_AssociationAfterDataSource(t *testing.T) { // Association mappings require entityContext from a prior DataSource mapping. // GenerateDefJSON must order datasource before association regardless of MPK order. @@ -632,20 +694,21 @@ func TestApplyColumnHeaderFallback(t *testing.T) { {PropertyKey: "attribute", Operation: "attribute", AttributePath: "Mod.Ent.Foo"}, }, } - applyColumnHeaderFallback(spec1) + applyColumnHeaderFallback(spec1, "col1", true) if len(spec1.Properties) != 2 { t.Errorf("Case 1 (header present): expected 2 properties, got %d", len(spec1.Properties)) } - // Case 2: no header, no attribute → no change + // Case 2: no header slot (a header-less object-list item like a chart series) + // → never touched, even with no attribute. spec2 := &backend.ObjectListItemSpec{ Properties: []backend.ObjectListItemProperty{ - {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "attribute"}, + {PropertyKey: "staticName", Operation: "primitive", PrimitiveVal: "x"}, }, } - applyColumnHeaderFallback(spec2) + applyColumnHeaderFallback(spec2, "series1", false) if len(spec2.Properties) != 1 { - t.Errorf("Case 2 (no attribute): expected 1 property, got %d", len(spec2.Properties)) + t.Errorf("Case 2 (no header slot): expected 1 property, got %d", len(spec2.Properties)) } // Case 3: no header, attribute set → synthesize header from attribute leaf @@ -654,7 +717,7 @@ func TestApplyColumnHeaderFallback(t *testing.T) { {PropertyKey: "attribute", Operation: "attribute", AttributePath: "Mod.Ent.OrderNumber"}, }, } - applyColumnHeaderFallback(spec3) + applyColumnHeaderFallback(spec3, "col3", true) if len(spec3.Properties) != 2 { t.Fatalf("Case 3 (fallback): expected 2 properties, got %d", len(spec3.Properties)) } @@ -669,10 +732,71 @@ func TestApplyColumnHeaderFallback(t *testing.T) { {PropertyKey: "attribute", Operation: "attribute", AttributePath: "BareName"}, }, } - applyColumnHeaderFallback(spec4) + applyColumnHeaderFallback(spec4, "col4", true) if len(spec4.Properties) != 2 || spec4.Properties[1].TextTemplate != "BareName" { t.Errorf("Case 4: fallback for unqualified path = %v", spec4.Properties) } + + // Case 5 (ledger #54): an explicit empty caption (`Caption: ''`) reaches the + // engine as a texttemplate header with empty text and no params. Studio Pro + // rejects an empty column header with CE0463, so it must be treated like an + // absent one — filled IN PLACE with the attribute leaf, not left empty and + // not duplicated. + spec5 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "header", Operation: "texttemplate", TextTemplate: ""}, + {PropertyKey: "attribute", Operation: "attribute", AttributePath: "Mod.Ent.Amount"}, + }, + } + applyColumnHeaderFallback(spec5, "col5", true) + if len(spec5.Properties) != 2 { + t.Fatalf("Case 5 (empty caption): expected 2 properties (no duplicate), got %d", len(spec5.Properties)) + } + if h := spec5.Properties[0]; h.PropertyKey != "header" || h.Operation != "texttemplate" || h.TextTemplate != "Amount" { + t.Errorf("Case 5: empty header not filled in place: %+v, want TextTemplate=Amount", spec5.Properties[0]) + } + + // Case 6: a template header WITH params (e.g. Caption: '{1}') is NOT empty — + // it must be left untouched even though its literal text may be blank. + spec6 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "header", Operation: "texttemplate", TextTemplate: "", Parameters: []*pages.ClientTemplateParameter{{}}}, + {PropertyKey: "attribute", Operation: "attribute", AttributePath: "Mod.Ent.Amount"}, + }, + } + applyColumnHeaderFallback(spec6, "col6", true) + if spec6.Properties[0].TextTemplate != "" { + t.Errorf("Case 6: param-bearing header must not be overwritten, got TextTemplate=%q", spec6.Properties[0].TextTemplate) + } + + // Case 7 (ledger #54, custom-content column): no attribute, empty header — + // fall back to the COLUMN NAME in place (a custom-content column has nothing + // to derive a header from, and an empty header is CE0463). + spec7 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "header", Operation: "texttemplate", TextTemplate: ""}, + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + }, + } + applyColumnHeaderFallback(spec7, "colActions", true) + if h := spec7.Properties[0]; h.PropertyKey != "header" || h.TextTemplate != "colActions" { + t.Errorf("Case 7: empty custom-content header not filled with column name: %+v", spec7.Properties[0]) + } + + // Case 8 (ledger #54): custom-content column with NO header prop at all — + // append a header carrying the column name (absent header is CE0463 too). + spec8 := &backend.ObjectListItemSpec{ + Properties: []backend.ObjectListItemProperty{ + {PropertyKey: "showContentAs", Operation: "primitive", PrimitiveVal: "customContent"}, + }, + } + applyColumnHeaderFallback(spec8, "colActions", true) + if len(spec8.Properties) != 2 { + t.Fatalf("Case 8 (absent custom-content header): expected 2 properties, got %d", len(spec8.Properties)) + } + if h := spec8.Properties[1]; h.PropertyKey != "header" || h.TextTemplate != "colActions" { + t.Errorf("Case 8: appended header = %+v, want TextTemplate=colActions", spec8.Properties[1]) + } } // TestObjectListItemPropertyParamsConvention documents the alias→params diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index 2f73dbd1c..eefb45363 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -70,7 +70,16 @@ const defaultSlotContainer = "template" // editorConfig-extracted rules instead of overwriting, so compound/ternary // guards the static extractor skips (e.g. Timeline title/description hidden // when customVisualization) still null their hidden textTemplates (CE0463). -const WidgetDefGeneratorVersion = 12 +// 13 — emit an `action` PropertyMapping for `type="action"` properties that MDL +// can author (`onClick`→the widget Action, `onChange`→OnChange). Previously +// no action operation was ever generated, so the writer had no mapping and +// silently dropped a widget-level action (e.g. DataGrid2 `onClick`). Bump +// forces existing projects to regenerate their defs with the action mapping. +// 14 — populate KnownProperties with every declared property that has no mapping +// in the generated def, so the checker warns "recognized but not persisted" +// instead of silently dropping or falsely rejecting it (general guard for the +// ledger #67 class). Bump forces regeneration to carry the new field. +const WidgetDefGeneratorVersion = 14 // WidgetDefinition describes how to construct a pluggable widget from MDL syntax. // Loaded from embedded JSON definition files (*.def.json). @@ -704,6 +713,15 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W ctx.Action = act } + case "OnChange": + if action := w.GetOnChange(); action != nil { + act, err := e.pageBuilder.buildClientActionV3(action) + if err != nil { + return nil, mdlerrors.NewBackend("build action", err) + } + ctx.Action = act + } + default: val := w.GetStringProp(source) if val == "" { @@ -1043,12 +1061,21 @@ func (e *PluggableWidgetEngine) buildObjectListItem(mapping *ObjectListMapping, spec.Properties = append(spec.Properties, prop) } - // DataGrid column header convention: when the user provides no Caption - // but does bind an Attribute, fall back to the attribute name as the - // header text. Mirrors buildDataGrid2ColumnObject in datagrid_builder.go - // (line 491-494). Without this, an attribute column with no Caption - // emits an empty header that Studio Pro flags as definition drift. - applyColumnHeaderFallback(&spec) + // DataGrid column header convention: when the user provides no Caption, + // fall back to the bound attribute name — or, for a custom-content column + // with no attribute, the column's own name. Mirrors buildDataGrid2ColumnObject + // in datagrid_builder.go. Without this, an empty/absent column header trips + // Studio Pro's CE0463 (ledger #54). Only applies to items whose template has + // a `header` slot (datagrid columns), never to header-less object-list items + // like chart series. + hasHeaderSlot := false + for _, ip := range mapping.ItemProperties { + if strings.EqualFold(ip.PropertyKey, "header") { + hasHeaderSlot = true + break + } + } + applyColumnHeaderFallback(&spec, child.Name, hasHeaderSlot) // DataGrid column sortable convention: attribute-less columns (typical // "Actions" custom-content columns) default to sortable=false, since @@ -1142,30 +1169,60 @@ func (e *PluggableWidgetEngine) buildObjectListItem(mapping *ObjectListMapping, // in datagrid_builder.go. The check is conservative: only fires when the // header slot is genuinely empty, so it's safe to call for any object-list // item kind. -func applyColumnHeaderFallback(spec *backend.ObjectListItemSpec) { - var hasHeader bool +func applyColumnHeaderFallback(spec *backend.ObjectListItemSpec, columnName string, hasHeaderSlot bool) { + // Only items whose template has a `header` slot (datagrid columns) get a + // header fallback; header-less object-list items (chart series, accordion + // groups) are left untouched. + if !hasHeaderSlot { + return + } + headerIdx := -1 + headerEmpty := false var attrPath string - for _, p := range spec.Properties { + for i, p := range spec.Properties { switch p.PropertyKey { case "header": - hasHeader = true + headerIdx = i + // An explicit `Caption: ''` reaches here as a texttemplate with no + // text and no params. Studio Pro rejects an empty column header with + // CE0463 "widget definition changed" (ledger #54) — the same failure + // an absent header would cause without this fallback. Treat empty as + // absent so the fallback applies either way. + headerEmpty = p.Operation == "texttemplate" && p.TextTemplate == "" && len(p.Parameters) == 0 case "attribute": attrPath = p.AttributePath } } - if hasHeader || attrPath == "" { + // A present, non-empty header needs nothing. + if headerIdx >= 0 && !headerEmpty { return } - // Extract the leaf attribute name from a fully-qualified path - // (Module.Entity.Attr → Attr). - attrName := attrPath - if idx := strings.LastIndex(attrName, "."); idx >= 0 { - attrName = attrName[idx+1:] + // Fallback header text: the bound attribute's leaf name (Module.Entity.Attr → + // Attr), or — for a custom-content / action column with no attribute — the + // column's own name. Every datagrid column needs a NON-empty header; an empty + // or absent one is CE0463 (ledger #54), and a custom-content column has no + // attribute to derive one from, so the column name is the sensible default. + fallback := attrPath + if idx := strings.LastIndex(fallback, "."); idx >= 0 { + fallback = fallback[idx+1:] + } + if fallback == "" { + fallback = columnName + } + if fallback == "" { + return + } + if headerIdx >= 0 { + // Fill the empty header in place rather than appending a duplicate. + spec.Properties[headerIdx].Operation = "texttemplate" + spec.Properties[headerIdx].TextTemplate = fallback + spec.Properties[headerIdx].EntityContext = "" + return } spec.Properties = append(spec.Properties, backend.ObjectListItemProperty{ PropertyKey: "header", Operation: "texttemplate", - TextTemplate: attrName, + TextTemplate: fallback, EntityContext: "", // literal text — no template params to resolve }) } diff --git a/mdl/exprcheck/func_checker.go b/mdl/exprcheck/func_checker.go index b3c9fb0ba..46e7aa41e 100644 --- a/mdl/exprcheck/func_checker.go +++ b/mdl/exprcheck/func_checker.go @@ -171,9 +171,14 @@ var funcTable = map[string]funcSig{ "dateTimeToEpoch": {args: []TypeKind{KindDateTime}, ret: KindLong}, "epochToDateTime": {args: []TypeKind{KindLong}, ret: KindDateTime}, - // DateTime — extraction → Integer - "year": {args: []TypeKind{KindDateTime}, ret: KindInteger}, - "month": {args: []TypeKind{KindDateTime}, ret: KindInteger}, + // DateTime — extraction → Integer. + // NOTE: `year(...)` and `month(...)` were previously listed here but Mendix + // has no such built-ins — `mxcli check` passed and the build then failed with + // CE0117 (ledger finding #16). The correct approach is date formatting/parsing + // (`parseInteger(formatDateTime($d, 'yyyy'))`). The sibling extraction names + // below are UNVERIFIED against a real Mendix build and are likely invalid too; + // remove any that a build rejects (do not add more without confirming against + // Studio Pro — this whole family looks speculative). "dayOfYear": {args: []TypeKind{KindDateTime}, ret: KindInteger}, "dayOfMonth": {args: []TypeKind{KindDateTime}, ret: KindInteger}, "weekOfYear": {args: []TypeKind{KindDateTime}, ret: KindInteger}, diff --git a/mdl/exprcheck/func_checker_test.go b/mdl/exprcheck/func_checker_test.go index 7696a182f..54a22bd82 100644 --- a/mdl/exprcheck/func_checker_test.go +++ b/mdl/exprcheck/func_checker_test.go @@ -362,7 +362,7 @@ func TestFuncChecker_FormatDecimal_CorrectArity_OK(t *testing.T) { } func TestFuncReturnKind_DateTimeExtraction(t *testing.T) { - intFuncs := []string{"year", "month", "dayOfYear", "dayOfMonth", + intFuncs := []string{"dayOfYear", "dayOfMonth", "weekOfYear", "dayOfWeek", "hour", "minute", "second", "millisecond"} for _, name := range intFuncs { k, ok := FuncReturnKind(name) @@ -376,6 +376,17 @@ func TestFuncReturnKind_DateTimeExtraction(t *testing.T) { } } +// TestFuncReturnKind_NoYearMonth guards ledger finding #16: Mendix has no +// year()/month() built-ins, so they must NOT be in the whitelist (else check +// passes and the build fails CE0117). +func TestFuncReturnKind_NoYearMonth(t *testing.T) { + for _, name := range []string{"year", "month"} { + if _, ok := FuncReturnKind(name); ok { + t.Errorf("%q must not be whitelisted — Mendix has no such function", name) + } + } +} + func TestFuncReturnKind_Unknown(t *testing.T) { _, ok := FuncReturnKind("nonExistentFunction") if ok { diff --git a/mdl/visitor/visitor_microflow_contains_test.go b/mdl/visitor/visitor_microflow_contains_test.go new file mode 100644 index 000000000..63e5ac1cb --- /dev/null +++ b/mdl/visitor/visitor_microflow_contains_test.go @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// TestContainsOverloadParsing covers ledger finding #53: `contains` is overloaded +// as both the LIST operation contains(list, object) and the STRING function +// contains(haystack, needle). When an argument is a literal or a computed +// expression the call is unambiguously the string function and must parse as a +// value expression (MfSetStmt), never a lossy List operation activity. When both +// arguments are plain variables the kind stays ambiguous at parse time and is +// kept as a ListOperationStmt for the flow builder to disambiguate downstream. +func TestContainsOverloadParsing(t *testing.T) { + cases := []struct { + name string + setExpr string + wantListOp bool // true → ListOperationStmt, false → MfSetStmt (string expression) + }{ + {"literal second arg is string contains", "contains($Email, '@')", false}, + {"literal first arg is string contains", "contains('needle in haystack', $Needle)", false}, + {"computed second arg is string contains", "contains($Text, $A + $B)", false}, + {"both plain variables stay a list op (ambiguous)", "contains($Items, $One)", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($Email: String, $Needle: String, $Text: String, $A: String, $B: String, $Items: list of M.Item, $One: M.Item)\n" + + "returns Boolean\nbegin\n declare $Match Boolean = false;\n set $Match = " + tc.setExpr + ";\n return $Match;\nend;" + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + + var stmt ast.MicroflowStatement + for _, s := range mf.Body { + switch v := s.(type) { + case *ast.ListOperationStmt: + if v.OutputVariable == "Match" { + stmt = s + } + case *ast.MfSetStmt: + if v.Target == "Match" { + stmt = s + } + } + } + if stmt == nil { + t.Fatalf("no statement assigning $Match found") + } + + _, isListOp := stmt.(*ast.ListOperationStmt) + if isListOp != tc.wantListOp { + t.Errorf("statement type = %T, wantListOp=%v (expr: %q)", stmt, tc.wantListOp, tc.setExpr) + } + }) + } +} + +// TestFindOverloadParsing covers ledger finding #63: `find` is overloaded as the +// LIST operation find(list, condition) and the STRING function +// find(haystack, needle) → index. A string-literal second argument is +// unambiguously the string function and must parse as a value expression +// (MfSetStmt), not a lossy List operation activity (whose created output variable +// collides → CE0111). A boolean condition means the list operation. +func TestFindOverloadParsing(t *testing.T) { + cases := []struct { + name string + setExpr string + wantListOp bool + }{ + {"string literal needle is string find", "find($Raw, '\"id\":\"')", false}, + {"both plain variables stay a list op (ambiguous)", "find($Items, $One)", true}, + {"condition second arg is a list find", "find($Items, Key = $One)", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "create microflow M.F ($Raw: String, $One: String, $Items: list of M.Item)\n" + + "returns Integer\nbegin\n declare $At Integer = 0;\n set $At = " + tc.setExpr + ";\n return $At;\nend;" + prog, errs := Build(src) + if len(errs) > 0 { + t.Fatalf("unexpected parse errors: %v", errs) + } + mf := prog.Statements[0].(*ast.CreateMicroflowStmt) + var stmt ast.MicroflowStatement + for _, s := range mf.Body { + switch v := s.(type) { + case *ast.ListOperationStmt: + if v.OutputVariable == "At" { + stmt = s + } + case *ast.MfSetStmt: + if v.Target == "At" { + stmt = s + } + } + } + if stmt == nil { + t.Fatalf("no statement assigning $At found") + } + _, isListOp := stmt.(*ast.ListOperationStmt) + if isListOp != tc.wantListOp { + t.Errorf("statement type = %T, wantListOp=%v (expr: %q)", stmt, tc.wantListOp, tc.setExpr) + } + }) + } +} diff --git a/mdl/visitor/visitor_microflow_statements.go b/mdl/visitor/visitor_microflow_statements.go index 76de69513..fbcaf5c35 100644 --- a/mdl/visitor/visitor_microflow_statements.go +++ b/mdl/visitor/visitor_microflow_statements.go @@ -707,12 +707,23 @@ func buildSetStatement(ctx parser.ISetStatementContext) ast.MicroflowStatement { InputVariable: extractVariableName(funcCall.Arguments, 0), } case "FIND": - return &ast.ListOperationStmt{ - OutputVariable: targetVar, - Operation: ast.ListOpFind, - InputVariable: extractVariableName(funcCall.Arguments, 0), - Condition: getArgumentExpression(funcCall.Arguments, 1), + // `find` is overloaded: the LIST operation find(list, condition) — which + // filters a list by a boolean condition — and the STRING function + // find(haystack, needle) → the index of a substring. A STRING-LITERAL + // second argument is unambiguously the string function (you never filter + // a list by a bare string literal); it must stay a value expression, not + // a lossy List operation activity whose output variable collides + // (CE0111). Ledger #63. When both arguments are plain variables the kind + // is ambiguous here; the flow builder disambiguates String-typed inputs. + if !isStringLiteralArg(funcCall.Arguments, 1) { + return &ast.ListOperationStmt{ + OutputVariable: targetVar, + Operation: ast.ListOpFind, + InputVariable: extractVariableName(funcCall.Arguments, 0), + Condition: getArgumentExpression(funcCall.Arguments, 1), + } } + // Falls through to the default MfSetStmt (string find expression). case "FILTER": return &ast.ListOperationStmt{ OutputVariable: targetVar, @@ -751,12 +762,24 @@ func buildSetStatement(ctx parser.ISetStatementContext) ast.MicroflowStatement { SecondVariable: extractVariableName(funcCall.Arguments, 1), } case "CONTAINS": - return &ast.ListOperationStmt{ - OutputVariable: targetVar, - Operation: ast.ListOpContains, - InputVariable: extractVariableName(funcCall.Arguments, 0), - SecondVariable: extractVariableName(funcCall.Arguments, 1), + // `contains` is overloaded: the LIST operation contains(list, object) + // and the STRING function contains(haystack, needle). A List operation + // activity requires two plain list/object variables; if either argument + // is a literal or a computed expression it is unambiguously the string + // function, which must stay a value expression (a Change Variable + // action) — serializing it as a List operation fails the build + // (CE0023/CE0097/CE0111). Ledger finding #53. When both arguments are + // plain variables the kind is still ambiguous here (no type info); the + // flow builder disambiguates String-typed inputs downstream. + if isPlainVariableArg(funcCall.Arguments, 0) && isPlainVariableArg(funcCall.Arguments, 1) { + return &ast.ListOperationStmt{ + OutputVariable: targetVar, + Operation: ast.ListOpContains, + InputVariable: extractVariableName(funcCall.Arguments, 0), + SecondVariable: extractVariableName(funcCall.Arguments, 1), + } } + // Falls through to the default MfSetStmt (string contains expression). case "EQUALS": return &ast.ListOperationStmt{ OutputVariable: targetVar, @@ -846,6 +869,32 @@ func extractVariableName(args []ast.Expression, index int) string { return "" } +// isPlainVariableArg reports whether the argument at the given index is a bare +// variable reference (`$x` or an unquoted identifier) rather than a literal or a +// computed expression. Used to distinguish the list-operation form of an +// overloaded function (e.g. contains(list, object)) from its string form. +func isPlainVariableArg(args []ast.Expression, index int) bool { + if index >= len(args) { + return false + } + switch args[index].(type) { + case *ast.VariableExpr, *ast.IdentifierExpr: + return true + } + return false +} + +// isStringLiteralArg reports whether the argument at the given index is a string +// literal — used to detect the string form of an overloaded function (e.g. +// find(haystack, 'needle')) that must not become a list operation. +func isStringLiteralArg(args []ast.Expression, index int) bool { + if index >= len(args) { + return false + } + lit, ok := args[index].(*ast.LiteralExpr) + return ok && lit.Kind == ast.LiteralString +} + // getArgumentExpression returns the expression at the given index, or nil if not present. func getArgumentExpression(args []ast.Expression, index int) ast.Expression { if index >= len(args) { @@ -1438,6 +1487,32 @@ func buildRetrieveWhereExpression(ctx parser.IExpressionContext) ast.Expression return expr } +func isDigitByte(c byte) bool { return c >= '0' && c <= '9' } + +func isIdentByte(c byte) bool { + return c == '_' || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || isDigitByte(c) +} + +// dotIsQualifiedNameSeparator reports whether the `.` at index i separates +// segments of a qualified name (`Module.Entity`, `L48.Transaction`) rather than +// being a decimal point. The token immediately before the `.` is a name segment +// (not a number) when it contains a letter or underscore — so a module/entity +// name that merely ends in a digit (`L48`, `Account2`) is not mistaken for a +// decimal. Symmetrically, a `.` that directly follows an identifier char and is +// followed by a name segment (a letter/underscore start) is a separator too. +func dotIsQualifiedNameSeparator(source string, i int) bool { + // Walk back over the run of identifier chars ending at i-1; if any is a + // letter or underscore, the preceding token is a name, not a number. + s := i + for s > 0 && isIdentByte(source[s-1]) { + s-- + if source[s] == '_' || (source[s] >= 'a' && source[s] <= 'z') || (source[s] >= 'A' && source[s] <= 'Z') { + return true + } + } + return false +} + func shouldPreserveExpressionSource(source string) bool { if strings.ContainsAny(source, "\r\n") { return true @@ -1455,6 +1530,48 @@ func shouldPreserveExpressionSource(source string) bool { if inString { continue } + // A `/` used as division with a variable right operand (`$a / $b`) parses + // as a member-access path (the `$` on the RHS is stripped), so the AST + // re-serializes to a bogus `$a/b`. Preserve the raw source ONLY for this + // unambiguous misuse — a `/` immediately followed (ignoring spaces) by `$`. + // A real association path never has `$` after `/`, so legitimate navigation + // (`$Order/Assoc/Name`) is NOT source-frozen. MDL045 then rejects it at + // check time using the preserved source. (#17, verification round) + if source[i] == '/' { + j := i + 1 + for j < len(source) && (source[j] == ' ' || source[j] == '\t') { + j++ + } + if j < len(source) && source[j] == '$' { + return true + } + } + // Otherwise `/` is the member-access separator (`$Order/Assoc/Name`), not + // division (which is `div`), so it is NOT a preservation trigger — that + // would wrongly source-freeze every association-navigation path. + // + // Decimal literal: the re-serializer truncates a zero fraction (`2.0` → `2`, + // which breaks Mendix's Decimal typing) and emits small values in scientific + // notation (`0.000001` → `1e-06`, which Mendix rejects). A `.` adjacent to a + // digit marks a numeric literal; preserving the source keeps the exact form. + // (#18, #19) + // + // BUT a `.` inside a QUALIFIED NAME (`L48.Transaction`, `Account2.Name`) + // must NOT be mistaken for a decimal point just because a name segment ends + // in a digit — modules/entities ending in a digit are common. Freezing such + // an expression bypasses association-target-entity resolution + // (`resolveAssociationPaths`), producing `$T/L48.Assoc/Attr` without the + // required entity step, which Mendix rejects with CE0117 (ledger #48 root + // cause). A decimal point's digit run is a standalone number — not preceded + // by an identifier char; a name separator's `.` follows a token that + // contains a letter or underscore. + if source[i] == '.' { + prevDigit := i > 0 && isDigitByte(source[i-1]) + nextDigit := i+1 < len(source) && isDigitByte(source[i+1]) + if (prevDigit || nextDigit) && !dotIsQualifiedNameSeparator(source, i) { + return true + } + } switch source[i] { case '=', '!', '<', '>', '+', '-', '*', ':', ',': if i > 0 && source[i-1] != ' ' && source[i-1] != '\t' { diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index f6575019a..f1cdcf1c0 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -2296,6 +2296,50 @@ func TestShouldPreserveExpressionSourceIgnoresStringLiteralPunctuation(t *testin } } +// TestShouldPreserveExpressionSource_Decimals guards ledger findings #18–19: the +// AST re-serializer truncates a zero-fraction decimal (`2.0`→`2`) and emits small +// values in scientific notation (`0.000001`→`1e-06`), so a decimal literal must +// preserve the raw source. A `/` (member-access separator, NOT division) must NOT +// trigger preservation — that would source-freeze every association path. +func TestShouldPreserveExpressionSource_Decimals(t *testing.T) { + mustPreserve := []string{ + "$Dec div 2.0", // #18: 2.0 → 2 on re-serialize + "$Dec * $I * $I * 0.000001", // #19: 0.000001 → 1e-06 + "2.0", // standalone decimal literal + "100.0", // zero-fraction decimal + "round($Dec * $I * 0.001)", // small decimal in an arg + "$Dec / $Dec2", // #17: `/ $` division misuse (kept so MDL045 sees it) + "$Dec/$Dec2", // #17: no-space division misuse + } + for _, s := range mustPreserve { + if !shouldPreserveExpressionSource(s) { + t.Errorf("expected source preservation for %q (decimal would be corrupted)", s) + } + } + // A `/` is the member-access separator in MDL, not division: an association + // path must NOT be source-frozen on account of its slashes (that regressed + // TestAssociationNavParsing). A qualified name's dot (letters) and a slash + // inside a string literal must also not force preservation. + mustNotPreserve := []string{ + "$Order/Module.Assoc/Name", // association navigation path + "$currentObject/Amount", // plain member access + "Ledger.Category", // qualified name dot (non-numeric) + "'a/b path'", // slash inside a string literal + // ledger #48: a qualified-name segment ending in a digit must NOT be read + // as a decimal (that froze the source and skipped association-entity + // resolution → CE0117). Modules/entities ending in a digit are common. + "$T/L48.Transaction_Account/Name", // module name ends in a digit + "$obj/App2.Entity/Attr", // module 'App2' + "$c/Mod.Account2/Name", // entity name ends in a digit + "L48.Transaction", // bare qualified name, digit module + } + for _, s := range mustNotPreserve { + if shouldPreserveExpressionSource(s) { + t.Errorf("did not expect source preservation for %q", s) + } + } +} + func TestRenameModule_ObjectTypeIsLowercase(t *testing.T) { prog, errs := Build("RENAME MODULE OldMod TO NewMod;") if len(errs) > 0 {