diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index ec0c4bd03..e3cbf1eda 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -33,6 +33,14 @@ to the symptom table below, so the next similar issue costs fewer reads. | `mxcli check` rejects a **valid** microflow: **MDL045** ("`/` is division") on `round($a div $obj/Attr * 100)` — division whose divisor is an association-attribute path — but `mx check` → 0 errors | The MDL grammar parses `div`/`*`/`/` at one precedence level, so `$a div $obj/Attr` mis-nests as `($a div $obj) / Attr`; MDL045 saw the `/ Attr` as division. But `Attr` is a bare member name — Mendix has no `/` division operator and re-parses the raw `$obj/Attr` as a path (serialized output preserves the `/`, so the build is clean) | `mdl/executor/validate_microflow.go` (`exprHasSlashDivision`) | Don't flag a `/` BinaryExpr whose RIGHT operand is a bare `IdentifierExpr` (member navigation); real division has a numeric/paren/variable divisor. Test `TestValidateMicroflow_SlashDivision` (div-by-assoc cases); repro `mdl-examples/bug-tests/52-53-microflow-check-false-positives.mdl`. FINDINGS #52 | | `describe microflow` prints `-- Empty action` for a `set task outcome` / `open user task` / `notify workflow` statement (default engine); a describe→drop→exec round-trip silently drops it. Legacy engine (`MXCLI_ENGINE=legacy`) describes it fine | The modelsdk read path (`actionFromGen`) had no case for the workflow microflow actions, so they read back as nil → "Empty action". The write path + DESCRIBE formatter already handled them; only the modelsdk read case was missing | `mdl/backend/modelsdk/microflow_read_actions.go` (`actionFromGen`) | Add cases for `genMf.SetTaskOutcomeAction` / `OpenUserTaskAction` / `NotifyWorkflowAction`, mirroring the legacy parsers. Test `TestActionFromGen_WorkflowActions`; repro `mdl-examples/bug-tests/54-describe-set-task-outcome.mdl`. FINDINGS #54 | | `create association X …` errors "association already exists" on re-run and aborts the script | Correct SQL-shaped semantics (like `CREATE TABLE`) — `create` is not idempotent. The idempotent form is `create or modify association`, but it was undiscoverable from the bare error | `mdl/executor/cmd_associations.go` (the `NewAlreadyExists("association", …)` sites) | Not a code bug in the write path — improve the error to name `create or modify association …` and `drop association …`. Repro `mdl-examples/bug-tests/51-create-or-modify-association.mdl`. FINDINGS #51 | +| `dynamictext x (Content: '')` builds with **CE0720** "Place holder index 1 is greater than 0" — `mxcli check` ✓, describe shows `Content: '{1}'` with no params | The builder unconditionally defaulted empty content to the template `{1}`, creating a placeholder with no matching parameter (orphaned) | `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildDynamicTextV3`, final `content == ""` guard) | Only default to `{1}` when there IS a parameter (`autoGeneratedParams`/`explicitParams`); empty content with no params is a literal empty template. Test `TestBuildDynamicTextV3_EmptyContent`; repro `mdl-examples/bug-tests/traceops-9-10-17-dynamictext-listview.mdl`. traceops #9 | +| `dynamictext s (Content: '$318')` builds with **CE0402/CE1613** ("attribute '$318' no longer exists") — the literal was turned into an unbound `{1}` param | The auto-bind check treated ANY `$`-prefixed content as a variable; `$318` (dollar + digits) is not a valid Mendix variable | `mdl/executor/cmd_pages_builder_v3_widgets.go` (`isDynamicTextVariableRef` / `dynamicTextVariableRe`) | Treat `$` as a variable ONLY when followed by a letter/underscore (`^\$[A-Za-z_]`); `$318` stays literal content. Tests `TestBuildDynamicTextV3_DollarDigitLiteral`, `TestIsDynamicTextVariableRef`. traceops #10 | +| `listview lv (… PageSize: 200)` always pages at 20 — `mxcli check` ✓, `mx check` ✓, describe shows no PageSize | The property parsed into the AST but three layers ignored it: `buildListViewV3` hardcoded `PageSize: 20`, the describe parse never read it, and the listview describe formatter never emitted it | `mdl/executor/cmd_pages_builder_v3_widgets.go` (`buildListViewV3`) + `cmd_pages_describe_parse.go` (Forms$ListView case) + `cmd_pages_describe_output.go` (listview case) | Read `w.GetIntProp("PageSize")` on write; read `w["PageSize"]` on describe; emit a non-default PageSize in the listview formatter. Test `TestBuildListViewV3_PageSize`. traceops #17 | +| A `/** … */` doc comment between `alter entity … add attribute` clauses is a **parse error** (`no viable alternative at input '/**'`) | `alterEntityAction` accepted a doc comment only INSIDE an `attributeDefinition` (after the ADD ATTRIBUTE keyword), not between clauses. `--` line comments are NOT an equivalent workaround — they are discarded, whereas a `/** */` doc comment is persisted as the attribute's Mendix documentation | `mdl/grammar/domains/MDLDomainModel.g4` (`alterEntityAction`) + `mdl/visitor/visitor_entity.go` (`ExitAlterEntityAction` ADD branch) | Add `docComment?` before `ADD ATTRIBUTE`/`ADD COLUMN`; the visitor attaches it as the added attribute's documentation when the attributeDefinition has none. `make grammar` regenerates the parser (not committed). Test `TestAlterEntityAddAttributeDocComment`; repro `mdl-examples/bug-tests/traceops-27-doc-comment-between-clauses.mdl`. traceops #27 | +| `combobox (Association: Mod.Ref, …)` drops the binding — `mxcli check` ✓ but MxBuild fails **CE0642** "Property 'Attribute' is required" | The widget engine's `Association` source read the reference only from the `attribute:` keyword (`w.GetAttribute()`), so an explicit `Association:` keyword was ignored and the widget fell back to enumeration mode | `mdl/executor/widget_engine.go` (`case "Association"`) + `mdl/executor/validate_widgets.go` (`validateComboBoxAssociation`) | Read the reference from `Association:` OR `attribute:`; and add MDL-WIDGET16 flagging an association combobox that lacks the required `datasource:` (option list). A complete association combobox needs reference + `datasource:` + `CaptionAttribute:`. Tests `TestValidateComboBoxAssociation`; repro `mdl-examples/bug-tests/traceops-23-combobox-association.mdl`. traceops #23 | +| A bare MDL keyword used as a WIDGET name (`container body`, `dynamictext content`) is a parse error (`mismatched input 'body' expecting {IDENTIFIER, QUOTED_IDENTIFIER}`) | `widgetV3`'s name only accepted `IDENTIFIER \| QUOTED_IDENTIFIER`, not `keyword` — unlike `attributeName`/placeholder names | `mdl/grammar/domains/MDLPage.g4` (`widgetV3`) + `mdl/visitor/visitor_page_v3.go` (`buildWidgetV3` name extraction) | Add `keyword` to the widget-name alternatives; the visitor reads `wCtx.Keyword()` too. `make grammar` regenerates the parser. Test `TestKeywordWidgetName`; repro `mdl-examples/bug-tests/traceops-11-12-16-strings-names.mdl`. traceops #12 | +| A `'…'` string literal spanning multiple lines fails to parse (`missing END at '…'`) — the newline terminated the token | `STRING_LITERAL` excluded `\r\n` (`~['\r\n\\]`) | `mdl/grammar/MDLLexer.g4` (`STRING_LITERAL`) | Drop `\r\n` from the exclusion (`~['\\]`); mxbuild accepts a multi-line String value (verified). A missing close-quote now spans lines — the standard multi-line-string trade-off. Test `TestMultiLineStringLiteral`. traceops #11 | +| `alter entity … add : ` (missing the `attribute` keyword) fails with an opaque `no viable alternative at input 'add'` | ALTER ENTITY requires the `attribute` keyword (SQL-shaped); the raw ANTLR error doesn't say so | `mdl/visitor/visitor.go` (`enhanceErrorMessage` / `addMissingAttributeRe`) | Source-aware hint: when the offending line is `add :` and `` isn't a real clause keyword (attribute/column/index/event/value/…), append the correct `add attribute : ` form. Gated to the primary "no viable alternative" error. Test `TestAlterEntityMissingAttributeKeywordHint`. traceops #16 | | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | | `grant view on page` / `grant execute on microflow\|nanoflow` / `grant access on odata\|published rest service` to a role from **another module** passes `mxcli check`/`exec` but fails the Mendix build with **CE0148 "reselect roles"** — the own-module role works | Document access (page/microflow/nanoflow/service `AllowedModuleRoles`) may only reference the document's **own** module roles; Studio Pro's picker only offers those. The grant path wrote `role.Module + "." + role.Name` verbatim with no same-module check (only `validateModuleRole` = role-exists-in-its-module), so a cross-module reference reached the model. The MOVE path already guarded this (`remapDocumentAccessRoles`) — GRANT didn't | `mdl/executor/cmd_security_defaults.go` (`checkDocumentAccessRolesSameModule`) + the 5 grant handlers in `mdl/executor/cmd_security_write.go` | Pre-check each grant: reject when any `role.Module != docModule` with an actionable message (name the doc's module + suggest the own-module role). Reject (don't silently remap) — a GRANT is explicit, so a wrong role/doc shouldn't be substituted. Wired into page/microflow/nanoflow/OData/published-REST grants. Repro `mdl-examples/bug-tests/ce0148-cross-module-grant.mdl` | | CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` | diff --git a/.claude/skills/mendix/create-page.md b/.claude/skills/mendix/create-page.md index 91332e9c3..ec1e931fa 100644 --- a/.claude/skills/mendix/create-page.md +++ b/.claude/skills/mendix/create-page.md @@ -553,8 +553,12 @@ datepicker dpCreated (label: 'Created Date', attribute: CreatedDate) -- Enumeration mode (attribute is an enum type): combobox cbCountry (label: 'Country', attribute: Country) --- Association mode (Attribute = association, DataSource = target entity, CaptionAttribute = display attr): -combobox cmbCustomer (label: 'Customer', attribute: Order_Customer, datasource: database MyModule.Customer, CaptionAttribute: Name) +-- Association mode: bind a reference. Requires the option DataSource (the target +-- entity whose objects fill the dropdown) AND a CaptionAttribute (display value). +-- The reference can be given as `Association:` or, equivalently, `attribute:`. +combobox cmbCustomer (label: 'Customer', Association: Order_Customer, datasource: database MyModule.Customer, CaptionAttribute: Name) +-- WRONG: `combobox (Association: X)` with no datasource — mxcli check errors +-- MDL-WIDGET16 (Mendix would otherwise drop the binding → CE0642). ``` ### DataView with Form Layout diff --git a/mdl-examples/bug-tests/traceops-11-12-16-strings-names.mdl b/mdl-examples/bug-tests/traceops-11-12-16-strings-names.mdl new file mode 100644 index 000000000..10087d18a --- /dev/null +++ b/mdl-examples/bug-tests/traceops-11-12-16-strings-names.mdl @@ -0,0 +1,48 @@ +-- ============================================================================ +-- TraceOps #11, #12, #16: string/identifier ergonomics +-- ============================================================================ +-- +-- #11 — MDL string literals may now span multiple lines. Before, a newline +-- inside '...' terminated the token ("missing END"). Verified: mxbuild +-- accepts a multi-line String value. +-- +-- #12 — A bare MDL keyword (body, content, search, as, …) may be used unquoted +-- as a WIDGET name (matching attribute/placeholder names). Before, the +-- parser rejected it ("mismatched input 'body'"). Names that collide with +-- a keyword are stored fine and round-trip (DESCRIBE quotes them). +-- +-- #16 — (parse-error, not shown here) `alter entity … add Name: type` without +-- the `attribute` keyword now yields an actionable hint pointing at +-- `add attribute Name: type`, instead of the opaque "no viable +-- alternative at input 'addName'". The correct form is used below. +-- +-- This script builds clean (`mx check` → 0 errors). +-- ============================================================================ + +create entity MyFirstModule.Note ( Name: String ); +/ + +-- #16: the correct ALTER ENTITY form (with the `attribute` keyword). +alter entity MyFirstModule.Note + add attribute Body: string(200); +/ + +-- #12: keyword widget names (body, content) used unquoted. +create page MyFirstModule.NoteView ( Title: 'Note', Layout: Atlas_Core.Atlas_Default ) { + container body { + container content { + dynamictext search (Content: 'Type to search') + } + } +} +/ + +-- #11: a multi-line string literal in a microflow. +create microflow MyFirstModule.Banner () returns String as $out +begin + declare $S String = 'Line one +Line two +Line three'; + return $S; +end; +/ diff --git a/mdl-examples/bug-tests/traceops-23-combobox-association.mdl b/mdl-examples/bug-tests/traceops-23-combobox-association.mdl new file mode 100644 index 000000000..5492dc850 --- /dev/null +++ b/mdl-examples/bug-tests/traceops-23-combobox-association.mdl @@ -0,0 +1,44 @@ +-- ============================================================================ +-- TraceOps #23: combobox could not bind an association via `Association:` +-- ============================================================================ +-- +-- Symptom: `combobox (Association: Mod.Ref, …)` — the parser accepted it, but +-- the writer only read the reference from the `attribute:` keyword, so +-- `Association:` was silently dropped and the build failed +-- [error] [CE0642] "Property 'Attribute' is required." +-- +-- Fixes: +-- 1) The widget engine now reads the reference from EITHER `Association:` or +-- `attribute:` (association-source), so the intuitive `Association:` keyword +-- works. Verified: the complete form below `mx check`s with 0 errors. +-- 2) An incomplete association combobox (a `Association:` with no `datasource:`) +-- is caught at check time with MDL-WIDGET16 and the full working syntax, +-- instead of silently dropping the binding and failing MxBuild with CE0642. +-- +-- A ComboBox in association mode needs THREE things: the reference +-- (`Association:`/`attribute:`), the option list (`datasource:` = the target +-- entity), and the display value (`CaptionAttribute:`). +-- ============================================================================ + +create entity MyFirstModule.Customer ( Name: String ); +create entity MyFirstModule.Order ( Number: String ); +create association MyFirstModule.Order_Customer + from MyFirstModule.Order to MyFirstModule.Customer; +/ + +create or replace page MyFirstModule.OrderEdit +( Title: 'Order', Layout: Atlas_Core.Atlas_Default, Params: { $Order: MyFirstModule.Order } ) +{ + dataview dv (datasource: $Order) { + -- Complete association combobox — builds clean. + combobox cmbCustomer ( + label: 'Customer', + Association: MyFirstModule.Order_Customer, + datasource: database MyFirstModule.Customer, + CaptionAttribute: Name + ) + } +} +/ + +describe page MyFirstModule.OrderEdit; diff --git a/mdl-examples/bug-tests/traceops-27-doc-comment-between-clauses.mdl b/mdl-examples/bug-tests/traceops-27-doc-comment-between-clauses.mdl new file mode 100644 index 000000000..1e2a60d03 --- /dev/null +++ b/mdl-examples/bug-tests/traceops-27-doc-comment-between-clauses.mdl @@ -0,0 +1,34 @@ +-- ============================================================================ +-- TraceOps #27: a /** … */ doc comment between `alter entity … add attribute` +-- clauses was a parse error +-- ============================================================================ +-- +-- Symptom: placing a doc comment between two add-attribute clauses failed with +-- line 4:2 no viable alternative at input '/** … */add' +-- The alterEntityAction list accepted a doc comment only INSIDE an +-- attributeDefinition (after the ADD ATTRIBUTE keyword), not between clauses. +-- +-- Why `--` line comments are NOT an acceptable workaround: `--` comments are +-- discarded by the lexer, so they never reach the Mendix model. A `/** … */` +-- doc comment, by contrast, is PERSISTED as the attribute's Mendix +-- documentation (visible in Studio Pro). Rewriting to `--` silently drops the +-- documentation the author intended to store. +-- +-- Fix: alterEntityAction now accepts an optional leading docComment on ADD +-- ATTRIBUTE / ADD COLUMN, and the visitor attaches it as the added attribute's +-- documentation (mirroring a CREATE ENTITY attribute doc comment). +-- +-- After the fix this builds clean (`mx check` → 0 errors) and the doc comment is +-- stored on Zz2 (DESCRIBE ENTITY round-trips it). +-- ============================================================================ + +create entity MyFirstModule.Requirement ( Name: String ); +/ + +alter entity MyFirstModule.Requirement + add attribute Zz1: string(10) + /** Documentation for the second column, persisted into the Mendix model */ + add attribute Zz2: string(10); +/ + +describe entity MyFirstModule.Requirement; diff --git a/mdl-examples/bug-tests/traceops-9-10-17-dynamictext-listview.mdl b/mdl-examples/bug-tests/traceops-9-10-17-dynamictext-listview.mdl new file mode 100644 index 000000000..1f59a8a91 --- /dev/null +++ b/mdl-examples/bug-tests/traceops-9-10-17-dynamictext-listview.mdl @@ -0,0 +1,37 @@ +-- ============================================================================ +-- TraceOps findings #9, #10, #17: DYNAMICTEXT content + ListView PageSize +-- silently produced broken or dropped output +-- ============================================================================ +-- +-- #9 — `dynamictext x (Content: '')` persisted as `Content: '{1}'` with NO +-- parameters: an orphaned placeholder. mxcli check passed, but MxBuild +-- failed CE0720 "Place holder index 1 is greater than 0". Fix: empty +-- content with no params is a literal EMPTY template — no synthetic {1}. +-- +-- #10 — `dynamictext s (Content: '$318')` was treated as a variable reference: +-- persisted as `Content: '{1}', ContentParams: [{1} = $318]` (unbound) → +-- CE0402/CE1613 "attribute '$318' no longer exists". A `$` is a variable +-- ONLY when followed by a letter/underscore; `$318` is literal content. +-- +-- #17 — `listview (... PageSize: 200)` parsed cleanly but the writer hardcoded +-- PageSize=20 and dropped the value; describe showed no PageSize. Fix: +-- honor the parsed PageSize on write and round-trip it through describe. +-- +-- All three build clean (`mx check` → 0 errors) after the fix, and describe +-- round-trips: `x1` (empty), `s4v (Content: '$318')`, and `PageSize: 200`. +-- ============================================================================ + +create entity MyFirstModule.Item ( Label: String ); +/ + +create or replace page MyFirstModule.TraceOpsWidgets ( Title: 'W', Layout: Atlas_Core.Atlas_Default ) +{ + listview lvTree (datasource: database MyFirstModule.Item, PageSize: 200) { + dynamictext x1 (Content: '') -- #9: empty → literal empty template + dynamictext s4v (Content: '$318') -- #10: dollar+digits → literal text + dynamictext lbl (Content: Label) -- control: real attribute still binds + } +} +/ + +describe page MyFirstModule.TraceOpsWidgets; diff --git a/mdl/executor/cmd_pages_builder_traceops_test.go b/mdl/executor/cmd_pages_builder_traceops_test.go new file mode 100644 index 000000000..35c149d0d --- /dev/null +++ b/mdl/executor/cmd_pages_builder_traceops_test.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/model" +) + +func newBuilderForWidgetTest() *pageBuilder { + return &pageBuilder{ + widgetScope: map[string]model.ID{}, + paramScope: map[string]model.ID{}, + paramEntityNames: map[string]string{}, + localVariables: map[string]bool{}, + } +} + +// TestBuildDynamicTextV3_EmptyContent guards traceops #9: `Content: ”` must +// produce an EMPTY template with no parameters — not an orphaned `{1}` +// placeholder, which Mendix rejects with CE0720 ("Place holder index 1 is +// greater than 0"). +func TestBuildDynamicTextV3_EmptyContent(t *testing.T) { + pb := newBuilderForWidgetTest() + w := &ast.WidgetV3{Name: "x1", Type: "dynamictext", Properties: map[string]any{"Content": ""}} + dt, err := pb.buildDynamicTextV3(w) + if err != nil { + t.Fatalf("build: %v", err) + } + if got := dt.Content.Template.Translations["en_US"]; got != "" { + t.Errorf("empty Content template = %q, want \"\" (no orphaned {1})", got) + } + if len(dt.Content.Parameters) != 0 { + t.Errorf("empty Content params = %d, want 0", len(dt.Content.Parameters)) + } +} + +// TestBuildDynamicTextV3_DollarDigitLiteral guards traceops #10: `Content: '$318'` +// (a `$` followed by digits) is NOT a valid variable reference and must be kept +// as literal content — not turned into an unbound `{1}` parameter (which failed +// the build with CE0402/CE1613 "attribute '$318' no longer exists"). +func TestBuildDynamicTextV3_DollarDigitLiteral(t *testing.T) { + pb := newBuilderForWidgetTest() + w := &ast.WidgetV3{Name: "s4v", Type: "dynamictext", Properties: map[string]any{"Content": "$318"}} + dt, err := pb.buildDynamicTextV3(w) + if err != nil { + t.Fatalf("build: %v", err) + } + if got := dt.Content.Template.Translations["en_US"]; got != "$318" { + t.Errorf("literal Content template = %q, want \"$318\"", got) + } + if len(dt.Content.Parameters) != 0 { + t.Errorf("literal '$318' params = %d, want 0 (not an unbound {1})", len(dt.Content.Parameters)) + } +} + +// TestBuildDynamicTextV3_VariableStillBinds ensures the #10 fix didn't break a +// genuine variable reference: `$currentObject/Attr` and `$var` must still +// auto-generate a `{1}` template with the value as a parameter. +func TestBuildDynamicTextV3_VariableStillBinds(t *testing.T) { + pb := newBuilderForWidgetTest() + pb.localVariables["v"] = true + w := &ast.WidgetV3{Name: "d", Type: "dynamictext", Properties: map[string]any{"Content": "$v"}} + dt, err := pb.buildDynamicTextV3(w) + if err != nil { + t.Fatalf("build: %v", err) + } + if got := dt.Content.Template.Translations["en_US"]; got != "{1}" { + t.Errorf("variable Content template = %q, want \"{1}\"", got) + } + if len(dt.Content.Parameters) != 1 { + t.Errorf("variable Content params = %d, want 1", len(dt.Content.Parameters)) + } +} + +func TestIsDynamicTextVariableRef(t *testing.T) { + cases := map[string]bool{ + "$var": true, + "$widget.Attr": true, + "$currentObject/A/Name": true, + "$_x": true, + "$318": false, // dollar + digit — literal, not a variable (#10) + "$": false, + "plain": false, + "": false, + } + for in, want := range cases { + if got := isDynamicTextVariableRef(in); got != want { + t.Errorf("isDynamicTextVariableRef(%q) = %v, want %v", in, got, want) + } + } +} + +// TestBuildListViewV3_PageSize guards traceops #17: an explicit PageSize must be +// honored, not silently discarded (the writer hardcoded 20 regardless of the +// parsed AST value). +func TestBuildListViewV3_PageSize(t *testing.T) { + pb := newBuilderForWidgetTest() + w := &ast.WidgetV3{Name: "lv", Type: "listview", Properties: map[string]any{"PageSize": 200}} + lv, err := pb.buildListViewV3(w) + if err != nil { + t.Fatalf("build: %v", err) + } + if lv.PageSize != 200 { + t.Errorf("PageSize = %d, want 200", lv.PageSize) + } + + // No PageSize property → default 20. + pb2 := newBuilderForWidgetTest() + w2 := &ast.WidgetV3{Name: "lv2", Type: "listview", Properties: map[string]any{}} + lv2, err := pb2.buildListViewV3(w2) + if err != nil { + t.Fatalf("build default: %v", err) + } + if lv2.PageSize != 20 { + t.Errorf("default PageSize = %d, want 20", lv2.PageSize) + } +} diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index dd2f21242..85d970337 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -241,6 +241,12 @@ func (pb *pageBuilder) buildListViewV3(w *ast.WidgetV3) (*pages.ListView, error) PageSize: 20, } + // Honor an explicit PageSize (the property was parsed but previously dropped — + // the writer hardcoded 20 regardless, so the app always paged at 20). (traceops #17) + if ps := w.GetIntProp("PageSize"); ps > 0 { + lv.PageSize = ps + } + // Handle DataSource if ds := w.GetDataSource(); ds != nil { dataSource, entityName, err := pb.buildDataSourceV3(ds) @@ -497,6 +503,19 @@ func (pb *pageBuilder) buildTextWidgetV3(w *ast.WidgetV3) (*pages.Text, error) { return st, nil } +// dynamicTextVariableRe matches a DYNAMICTEXT Content value that is a variable +// reference: a `$` followed by a valid identifier start (letter or underscore), +// e.g. `$var`, `$widget.Attr`, `$currentObject/Assoc/Attr`. It deliberately does +// NOT match `$` followed by a digit (`$318`) — that is not a valid Mendix +// variable and must be treated as literal content. (traceops #10) +var dynamicTextVariableRe = regexp.MustCompile(`^\$[A-Za-z_]`) + +// isDynamicTextVariableRef reports whether a DYNAMICTEXT Content value should be +// auto-bound as a variable parameter rather than emitted as literal text. +func isDynamicTextVariableRef(content string) bool { + return dynamicTextVariableRe.MatchString(content) +} + func (pb *pageBuilder) buildDynamicTextV3(w *ast.WidgetV3) (*pages.DynamicText, error) { dt := &pages.DynamicText{ BaseWidget: pages.BaseWidget{ @@ -537,7 +556,12 @@ func (pb *pageBuilder) buildDynamicTextV3(w *ast.WidgetV3) (*pages.DynamicText, // This avoids matching strings like "Version 1.0" or "Dashboard - V2.1" isEntityPath = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*\.[A-Za-z_][A-Za-z0-9_]*$`).MatchString(content) } - if strings.HasPrefix(content, "$") || isEntityPath { + // A `$`-prefixed value is a variable reference ONLY when the `$` is followed + // by a valid identifier start (letter/underscore): `$var`, `$widget.Attr`. + // `$318` (dollar + digits) is not a valid Mendix variable, so treat it as + // LITERAL content — otherwise it became an unbound `{1}` param and the build + // failed CE0402/CE1613 ("attribute '$318' no longer exists"). (traceops #10) + if isDynamicTextVariableRef(content) || isEntityPath { autoGeneratedParams = append(autoGeneratedParams, content) content = "{1}" } @@ -554,7 +578,12 @@ func (pb *pageBuilder) buildDynamicTextV3(w *ast.WidgetV3) (*pages.DynamicText, } } - if content == "" { + // Empty content with NO parameters is a literal empty template. Do NOT + // synthesize an orphaned `{1}` placeholder — Mendix rejects a template whose + // highest placeholder index exceeds its parameter count with CE0720 ("Place + // holder index 1 is greater than 0"). Only default to `{1}` when there is a + // parameter to bind to it. (traceops #9) + if content == "" && (len(autoGeneratedParams) > 0 || len(explicitParams) > 0) { content = "{1}" } diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index a194e5660..02d8d3786 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -791,6 +791,10 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { props = append(props, fmt.Sprintf("DataSource: %s", associationDataSourceExpr(w.DataSource))) } } + // Emit a non-default PageSize so it round-trips (Studio Pro's default is 20). + if w.PageSize != "" && w.PageSize != "20" { + props = append(props, fmt.Sprintf("PageSize: %s", w.PageSize)) + } props = appendAppearanceProps(props, w) if len(w.Children) > 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 9fabb10b2..87b9f4c9f 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -3,6 +3,7 @@ package executor import ( + "strconv" "strings" "github.com/mendixlabs/mxcli/model" @@ -387,6 +388,10 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s } else if inheritedCtx != "" { widget.EntityContext = inheritedCtx } + // Round-trip a non-default PageSize (the output formatter suppresses "20"). + if ps := extractInt(w["PageSize"]); ps > 0 { + widget.PageSize = strconv.Itoa(ps) + } widget.Children = parseListViewContent(ctx, w, widget.EntityContext) return []rawWidget{widget} diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index aa3afca31..ca64f76ad 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -120,6 +120,7 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc out = append(out, validateWidgetVisibility(w, registry, locationPrefix)...) out = append(out, validateStaticWidget(w, locationPrefix)...) out = append(out, validateDatasourceXPathAssociationEmpty(w, locationPrefix)...) + out = append(out, validateComboBoxAssociation(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. @@ -162,6 +163,33 @@ func validateDatasourceXPathAssociationEmpty(w *ast.WidgetV3, locationPrefix str return out } +// validateComboBoxAssociation flags an incomplete association-mode ComboBox. +// A ComboBox that binds an association (`Association:`) needs an options +// datasource (`DataSource:`, the entity whose objects populate the dropdown) and +// a caption attribute (`CaptionAttribute:`) — without a datasource the writer +// falls back to enumeration mode and drops the association, so the build fails +// CE0642 ("Property 'Attribute' is required"). Flag it at check time with the +// full, working syntax instead. (traceops #23) +func validateComboBoxAssociation(w *ast.WidgetV3, locationPrefix string) []linter.Violation { + if w == nil || !strings.EqualFold(w.Type, "combobox") { + return nil + } + if w.GetStringProp("Association") == "" { + return nil + } + if w.GetDataSource() != nil { + return nil // has an options datasource — association mode is complete enough + } + return []linter.Violation{{ + RuleID: "MDL-WIDGET16", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "%s: combobox `%s` binds an association (`Association:`) but has no `datasource:` — Mendix drops the binding and fails the build with CE0642 (\"Property 'Attribute' is required\")", + locationPrefix, w.Name), + Suggestion: "Association mode needs the option list and a caption: `combobox " + w.Name + " (Association: Module.Ref, datasource: database Module.TargetEntity, CaptionAttribute: Name)`.", + }} +} + // headingRenderModeRe matches the block-level dynamictext render modes H1–H6. var headingRenderModeRe = regexp.MustCompile(`(?i)^h[1-6]$`) diff --git a/mdl/executor/validate_widgets_test.go b/mdl/executor/validate_widgets_test.go index 8c49b3990..de1e8c594 100644 --- a/mdl/executor/validate_widgets_test.go +++ b/mdl/executor/validate_widgets_test.go @@ -356,3 +356,33 @@ func TestValidateWidgetVisibility(t *testing.T) { t.Errorf("unset property → got %d violations, want 0", len(v)) } } + +// TestValidateComboBoxAssociation guards traceops #23 (the MDL-WIDGET16 check): +// a combobox that binds an association needs an options datasource; without one +// Mendix drops the binding and fails the build with CE0642. +func TestValidateComboBoxAssociation(t *testing.T) { + dbDS := &ast.DataSourceV3{Type: "database", Reference: "M.Customer"} + cases := []struct { + name string + widget *ast.WidgetV3 + want bool // expect MDL-WIDGET16 + }{ + {"association, no datasource → flagged", &ast.WidgetV3{Type: "combobox", Name: "cb", Properties: map[string]any{"Association": "M.Order_Customer"}}, true}, + {"association + datasource → ok", &ast.WidgetV3{Type: "combobox", Name: "cb", Properties: map[string]any{"Association": "M.Order_Customer", "DataSource": dbDS}}, false}, + {"enumeration attribute only → ok", &ast.WidgetV3{Type: "combobox", Name: "cb", Properties: map[string]any{"Attribute": "Status"}}, false}, + {"not a combobox → ignored", &ast.WidgetV3{Type: "textbox", Name: "tb", Properties: map[string]any{"Association": "M.Order_Customer"}}, false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := false + for _, v := range validateComboBoxAssociation(c.widget, "page X") { + if v.RuleID == "MDL-WIDGET16" { + got = true + } + } + if got != c.want { + t.Errorf("MDL-WIDGET16 present = %v, want %v", got, c.want) + } + }) + } +} diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index eefb45363..bfd877c39 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -696,7 +696,16 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W } case "Association": - if attr := w.GetAttribute(); attr != "" { + // Accept either the explicit `Association:` keyword or the generic + // `attribute:` keyword for the reference the widget binds (e.g. a ComboBox + // in association mode). Before this, only `attribute:` was read, so a + // natural `combobox (Association: Mod.A_B, …)` silently dropped the binding + // and failed the build with CE0642. (traceops #23) + attr := w.GetStringProp("Association") + if attr == "" { + attr = w.GetAttribute() + } + if attr != "" { ctx.AssocPath = e.pageBuilder.resolveAssociationPath(attr) } ctx.EntityName = e.pageBuilder.entityContext diff --git a/mdl/grammar/MDLLexer.g4 b/mdl/grammar/MDLLexer.g4 index 93bbd5a69..cd9f1f98c 100644 --- a/mdl/grammar/MDLLexer.g4 +++ b/mdl/grammar/MDLLexer.g4 @@ -783,7 +783,7 @@ MENDIX_TOKEN: '[%' .*? '%]'; // String literals (single-quoted, with escape support) STRING_LITERAL - : '\'' ( ~['\r\n\\] | '\\' . | '\'\'' )* '\'' + : '\'' ( ~['\\] | '\\' . | '\'\'' )* '\'' ; // Dollar-quoted string literal (PostgreSQL style) for embedding code blocks diff --git a/mdl/grammar/domains/MDLDomainModel.g4 b/mdl/grammar/domains/MDLDomainModel.g4 index 8d0fd950b..8a83b361e 100644 --- a/mdl/grammar/domains/MDLDomainModel.g4 +++ b/mdl/grammar/domains/MDLDomainModel.g4 @@ -197,8 +197,8 @@ deleteBehavior // ============================================================================= alterEntityAction - : ADD ATTRIBUTE ifNotExists? attributeDefinition - | ADD COLUMN ifNotExists? attributeDefinition + : docComment? ADD ATTRIBUTE ifNotExists? attributeDefinition + | docComment? ADD COLUMN ifNotExists? attributeDefinition | RENAME ATTRIBUTE attributeName TO attributeName | RENAME COLUMN attributeName TO attributeName | MODIFY ATTRIBUTE attributeName COLON? dataType attributeConstraint* diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index 9d9f8710b..3d5eb935c 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -260,9 +260,9 @@ blockOverride // after a reserved keyword (e.g. "List", "Column") can be expressed. DESCRIBE // emits the quoted form for such names so its output re-parses. See issue #619. widgetV3 - : widgetTypeV3 (IDENTIFIER | QUOTED_IDENTIFIER) widgetPropertiesV3? widgetBodyV3? - | PLUGGABLEWIDGET STRING_LITERAL (IDENTIFIER | QUOTED_IDENTIFIER) widgetPropertiesV3? widgetBodyV3? // PLUGGABLEWIDGET 'widget.id' name - | CUSTOMWIDGET STRING_LITERAL (IDENTIFIER | QUOTED_IDENTIFIER) widgetPropertiesV3? widgetBodyV3? // CUSTOMWIDGET 'widget.id' name (legacy) + : widgetTypeV3 (IDENTIFIER | QUOTED_IDENTIFIER | keyword) widgetPropertiesV3? widgetBodyV3? + | PLUGGABLEWIDGET STRING_LITERAL (IDENTIFIER | QUOTED_IDENTIFIER | keyword) widgetPropertiesV3? widgetBodyV3? // PLUGGABLEWIDGET 'widget.id' name + | CUSTOMWIDGET STRING_LITERAL (IDENTIFIER | QUOTED_IDENTIFIER | keyword) widgetPropertiesV3? widgetBodyV3? // CUSTOMWIDGET 'widget.id' name (legacy) ; // V3 Widget types (same as V2) diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index 975f92571..0f5b1effb 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -194,12 +194,33 @@ func enhanceErrorMessage(msg, offendingLine string) string { } } + // `alter entity … add : ` without the `attribute` keyword. The raw + // error is an unhelpful "no viable alternative at input 'add'"; key off + // the source line, which unambiguously shows `add :` with a non-clause + // word. (traceops #16) + if m := addMissingAttributeRe.FindStringSubmatch(offendingLine); m != nil && strings.Contains(msg, "no viable alternative") { + switch strings.ToLower(m[1]) { + case "attribute", "column", "index", "event", "association", "value", "role", "handler": + // a real clause keyword — not the missing-keyword mistake + default: + return fmt.Sprintf("%s\n\n ALTER ENTITY needs the `attribute` keyword before a new attribute:\n"+ + " alter entity Module.Entity add attribute %s: ; (correct)\n"+ + " alter entity Module.Entity add %s: ; (wrong)", msg, m[1], m[1]) + } + } + // Nothing matched a specific pattern — the location is precise but the fix // isn't spelled out. Point at the syntax reference so the correct form is one // command away. (Kept to one line since it can repeat across cascading errors.) return msg + " [see: mxcli syntax , e.g. entity | microflow | page]" } +// addMissingAttributeRe matches `add :` on a source line — the shape of an +// ALTER ENTITY add-attribute clause missing its `attribute` keyword. The captured +// word is checked against the real clause keywords in enhanceErrorMessage so a +// valid `add index`/`add event handler`/etc. is not mis-hinted. (traceops #16) +var addMissingAttributeRe = regexp.MustCompile(`(?i)\badd\s+([A-Za-z_]\w*)\s*:`) + // bareNotRe matches a bare `not $…` (not followed by `(`) on a source line — the // exact shape of the unparenthesized-negation mistake. Scoped to `not $var` to // stay false-positive-free (it won't fire on `not(...)`, `is not null`, etc.). diff --git a/mdl/visitor/visitor_entity.go b/mdl/visitor/visitor_entity.go index 993186d6d..0dc368b5c 100644 --- a/mdl/visitor/visitor_entity.go +++ b/mdl/visitor/visitor_entity.go @@ -534,6 +534,16 @@ func (b *Builder) ExitAlterEntityAction(ctx *parser.AlterEntityActionContext) { if ctx.ADD() != nil && (ctx.ATTRIBUTE() != nil || ctx.COLUMN() != nil) { if attrDef := ctx.AttributeDefinition(); attrDef != nil { attr := buildSingleAttribute(attrDef.(*parser.AttributeDefinitionContext)) + // A `/** … */` doc comment written BETWEEN clauses (before the + // ADD ATTRIBUTE keyword) documents the added attribute — same as a + // doc comment on a CREATE ENTITY attribute. `--` line comments are + // NOT an equivalent: they are discarded, whereas a doc comment is + // persisted as the attribute's Mendix documentation. (traceops #27) + if attr != nil && attr.Documentation == "" { + if docCtx := ctx.DocComment(); docCtx != nil { + attr.Documentation = extractDocComment(docCtx.GetText()) + } + } if attr != nil { b.statements = append(b.statements, &ast.AlterEntityStmt{ Name: name, diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 7aec490e9..795174d52 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -523,11 +523,16 @@ func buildWidgetV3(ctx parser.IWidgetV3Context, b *Builder) *ast.WidgetV3 { } // Get required identifier. The name may be quoted (QUOTED_IDENTIFIER) when it - // collides with a reserved keyword, e.g. a widget named "List". See issue #619. + // collides with a reserved keyword, e.g. a widget named "List" (issue #619), or + // a bare MDL keyword used unquoted as a name (`container body`, `dynamictext + // content`) — accepted by the grammar so a natural widget name is not blocked + // (traceops #12). if id := wCtx.IDENTIFIER(); id != nil { widget.Name = id.GetText() } else if qid := wCtx.QUOTED_IDENTIFIER(); qid != nil { widget.Name = unquoteIdentifier(qid.GetText()) + } else if kw := wCtx.Keyword(); kw != nil { + widget.Name = kw.GetText() } // Parse properties diff --git a/mdl/visitor/visitor_test.go b/mdl/visitor/visitor_test.go index f1cdcf1c0..5868dd1ff 100644 --- a/mdl/visitor/visitor_test.go +++ b/mdl/visitor/visitor_test.go @@ -1086,6 +1086,37 @@ ALTER ENTITY M.E DROP ATTRIBUTE PlainCol;`) } } +// TestAlterEntityAddAttributeDocComment guards traceops #27: a `/** … */` doc +// comment written BETWEEN `add attribute` clauses (before the ADD keyword) must +// parse and document the FOLLOWING attribute — `--` line comments are discarded, +// so they are not an equivalent workaround. +func TestAlterEntityAddAttributeDocComment(t *testing.T) { + prog, errs := Build(`alter entity M.E + add attribute Zz1: string(10) + /** doc for the second column */ + add attribute Zz2: string(10);`) + if len(errs) > 0 { + t.Fatalf("parse error: %v", errs[0]) + } + if len(prog.Statements) != 2 { + t.Fatalf("expected 2 statements (one per add attribute), got %d", len(prog.Statements)) + } + zz1 := prog.Statements[0].(*ast.AlterEntityStmt) + if zz1.Attribute == nil || zz1.Attribute.Name != "Zz1" { + t.Fatalf("statement 0 is not ADD Zz1: %+v", zz1) + } + if zz1.Attribute.Documentation != "" { + t.Errorf("Zz1 should have no doc, got %q", zz1.Attribute.Documentation) + } + zz2 := prog.Statements[1].(*ast.AlterEntityStmt) + if zz2.Attribute == nil || zz2.Attribute.Name != "Zz2" { + t.Fatalf("statement 1 is not ADD Zz2: %+v", zz2) + } + if zz2.Attribute.Documentation != "doc for the second column" { + t.Errorf("Zz2 documentation = %q, want %q", zz2.Attribute.Documentation, "doc for the second column") + } +} + // TestAlterEntityAddAttribute verifies ALTER ENTITY ADD ATTRIBUTE produces correct AST. func TestAlterEntityAddAttribute(t *testing.T) { input := `ALTER ENTITY MyModule.Customer @@ -2520,3 +2551,69 @@ func TestEnhanceErrorMessage_XPathArithmetic(t *testing.T) { t.Errorf("did not expect XPath hint for a non-constraint arithmetic error:\n%s", got) } } + +// TestKeywordWidgetName guards traceops #12: an MDL keyword (body, content, +// search, as) used unquoted as a widget name must parse and keep the name, +// instead of being rejected ("mismatched input 'body'") or silently dropped. +func TestKeywordWidgetName(t *testing.T) { + prog, errs := Build(`create page M.P ( Title: 'P', Layout: Atlas_Core.Atlas_Default ) { + container body { + dynamictext content (Content: 'x') + } +}`) + if len(errs) > 0 { + t.Fatalf("parse error: %v", errs[0]) + } + page := prog.Statements[0].(*ast.CreatePageStmtV3) + if len(page.Widgets) != 1 || page.Widgets[0].Name != "body" { + t.Fatalf("outer widget name = %q, want body", widgetName(page.Widgets)) + } + inner := page.Widgets[0].Children + if len(inner) != 1 || inner[0].Name != "content" { + t.Errorf("inner widget name = %q, want content", widgetName(inner)) + } +} + +func widgetName(ws []*ast.WidgetV3) string { + if len(ws) == 0 { + return "" + } + return ws[0].Name +} + +// TestMultiLineStringLiteral guards traceops #11: a single-quoted string literal +// may span lines. Before this the newline terminated the token ("missing END"). +func TestMultiLineStringLiteral(t *testing.T) { + prog, errs := Build("create microflow M.T () returns String as $out\nbegin\n declare $S String = 'line one\nline two';\n return $S;\nend;") + if len(errs) > 0 { + t.Fatalf("parse error: %v", errs[0]) + } + if len(prog.Statements) != 1 { + t.Fatalf("expected 1 statement, got %d", len(prog.Statements)) + } +} + +// TestAlterEntityMissingAttributeKeywordHint guards traceops #16: the parse +// error for `alter entity … add : ` (missing the `attribute` +// keyword) carries an actionable hint, and a real clause (`add index …`) does not. +func TestAlterEntityMissingAttributeKeywordHint(t *testing.T) { + _, errs := Build(`alter entity M.E add Name: string(20);`) + if len(errs) == 0 { + t.Fatal("expected a parse error for the missing `attribute` keyword") + } + var joined string + for _, e := range errs { + joined += e.Error() + "\n" + } + if !strings.Contains(joined, "needs the `attribute` keyword") { + t.Errorf("error missing the attribute-keyword hint:\n%s", joined) + } + + // A valid `add index` clause must NOT be mis-hinted. + _, errs2 := Build(`alter entity M.E add index idx (Name);`) + for _, e := range errs2 { + if strings.Contains(e.Error(), "needs the `attribute` keyword") { + t.Errorf("add index wrongly hinted: %v", e) + } + } +}