From c1f8c899bde115e7a92011a1611b67d1d61fe497 Mon Sep 17 00:00:00 2001 From: Ako Date: Fri, 25 Sep 2026 14:15:25 +0000 Subject: [PATCH] fix(pages): refuse an attribute binding with no object to bind to (MDL-WIDGET34) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `textbox t (Attribute: FullName)` at the top of a page, snippet or plain container has no entity to qualify the name with; the writer stores anything shorter than Module.Entity.Attribute as `AttributeRef: null`. Plain `check` passed, `exec --no-check` and `alter page … insert` said success, and mxbuild 11.13.0 failed the page: CE0544 + CE7005 on text box, text area, date picker, check box, radio buttons and drop-down, CE0402 on a dynamic text, CE0642 on a combo box. A qualified attribute there is stored and fails as well (CE0544 / CE2421 / CE1365 / CE7247 + CE7006). `Attribute: $P/Name` and `$currentObject/Name` parse as a data-source expression no builder reads, so they were dropped even inside a data view. - check: MDL-WIDGET34 in the widget-tree walk (no project needed), using the MDL-PAGEARG01 three-state context — refuses bare and qualified bindings at a document root outside any data widget, and the `$x/Attr` spelling anywhere. ALTER's subtree walk (unknown context) stands down. - build: the six input builders, dynamic text and the pluggable engine's primary `Attribute:` mapping refuse with the widget named, so nothing is written; with an unknown context only a bare name with no entity is refused, so qualified bindings inside an unresolvable flow source (excluded ShareFeedback_Logo) keep building. Two unit tests built inputs with no entity in scope and one asserted the bare `Title` reference counted as bound; they now set an entity context. Verified on a copy of a Mendix 11.13.0 project: 22 mxbuild errors before, every case refused after with nothing written; controls (data view, list view, gallery, data grid, snippet data view, ALTER into a data view) build at 0 errors; describe -> exec round trip 17/17 pages, 4/4 snippets. Co-Authored-By: Claude Opus 5.5 --- .../fix-issue/findings/mdl-executor.jsonl | 1 + CHANGELOG.md | 1 + .../input-binding-without-context.fail.mdl | 36 ++++ .../input-binding-without-context.mdl | 49 ++++++ .../cmd_pages_builder_onchange_test.go | 3 + mdl/executor/cmd_pages_builder_v3_widgets.go | 24 +++ .../cmd_pages_input_binding_context.go | 150 ++++++++++++++++ .../cmd_pages_input_binding_context_test.go | 163 ++++++++++++++++++ mdl/executor/cmd_pages_popup_test.go | 7 + mdl/executor/validate_widgets.go | 2 + mdl/executor/widget_engine.go | 8 + 11 files changed, 444 insertions(+) create mode 100644 mdl-examples/bug-tests/input-binding-without-context.fail.mdl create mode 100644 mdl-examples/bug-tests/input-binding-without-context.mdl create mode 100644 mdl/executor/cmd_pages_input_binding_context.go create mode 100644 mdl/executor/cmd_pages_input_binding_context_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor.jsonl b/.claude/skills/fix-issue/findings/mdl-executor.jsonl index ed70b75f46..766f97e6b4 100644 --- a/.claude/skills/fix-issue/findings/mdl-executor.jsonl +++ b/.claude/skills/fix-issue/findings/mdl-executor.jsonl @@ -706,3 +706,4 @@ {"area": "mdl/executor", "symptom": "describe → exec of Feedback v4.0.2's EXCLUDED FeedbackModule.ShareFeedback_Logo refused `nanoflow not found: FeedbackModule.DS_FeedbackForm (data source)`; forcing it through left a project `mx check` could not LOAD (ArgumentNullException setting 'Attribute')", "cause": "The data view's flow is not in the project, so DESCRIBE had no context entity and printed every binding inside it bare (`Attribute: Subject`, `{1} = ImageB64`, `Visible: _showEmail in (…)`); exec had nothing to qualify them against, and a bare DomainModels$AttributeRef makes Mendix's loader throw. The stored model always had the full names", "file": "`mdl/executor/cmd_pages_describe_flowcontext.go` (`withQualifiedAttrs`, `describeAttr`), `mdl/executor/validate.go` (`unscopedBindings`), `mdl/executor/cmd_pages_builder_v3.go` (dangling DS flow kept by name), `mdl/backend/modelsdk/page_bare_attributeref.go` (`refuseBareAttributeRefs`)", "insight": "**The information was never lost — DESCRIBE threw it away**: every AttributeRef inside the unresolvable container still stored Module.Entity.Attr; shortening to the bare name is only safe where the reader can re-derive the entity, so key the shortening on whether the context resolved, not on habit. Measure the loader's tolerance before adding a write guard: 72 of 72 Studio Pro AttributeRefs in the project are qualified, so refusing a bare one refuses only writes that were already fatal — and turns a load-time stack trace into a statement-level error naming the attribute. Pair a blanket AST-level refusal with the exact failing slots (Attribute, CaptionAttribute, Visible-in, *Params) so the refusal names the widget, and keep the writer guard as the net for slots the walk does not know. Forced-fault run: hand-edit one qualified binding back to bare and confirm the refusal names it", "refs": ["ako/mxcli#675", "FeedbackModule.ShareFeedback_Logo"], "date": "2026-09-25"} {"area": "mdl/executor", "symptom": "describe → exec of Administration.Account_New fails `mx check` with [CE0642] \"Property 'Caption' is required.\" at Combo box 'comboBox2' (Account_Edit comboBox4 too); check and exec report success", "cause": "The ComboBox caption is an EXPRESSION (optionsSourceAssociationCaptionType=expression, optionsSourceAssociationCaptionExpression='$currentObject/Description'); describe read only the attribute caption, so the widget was rewritten with none. The write side already worked via the explicit-property pass, but MDL-WIDGET06 claimed both keys 'will be dropped'", "file": "`mdl/executor/cmd_pages_describe_parse.go` (combobox branch), `cmd_pages_describe_output.go`, `validate_widget_explicit_writable.go` (`persistedByExplicitPass`), `validate_widgets.go` (MDL-WIDGET06)", "insight": "**Resolve pluggable-widget properties by key before theorising**: a 20-line script mapping each Property's TypePointer to its PropertyKey through the widget's own Type.ObjectType settled the cause in one run, where the ndsl dump shows only anonymous values. Then TEST THE WRITE SIDE before building one: adding the two storage keys to the describe output by hand persisted both and built clean, which shrank the fix to describe + a false warning — no new syntax. A validator rule that asserts 'not persisted' must be tied to what the write path handles (here: the explicit pass writes Expression/TextTemplate/Attribute and scalar types), or it goes stale when the writer grows; the #643 test pinned the stale claim. A dedicated extractor for a 'known' pluggable widget silently drops every property it does not map — generic widgets emit them, known ones do not", "refs": ["ako/mxcli#664", "ako/mxcli#643"], "ce": ["CE0642"], "rules": ["MDL-WIDGET06"], "date": "2026-09-25"} {"area": "mdl/executor", "date": "2026-09-25", "symptom": "`mxcli check` passed a microflow with a commit inside a loop \u2014 one database round trip per iteration \u2014 that `mxcli lint` already flagged as CONV011. The defect surfaced only at project-wide lint time, long after the write.", "cause": "CONV011 reads the STORED model, so it cannot speak until `exec` has written the microflow; `check` reads the MDL and had no equivalent rule. The gap is temporal, not a missing capability on either side.", "file": "`mdl/executor/validate_commit_in_loop.go` (MDL-PERF01, hooked in `validate_microflow.go`), test `validate_commit_in_loop_test.go`, example `mdl-examples/bug-tests/1186-commit-in-loop.mdl`", "insight": "**When adding a check-time rule that anticipates an existing lint rule, pin the BOUNDARY to the lint rule's, not to the better one, and say why in the code.** A `while true` is built as an ExclusiveMerge back-edge rather than a LoopedActivity, so CONV011 (which walks LoopedActivity) does not flag a commit inside one. A commit there is arguably still N+1, and the tempting move is to be more correct \u2014 but two rules for one concept that disagree on what counts is precisely how a pair drifts, and this repo already has CONV010's three successive short allowlists as the worked example. If the case is worth reporting it is worth reporting in BOTH, and the stored-model rule is the one that sees the built flow. The test that pins this carries a control on the control: `while true` is exempt, `while ` is not, so the exemption cannot silently become 'never flag a while'. Name the sibling rule in the message (`lint reports this as CONV011`) so a reader hitting one recognises the other rather than filing it twice. Also worth reusing: `checkReturnInLoop` already had the depth-tracking walk over Loop/While/If/EnumSplit/InheritanceSplit \u2014 copying its shape got the nesting cases right for free.", "refs": ["ako/mxcli#681", "mendixlabs/mxcli#1186"], "rules": ["MDL-PERF01"]} +{"area": "mdl/executor", "date": "2026-09-25", "symptom": "`textbox t (Attribute: FullName)` at the top of a page (CREATE PAGE/SNIPPET, a plain container, or ALTER PAGE … INSERT at page level) passed plain `mxcli check`, `exec --no-check`/ALTER reported success, and `bson dump` showed `AttributeRef: null` — mxbuild 11.13.0: CE0544 \"This widget can only function inside a data context\" + CE7005 (textbox/textarea/datepicker/checkbox/radiobuttons/dropdown), CE0402 (dynamictext Attribute:), CE0642 (combobox). Qualified `Mod.Ent.Attr` there is stored and fails CE0544/CE2421/CE1365/CE7247 \"Move this widget into a data container\" + CE7006. `Attribute: $P/Attr` / `$currentObject/Attr` dropped even INSIDE a data view.", "cause": "resolveAttributePath returns the bare name when entityContext is \"\", and attributeRefToGen (and widgetobj setAttributeRefField) write nil for any path with < 2 dots, so the binding vanished between builder and writer; refuseBareAttributeRefs never sees it because no Attribute string is emitted. The only refusal (validatePageContextTree) runs in the --references phase for CREATE PAGE/SNIPPET, so plain check, --no-check and ALTER were unguarded. `$x/Attr` parses via the generic property rule as an *ast.DataSourceV3, so GetAttribute() returns \"\" and every builder skipped it.", "file": "mdl/executor/cmd_pages_input_binding_context.go (inputBindingProblem, checkInputBinding, validateInputBindingContext = MDL-WIDGET34), wired in cmd_pages_builder_v3_widgets.go (6 input builders + buildDynamicTextV3), widget_engine.go (primary Attribute mapping), validate_widgets.go (validateWidgetTreeIn); tests cmd_pages_input_binding_context_test.go; bug-tests input-binding-without-context{,.fail}.mdl", "insight": "Reuse the MDL-PAGEARG01 three-state context (pageArgContext known/present) rather than entityContext==\"\" as the 'outside a data container' signal: entityContext is also empty INSIDE a container whose flow cannot be resolved (excluded ShareFeedback_Logo), where DESCRIBE writes qualified names that must keep building — refusing qualified-on-empty-entity would have broken that round trip. So known-absent context refuses bare AND qualified; unknown context (ALTER) refuses only the bare name the writer provably nulls. Two existing unit tests (OnChangeSurvivesBuilder, DynamicTextV3_AttributeBinds) built inputs with NO entity and passed — the second asserted a bare `Title` AttributeRef counted as 'bound', i.e. it pinned the bug: when a fixture has no entity context, ask what the writer does with its output. The `$P/Attr` drop was found only by dumping the control page, not from the report — print the AST value type with a probe test before assuming a spelling reaches the builder. Evidence: 22 mxbuild errors before on the probe matrix; after, every case refused with nothing written, controls (dataview/listview/gallery/datagrid/snippet dataview/ALTER into dataview) 0 errors, 17/17 stock pages + 4/4 snippets describe→exec round trip.", "refs": ["MDL-WIDGET34"], "ce": ["CE0544", "CE7005", "CE0402", "CE0642", "CE2421", "CE1365", "CE7247", "CE7006"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ac4a90451..c82d830053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **An input bound to an attribute outside any data container was written with no binding** — `textbox t (Attribute: FullName)` at the top of a page (or of a snippet, or inside a plain container) has no entity to qualify the name with, and the writer stored it as `AttributeRef: null`. Plain `check` passed, `exec --no-check` and `alter page … insert` at page level said success, and mxbuild 11.13.0 failed the page with CE0544 "This widget can only function inside a data context" + CE7005 (text box, text area, date picker, check box, radio buttons, drop-down), CE0402 (dynamic text `Attribute:`) or CE0642 (combo box). A qualified attribute there is stored and fails the same way (CE0544 / CE2421 / CE1365 / CE7247 "Move this widget into a data container"). `Attribute: $P/Name` and `Attribute: $currentObject/Name` never parsed as an attribute path and were dropped even inside a data view. `check` now reports all three as **MDL-WIDGET34** (no project needed), and the page builder refuses them with the widget named, so nothing is written; `alter page` refuses a bare name it has no entity for. Place the widget in a data view, list view, gallery or data grid and bind the attribute by name. - **An expression property written in brackets was silently dropped** (mendixlabs/mxcli#750) — `dynamicclasses: [ if $currentObject/Featured then 'a' else 'b' ]`, the spelling #750 proposes, parsed as a list that no writer reads: `check` was clean, `exec` said `Created page`, and the widget was stored with no dynamic class. `alter page … set DynamicClasses = [ … ]` said `Altered page` and changed nothing, and a column's `DynamicCellClass` stored the list's text — tokens fused, `[if$x/Ythen'a'else'b']` — as its expression. Measured on a copy of a Mendix 11.14.0 project with the pre-fix binary. `mxcli check` now reports **MDL-WIDGET32** for `DynamicClasses` and `DynamicCellClass` written as a list (no project needed), and ALTER refuses it, so `check -p` reports that too. Write the expression quoted. - **`describe odata client` lost a quote level on a literal credential** — Studio Pro stores a literal user name as the expression `'abc'`, quotes included. `describe` printed `HttpUsername: 'abc'`, and re-executing that output stored `abc`, an identifier. `ClientCertificate`, header keys, `Version`, `MetadataUrl` and `Folder` were printed unescaped and did not re-parse when they held a quote. Every value is now quoted so a re-exec stores exactly what was read; measured against a Studio Pro-authored client decoded before and after a round trip. - **An OData client's proxy constant written `@Module.Const` was stored with the `@`** — `ProxyHost` / `ProxyPort` / `ProxyUsername` / `ProxyPassword` are by-name references to a constant, and Studio Pro stores the bare name (with `ProxyType: Override`). `"@Module.Const"` named no constant, so the proxy resolved to nothing. `create`, `create or modify` and `alter` now store the bare name for the bare, `@` and quoted-`@` spellings. The constant may be a String or an Integer. diff --git a/mdl-examples/bug-tests/input-binding-without-context.fail.mdl b/mdl-examples/bug-tests/input-binding-without-context.fail.mdl new file mode 100644 index 0000000000..16245c9d62 --- /dev/null +++ b/mdl-examples/bug-tests/input-binding-without-context.fail.mdl @@ -0,0 +1,36 @@ +-- An input widget bound to an attribute where nothing supplies an object. +-- +-- `textbox t (Attribute: FullName)` at the top of a page — outside any data +-- view, list view, gallery or data grid — has no entity to qualify `FullName` +-- with, and the writer stores an unqualified name as `AttributeRef: null`. Plain +-- `mxcli check` said "Check passed!", `exec --no-check` (and ALTER PAGE … INSERT +-- at page level, which `check --references` never saw) reported success, and +-- mxbuild 11.13.0 then failed the page: +-- +-- [CE0544] "This widget can only function inside a data context — like a data +-- view, list view, or a page with parameters or variables." +-- [CE7005] "No value selection has been made. Please select a value." +-- +-- The same drop, per kind: textarea / datepicker / checkbox / radiobuttons / +-- dropdown (CE0544 + CE7005), dynamictext `Attribute:` (CE0402 "No value +-- specified."), combobox (CE0642 "Property 'Attribute' is required."). A +-- QUALIFIED attribute there is stored and fails all the same (CE0544 / CE2421 / +-- CE1365 / CE7247 "Move this widget into a data container", with CE7006), and +-- `Attribute: $P/Name` never parsed as an attribute path, so it was dropped even +-- inside a data view. +-- +-- MDL-WIDGET34 now refuses each of these at check time, and the page builder +-- refuses them with the widget named, so nothing is written. This script must +-- FAIL check. The valid forms are in input-binding-without-context.mdl. +create module ProbeInBind; + +create persistent entity ProbeInBind.Person ( + FullName: String(100) +); + +create page ProbeInBind.Edit ( + title: 'Edit', + layout: Atlas_Core.Atlas_Default +) { + textbox t (Attribute: FullName) +}; diff --git a/mdl-examples/bug-tests/input-binding-without-context.mdl b/mdl-examples/bug-tests/input-binding-without-context.mdl new file mode 100644 index 0000000000..f819cdba67 --- /dev/null +++ b/mdl-examples/bug-tests/input-binding-without-context.mdl @@ -0,0 +1,49 @@ +-- What the MDL-WIDGET34 refusal must NOT touch: every binding below has an +-- object to bind to. Executed on a copy of a Mendix 11.13.0 project, the pages +-- build at 0 errors. This script must PASS check. +-- +-- The refused forms are in input-binding-without-context.fail.mdl. +create module ProbeInBindOk; + +create enumeration ProbeInBindOk.Color ( Red 'Red', Blue 'Blue' ); + +create persistent entity ProbeInBindOk.Person ( + FullName: String(100), + Fav: Enumeration(ProbeInBindOk.Color) +); + +create page ProbeInBindOk.Edit ( + params: { $P: ProbeInBindOk.Person }, + title: 'Edit', + layout: Atlas_Core.Atlas_Default +) { + layoutgrid lg { + row r1 { + column c1 (desktopwidth: autofill) { + -- The data view supplies the object; bare and qualified both bind. + dataview dv (DataSource: $P) { + textbox tName (Attribute: FullName) + textbox tQualified (Attribute: ProbeInBindOk.Person.FullName) + combobox cbFav (Attribute: Fav) + dynamictext dtName (Attribute: FullName) + } + } + } + } + -- A list widget's rows are the object. + listview lv (DataSource: database ProbeInBindOk.Person) { + dynamictext lvName (Attribute: FullName) + } + datagrid dg (DataSource: database ProbeInBindOk.Person) { + column cName (Attribute: FullName, Caption: 'Name') + } +}; + +-- A snippet binds through a data view over its parameter, exactly as a page does. +create snippet ProbeInBindOk.PersonFields ( + params: { $P: ProbeInBindOk.Person } +) { + dataview sdv (DataSource: $P) { + textbox sName (Attribute: FullName) + } +}; diff --git a/mdl/executor/cmd_pages_builder_onchange_test.go b/mdl/executor/cmd_pages_builder_onchange_test.go index a1c154607c..79b3aaffd5 100644 --- a/mdl/executor/cmd_pages_builder_onchange_test.go +++ b/mdl/executor/cmd_pages_builder_onchange_test.go @@ -62,6 +62,9 @@ func TestBuildWidgetV3_OnChangeSurvivesBuilder(t *testing.T) { h := mkHierarchy(mod) withContainer(h, mod.ID, mod.ID) pb := newPageBuilder(&mock.MockBackend{}, h, "Mod") + // Inside a data container: with no entity in scope the binding + // has nothing to resolve against and the widget is refused. + pb.entityContext = "Mod.Ent" w, err := pb.buildWidgetV3(mkOnChangeWidget(mdlType, "w1")) if err != nil { diff --git a/mdl/executor/cmd_pages_builder_v3_widgets.go b/mdl/executor/cmd_pages_builder_v3_widgets.go index 3a02960f8b..8e27eaf987 100644 --- a/mdl/executor/cmd_pages_builder_v3_widgets.go +++ b/mdl/executor/cmd_pages_builder_v3_widgets.go @@ -456,6 +456,9 @@ func (pb *pageBuilder) buildTextBoxV3(w *ast.WidgetV3) (*pages.TextBox, error) { if attr := w.GetAttribute(); attr != "" { tb.AttributePath, tb.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Forms$TextBox.IsPasswordBox. The writer always carried it; nothing parsed // it, so a describe → exec round trip turned a password field into a @@ -516,6 +519,9 @@ func (pb *pageBuilder) buildTextAreaV3(w *ast.WidgetV3) (*pages.TextArea, error) if attr := w.GetAttribute(); attr != "" { ta.AttributePath, ta.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Handle Label if label := w.GetLabel(); label != "" { @@ -549,6 +555,9 @@ func (pb *pageBuilder) buildDatePickerV3(w *ast.WidgetV3) (*pages.DatePicker, er if attr := w.GetAttribute(); attr != "" { dp.AttributePath, dp.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Handle Label if label := w.GetLabel(); label != "" { @@ -582,6 +591,9 @@ func (pb *pageBuilder) buildDropdownV3(w *ast.WidgetV3) (*pages.DropDown, error) if attr := w.GetAttribute(); attr != "" { dd.AttributePath, dd.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Handle Label if label := w.GetLabel(); label != "" { @@ -615,6 +627,9 @@ func (pb *pageBuilder) buildCheckBoxV3(w *ast.WidgetV3) (*pages.CheckBox, error) if attr := w.GetAttribute(); attr != "" { cb.AttributePath, cb.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Handle Label if label := w.GetLabel(); label != "" { @@ -683,6 +698,9 @@ func (pb *pageBuilder) buildRadioButtonsV3(w *ast.WidgetV3) (*pages.RadioButtons if attr := w.GetAttribute(); attr != "" { rb.AttributePath, rb.AttributeRefSteps = pb.resolveInputAttribute(attr) } + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } // Handle OnChange (the "On change" client action) if err := pb.applyOnChangeV3(w, &rb.OnChangeAction); err != nil { @@ -781,6 +799,12 @@ func (pb *pageBuilder) buildDynamicTextV3(w *ast.WidgetV3) (*pages.DynamicText, // to `ContentParams: [{1} = X]`. Without this the Attribute was dropped, leaving // an orphaned `{1}` template with no parameter — which Studio Pro can't open // (NullReferenceException in ClientTemplateFormPart.CollectControls). + // Outside a data container that parameter binds nothing (CE0402). + if explicitParams == nil && len(autoGeneratedParams) == 0 { + if err := pb.checkInputBinding(w, pb.entityContext); err != nil { + return nil, err + } + } if attr := w.GetAttribute(); attr != "" && explicitParams == nil && len(autoGeneratedParams) == 0 { autoGeneratedParams = append(autoGeneratedParams, attr) if content == "" { diff --git a/mdl/executor/cmd_pages_input_binding_context.go b/mdl/executor/cmd_pages_input_binding_context.go new file mode 100644 index 0000000000..b16b114b50 --- /dev/null +++ b/mdl/executor/cmd_pages_input_binding_context.go @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "strings" + + "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/linter" +) + +// An `Attribute:` binding is only storable when there is an object to bind to. +// +// The writer stores a binding as a DomainModels$AttributeRef holding +// Module.Entity.Attribute and writes a null reference for anything shorter +// (attributeRefToGen), so a bare name nothing could qualify — `textbox t +// (Attribute: FullName)` at the top of a page, where there is no entity in +// scope — went out as `AttributeRef: null`. `exec` said "Created page", and +// mxbuild 11.13.0 reported, per widget kind: +// +// textbox/textarea/datepicker/checkbox/radiobuttons/dropdown +// CE0544 "This widget can only function inside a data context" + CE7005 +// dynamictext CE0402 "No value specified." +// combobox CE0642 "Property 'Attribute' is required." +// +// Qualifying the name does not rescue it: the reference is stored, but outside a +// data container there is no object of that entity to edit, and mxbuild rejects +// that too — CE0544/CE2421 on a text box, CE1365 on a dynamic text, CE7247 on a +// combo box ("Move this widget into a data container"), each with CE7006. +// +// `Attribute: $P/Name` is a third shape of the same drop: it does not parse as +// an attribute path but as a data-source expression, which no input builder +// reads, so it was dropped INSIDE a data view as well as outside one. +// +// `mxcli check -p … --references` already refused the first two on CREATE +// PAGE/SNIPPET (validatePageContextTree), but plain `mxcli check`, `exec +// --no-check` and ALTER PAGE did not. The check-time rule below needs no +// project; the builder guard catches what reaches the writer by any route. + +// inputBindingProblem says why w's `Attribute:` binding cannot be stored, or "" +// when it can (or the widget has none). +// +// c is the context the widget sits in. noEntity says that nothing could qualify +// a bare name here — the builder knows that; the check-time walk has no project +// to ask, passes false, and lets the context alone decide. +func inputBindingProblem(w *ast.WidgetV3, c pageArgContext, noEntity bool) string { + raw, present := lookupPropCI(w, "Attribute") + if !present || raw == nil { + return "" + } + kind := strings.ToLower(w.Type) + attr, isString := raw.(string) + if !isString { + return fmt.Sprintf("%s `%s`: `Attribute: %s` is not an attribute binding MDL can store — the widget "+ + "would be written with no binding at all, inside a data view or outside one. Bind the attribute by "+ + "name inside a data container over that object: `dataview dv (DataSource: $Param) { %s %s "+ + "(Attribute: Name) }` ($currentObject/Name is written `Name`)", + kind, w.Name, nonStringAttributeText(raw), kind, w.Name) + } + if attr == "" { + return "" + } + qualified := !strings.Contains(attr, "/") && strings.Count(attr, ".") >= 2 + if c.known && !c.present { + if qualified { + return fmt.Sprintf("%s `%s`: attribute `%s` is qualified, but the widget is not inside a data view, "+ + "list view, gallery or data grid, so there is no object of that entity for it to show or edit — "+ + "mxbuild rejects it (CE0544 \"This widget can only function inside a data context\", or \"Move "+ + "this widget into a data container\"). Place it inside a data container over that entity", + kind, w.Name, attr) + } + return fmt.Sprintf("%s `%s`: attribute `%s` has no entity to bind against — the widget is not inside "+ + "a data view, list view, gallery or data grid, so it would be written with no binding "+ + "(AttributeRef: null; mxbuild: %s). "+ + "Place it inside a data container, e.g. `dataview dv (DataSource: $Param) { %s %s (Attribute: %s) }`", + kind, w.Name, attr, unboundBuildError(kind), kind, w.Name, attr) + } + if noEntity && !qualified { + return fmt.Sprintf("%s `%s`: attribute `%s` has no entity to bind against — no enclosing data source "+ + "with a resolvable entity was found, so it would be written with no binding (AttributeRef: null). "+ + "Place it inside a data container or qualify it (Module.Entity.Attribute)", + kind, w.Name, attr) + } + return "" +} + +// unboundBuildError is what mxbuild 11.13.0 reports for a widget of this kind +// written with a null binding at the top of a page — measured per kind. +func unboundBuildError(kind string) string { + switch kind { + case "dynamictext": + return `CE0402 "No value specified."` + case "combobox": + return `CE0642 "Property 'Attribute' is required."` + } + return `CE0544 "This widget can only function inside a data context"` +} + +// nonStringAttributeText renders an `Attribute:` value that did not parse as an +// attribute path back into the spelling the author wrote, for the message. +func nonStringAttributeText(v any) string { + ds, ok := v.(*ast.DataSourceV3) + if !ok { + return fmt.Sprintf("%v", v) + } + switch { + case ds.ContextVariable != "": + return "$" + ds.ContextVariable + "/" + ds.Reference + case strings.HasPrefix(ds.Reference, "$"): + return ds.Reference + } + return ds.Type + " " + ds.Reference +} + +// checkInputBinding is the builder's refusal: the widget being built must not +// reach the writer with a binding the writer will turn into nothing. +func (pb *pageBuilder) checkInputBinding(w *ast.WidgetV3, entity string) error { + if msg := inputBindingProblem(w, pb.argCtx, entity == ""); msg != "" { + return mdlerrors.NewValidation(msg) + } + return nil +} + +// validateInputBindingContext is the check-time mirror (MDL-WIDGET34). It runs +// in the widget-tree walk, which has no project and so no entity: only the +// document-root context — known, and empty — refuses a string binding. ALTER PAGE's subtree walk has an unknown +// context and is left to the builder. +// +// A widget with a data source of its own binds its attributes against that +// source, so it is judged in the context it creates, not the one it sits in. +func validateInputBindingContext(w *ast.WidgetV3, c pageArgContext, locationPrefix string) []linter.Violation { + if w == nil { + return nil + } + if own := argContextForOwnAction(w, c); own != c { + c = own + } + msg := inputBindingProblem(w, c, false) + if msg == "" { + return nil + } + return []linter.Violation{{ + RuleID: "MDL-WIDGET34", + Severity: linter.SeverityError, + Message: locationPrefix + ": " + msg, + Suggestion: "An input or dynamic text shows an attribute of the object a data view, list view, gallery or data grid supplies — wrap it in one.", + }} +} diff --git a/mdl/executor/cmd_pages_input_binding_context_test.go b/mdl/executor/cmd_pages_input_binding_context_test.go new file mode 100644 index 0000000000..26b3ecd285 --- /dev/null +++ b/mdl/executor/cmd_pages_input_binding_context_test.go @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/model" +) + +// An input widget bound to an attribute where nothing supplies an object — +// `textbox t (Attribute: FullName)` at the top of a page — was built with its +// binding resolved to the bare name, which the writer cannot store: the widget +// went out with `AttributeRef: null`. `exec` said "Created page"; mxbuild +// 11.13.0 then reported CE0544 "This widget can only function inside a data +// context" plus CE7005 "No value selection has been made", CE0402 on a dynamic +// text and CE0642 "Property 'Attribute' is required" on a combo box. A +// QUALIFIED attribute there is stored, and is just as wrong: CE0544 / CE2421 / +// CE1365 / CE7247 "Move this widget into a data container". + +func testInputPB(entity string, argCtx pageArgContext) *pageBuilder { + return &pageBuilder{entityContext: entity, argCtx: argCtx, widgetScope: map[string]model.ID{}} +} + +func inputWidget(kind, name string, attr any) *ast.WidgetV3 { + return &ast.WidgetV3{Type: kind, Name: name, Properties: map[string]any{"Attribute": attr}} +} + +// The builder is the last gate before the writer, and ALTER PAGE reaches it +// without passing the check-time walk. It must refuse, naming the widget, rather +// than hand the writer a binding it will turn into null. +func TestBuildInputWithoutDataContextIsRefused(t *testing.T) { + cases := []struct { + kind, attr string + want []string + }{ + {"textbox", "FullName", []string{"`t`", "FullName", "data container"}}, + {"textarea", "Notes", []string{"`t`", "Notes", "data container"}}, + {"datepicker", "Born", []string{"`t`", "data container"}}, + {"checkbox", "Active", []string{"`t`", "data container"}}, + {"radiobuttons", "Fav", []string{"`t`", "data container"}}, + {"dropdown", "Fav", []string{"`t`", "data container"}}, + {"dynamictext", "FullName", []string{"`t`", "data container"}}, + // Qualified is stored, and fails the build all the same. + {"textbox", "AN.Person.FullName", []string{"`t`", "AN.Person.FullName", "data container"}}, + } + for _, c := range cases { + t.Run(c.kind+"/"+c.attr, func(t *testing.T) { + pb := testInputPB("", atDocumentRoot()) + _, err := pb.buildWidgetV3(inputWidget(c.kind, "t", c.attr)) + if err == nil { + t.Fatalf("%s bound to %q at page level was built — a bare name is written AttributeRef: null, a qualified one fails CE0544", c.kind, c.attr) + } + for _, w := range c.want { + if !strings.Contains(err.Error(), w) { + t.Errorf("refusal does not mention %q: %v", w, err) + } + } + }) + } +} + +// ALTER PAGE builds with an UNKNOWN context (the stored page is never walked). +// A qualified binding may be legitimate there — inside a container whose flow +// the project lacks, it is how DESCRIBE writes it — so only the binding the +// writer provably cannot store, a bare name with no entity, is refused. +func TestBuildInputUnknownContext(t *testing.T) { + pb := testInputPB("", pageArgContext{}) + if _, err := pb.buildWidgetV3(inputWidget("textbox", "t", "FullName")); err == nil { + t.Fatal("bare attribute with no entity in scope was built — the writer stores AttributeRef: null") + } else if !strings.Contains(err.Error(), "Module.Entity.Attribute") { + t.Errorf("refusal should offer the qualified form: %v", err) + } + if _, err := pb.buildWidgetV3(inputWidget("textbox", "t", "AN.Person.FullName")); err != nil { + t.Errorf("qualified attribute in an unknown context was refused: %v", err) + } +} + +// The control: inside a data container the same widgets build. +func TestBuildInputInsideDataContext(t *testing.T) { + for _, kind := range []string{"textbox", "textarea", "datepicker", "checkbox", "radiobuttons", "dropdown", "dynamictext"} { + pb := testInputPB("AN.Person", pageArgContext{known: true, present: true}) + if _, err := pb.buildWidgetV3(inputWidget(kind, "t", "FullName")); err != nil { + t.Errorf("%s inside a data container was refused: %v", kind, err) + } + } +} + +// `Attribute: $P/FullName` does not parse as an attribute path at all — it lands +// as a data-source expression, which no input builder reads — so the binding +// was dropped INSIDE a data view as well as outside one. +func TestBuildInputVariableRootedAttributeIsRefused(t *testing.T) { + ds := &ast.DataSourceV3{Type: "parameter", Reference: "$P"} + pb := testInputPB("AN.Person", pageArgContext{known: true, present: true}) + _, err := pb.buildWidgetV3(inputWidget("textbox", "t", ds)) + if err == nil { + t.Fatal("`Attribute: $P/…` was built — no builder reads it, so the binding is dropped") + } + if !strings.Contains(err.Error(), "`t`") { + t.Errorf("refusal does not name the widget: %v", err) + } +} + +// Check time: the same refusal from the widget-tree walk, which runs with no +// project, so `mxcli check script.mdl` reports it. +func TestValidateInputBindingWithoutDataContext(t *testing.T) { + registry := LoadWidgetRegistry("") + if registry == nil { + t.Fatal("LoadWidgetRegistry returned nil") + } + hits := func(tree []*ast.WidgetV3, subtree bool) (n int, msg string) { + vs := validateWidgetTree(tree, registry, "page AN.P") + if subtree { + vs = validateWidgetSubtree(tree, registry, "alter AN.P") + } + for _, v := range vs { + if v.RuleID == "MDL-WIDGET34" { + n++ + msg = v.Message + } + } + return + } + + for _, kind := range []string{"textbox", "textarea", "datepicker", "checkbox", "radiobuttons", "dropdown", "dynamictext", "combobox"} { + if n, _ := hits([]*ast.WidgetV3{inputWidget(kind, "t", "FullName")}, false); n != 1 { + t.Errorf("%s at page level: MDL-WIDGET34 = %d, want 1", kind, n) + } + } + if n, msg := hits([]*ast.WidgetV3{inputWidget("textbox", "t", "AN.Person.FullName")}, false); n != 1 { + t.Errorf("qualified textbox at page level: MDL-WIDGET34 = %d, want 1", n) + } else if !strings.Contains(msg, "CE0544") { + t.Errorf("message should name the build error: %s", msg) + } + // A plain container supplies nothing. + nested := []*ast.WidgetV3{{Type: "container", Name: "c", Children: []*ast.WidgetV3{inputWidget("textbox", "t", "FullName")}}} + if n, _ := hits(nested, false); n != 1 { + t.Errorf("textbox in a plain container: MDL-WIDGET34 = %d, want 1", n) + } + // Controls: a data view supplies the object; ALTER cannot say. + dv := []*ast.WidgetV3{{ + Type: "dataview", Name: "dv", + Properties: map[string]any{"DataSource": &ast.DataSourceV3{Type: "parameter", Reference: "$P"}}, + Children: []*ast.WidgetV3{inputWidget("textbox", "t", "FullName")}, + }} + if n, msg := hits(dv, false); n != 0 { + t.Errorf("textbox inside a data view was flagged: %s", msg) + } + if n, msg := hits([]*ast.WidgetV3{inputWidget("textbox", "t", "FullName")}, true); n != 0 { + t.Errorf("ALTER PAGE insert was flagged at check time, where the enclosing context is unknown: %s", msg) + } + // `$P/Attr` is dropped wherever it is written. + varRooted := []*ast.WidgetV3{{ + Type: "dataview", Name: "dv", + Properties: map[string]any{"DataSource": &ast.DataSourceV3{Type: "parameter", Reference: "$P"}}, + Children: []*ast.WidgetV3{inputWidget("textbox", "t", &ast.DataSourceV3{Type: "parameter", Reference: "$P"})}, + }} + if n, _ := hits(varRooted, false); n != 1 { + t.Errorf("`Attribute: $P/…` inside a data view: MDL-WIDGET34 = %d, want 1", n) + } +} diff --git a/mdl/executor/cmd_pages_popup_test.go b/mdl/executor/cmd_pages_popup_test.go index 65d69ec79b..1ed0df64ca 100644 --- a/mdl/executor/cmd_pages_popup_test.go +++ b/mdl/executor/cmd_pages_popup_test.go @@ -56,6 +56,10 @@ func TestBuildPageV3_PopupDefaults(t *testing.T) { // ClientTemplateParameter. func TestBuildDynamicTextV3_AttributeBinds(t *testing.T) { pb := newPopupPageBuilder() + // Inside a data container over M.Item. Without one there is no entity to + // qualify `Title` with, and the parameter used to be "bound" to the bare + // name — which the writer stores as a null AttributeRef (CE0402). + pb.entityContext = "M.Item" w := &ast.WidgetV3{Type: "dynamictext", Name: "txt", Properties: map[string]any{"Attribute": "Title"}} dt, err := pb.buildDynamicTextV3(w) if err != nil { @@ -74,6 +78,9 @@ func TestBuildDynamicTextV3_AttributeBinds(t *testing.T) { if p.AttributeRef == "" && p.Expression == "" && p.SourceVariable == "" { t.Error("parameter has no binding (AttributeRef/Expression/SourceVariable all empty)") } + if p.AttributeRef != "M.Item.Title" { + t.Errorf("AttributeRef = %q, want M.Item.Title — anything shorter is written as null", p.AttributeRef) + } } // A content-less dynamictext with no binding is unchanged (no panic, no params). diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index 9ef2ee5c3a..32d5ab4d6a 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -211,6 +211,8 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc // The widget's OWN action is judged in the context IT establishes, not the // one it sits in — a list widget's onClick is row-scoped (ako/mxcli#552). out = append(out, validateShowPageArguments(w, argContextForOwnAction(w, argCtx), locationPrefix)...) + // An `Attribute:` binding with no object to bind to is written empty. + out = append(out, validateInputBindingContext(w, argCtx, 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. diff --git a/mdl/executor/widget_engine.go b/mdl/executor/widget_engine.go index 5dd93bfb84..50f8fc2c80 100644 --- a/mdl/executor/widget_engine.go +++ b/mdl/executor/widget_engine.go @@ -1297,6 +1297,14 @@ func (e *PluggableWidgetEngine) resolveMapping(mapping PropertyMapping, w *ast.W // for those three and the hidden-property guard cannot catch it. // Not writing them in the first place does not depend on that data. attr = w.GetAttribute() + // Bound against the enclosing object rather than a source of the + // widget's own: with none, the binding is written empty (a combo + // box's CE0642 "Property 'Attribute' is required"). + if entity := e.entityContextFor(mapping.PropertyKey); entity == e.pageBuilder.entityContext { + if err := e.pageBuilder.checkInputBinding(w, entity); err != nil { + return nil, err + } + } } if attr != "" { // Against THIS property's datasource entity, which is the shared