From f1787d22512cec243f1ab91fb35942da2d387ac5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:25:30 +0000 Subject: [PATCH 01/13] fix(security): echo the rule a GRANT wrote, not a shared one After `grant write (Country) on entity FieldService.Customer to FieldService.Coordinator`, the Result line printed `read *` - the rights of the shared FabUser/Coordinator/Engineer rule - while the grant had written a separate rule for Coordinator alone. formatAccessRuleResult picked the first rule naming ANY granted role with the same XPath, but AddEntityAccessRule upserts by the exact role set plus XPath. On the GRANT path the echo now selects by that same key (sameRoleSet mirrors the backend's order-insensitive sameStringSet). REVOKE keeps the any-overlap match, which it wants. Finding: .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-grant-result-line-describes-shared-rule.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...ant-result-line-describes-shared-rule.json | 1 + mdl/executor/cmd_entities_access.go | 30 +++++-- mdl/executor/cmd_security_mock_test.go | 86 +++++++++++++++++++ 3 files changed, 110 insertions(+), 7 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-grant-result-line-describes-shared-rule.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-grant-result-line-describes-shared-rule.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-grant-result-line-describes-shared-rule.json new file mode 100644 index 0000000000..981e59ec4d --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-grant-result-line-describes-shared-rule.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-09","symptom":"`grant write (Country) on entity FieldService.Customer to FieldService.Coordinator` prints `Result: read *` — the rights of the shared FabUser/Coordinator/Engineer rule — while the grant actually wrote a separate rule for Coordinator alone (`show access` rule 3: read, write Country)","cause":"`formatAccessRuleResult` echoed the first rule naming ANY of the granted roles with the same XPath, but `AddEntityAccessRule` upserts by the EXACT role set plus XPath (`sameStringSet` in the backend), so whenever a role also sits in a broader rule the echo reported that rule instead","file":"`mdl/executor/cmd_entities_access.go` (`formatAccessRuleResult`, `sameRoleSet`)","insight":"**A post-write echo must select by the key the write used, not a looser one.** #936 already made the XPath part of the echo key; the role-set half stayed an any-overlap match, so the report and the write disagreed exactly when rules are additive — the case #936 made common. REVOKE keeps the any-overlap match deliberately (it narrows every rule a role appears in). Guard `TestGrantEntityAccess_ResultDescribesExactRoleSetRule` (control: a grant to the shared rule's exact role set still echoes it); stubbing the role-set check makes it print `Result: read *` again. Measured on chipcov6 (11.15.0): old binary `Result: read *`, fixed `Result: read (Country), write (Country)`","refs":["mendixlabs/mxcli#936"],"ce":[]} diff --git a/mdl/executor/cmd_entities_access.go b/mdl/executor/cmd_entities_access.go index 9c7bb1ce5c..1f06591eab 100644 --- a/mdl/executor/cmd_entities_access.go +++ b/mdl/executor/cmd_entities_access.go @@ -5,6 +5,7 @@ package executor import ( "fmt" + "slices" "strings" "github.com/mendixlabs/mxcli/mdl/visitor" @@ -200,9 +201,13 @@ func quoteMembers(members []string) []string { return out } -// xpath selects among the rules that name the same role — Mendix allows one per -// constraint — and anyXPath takes the first of them regardless, which is what -// REVOKE wants since it narrows every rule the roles appear in. +// With anyXPath false (GRANT) the rule is the one the backend upserted: exactly +// roleNames as a set, and the same xpath — Mendix allows one rule per constraint, +// and several rules may name one role. Matching on any overlap reported a shared +// rule (FabUser, Coordinator, Engineer) after a GRANT that wrote a separate rule +// for Coordinator alone. anyXPath takes the first rule naming any of the roles, +// whatever its constraint, which is what REVOKE wants since it narrows every rule +// the roles appear in. // // formatAccessRuleResult re-reads the entity and formats the resulting access state // for the given roles. Returns a string like " Result: CREATE, READ (Name, Price)\n". @@ -246,10 +251,10 @@ func formatAccessRuleResult(ctx *ExecContext, moduleName, entityName string, rol if matchCount == 0 { continue } - // A role may hold one rule per XPath constraint (#936), so the roles - // alone no longer identify the rule the statement touched — echoing the - // first match would report a different rule's rights back to the user. - if !anyXPath && rule.XPathConstraint != xpath { + // A role may hold one rule per XPath constraint (#936), and the backend + // keys the rule by role set plus constraint — echoing any other match + // would report a different rule's rights back to the user. + if !anyXPath && (rule.XPathConstraint != xpath || !sameRoleSet(rule.ModuleRoleNames, roleNames)) { continue } // Found a matching rule @@ -263,4 +268,15 @@ func formatAccessRuleResult(ctx *ExecContext, moduleName, entityName string, rol return " Result: (no access)\n" } +// sameRoleSet reports whether a and b hold the same role names, order-insensitive. +// It is the comparison AddEntityAccessRule upserts by (sameStringSet in +// mdl/backend/modelsdk), so the GRANT echo finds the rule that was written. +func sameRoleSet(a, b []string) bool { + ac := slices.Clone(a) + bc := slices.Clone(b) + slices.Sort(ac) + slices.Sort(bc) + return slices.Equal(ac, bc) +} + // --- Executor method wrappers for callers not yet migrated --- diff --git a/mdl/executor/cmd_security_mock_test.go b/mdl/executor/cmd_security_mock_test.go index 2e21bd6c8b..11d372f547 100644 --- a/mdl/executor/cmd_security_mock_test.go +++ b/mdl/executor/cmd_security_mock_test.go @@ -553,3 +553,89 @@ func TestRevokeEntityAccess_FakeRole_Issue399(t *testing.T) { assertContainsStr(t, err.Error(), "module role") assertContainsStr(t, err.Error(), "GhostRole") } + +// grantResultFixture builds an entity holding a shared read-only rule for +// {FabUser, Coordinator, Engineer} and a separate rule for {Coordinator} alone +// that writes Country — the state after `grant write (Country) … to Coordinator` +// against an entity that already had the shared rule. +func grantResultFixture(t *testing.T, grantRoles []string) string { + t.Helper() + mod := mkModule("FieldService") + h := mkHierarchy(mod) + + country := &domainmodel.Attribute{BaseElement: model.BaseElement{ID: nextID("attr")}, Name: "Country"} + entity := &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: nextID("ent")}, + ContainerID: mod.ID, + Name: "Customer", + Persistable: true, + Attributes: []*domainmodel.Attribute{country}, + AccessRules: []*domainmodel.AccessRule{ + { + ModuleRoleNames: []string{"FieldService.FabUser", "FieldService.Coordinator", "FieldService.Engineer"}, + DefaultMemberAccessRights: domainmodel.MemberAccessRightsReadOnly, + MemberAccesses: []*domainmodel.MemberAccess{ + {AttributeName: "FieldService.Customer.Country", AccessRights: domainmodel.MemberAccessRightsReadOnly}, + }, + }, + { + ModuleRoleNames: []string{"FieldService.Coordinator"}, + DefaultMemberAccessRights: domainmodel.MemberAccessRightsNone, + MemberAccesses: []*domainmodel.MemberAccess{ + {AttributeName: "FieldService.Customer.Country", AccessRights: domainmodel.MemberAccessRightsReadWrite}, + }, + }, + }, + } + dm := &domainmodel.DomainModel{ + BaseElement: model.BaseElement{ID: nextID("dm")}, + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{entity}, + } + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { return mod, nil }, + GetModuleSecurityFunc: func(moduleID model.ID) (*security.ModuleSecurity, error) { + return &security.ModuleSecurity{ModuleRoles: []*security.ModuleRole{ + {Name: "FabUser"}, {Name: "Coordinator"}, {Name: "Engineer"}, + }}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + AddEntityAccessRuleFunc: func(params backend.EntityAccessRuleParams) error { return nil }, + ReconcileMemberAccessesFunc: func(unitID model.ID, moduleName string) (int, error) { return 0, nil }, + } + + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + var roles []ast.QualifiedName + for _, r := range grantRoles { + roles = append(roles, ast.QualifiedName{Module: "FieldService", Name: r}) + } + rights := []ast.EntityAccessRight{{Type: ast.EntityAccessReadAll}} + if len(grantRoles) == 1 { + rights = []ast.EntityAccessRight{{Type: ast.EntityAccessWriteMembers, Members: []string{"Country"}}} + } + assertNoError(t, execGrantEntityAccess(ctx, &ast.GrantEntityAccessStmt{ + Entity: ast.QualifiedName{Module: "FieldService", Name: "Customer"}, + Roles: roles, + Rights: rights, + })) + return buf.String() +} + +// TestGrantEntityAccess_ResultDescribesExactRoleSetRule: the Result line after a +// GRANT must describe the rule the backend upserted — the one keyed by the exact +// role set plus XPath — not the first rule that merely mentions one of the +// granted roles. Granting to Coordinator alone used to echo the shared +// FabUser/Coordinator/Engineer rule ("read *"). +func TestGrantEntityAccess_ResultDescribesExactRoleSetRule(t *testing.T) { + out := grantResultFixture(t, []string{"Coordinator"}) + assertContainsStr(t, out, "Result: read (Country), write (Country)") + assertNotContainsStr(t, out, "Result: read *") + + // Control: granting to the shared rule's exact role set still describes it. + out = grantResultFixture(t, []string{"Engineer", "FabUser", "Coordinator"}) + assertContainsStr(t, out, "Result: read *") + assertNotContainsStr(t, out, "write (Country)") +} From ea92fc03b7fcce4c473efd8e8362513cf76d44e0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:27:55 +0000 Subject: [PATCH 02/13] fix(check): no MDL-WIDGET07 for an explicit pluggable widget id without a project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `mxcli check` with no project reported every property of a project's own pluggable widget (`pluggablewidget '' w (…)`) as "not recognized and will be silently dropped on write", although exec writes them all and the same check with -p is clean. 58 false warnings across the mxcli-ledger scripts. Cause: with no project the widget registry holds only the embedded definitions, so the explicit id resolves to no definition. TypeIsGeneric is set only for a bare-identifier type, so this form fell through to the built-in static allow-list. MDL-WIDGET25 already returns early for an explicit id with no project. Fix: skip the built-in checks for any widget carrying an explicit widget id, detected by a new explicitWidgetID helper now shared with MDL-WIDGET25. With a project an unknown id is still MDL-WIDGET25. Control test: a built-in `container c (bogus: 1)` still raises MDL-WIDGET07. Finding: .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-check-no-project-widget07-explicit-widget-id.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...o-project-widget07-explicit-widget-id.json | 1 + mdl/executor/validate_widget_kind.go | 9 +++- mdl/executor/validate_widgets.go | 8 +++- .../validate_widgets_explicit_id_test.go | 46 +++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-check-no-project-widget07-explicit-widget-id.json create mode 100644 mdl/executor/validate_widgets_explicit_id_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-check-no-project-widget07-explicit-widget-id.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-check-no-project-widget07-explicit-widget-id.json new file mode 100644 index 0000000000..15b1691ba2 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-check-no-project-widget07-explicit-widget-id.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-09", "symptom": "`mxcli check` with no project warns MDL-WIDGET07 \"property `p` is not recognized and will be silently dropped on write\" for every property of a project's own pluggable widget (`pluggablewidget 'ledger.widget.web.vegachart.VegaChart' chartSpark (spec: …, chartData: …)` drew 6), yet with `-p` it is clean and exec writes them all. 58 false warnings across the 41 mxcli-ledger scripts", "cause": "With no project `LoadWidgetRegistry(\"\")` holds only embedded definitions, so the explicit id resolves to nothing (def == nil); TypeIsGeneric is only set for a bare-identifier type, so the `pluggablewidget ''` form fell through to the BUILT-IN static allow-list (`validateStaticWidgetUnknownProps`). MDL-WIDGET25 already returned early for exactly this case", "file": "`mdl/executor/validate_widgets.go` (WIDGET07 gate), helper `explicitWidgetID` in `mdl/executor/validate_widget_kind.go`", "insight": "Three ways to name a widget (keyword, bare generic identifier, explicit id string) and the 'is this a built-in?' gate only excluded two. #1036 fixed the generic-identifier leg; the explicit-id leg had the same fall-through. Every rule that branches on 'no definition found' must say which of the three forms it means — and the no-project case is the one CI exercises, so it is where the gap shows. CONTROL: built-in `container c (bogus: 1)` still raises MDL-WIDGET07", "refs": ["#1036"]} diff --git a/mdl/executor/validate_widget_kind.go b/mdl/executor/validate_widget_kind.go index a2e4a0532e..483a4d4a63 100644 --- a/mdl/executor/validate_widget_kind.go +++ b/mdl/executor/validate_widget_kind.go @@ -29,6 +29,13 @@ import ( // def-driven body (slices 2-3) safe: those give up the parser's enforcement, so // the semantic check has to exist first. +// explicitWidgetID returns the widget id a `pluggablewidget ''` / +// `customwidget ''` form names, or "" for any other widget. +func explicitWidgetID(w *ast.WidgetV3) string { + id, _ := w.Properties["WidgetType"].(string) + return id +} + // validateWidgetKind reports a widget whose kind mxcli cannot resolve, and a // container keyword the parent's definition does not declare. func validateWidgetKind(w *ast.WidgetV3, registry *WidgetRegistry, parentDef *WidgetDefinition, @@ -40,7 +47,7 @@ func validateWidgetKind(w *ast.WidgetV3, registry *WidgetRegistry, parentDef *Wi // An explicit widget id that resolves to nothing. Only reachable through the // `pluggablewidget ''` / `customwidget ''` forms, where the id is a // string literal the parser cannot check. - if id, ok := w.Properties["WidgetType"].(string); ok && id != "" { + if id := explicitWidgetID(w); id != "" { // With no project there is nothing to be unknown RELATIVE TO: the // registry holds only the embedded widgets, so every real project // widget would be reported. `mxcli check` with no -p is the common diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index f4b4a9d329..f79be81059 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -291,8 +291,12 @@ func validateWidgetTreeIn(widgets []*ast.WidgetV3, registry *WidgetRegistry, loc // the wrong token — measured on `htmlelemnt frame (tagName: 'div')`, // which drew a `tagName` warning beside the real error. A built-in // (TypeIsGeneric false) keeps the check, since its properties are the - // only thing that can be wrong about it. - if def == nil && !isObjectListItem && !w.TypeIsGeneric { + // only thing that can be wrong about it. An explicit widget id + // (`pluggablewidget ''`) is never a built-in either: with a project + // an unknown one is MDL-WIDGET25, and with none the registry simply has + // no definition for the project's own widget — the built-in allow-list + // would report every property it has. + if def == nil && !isObjectListItem && !w.TypeIsGeneric && explicitWidgetID(w) == "" { out = append(out, validateStaticWidgetUnknownProps(w, locationPrefix)...) // #928: `editable:` on a widget Mendix gives no editability — same // "silently dropped on write" family, but the flat property diff --git a/mdl/executor/validate_widgets_explicit_id_test.go b/mdl/executor/validate_widgets_explicit_id_test.go new file mode 100644 index 0000000000..ca347853bc --- /dev/null +++ b/mdl/executor/validate_widgets_explicit_id_test.go @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/visitor" +) + +// widget07Hits checks one page against the no-project registry and returns +// the MDL-WIDGET07 violations it draws. +func widget07Hits(t *testing.T, body string) int { + t.Helper() + prog, errs := visitor.Build("create page Shop.P (title: 'P', layout: Atlas_Core.Atlas_Default) {\n" + body + "\n}") + if len(errs) > 0 { + t.Fatalf("parse: %v", errs) + } + n := 0 + for _, v := range ValidateWidgetPropertiesForStatement(prog.Statements[0], LoadWidgetRegistry("")) { + if v.RuleID == "MDL-WIDGET07" { + n++ + t.Logf("%s", v.Message) + } + } + return n +} + +// TestExplicitWidgetIDSkipsBuiltinAllowList: with no project, a project's own +// pluggable widget (`pluggablewidget ''`) has no definition, and fell +// through to the BUILT-IN allow-list — so every one of its properties was +// "silently dropped on write" while exec wrote them all. With a project an +// unknown id is MDL-WIDGET25, so the built-in list is never the right judge. +func TestExplicitWidgetIDSkipsBuiltinAllowList(t *testing.T) { + if n := widget07Hits(t, `pluggablewidget 'com.acme.Unknown' w (foo: 1, bar: 'x')`); n != 0 { + t.Errorf("explicit widget id with no project: %d MDL-WIDGET07, want 0", n) + } +} + +// TestBuiltinWidgetStillGetsAllowList is the control: the built-in check must +// still fire for a built-in widget under the same conditions. +func TestBuiltinWidgetStillGetsAllowList(t *testing.T) { + if n := widget07Hits(t, `container c (bogus: 1)`); n != 1 { + t.Errorf("built-in container with a bogus property: %d MDL-WIDGET07, want 1", n) + } +} From 4b1cfde330a2d05906bbdecfdab05d686eb6843a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:41:02 +0000 Subject: [PATCH 03/13] fix(navigation): report Unchanged when a navigation rewrite was elided Re-running `create or modify navigation Responsive ...` printed "Navigation profile 'Responsive' updated." on every run, although canon.Reconcile elided the write and no .mxunit changed. The handler printed its sentence with fmt.Fprintf after UpdateNavigationProfile returned, so it claimed a write it had no evidence for; the #890 sweep moved security and settings onto ctx.reportWrite but missed navigation. The update branch now reports through ctx.reportWrite, which says "Unchanged navigation profile ''" (via the run tally) when the write was offered and elided. The kept-menu-action note follows the write, as reportWrite's follow-up lines do elsewhere. Creating a profile always writes and is unchanged. Finding: .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-create-or-modify-navigation-reports-updated-when-write-elided.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...ion-reports-updated-when-write-elided.json | 1 + mdl/executor/cmd_navigation.go | 7 +- .../navigation_report_mutation_test.go | 65 +++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-create-or-modify-navigation-reports-updated-when-write-elided.json create mode 100644 mdl/executor/navigation_report_mutation_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-create-or-modify-navigation-reports-updated-when-write-elided.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-create-or-modify-navigation-reports-updated-when-write-elided.json new file mode 100644 index 0000000000..fa88b0d240 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-create-or-modify-navigation-reports-updated-when-write-elided.json @@ -0,0 +1 @@ +{"date": "2026-10-09", "area": "mdl/executor", "symptom": "Re-running `create or modify navigation Responsive ...` (chipcov6 04-security-navigation.mdl) printed \"Navigation profile 'Responsive' updated.\" on every run although canon.Reconcile elided the write and every .mxunit md5 was identical; every other statement in the script already reported Unchanged, so the navigation line was the only thing stopping the rerun output from serving as the idempotency gate.", "cause": "execAlterNavigation printed its 'updated' sentence with fmt.Fprintf after UpdateNavigationProfile returned, instead of going through ctx.reportWrite's write-stats evidence (offered vs written) - the #890 sweep converted security and settings but missed navigation. The kept-menu-action note was likewise printed for a rewrite that never happened.", "file": "mdl/executor/cmd_navigation.go", "fix": "The update branch reports through ctx.reportWrite(\"navigation profile ''\", ...), which prints Unchanged (through the run tally) when the write was elided; reportKeptMenuActions now runs only when the write landed (the created-profile branch is untouched: AddNavigationProfile always writes).", "test": "mdl/executor/navigation_report_mutation_test.go TestCreateOrModifyNavigation_ReportsWhatHappened (countingBackend: written=1 reports 'updated' + kept note = control; written=0 reports 'Unchanged navigation profile' and no kept note). Revert check: the elided case fails with \"Navigation profile 'Responsive' updated.\". E2E on a chipcov6 copy: base binary rerun printed 'updated' + 22 in sync, fixed binary 23 in sync; a renamed menu item prints 'updated' then 'Unchanged' on rerun.", "insight": "A statement-specific sentence printed after a backend write is the #890 class again; grep the executor for fmt.Fprintf lines containing 'updated'/'set'/'added' that follow a ctx.Backend.Update* call rather than auditing statement by statement."} diff --git a/mdl/executor/cmd_navigation.go b/mdl/executor/cmd_navigation.go index a01f844474..6adde2448a 100644 --- a/mdl/executor/cmd_navigation.go +++ b/mdl/executor/cmd_navigation.go @@ -137,8 +137,11 @@ func execAlterNavigation(ctx *ExecContext, s *ast.AlterNavigationStmt) error { // An offline profile changes what the platform demands of pages this // statement never mentioned. Say so now, not at the next build. warnOfflineIncompatiblePages(ctx, createdKind) - } else { - fmt.Fprintf(ctx.Output, "Navigation profile %s updated.\n", mdlQuoted(s.ProfileName)) + } else if !ctx.reportWrite("navigation profile "+mdlQuoted(s.ProfileName), + "Navigation profile %s updated.", mdlQuoted(s.ProfileName)) { + // A re-run whose write was elided rewrote nothing, so it carried nothing + // either (ako/mxcli#890 is the same rule for security and settings). + return nil } reportKeptMenuActions(ctx, kept) return nil diff --git a/mdl/executor/navigation_report_mutation_test.go b/mdl/executor/navigation_report_mutation_test.go new file mode 100644 index 0000000000..bc868525d1 --- /dev/null +++ b/mdl/executor/navigation_report_mutation_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" +) + +// Re-running `create or modify navigation Responsive …` printed "Navigation +// profile 'Responsive' updated." on every run, while canon.Reconcile elided the +// unit write and no file changed. The statement reports through reportWrite like +// security and settings do (ako/mxcli#890), so the second run says Unchanged. +func TestCreateOrModifyNavigation_ReportsWhatHappened(t *testing.T) { + for _, tc := range []struct { + name string + written int + want string + wantKept bool + }{ + {"rewrite that landed (control)", 1, "Navigation profile 'Responsive' updated.", true}, + {"elided rewrite", 0, "Unchanged navigation profile 'Responsive'", false}, + } { + t.Run(tc.name, func(t *testing.T) { + cb := &countingBackend{} + cb.MockBackend = &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{Profiles: []*types.NavigationProfile{{ + Name: "Responsive", Kind: "Responsive", + MenuItems: []*types.NavMenuItem{{ + Caption: "Reports", ActionType: "Forms$UnknownFutureClientAction", + StoredAction: []byte("stored"), ActionDoc: map[string]any{"$Type": "Forms$UnknownFutureClientAction"}, + }}, + }}}, nil + }, + UpdateNavigationProfileFunc: func(model.ID, string, types.NavigationProfileSpec) error { + cb.offer(1, tc.written) + return nil + }, + } + ctx, out := newMockCtx(t) + ctx.Backend = cb + prog := parseMDL(t, "create or modify navigation Responsive {\n menu item 'Reports'\n};") + assertNoError(t, execAlterNavigation(ctx, prog.Statements[0].(*ast.AlterNavigationStmt))) + got := out.String() + if !strings.Contains(got, tc.want) { + t.Fatalf("output %q, want it to contain %q", got, tc.want) + } + if tc.written == 0 && strings.Contains(got, "updated") { + t.Errorf("an elided rewrite must not report a write, got %q", got) + } + // The kept-action note describes what a rewrite carried; a run that + // rewrote nothing carried nothing. + if kept := strings.Contains(got, "kept the stored action"); kept != tc.wantKept { + t.Errorf("kept-action note printed=%v, want %v; output %q", kept, tc.wantKept, got) + } + }) + } +} From a1f046312577cc2aa6b2676717e3e1b2971e2710 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 20:50:48 +0000 Subject: [PATCH 04/13] fix(pages): carry layout-grid row/column appearance and alignment through describe -> exec A describe -> exec round trip of a Studio Pro page lost every design property, class and style on its layout-grid rows and columns, and reset row/column alignment. Measured on ledger MyFirstModule.Home_Web: Forms$DesignPropertyValue entries 46 -> 33 ('Flex container' x7 on columns, 'Column gap' x3 and 'Cards style' x3 on rows), and LayoutGridRow.VerticalAlignment Center x6 came back None. check, exec and mx check were all green. Cause: none of the three layers carried it. parseLayoutGridRows read only columns/weights/widgets; buildLayoutGridRowV3/ColumnV3 ignored all properties but widths (the validator classified row/column as slotDropped); layoutGridRowToGen/ColumnToGen hardcoded an empty Forms$Appearance, alignment "None" and SpacingBetweenColumns true. Fix: sdk/pages LayoutGridRow/Column gain Class, Style, DynamicClasses, DesignProperties and alignment (row: Vertical/HorizontalAlignment, NoSpacingBetweenColumns; column: VerticalAlignment). Describe reads the row's and column's Appearance and alignments and prints them only when set (`row (VerticalAlignment: Center, DesignProperties: (...)) {`), so a plain grid describes as before. The builder types design properties against the theme's LayoutGridRow / LayoutGridColumn groups (shared designPropertyValuesV3, split out of applyWidgetAppearance), and the validator checks them there instead of reporting them dropped. A top-level `row`/`column` keeps its appearance on the wrapping container only. Writers (modelsdk and mcp) write the carried values, defaults unchanged when unset. Finding: .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-layout-grid-row-column-appearance-lost-on-describe-exec.json (follows 2026-09-29-re-running-describe-page-output-resets-every-layout-grid). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...lumn-appearance-lost-on-describe-exec.json | 1 + mdl/backend/mcp/page_widgets.go | 21 ++- mdl/backend/modelsdk/widget_write.go | 26 ++- ...widget_write_layoutgrid_appearance_test.go | 100 +++++++++++ mdl/executor/cmd_pages_builder_v3.go | 71 ++++---- mdl/executor/cmd_pages_builder_v3_layout.go | 59 +++++- mdl/executor/cmd_pages_describe.go | 16 +- mdl/executor/cmd_pages_describe_output.go | 29 ++- mdl/executor/cmd_pages_describe_parse.go | 45 +++-- .../cmd_pages_layoutgrid_appearance_test.go | 170 ++++++++++++++++++ .../design_property_keyword_keys_test.go | 59 ++++-- mdl/executor/validate_design_properties.go | 24 ++- mdl/executor/validate_widgets.go | 3 + sdk/pages/pages_widgets_container.go | 31 +++- 14 files changed, 563 insertions(+), 92 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-layout-grid-row-column-appearance-lost-on-describe-exec.json create mode 100644 mdl/backend/modelsdk/widget_write_layoutgrid_appearance_test.go create mode 100644 mdl/executor/cmd_pages_layoutgrid_appearance_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-layout-grid-row-column-appearance-lost-on-describe-exec.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-layout-grid-row-column-appearance-lost-on-describe-exec.json new file mode 100644 index 0000000000..ce87bf0f94 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-layout-grid-row-column-appearance-lost-on-describe-exec.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-09", "symptom": "`describe page` -> `exec` of a Studio Pro page loses its layout-grid row/column appearance: ledger MyFirstModule.Home_Web went from 46 Forms$DesignPropertyValue entries to 33 ('Flex container' = 'Vertical (column)' x7 on LayoutGridColumn, 'Column gap' = 'Large' x3 and 'Cards style' toggle x3 on LayoutGridRow), and LayoutGridRow.VerticalAlignment Center x6 came back None. check, exec and mx check all green; check even warned MDL-WIDGET07 'silently dropped' for design properties written on a row.", "cause": "Three layers, none carried it: parseLayoutGridRows read only Columns/weights/Widgets (never Appearance or the alignment enums); buildLayoutGridRowV3/ColumnV3 ignored every property but widths (validate_design_properties classified row/column as slotDropped); layoutGridRowToGen/ColumnToGen hardcoded newAppearance(\"\",\"\",\"\",nil), alignment \"None\" and SpacingBetweenColumns true. sdk/pages.LayoutGridRow/Column had no field to hold any of it.", "file": "sdk/pages/pages_widgets_container.go, mdl/executor/cmd_pages_describe_parse.go (extractAppearance), cmd_pages_describe_output.go (row (...) / column (...) via formatWidgetProps), cmd_pages_builder_v3_layout.go (designPropertyValuesV3 with theme key LayoutGridRow/LayoutGridColumn, layoutGridAlignment), cmd_pages_builder_v3.go, validate_design_properties.go (slotLayoutGridRow/Column), validate_widgets.go (known props), mdl/backend/modelsdk/widget_write.go, mdl/backend/mcp/page_widgets.go", "insight": "A hardcoded literal in a *ToGen helper (newAppearance(\"\", \"\", \"\", nil), \"None\") is the writer-side tell of a describe round-trip gap, the same way a zero-arg constructor is. Count design-property values over the whole page dump before/after rather than diffing: the loss is invisible in mx check. The theme key matters: a row/column's design properties live under the Atlas classes LayoutGridRow / LayoutGridColumn, not DivContainer (what resolveDesignPropsKey('row') gives, since a top-level `row` builds a container), so ToggleButtonGroup typing and MDL-WIDGET11/12 validation need the explicit key. `VerticalAlignment: End` parses although END is a keyword (qualifiedName admits it).", "refs": "ledger Home_Web round trip; prior: 2026-09-29-re-running-describe-page-output-resets-every-layout-grid", "test": "mdl/executor/cmd_pages_layoutgrid_appearance_test.go, mdl/backend/modelsdk/widget_write_layoutgrid_appearance_test.go; control: both fail with the parse/build/writer changes stashed; e2e Home_Web 46 -> 46 entries, VerticalAlignment Center x6 kept, second exec 'Unchanged page'"} diff --git a/mdl/backend/mcp/page_widgets.go b/mdl/backend/mcp/page_widgets.go index 03d700b754..03c7e26160 100644 --- a/mdl/backend/mcp/page_widgets.go +++ b/mdl/backend/mcp/page_widgets.go @@ -158,21 +158,21 @@ func (b *Backend) mapPageWidgetBody(w pages.Widget) (map[string]any, error) { } cols = append(cols, map[string]any{ "$Type": "Pages$LayoutGridColumn", - "appearance": pageAppearance("", ""), + "appearance": pageAppearance(c.Class, c.Style), "weight": weight, "tabletWeight": tablet, "phoneWeight": phone, "previewWidth": -1, - "verticalAlignment": "None", + "verticalAlignment": alignmentOrNone(c.VerticalAlignment), "widgets": kids, }) } rows = append(rows, map[string]any{ "$Type": "Pages$LayoutGridRow", - "appearance": pageAppearance("", ""), - "verticalAlignment": "None", - "horizontalAlignment": "None", - "spacingBetweenColumns": true, + "appearance": pageAppearance(r.Class, r.Style), + "verticalAlignment": alignmentOrNone(r.VerticalAlignment), + "horizontalAlignment": alignmentOrNone(r.HorizontalAlignment), + "spacingBetweenColumns": !r.NoSpacingBetweenColumns, "columns": cols, }) } @@ -603,3 +603,12 @@ func clientTemplateParam(p *pages.ClientTemplateParameter) map[string]any { } return param } + +// alignmentOrNone maps an unset layout-grid row/column alignment to Mendix's +// default. +func alignmentOrNone(a string) string { + if a == "" { + return "None" + } + return a +} diff --git a/mdl/backend/modelsdk/widget_write.go b/mdl/backend/modelsdk/widget_write.go index e6a03b2aae..732da93c97 100644 --- a/mdl/backend/modelsdk/widget_write.go +++ b/mdl/backend/modelsdk/widget_write.go @@ -998,18 +998,20 @@ func navListItemToGen(item *pages.NavigationListItem) (element.Element, error) { return g, nil } -// layoutGridRowToGen converts a LayoutGridRow (alignment defaults match the -// legacy serializer; not a full widget, so no name/tabindex). +// layoutGridRowToGen converts a LayoutGridRow (not a full widget, so no +// name/tabindex). Its appearance and alignment are carried: hardcoding them +// dropped every row's design properties and reset a centred row on each +// describe → exec. Unset values write the defaults the legacy serializer did. func layoutGridRowToGen(row *pages.LayoutGridRow) (element.Element, error) { g := genPg.NewLayoutGridRow() if row.ID != "" { g.SetID(element.ID(row.ID)) } assignID(g) - g.SetAppearance(newAppearance("", "", "", nil)) - g.SetHorizontalAlignment("None") - g.SetSpacingBetweenColumns(true) - g.SetVerticalAlignment("None") + g.SetAppearance(newAppearance(row.Class, row.Style, row.DynamicClasses, row.DesignProperties)) + g.SetHorizontalAlignment(alignmentOrNone(row.HorizontalAlignment)) + g.SetSpacingBetweenColumns(!row.NoSpacingBetweenColumns) + g.SetVerticalAlignment(alignmentOrNone(row.VerticalAlignment)) for _, col := range row.Columns { cg, err := layoutGridColumnToGen(col) if err != nil { @@ -1028,12 +1030,12 @@ func layoutGridColumnToGen(col *pages.LayoutGridColumn) (element.Element, error) g.SetID(element.ID(col.ID)) } assignID(g) - g.SetAppearance(newAppearance("", "", "", nil)) + g.SetAppearance(newAppearance(col.Class, col.Style, col.DynamicClasses, col.DesignProperties)) g.SetWeight(int32(columnWeight(col.Weight))) g.SetTabletWeight(int32(columnWeight(col.TabletWeight))) g.SetPhoneWeight(int32(columnWeight(col.PhoneWeight))) g.SetPreviewWidth(-1) - g.SetVerticalAlignment("None") + g.SetVerticalAlignment(alignmentOrNone(col.VerticalAlignment)) for _, w := range col.Widgets { wg, err := widgetToGen(w) if err != nil { @@ -1044,6 +1046,14 @@ func layoutGridColumnToGen(col *pages.LayoutGridColumn) (element.Element, error) return g, nil } +// alignmentOrNone maps an unset layout-grid alignment to Mendix's default. +func alignmentOrNone(a string) string { + if a == "" { + return "None" + } + return a +} + // columnWeight maps an unset weight (0) to -1 (auto-fill), matching the legacy // serializer's columnWeight. func columnWeight(w int) int32 { diff --git a/mdl/backend/modelsdk/widget_write_layoutgrid_appearance_test.go b/mdl/backend/modelsdk/widget_write_layoutgrid_appearance_test.go new file mode 100644 index 0000000000..af92a424f5 --- /dev/null +++ b/mdl/backend/modelsdk/widget_write_layoutgrid_appearance_test.go @@ -0,0 +1,100 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import ( + "testing" + + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/sdk/pages" +) + +// designPropKeys lists the Key of each Forms$DesignPropertyValue in an encoded +// Forms$Appearance. +func designPropKeys(t *testing.T, appearance any) []string { + t.Helper() + a, ok := appearance.(map[string]any) + if !ok { + t.Fatalf("Appearance is %T, want a document", appearance) + } + list, _ := a["DesignProperties"].(primitive.A) + var keys []string + for _, item := range list { + if m, ok := item.(map[string]any); ok { + k, _ := m["Key"].(string) + keys = append(keys, k) + } + } + return keys +} + +// A layout-grid row and column write their appearance and alignment; the +// writer hardcoded an empty appearance and "None" for both, which dropped +// every grid design property on describe → exec. +func TestLayoutGridRowColumnToGen_CarriesAppearanceAndAlignment(t *testing.T) { + row := &pages.LayoutGridRow{ + Class: "row-class", + VerticalAlignment: "Center", + HorizontalAlignment: "End", + NoSpacingBetweenColumns: true, + DesignProperties: []pages.DesignPropertyValue{ + {Key: "Column gap", ValueType: "option", Option: "Large"}, + {Key: "Cards style", ValueType: "toggle"}, + }, + Columns: []*pages.LayoutGridColumn{{ + Weight: 6, + Style: "padding: 0", + VerticalAlignment: "Center", + DesignProperties: []pages.DesignPropertyValue{ + {Key: "Flex container", ValueType: "option", Option: "Vertical (column)"}, + }, + }}, + } + g, err := layoutGridRowToGen(row) + if err != nil { + t.Fatal(err) + } + doc := encodeToMap(t, g) + if doc["VerticalAlignment"] != "Center" || doc["HorizontalAlignment"] != "End" || doc["SpacingBetweenColumns"] != false { + t.Errorf("row alignment: %v / %v / %v, want Center / End / false", + doc["VerticalAlignment"], doc["HorizontalAlignment"], doc["SpacingBetweenColumns"]) + } + if got := doc["Appearance"].(map[string]any)["Class"]; got != "row-class" { + t.Errorf("row Appearance.Class = %v, want row-class", got) + } + if keys := designPropKeys(t, doc["Appearance"]); len(keys) != 2 || keys[0] != "Column gap" || keys[1] != "Cards style" { + t.Errorf("row design properties %v, want [Column gap Cards style]", keys) + } + + col := doc["Columns"].(primitive.A)[1].(map[string]any) // [0] is the list marker + if col["VerticalAlignment"] != "Center" { + t.Errorf("column VerticalAlignment = %v, want Center", col["VerticalAlignment"]) + } + if got := col["Appearance"].(map[string]any)["Style"]; got != "padding: 0" { + t.Errorf("column Appearance.Style = %v, want padding: 0", got) + } + if keys := designPropKeys(t, col["Appearance"]); len(keys) != 1 || keys[0] != "Flex container" { + t.Errorf("column design properties %v, want [Flex container]", keys) + } +} + +// Nothing set writes exactly the defaults the writer always wrote. +func TestLayoutGridRowColumnToGen_DefaultsUnchanged(t *testing.T) { + g, err := layoutGridRowToGen(&pages.LayoutGridRow{Columns: []*pages.LayoutGridColumn{{}}}) + if err != nil { + t.Fatal(err) + } + doc := encodeToMap(t, g) + if doc["VerticalAlignment"] != "None" || doc["HorizontalAlignment"] != "None" || doc["SpacingBetweenColumns"] != true { + t.Errorf("row defaults: %v / %v / %v, want None / None / true", + doc["VerticalAlignment"], doc["HorizontalAlignment"], doc["SpacingBetweenColumns"]) + } + if keys := designPropKeys(t, doc["Appearance"]); len(keys) != 0 { + t.Errorf("row design properties %v, want none", keys) + } + col := doc["Columns"].(primitive.A)[1].(map[string]any) + if col["VerticalAlignment"] != "None" { + t.Errorf("column VerticalAlignment = %v, want None", col["VerticalAlignment"]) + } +} diff --git a/mdl/executor/cmd_pages_builder_v3.go b/mdl/executor/cmd_pages_builder_v3.go index a01746fecb..5b8c855834 100644 --- a/mdl/executor/cmd_pages_builder_v3.go +++ b/mdl/executor/cmd_pages_builder_v3.go @@ -637,42 +637,53 @@ func applyWidgetAppearance(widget pages.Widget, w *ast.WidgetV3, theme *ThemeReg } // Apply design properties - astProps := w.GetDesignProperties() - if len(astProps) > 0 { - // Resolve the widget's design-property definitions from the theme registry - // (when loaded) so each value's BSON type is taken from metadata - // (ColorPicker/ToggleButtonGroup → custom) rather than guessed. Nil/empty - // when no project/themesource — the converter then falls back to option. - var themeProps []ThemeProperty - if theme != nil { - themeProps = theme.GetPropertiesForWidget(resolveDesignPropsKey(w.Type)) - } - var dpValues []pages.DesignPropertyValue - for _, p := range astProps { - // Refuse rather than write, when the theme proves the shape wrong — - // a flat value on a multi-select property (ako/mxcli#511). Silently - // writing it produced a document mxbuild rejects with CE6084, whose - // wording names a type mismatch and not the spelling that fixes it. - dp, ok, err := astDesignPropToValueChecked(p, themeProps) - if err != nil { - return fmt.Errorf("widget %q: %w", w.Name, err) - } - if ok { - dpValues = append(dpValues, dp) - } + dpValues, err := designPropertyValuesV3(w, resolveDesignPropsKey(w.Type), theme) + if err != nil { + return fmt.Errorf("widget %q: %w", w.Name, err) + } + if len(dpValues) > 0 { + type designPropSetter interface { + SetDesignProperties(props []pages.DesignPropertyValue) } - if len(dpValues) > 0 { - type designPropSetter interface { - SetDesignProperties(props []pages.DesignPropertyValue) - } - if setter, ok := widget.(designPropSetter); ok { - setter.SetDesignProperties(dpValues) - } + if setter, ok := widget.(designPropSetter); ok { + setter.SetDesignProperties(dpValues) } } return nil } +// designPropertyValuesV3 converts w's DesignProperties entries, typed against +// the theme's definitions for themeKey (a design-properties.json group). +func designPropertyValuesV3(w *ast.WidgetV3, themeKey string, theme *ThemeRegistry) ([]pages.DesignPropertyValue, error) { + astProps := w.GetDesignProperties() + if len(astProps) == 0 { + return nil, nil + } + // Resolve the widget's design-property definitions from the theme registry + // (when loaded) so each value's BSON type is taken from metadata + // (ColorPicker/ToggleButtonGroup → custom) rather than guessed. Nil/empty + // when no project/themesource — the converter then falls back to option. + var themeProps []ThemeProperty + if theme != nil { + themeProps = theme.GetPropertiesForWidget(themeKey) + } + var dpValues []pages.DesignPropertyValue + for _, p := range astProps { + // Refuse rather than write, when the theme proves the shape wrong — + // a flat value on a multi-select property (ako/mxcli#511). Silently + // writing it produced a document mxbuild rejects with CE6084, whose + // wording names a type mismatch and not the spelling that fixes it. + dp, ok, err := astDesignPropToValueChecked(p, themeProps) + if err != nil { + return nil, err + } + if ok { + dpValues = append(dpValues, dp) + } + } + return dpValues, nil +} + // astDesignPropToValue converts one MDL design-property entry to a // pages.DesignPropertyValue. Compound entries (a key whose value is a nested // list, e.g. 'Spacing': ['margin-top': 'Large', …]) recurse into sub-properties. diff --git a/mdl/executor/cmd_pages_builder_v3_layout.go b/mdl/executor/cmd_pages_builder_v3_layout.go index a0591a92de..cb15a1b8a7 100644 --- a/mdl/executor/cmd_pages_builder_v3_layout.go +++ b/mdl/executor/cmd_pages_builder_v3_layout.go @@ -3,9 +3,11 @@ package executor import ( + "fmt" "strings" "github.com/mendixlabs/mxcli/mdl/ast" + mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/pages" @@ -42,6 +44,26 @@ func (pb *pageBuilder) buildLayoutGridRowV3(w *ast.WidgetV3) (*pages.LayoutGridR ID: model.ID(types.GenerateID()), TypeName: "Forms$LayoutGridRow", }, + Class: w.GetClass(), + Style: w.GetStyle(), + DynamicClasses: w.GetDynamicClasses(), + } + var err error + if row.DesignProperties, err = designPropertyValuesV3(w, "LayoutGridRow", pb.themeRegistry); err != nil { + return nil, fmt.Errorf("layout grid row: %w", err) + } + if row.VerticalAlignment, err = layoutGridAlignment(w, "VerticalAlignment"); err != nil { + return nil, err + } + if row.HorizontalAlignment, err = layoutGridAlignment(w, "HorizontalAlignment"); err != nil { + return nil, err + } + if raw, ok := lookupPropCI(w, "SpacingBetweenColumns"); ok { + v, err := propBool(raw) + if err != nil { + return nil, fmt.Errorf("layout grid row SpacingBetweenColumns: %w", err) + } + row.NoSpacingBetweenColumns = !v } // Build columns from children @@ -64,7 +86,17 @@ func (pb *pageBuilder) buildLayoutGridColumnV3(w *ast.WidgetV3) (*pages.LayoutGr ID: model.ID(types.GenerateID()), TypeName: "Forms$LayoutGridColumn", }, - Weight: 1, + Weight: 1, + Class: w.GetClass(), + Style: w.GetStyle(), + DynamicClasses: w.GetDynamicClasses(), + } + var err error + if col.DesignProperties, err = designPropertyValuesV3(w, "LayoutGridColumn", pb.themeRegistry); err != nil { + return nil, fmt.Errorf("layout grid column: %w", err) + } + if col.VerticalAlignment, err = layoutGridAlignment(w, "VerticalAlignment"); err != nil { + return nil, err } if dw := w.GetDesktopWidth(); dw != nil { @@ -89,6 +121,25 @@ func (pb *pageBuilder) buildLayoutGridColumnV3(w *ast.WidgetV3) (*pages.LayoutGr return col, nil } +// layoutGridAlignment reads a row's or column's VerticalAlignment / +// HorizontalAlignment: Start, Center or End (any case), "" when unset — which +// the writer stores as Mendix's default, None. +func layoutGridAlignment(w *ast.WidgetV3, key string) (string, error) { + raw, ok := lookupPropCI(w, key) + if !ok { + return "", nil + } + if s, ok := raw.(string); ok { + for _, a := range []string{"None", "Start", "Center", "End"} { + if strings.EqualFold(s, a) { + return a, nil + } + } + } + return "", mdlerrors.NewValidationf("layout grid %s %s: invalid value %v (expected Start, Center or End)", + strings.ToLower(w.Type), key, raw) +} + // Stored layout-grid column weights besides 1..12. const ( layoutGridWeightAutoFill = -1 @@ -139,6 +190,10 @@ func (pb *pageBuilder) buildContainerWithRowV3(w *ast.WidgetV3) (*pages.Containe if err != nil { return nil, err } + // The appearance written on a top-level `row` is the container's + // (applyWidgetAppearance sets it there); writing it on the row too would + // apply every class and design property twice. + row.Class, row.Style, row.DynamicClasses, row.DesignProperties = "", "", "", nil lg.Rows = append(lg.Rows, row) container.Widgets = append(container.Widgets, lg) @@ -178,6 +233,8 @@ func (pb *pageBuilder) buildContainerWithColumnV3(w *ast.WidgetV3) (*pages.Conta if err != nil { return nil, err } + // The container carries a top-level `column`'s appearance, as for `row`. + col.Class, col.Style, col.DynamicClasses, col.DesignProperties = "", "", "", nil row.Columns = append(row.Columns, col) lg.Rows = append(lg.Rows, row) container.Widgets = append(container.Widgets, lg) diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index ac22d188e0..7013d648c7 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -858,13 +858,21 @@ type rawDesignProp struct { type rawWidgetRow struct { Columns []rawWidgetColumn + // Appearance holds only Class, Style, DynamicClasses and DesignProperties — + // a row is not a widget, but its Forms$Appearance is a widget's. + Appearance rawWidget + VerticalAlignment string // "" or "None" is the default + HorizontalAlignment string + SpacingBetweenColumns bool } type rawWidgetColumn struct { - Width int - TabletWidth int - PhoneWidth int - Widgets []rawWidget + Width int + TabletWidth int + PhoneWidth int + Widgets []rawWidget + Appearance rawWidget // Class, Style, DynamicClasses, DesignProperties + VerticalAlignment string } // toBsonArray converts various BSON array types to []interface{}. diff --git a/mdl/executor/cmd_pages_describe_output.go b/mdl/executor/cmd_pages_describe_output.go index 962a9bcc5e..c4d8e8c0f1 100644 --- a/mdl/executor/cmd_pages_describe_output.go +++ b/mdl/executor/cmd_pages_describe_output.go @@ -453,7 +453,19 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { // Mendix stores no name on a row or a column (R12, #749), so // describe writes none: an invented `row1` / `col3` churned when a // row or column was inserted, and meant nothing on re-execution. - fmt.Fprintf(ctx.Output, "%s row {\n", prefix) + // Alignment and appearance only when set, so a plain row stays `row {`. + var rowProps []string + if a := layoutGridAlignmentMDL(row.VerticalAlignment); a != "" { + rowProps = append(rowProps, "VerticalAlignment: "+a) + } + if a := layoutGridAlignmentMDL(row.HorizontalAlignment); a != "" { + rowProps = append(rowProps, "HorizontalAlignment: "+a) + } + if !row.SpacingBetweenColumns { + rowProps = append(rowProps, "SpacingBetweenColumns: false") + } + rowProps = appendAppearanceProps(ctx, rowProps, row.Appearance) + formatWidgetProps(ctx.Output, prefix+" ", "row", rowProps, " {\n") for _, col := range row.Columns { // The desktop width is always printed. A tablet or phone width // is printed unless it is auto-fill, which is what the builder @@ -465,7 +477,11 @@ func outputWidgetMDLV3(ctx *ExecContext, w rawWidget, indent int) { if w := layoutGridWidthMDL(col.PhoneWidth); w != "AutoFill" { colProps = append(colProps, "PhoneWidth: "+w) } - fmt.Fprintf(ctx.Output, "%s column (%s) {\n", prefix, strings.Join(colProps, ", ")) + if a := layoutGridAlignmentMDL(col.VerticalAlignment); a != "" { + colProps = append(colProps, "VerticalAlignment: "+a) + } + colProps = appendAppearanceProps(ctx, colProps, col.Appearance) + formatWidgetProps(ctx.Output, prefix+" ", "column", colProps, " {\n") for _, cw := range col.Widgets { outputWidgetMDLV3(ctx, cw, indent+3) } @@ -2127,3 +2143,12 @@ func layoutGridWidthMDL(weight int) string { } return "AutoFill" } + +// layoutGridAlignmentMDL spells a stored layout-grid row or column alignment, +// or "" for Mendix's default ("None", or absent on an older model). +func layoutGridAlignmentMDL(a string) string { + if a == "" || a == "None" { + return "" + } + return a +} diff --git a/mdl/executor/cmd_pages_describe_parse.go b/mdl/executor/cmd_pages_describe_parse.go index 41e1967acb..994ed63d4d 100644 --- a/mdl/executor/cmd_pages_describe_parse.go +++ b/mdl/executor/cmd_pages_describe_parse.go @@ -277,18 +277,7 @@ func parseRawWidget(ctx *ExecContext, w map[string]any, parentEntityContext ...s extractConditionalSettings(ctx, &widget, w) // Extract CSS class, style, and design properties from Appearance - if appearance, ok := w["Appearance"].(map[string]any); ok { - if class, ok := appearance["Class"].(string); ok && class != "" { - widget.Class = class - } - if style, ok := appearance["Style"].(string); ok && style != "" { - widget.Style = style - } - if dc, ok := appearance["DynamicClasses"].(string); ok && dc != "" { - widget.DynamicClasses = dc - } - widget.DesignProperties = extractDesignProperties(appearance) - } + extractAppearance(&widget, w) switch typeName { case "Forms$LayoutGrid", "Pages$LayoutGrid": @@ -785,7 +774,16 @@ func parseLayoutGridRows(ctx *ExecContext, w map[string]any, entityContext ...st if !ok { continue } - row := rawWidgetRow{} + row := rawWidgetRow{SpacingBetweenColumns: true} + // A row's and a column's appearance and alignment were never read, so a + // describe → exec reset every one of them (the Atlas "Flex container" / + // "Column gap" / "Cards style" a Studio Pro page sets on its grid). + extractAppearance(&row.Appearance, rMap) + row.VerticalAlignment, _ = rMap["VerticalAlignment"].(string) + row.HorizontalAlignment, _ = rMap["HorizontalAlignment"].(string) + if v, ok := rMap["SpacingBetweenColumns"].(bool); ok { + row.SpacingBetweenColumns = v + } cols := getBsonArrayElements(rMap["Columns"]) for _, c := range cols { cMap, ok := c.(map[string]any) @@ -793,6 +791,8 @@ func parseLayoutGridRows(ctx *ExecContext, w map[string]any, entityContext ...st continue } col := rawWidgetColumn{} + extractAppearance(&col.Appearance, cMap) + col.VerticalAlignment, _ = cMap["VerticalAlignment"].(string) // Widths: 1..12, -1 auto-fill, -2 auto-fit content. Studio Pro // stores them as int64, so read them width-agnostically — an // `.(int32)` here missed every stored width and describe printed @@ -818,6 +818,25 @@ func parseLayoutGridRows(ctx *ExecContext, w map[string]any, entityContext ...st return result } +// extractAppearance copies the CSS class, inline style, dynamic classes and +// design properties of an element's Forms$Appearance onto dst. +func extractAppearance(dst *rawWidget, w map[string]any) { + appearance, ok := w["Appearance"].(map[string]any) + if !ok { + return + } + if class, ok := appearance["Class"].(string); ok && class != "" { + dst.Class = class + } + if style, ok := appearance["Style"].(string); ok && style != "" { + dst.Style = style + } + if dc, ok := appearance["DynamicClasses"].(string); ok && dc != "" { + dst.DynamicClasses = dc + } + dst.DesignProperties = extractDesignProperties(appearance) +} + // parseNavigationListItems extracts items from a NavigationList widget. func parseNavigationListItems(ctx *ExecContext, w map[string]any) []rawWidget { items := getBsonArrayElements(w["Items"]) diff --git a/mdl/executor/cmd_pages_layoutgrid_appearance_test.go b/mdl/executor/cmd_pages_layoutgrid_appearance_test.go new file mode 100644 index 0000000000..9a78839622 --- /dev/null +++ b/mdl/executor/cmd_pages_layoutgrid_appearance_test.go @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: Apache-2.0 + +// A layout-grid row's and column's appearance and alignment did not survive +// describe → exec. Describe never read them, the builder never set them and the +// writer hardcoded an empty Forms$Appearance with alignment "None", so a +// Studio Pro page lost every Atlas "Flex container" / "Column gap" / "Cards +// style" on its grid and each centred row came back uncentred (ledger +// MyFirstModule.Home_Web: 46 design-property values → 33). + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/pages" +) + +func storedDesignProp(key, valueType, option string) map[string]any { + v := map[string]any{"$Type": valueType} + if option != "" { + v["Option"] = option + } + return map[string]any{"$Type": "Forms$DesignPropertyValue", "Key": key, "Value": v} +} + +// buildDescribedGrid describes a stored layout grid, parses the output and +// builds its rows the way exec does. +func buildDescribedGrid(t *testing.T, rows ...any) ([]*pages.LayoutGridRow, string) { + t.Helper() + ctx, _ := newMockCtx(t) + raw := map[string]any{"$Type": "Forms$LayoutGrid", "Name": "lg", "Rows": rows} + widgets := parseRawWidget(ctx, raw) + if len(widgets) != 1 { + t.Fatalf("parseRawWidget: %d widgets, want 1", len(widgets)) + } + var buf bytes.Buffer + outputWidgetMDLV3(&ExecContext{Output: &buf}, widgets[0], 1) + mdl := buf.String() + src := "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) {\n" + mdl + "};" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("described layout grid does not parse: %v\n%s", errs, src) + } + page := prog.Statements[0].(*ast.CreatePageStmtV3) + lg, err := (&pageBuilder{}).buildLayoutGridV3(page.Widgets[0]) + if err != nil { + t.Fatalf("build: %v\n%s", err, mdl) + } + return lg.Rows, mdl +} + +func TestLayoutGridRowColumnAppearanceRoundTrip(t *testing.T) { + row := map[string]any{ + "$Type": "Forms$LayoutGridRow", + "VerticalAlignment": "Center", + "HorizontalAlignment": "End", + "SpacingBetweenColumns": false, + "Appearance": map[string]any{ + "$Type": "Forms$Appearance", + "Class": "row-class", + "DesignProperties": []any{int32(2), + storedDesignProp("Column gap", "Forms$OptionDesignPropertyValue", "Large"), + storedDesignProp("Cards style", "Forms$ToggleDesignPropertyValue", ""), + }, + }, + "Columns": []any{map[string]any{ + "$Type": "Forms$LayoutGridColumn", + "Weight": int64(6), + "VerticalAlignment": "Center", + "Appearance": map[string]any{ + "$Type": "Forms$Appearance", + "Style": "padding: 0", + "DesignProperties": []any{int32(2), + storedDesignProp("Flex container", "Forms$OptionDesignPropertyValue", "Vertical (column)"), + }, + }, + }}, + } + rows, mdl := buildDescribedGrid(t, row) + r := rows[0] + if r.VerticalAlignment != "Center" || r.HorizontalAlignment != "End" || !r.NoSpacingBetweenColumns { + t.Errorf("row alignment: vertical %q, horizontal %q, no spacing %v; want Center, End, true\n%s", + r.VerticalAlignment, r.HorizontalAlignment, r.NoSpacingBetweenColumns, mdl) + } + if r.Class != "row-class" { + t.Errorf("row class %q, want row-class\n%s", r.Class, mdl) + } + wantRowDPs := []pages.DesignPropertyValue{ + {Key: "Column gap", ValueType: "option", Option: "Large"}, + {Key: "Cards style", ValueType: "toggle"}, + } + assertDesignProps(t, "row", r.DesignProperties, wantRowDPs, mdl) + + c := r.Columns[0] + if c.VerticalAlignment != "Center" || c.Style != "padding: 0" || c.Weight != 6 { + t.Errorf("column: vertical %q, style %q, weight %d; want Center, padding: 0, 6\n%s", + c.VerticalAlignment, c.Style, c.Weight, mdl) + } + assertDesignProps(t, "column", c.DesignProperties, + []pages.DesignPropertyValue{{Key: "Flex container", ValueType: "option", Option: "Vertical (column)"}}, mdl) +} + +// A row and column with nothing set still describe as before and build with +// every value at its default — what the writer turns into the old fixed output. +func TestLayoutGridRowColumnAppearanceDefaultsQuiet(t *testing.T) { + row := map[string]any{ + "$Type": "Forms$LayoutGridRow", + "VerticalAlignment": "None", + "HorizontalAlignment": "None", + "SpacingBetweenColumns": true, + "Appearance": map[string]any{"$Type": "Forms$Appearance", "DesignProperties": []any{int32(3)}}, + "Columns": []any{map[string]any{ + "$Type": "Forms$LayoutGridColumn", "Weight": int64(-1), "VerticalAlignment": "None", + "Appearance": map[string]any{"$Type": "Forms$Appearance"}, + }}, + } + rows, mdl := buildDescribedGrid(t, row) + if !strings.Contains(mdl, " row {\n") || !strings.Contains(mdl, "column (DesktopWidth: AutoFill) {") { + t.Errorf("plain row/column no longer describes plainly:\n%s", mdl) + } + r, c := rows[0], rows[0].Columns[0] + if r.VerticalAlignment != "" || r.HorizontalAlignment != "" || r.NoSpacingBetweenColumns || + r.Class != "" || r.Style != "" || len(r.DesignProperties) != 0 || + c.VerticalAlignment != "" || c.Class != "" || c.Style != "" || len(c.DesignProperties) != 0 { + t.Errorf("plain row/column built with non-default values: row %+v, column %+v", r, c) + } +} + +// A top-level `row` builds a container around a one-row grid, and the +// appearance written on it is the container's: the inner row must not carry it +// a second time. +func TestTopLevelRowAppearanceStaysOnContainer(t *testing.T) { + prog, errs := visitor.Build(`create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) { + row r (Class: 'outer', DesignProperties: ['Card style': on]) { column (DesktopWidth: 6) { } } +};`) + if len(errs) > 0 { + t.Fatal(errs) + } + pb := &pageBuilder{backend: &mock.MockBackend{}, widgetScope: map[string]model.ID{}} + built, err := pb.buildWidgetV3(prog.Statements[0].(*ast.CreatePageStmtV3).Widgets[0]) + if err != nil { + t.Fatal(err) + } + c := built.(*pages.Container) + if c.Class != "outer" || len(c.DesignProperties) != 1 { + t.Errorf("container: class %q, %d design properties; want outer, 1", c.Class, len(c.DesignProperties)) + } + row := c.Widgets[0].(*pages.LayoutGrid).Rows[0] + if row.Class != "" || len(row.DesignProperties) != 0 { + t.Errorf("inner row repeats the container's appearance: class %q, %+v", row.Class, row.DesignProperties) + } +} + +func assertDesignProps(t *testing.T, what string, got, want []pages.DesignPropertyValue, mdl string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("%s design properties: got %+v, want %+v\n%s", what, got, want, mdl) + } + for i := range want { + if got[i].Key != want[i].Key || got[i].ValueType != want[i].ValueType || got[i].Option != want[i].Option { + t.Errorf("%s design property %d: got %+v, want %+v\n%s", what, i, got[i], want[i], mdl) + } + } +} diff --git a/mdl/executor/design_property_keyword_keys_test.go b/mdl/executor/design_property_keyword_keys_test.go index 534447fbb7..5f47d2aba5 100644 --- a/mdl/executor/design_property_keyword_keys_test.go +++ b/mdl/executor/design_property_keyword_keys_test.go @@ -140,12 +140,16 @@ func TestBuildWidget_OwnGroupColourIsCustom(t *testing.T) { } } -// A row inside a layoutgrid, a column inside a row, and a dataview's footer are -// SLOTS, not widgets: buildLayoutGridRowV3 / buildLayoutGridColumnV3 and the -// dataview footer split never call applyWidgetAppearance, so their design -// properties are dropped on write. Resolving the keyword as a DivContainer there -// would validate against a group the value never reaches; staying silent reads -// as approval of a value that is thrown away. +// A dataview's footer is a SLOT, not a widget: the dataview footer split never +// calls applyWidgetAppearance, so its design properties are dropped on write. +// Resolving the keyword as a DivContainer there would validate against a group +// the value never reaches; staying silent reads as approval of a value that is +// thrown away. +// +// A layout grid's row and a row's column were the same until their appearance +// was written: they are validated against the theme's LayoutGridRow / +// LayoutGridColumn groups now (never as a DivContainer), and are not reported +// dropped. func TestValidateDesignProperties_SlotDesignPropsAreReportedDropped(t *testing.T) { reg := ownGroupThemeRegistry(t) vs := allDesignPropViolations(t, `create page M.P (layout: Atlas_Core.Atlas_Default) { @@ -164,25 +168,44 @@ func TestValidateDesignProperties_SlotDesignPropsAreReportedDropped(t *testing.T // A layout grid's rows and columns, and a data view's footer, have no // stored name, so the visitor drops the one written here (MDL-DEPR005, // #749, ako/mxcli#528); the message names them by kind and parent instead. - for _, name := range []string{`row inside layoutgrid "lg"`, `column inside row sets`, `column inside row "topRow"`, `footer inside dataview "dv"`} { - var found bool - for _, v := range vs { - if v.RuleID == "MDL-WIDGET07" && strings.Contains(v.Message, name) && - strings.Contains(v.Message, "dropped") { - found = true - } - } - if !found { - t.Errorf("slot %q: design properties dropped on write but not reported (%d violations)", name, len(vs)) + var found bool + for _, v := range vs { + if v.RuleID == "MDL-WIDGET07" && strings.Contains(v.Message, `footer inside dataview "dv"`) && + strings.Contains(v.Message, "dropped") { + found = true } } + if !found { + t.Errorf("footer: design properties dropped on write but not reported (%d violations)", len(vs)) + } for _, v := range vs { - if v.RuleID != "MDL-WIDGET07" { - t.Errorf("a slot must not be validated as a widget, got %s: %s", v.RuleID, v.Message) + if v.RuleID != "MDL-WIDGET07" || !strings.Contains(v.Message, "footer") { + t.Errorf("only the footer may be reported, got %s: %s", v.RuleID, v.Message) } } } +// A layout grid's row and column are validated against their own theme groups: +// a key the LayoutGridColumn group does not define is flagged, one it defines +// is not — and DivContainer's "Card style" is not borrowed for them. +func TestValidateDesignProperties_LayoutGridRowColumnUseOwnGroups(t *testing.T) { + reg := ownGroupThemeRegistry(t) + reg.WidgetProperties["LayoutGridRow"] = []ThemeProperty{{Name: "Cards style", Type: "Toggle"}} + reg.WidgetProperties["LayoutGridColumn"] = []ThemeProperty{{Name: "Flex container", Type: "ToggleButtonGroup", + Options: []ThemeOption{{Name: "Horizontal (row)"}, {Name: "Vertical (column)"}}}} + vs := allDesignPropViolations(t, `create page M.P (layout: Atlas_Core.Atlas_Default) { + layoutgrid lg { + row (DesignProperties: ['Cards style': on]) { + column (DesignProperties: ['Flex container': 'Vertical (column)']) { } + column (DesignProperties: ['Card style': on]) { } + } + } +}`, reg) + if len(vs) != 1 || vs[0].RuleID != "MDL-WIDGET11" || !strings.Contains(vs[0].Message, `"Card style"`) { + t.Errorf("want one MDL-WIDGET11 for the column's \"Card style\", got %v", vs) + } +} + // CONTROL: the same keywords as a CHILD of a pluggable widget belong to the // widget's own object lists (a data grid `column`), which the pluggable engine // builds — not a DivContainer and not a layout-grid slot. They must stay diff --git a/mdl/executor/validate_design_properties.go b/mdl/executor/validate_design_properties.go index 7e13b91f1b..14c2d00fa1 100644 --- a/mdl/executor/validate_design_properties.go +++ b/mdl/executor/validate_design_properties.go @@ -92,6 +92,10 @@ func validateDesignPropsSubtree(parent *ast.WidgetV3, widgets []*ast.WidgetV3, r switch designPropsSlotOf(parent, w) { case slotDropped: out = append(out, droppedSlotDesignProps(parent, w, locationPrefix)...) + case slotLayoutGridRow: + out = append(out, validateDesignPropsAs(w, "LayoutGridRow", reg, locationPrefix)...) + case slotLayoutGridColumn: + out = append(out, validateDesignPropsAs(w, "LayoutGridColumn", reg, locationPrefix)...) case slotOfPluggable: // Built by the pluggable engine from the parent's object lists; the // keyword names no native widget here, so there is nothing to resolve. @@ -113,6 +117,11 @@ const ( // assembles itself, never through buildWidgetV3, so applyWidgetAppearance // never sees its design properties. slotDropped + // slotLayoutGridRow / slotLayoutGridColumn: a layout grid's row and a row's + // column. Not widgets either, but the builder writes their appearance, and + // the theme defines their design properties under their own class names. + slotLayoutGridRow + slotLayoutGridColumn // slotOfPluggable: a keyword that is a native widget elsewhere, used as a // child of a pluggable widget — one of its object-list entries or slots. slotOfPluggable @@ -136,9 +145,11 @@ func designPropsSlotOf(parent, child *ast.WidgetV3) designPropsSlot { } p, c := strings.ToLower(parent.Type), strings.ToLower(child.Type) switch { - case p == "layoutgrid" && c == "row", // buildLayoutGridRowV3 - p == "row" && c == "column", // buildLayoutGridColumnV3 - p == "dataview" && c == "footer": // children moved into FooterWidgets + case p == "layoutgrid" && c == "row": // buildLayoutGridRowV3 + return slotLayoutGridRow + case p == "row" && c == "column": // buildLayoutGridColumnV3 + return slotLayoutGridColumn + case p == "dataview" && c == "footer": // children moved into FooterWidgets return slotDropped } if !slotKeywords[c] { @@ -178,11 +189,16 @@ func elementLabel(w *ast.WidgetV3) string { } func validateWidgetDesignProps(w *ast.WidgetV3, reg *ThemeRegistry, locationPrefix string) []linter.Violation { + return validateDesignPropsAs(w, resolveDesignPropsKey(w.Type), reg, locationPrefix) +} + +// validateDesignPropsAs validates w's design properties against the theme's +// definitions for key (a design-properties.json group). +func validateDesignPropsAs(w *ast.WidgetV3, key string, reg *ThemeRegistry, locationPrefix string) []linter.Violation { astProps := w.GetDesignProperties() if len(astProps) == 0 { return nil } - key := resolveDesignPropsKey(w.Type) // Only validate widgets we have type-specific metadata for. For an unknown // widget type (e.g. a pluggable widget not in the theme registry) we don't // know the full property set, so we must not flag its keys as unknown. diff --git a/mdl/executor/validate_widgets.go b/mdl/executor/validate_widgets.go index f79be81059..0be0242695 100644 --- a/mdl/executor/validate_widgets.go +++ b/mdl/executor/validate_widgets.go @@ -790,6 +790,9 @@ var staticWidgetKnownProps = func() map[string]bool { // menu source and orientation of navigationtree / menubar / // simplemenubar, and a scroll-container region's size mode. "ShowFooter", "Menu", "Profile", "Orientation", "SizeMode", + // a layout-grid row's and column's alignment, read by + // buildLayoutGridRowV3 / buildLayoutGridColumnV3 and emitted by describe. + "VerticalAlignment", "HorizontalAlignment", "SpacingBetweenColumns", // fragment / building-block sentinel-internal keys (USE_FRAGMENT / // USE_BUILDING_BLOCK), consumed by the expander, never serialized "Args", "DataSourceOverride", "ActionOverride", diff --git a/sdk/pages/pages_widgets_container.go b/sdk/pages/pages_widgets_container.go index 4d3a96e45e..cc54e8e33c 100644 --- a/sdk/pages/pages_widgets_container.go +++ b/sdk/pages/pages_widgets_container.go @@ -15,18 +15,37 @@ type LayoutGrid struct { } // LayoutGridRow represents a row in a layout grid. +// +// A row is not a widget — it has no name — but it carries a Forms$Appearance +// (class, style, design properties) and its own alignment, exactly like one. +// Empty values mean Mendix's defaults: no appearance, alignments "None", and +// SpacingBetweenColumns true (NoSpacingBetweenColumns is its negation, so the +// zero value is the default). type LayoutGridRow struct { model.BaseElement - Columns []*LayoutGridColumn `json:"columns,omitempty"` + Columns []*LayoutGridColumn `json:"columns,omitempty"` + Class string `json:"class,omitempty"` + Style string `json:"style,omitempty"` + DynamicClasses string `json:"dynamicClasses,omitempty"` + DesignProperties []DesignPropertyValue `json:"designProperties,omitempty"` + VerticalAlignment string `json:"verticalAlignment,omitempty"` // "None" (default), "Start", "Center", "End" + HorizontalAlignment string `json:"horizontalAlignment,omitempty"` // "None" (default), "Start", "Center", "End" + NoSpacingBetweenColumns bool `json:"noSpacingBetweenColumns,omitempty"` } -// LayoutGridColumn represents a column in a layout grid. +// LayoutGridColumn represents a column in a layout grid. Like a row it has an +// appearance and a vertical alignment; empty means Mendix's default. type LayoutGridColumn struct { model.BaseElement - Weight int `json:"weight"` - TabletWeight int `json:"tabletWeight"` - PhoneWeight int `json:"phoneWeight"` - Widgets []Widget `json:"widgets,omitempty"` + Weight int `json:"weight"` + TabletWeight int `json:"tabletWeight"` + PhoneWeight int `json:"phoneWeight"` + Widgets []Widget `json:"widgets,omitempty"` + Class string `json:"class,omitempty"` + Style string `json:"style,omitempty"` + DynamicClasses string `json:"dynamicClasses,omitempty"` + DesignProperties []DesignPropertyValue `json:"designProperties,omitempty"` + VerticalAlignment string `json:"verticalAlignment,omitempty"` // "None" (default), "Start", "Center", "End" } // Container represents a generic container widget. From 5062de08310f11cf6b5fa2fc3685713d9a788ac8 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 21:07:08 +0000 Subject: [PATCH 05/13] fix(deprecation): MDL-DEPR081 names the $currentObject form it means MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MDL-DEPR081 told the author to write `Visible: / Editable: ` — "same meaning". It is not: inside the brackets a bare attribute is rooted in $currentObject, so dropping the brackets alone rebinds it (`Visible: ["N1"]` stores $currentObject/N1, `Visible: "N1"` does not). A hand migration from the message changed every notes-mode cell in sudoku with check, exec, mx check and tests all green (sudoku FINDINGS #63). fmt --upgrade and the Structural rewrite were already right; the one-line message contradicted its own suggestion. The entry's Canonical now names the binding. It is the text shown by the check/exec warning, the mdl-2 refusal, the LSP, fmt notes, help and the generated migration table (versions.md regenerated). Finding: .claude/skills/fix-issue/findings/mdl-other/2026-10-09-depr081-message-canonical-drops-currentobject.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...message-canonical-drops-currentobject.json | 1 + docs-site/src/language/versions.md | 2 +- mdl/deprecation/deprecation.go | 9 ++++++--- mdl/executor/validate_deprecations_test.go | 20 +++++++++++++++++++ 4 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-other/2026-10-09-depr081-message-canonical-drops-currentobject.json diff --git a/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-depr081-message-canonical-drops-currentobject.json b/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-depr081-message-canonical-drops-currentobject.json new file mode 100644 index 0000000000..be5ef1b114 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-depr081-message-canonical-drops-currentobject.json @@ -0,0 +1 @@ +{"area":"mdl/deprecation","date":"2026-10-09","symptom":"MDL-DEPR081 said `write Visible: / Editable: — same meaning`; following it by hand turns `Visible: [\"Value\" != empty]` into `Visible: Value != empty` (unbound) and `Visible: [\"N1\"]` into a non-attribute — every notes-mode cell in sudoku rebound, with check, exec, mx check and tests all green (sudoku FINDINGS #63)","cause":"the registry entry's `Canonical` was the bare form, but the brackets root a bare attribute in $currentObject; the Structural rewrite and `fmt --upgrade` already wrote `$currentObject/Attr`, so the one-line message contradicted its own suggestion","file":"`mdl/deprecation/deprecation.go` (BracketedWidgetCondition entry)","insight":"**`Canonical` is shown verbatim in six places (check/exec warning, mdl-2 refusal, LSP, fmt notes, help, migration table) behind a universal \"same meaning\" — so it must be the form that stores the same thing, not the form that merely parses.** Guard `TestBracketedWidgetConditionMessageNamesCurrentObject` reads the text between `write` and `same meaning`; on the old entry it fails with the bare form. `make gen-migration-reference` regenerates versions.md","refs":[],"ce":[]} diff --git a/docs-site/src/language/versions.md b/docs-site/src/language/versions.md index a66e7c2ad5..2f2df8a30b 100644 --- a/docs-site/src/language/versions.md +++ b/docs-site/src/language/versions.md @@ -287,7 +287,7 @@ refuse the spelling; until then it only warns. | `MDL-DEPR073` | `message definition collection M.C ( definition D for M.E ( A, M.E_B/M.B ( C ) ) )` | `message definition collection M.C { definition D for M.E { A, M.E_B/M.B { C } } }` | yes: message trees: each parenthesised definition list and member tree moves into { } | mdl 2 | | `MDL-DEPR074` | `alter microflow M.F { insert after $X { … } }` | `alter microflow M.F { insert after $X begin … end; }` | yes: fragment's braces: `{` becomes `begin` and `}` becomes `end` | mdl 2 | | `MDL-DEPR080` | `decision '' / timer '' / due date ''` | `decision / timer / due date ` | yes: expression out of its string: `decision '$Ctx/Total > 1000'` becomes `decision $Ctx/Total > 1000` | mdl 2 | -| `MDL-DEPR081` | `Visible: [] / Editable: []` | `Visible: / Editable: ` | yes: brackets into the expression they store: `Visible: [Active]` becomes `Visible: $currentObject/Active` | mdl 2 | +| `MDL-DEPR081` | `Visible: [] / Editable: []` | `Visible: / Editable: , each attribute as $currentObject/Attr` | yes: brackets into the expression they store: `Visible: [Active]` becomes `Visible: $currentObject/Active` | mdl 2 | | `MDL-DEPR082` | `revoke M.Role on M.E [(rights)]` | `revoke rights\|all on entity M.E from M.Role` | yes: rights (or `all` when none are listed) before `on entity`, roles after `from`: `revoke R on M.E (write *)` becomes `revoke write * on entity M.E from R`, `revoke R on M.E` becomes `revoke all on entity M.E from R` | mdl 2 | | `MDL-DEPR083` | `Username: $Const` | `Username: @Module.Const` | yes: `$Const` becomes `@.Const` | mdl 2 | | `MDL-DEPR084` | `Key: Module.Const` | `Key: @Module.Const` | yes: `@` before the constant's name | mdl 2 | diff --git a/mdl/deprecation/deprecation.go b/mdl/deprecation/deprecation.go index b236d8afc1..e6d2471084 100644 --- a/mdl/deprecation/deprecation.go +++ b/mdl/deprecation/deprecation.go @@ -928,9 +928,12 @@ var r5Entries = []Entry{ "decision $WorkflowContext/Total > 1000 outcomes true -> { } false -> { }; end workflow;", }, { - Code: BracketedWidgetCondition, - Old: "Visible: [] / Editable: []", - Canonical: "Visible: / Editable: ", + Code: BracketedWidgetCondition, + Old: "Visible: [] / Editable: []", + // The bare form alone is not the same meaning: inside the brackets a bare + // attribute is rooted in $currentObject, so the canonical form names it + // (sudoku FINDINGS #63 migrated from this text by hand and rebound them). + Canonical: "Visible: / Editable: , each attribute as $currentObject/Attr", Rewrite: Rewrite{Structural: "brackets into the expression they store: `Visible: [Active]` becomes " + "`Visible: $currentObject/Active`"}, RemovedIn: 2, diff --git a/mdl/executor/validate_deprecations_test.go b/mdl/executor/validate_deprecations_test.go index 0456d464d1..a828f44e98 100644 --- a/mdl/executor/validate_deprecations_test.go +++ b/mdl/executor/validate_deprecations_test.go @@ -105,3 +105,23 @@ func TestDeprecationWarningsEndWithHelpPointer(t *testing.T) { } } } + +// MDL-DEPR081's message says "same meaning", so the form it tells you to write +// must be the one the brackets stored. Dropping the brackets alone rebinds a +// bare attribute: `Visible: [Active]` roots Active in $currentObject, while +// `Visible: Active` does not — a hand migration from the old text changed every +// such condition (sudoku FINDINGS #63). fmt --upgrade was already right. +func TestBracketedWidgetConditionMessageNamesCurrentObject(t *testing.T) { + src := "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) { dataview dv (DataSource: $E) { " + + "textbox t (Attribute: Name, Visible: [Active]) } };" + got := deprecationViolations(t, src, deprecation.Warn) + if len(got) != 1 || got[0].RuleID != deprecation.BracketedWidgetCondition { + t.Fatalf("got %+v, want one %s", got, deprecation.BracketedWidgetCondition) + } + msg := got[0].Message + write := msg[strings.Index(msg, "write `"):] + write = write[:strings.Index(write, "— same meaning")] + if !strings.Contains(write, "$currentObject/") { + t.Errorf("the form the message says to write drops the $currentObject binding: %q", msg) + } +} From f116cb8e20422cb8a0c5988a742a6e81df083068 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 21:07:08 +0000 Subject: [PATCH 06/13] fix(visitor): hint the `if exists` order, and no false missing-`;` after recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `drop microflow M.F if exists;` (SQL order) gave only `extraneous input 'if'`, and under `mdl 1;` a second error claimed the statement "has no terminating `;`" — ANTLR's recovery ended the drop at the name and discarded `if exists;`, and the mdl-1 terminator check read that truncated statement (ledger FINDINGS #166). enhanceErrorMessage now names the order (`drop microflow if exists M.F;`). The error listener records the lines it reported on, and ExitStatement does not add a terminator error on such a line: the statement there is what recovery left, not what was written. A missing `;` on a clean line is still refused. Finding: .claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-drop-if-exists-after-name-blames-missing-semicolon.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...s-after-name-blames-missing-semicolon.json | 1 + mdl/visitor/drop_if_exists_hint_test.go | 47 +++++++++++++++++++ mdl/visitor/visitor.go | 20 ++++++++ mdl/visitor/visitor_strict_terminators.go | 6 ++- 4 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-drop-if-exists-after-name-blames-missing-semicolon.json create mode 100644 mdl/visitor/drop_if_exists_hint_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-drop-if-exists-after-name-blames-missing-semicolon.json b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-drop-if-exists-after-name-blames-missing-semicolon.json new file mode 100644 index 0000000000..7b47320d23 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-drop-if-exists-after-name-blames-missing-semicolon.json @@ -0,0 +1 @@ +{"area":"mdl/visitor","date":"2026-10-09","symptom":"`drop microflow M.F if exists;` (SQL order) gave `extraneous input 'if'` with no hint, and under `mdl 1;` a second error, `the statement ending at \"F\" has no terminating ;`, blaming a `;` that is present (ledger FINDINGS #166)","cause":"the grammar takes `if exists` before the name; ANTLR's single-token deletion ends the drop at the name and discards `if exists;`, and ExitStatement's mdl-1 terminator check then read the recovery-truncated statement as unterminated","file":"`mdl/visitor/visitor.go` (`dropIfExistsAfterNameRe` in enhanceErrorMessage; errorListener.lines -> Builder.syntaxErrorLines), `mdl/visitor/visitor_strict_terminators.go` (ExitStatement)","insight":"**A builder check that reads a statement's last tokens is reading the error-recovered tree on a line with a syntax error, so it must stand down there or it reports the recovery as a second mistake.** The parser finishes before the walk, so the listener's error lines are available to every Exit*. Guard `TestDropIfExistsAfterNameHint` (control `TestDropIfExistsControls`: canonical order parses; a genuinely missing `;` on a clean line is still refused); reverting only the terminator half brings back both `no terminating ;` errors","refs":[],"ce":[]} diff --git a/mdl/visitor/drop_if_exists_hint_test.go b/mdl/visitor/drop_if_exists_hint_test.go new file mode 100644 index 0000000000..c846e0f8a2 --- /dev/null +++ b/mdl/visitor/drop_if_exists_hint_test.go @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" +) + +const dropIfExistsHint = "`if exists` goes before the name" + +// TestDropIfExistsAfterNameHint covers ledger FINDINGS #166: the SQL order +// `drop microflow M.F if exists;` is a syntax error, and under `mdl 1;` the +// parser's recovery swallowed `if exists;` so a second error blamed a missing +// `;` that is there. The fix is one sentence: name the order, and do not claim +// a missing terminator on a line that already has a syntax error. +func TestDropIfExistsAfterNameHint(t *testing.T) { + for _, src := range []string{ + "drop microflow M.F if exists;", + "mdl 1;\ndrop microflow M.F if exists;", + "mdl 1;\ndrop module role M.Admin if exists;", + } { + _, errs := Build(src) + if len(errs) == 0 { + t.Fatalf("%q: expected a syntax error", src) + } + joined := errsText(errs) + if !strings.Contains(joined, dropIfExistsHint) { + t.Errorf("%q: no if-exists hint in:\n%s", src, joined) + } + if strings.Contains(joined, "no terminating `;`") { + t.Errorf("%q: blames a missing `;` that is present:\n%s", src, joined) + } + } +} + +// The control: the right order parses, and a statement that really lacks its +// `;` under mdl 1 is still refused for it. +func TestDropIfExistsControls(t *testing.T) { + if _, errs := Build("mdl 1;\ndrop microflow if exists M.F;"); len(errs) != 0 { + t.Errorf("canonical order refused: %v", errs) + } + _, errs := Build("mdl 1;\ndrop microflow if exists M.F\nlist modules;") + if !strings.Contains(errsText(errs), "no terminating `;`") { + t.Errorf("a genuinely missing `;` is no longer reported: %v", errs) + } +} diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index ff42c02218..2ad405417b 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -24,6 +24,9 @@ type errorListener struct { // hinted records the (line, hint) pairs already reported, so one mistake // that cascades into several ANTLR errors carries its explanation once. hinted map[string]bool + // lines holds every line a syntax error was reported on, so a builder + // check does not add a second, recovery-made error on the same line. + lines map[int]bool } func newErrorListener() *errorListener { @@ -31,6 +34,7 @@ func newErrorListener() *errorListener { DefaultErrorListener: antlr.NewDefaultErrorListener(), errors: make([]error, 0), hinted: make(map[string]bool), + lines: make(map[int]bool), } } @@ -45,6 +49,7 @@ func (l *errorListener) SyntaxError(rec antlr.Recognizer, sym any, line, column } enhancedMsg := l.deduplicateHint(enhanceErrorMessage(msg, offending), line) l.errors = append(l.errors, fmt.Errorf("line %d:%d %s", line, column, enhancedMsg)) + l.lines[line] = true } // deduplicateHint strips the explanatory block from an enhanced message when the @@ -124,6 +129,13 @@ func enhanceErrorMessage(msg, offendingLine string) string { " mdl 1;\n"+ " create entity Shop.Customer ( Name: String(200) ); (correct)", msg) } + // `drop microflow M.F if exists;` — the SQL order. The grammar takes the + // clause before the name, and ANTLR reports only an extraneous `if`. + if dropIfExistsAfterNameRe.MatchString(offendingLine) { + return fmt.Sprintf("%s\n\n `if exists` goes before the name, not after it:\n"+ + " drop microflow if exists M.F; (correct)\n"+ + " drop microflow M.F if exists; (SQL order, not MDL)", msg) + } // Grammar removed as dead (ako/mxcli#756): it parsed, and could never // succeed. The parse error is where the explanation now has to live. if workflowAccessRe.MatchString(offendingLine) { @@ -349,6 +361,10 @@ func enhanceErrorMessage(msg, offendingLine string) string { } // workflowAccessRe matches the removed `grant|revoke execute on workflow`. +// dropIfExistsAfterNameRe matches `drop if exists` — +// the clause after the name rather than before it. +var dropIfExistsAfterNameRe = regexp.MustCompile(`(?i)^\s*drop\s+[a-z][a-z ]*?\s+[\w."]+\.[\w."]+\s+if\s+exists\b`) + var workflowAccessRe = regexp.MustCompile(`(?i)^\s*(grant|revoke)\s+execute\s+on\s+workflow\b`) // addMissingAttributeRe matches `add :` on a source line — the shape of an @@ -550,6 +566,9 @@ type Builder struct { deprecations []ast.DeprecatedSpelling // flowCommits are the commits in create-or-modify flows (visitor_flow_commits.go). flowCommits []ast.FlowCommit + // syntaxErrorLines are the lines the parser already reported a syntax error + // on; ExitStatement does not add a terminator error there (see below). + syntaxErrorLines map[int]bool // detachedDocs are the doc comments on statements that do not store them, // and docsAwaitingNext the first of them still waiting for the next // statement that would; docStmtStart is where the current statement's @@ -684,6 +703,7 @@ func build(input string, opts buildOptions, listen func(*Builder) antlr.ParseTre // Create builder and walk the tree builder := NewBuilder() builder.session = opts.session + builder.syntaxErrorLines = errListener.lines tree := p.Program() antlr.ParseTreeWalkerDefault.Walk(listen(builder), tree) builder.noteBackslashEscapes(stream.GetAllTokens()) diff --git a/mdl/visitor/visitor_strict_terminators.go b/mdl/visitor/visitor_strict_terminators.go index 69c4687dbf..f5d0c838a3 100644 --- a/mdl/visitor/visitor_strict_terminators.go +++ b/mdl/visitor/visitor_strict_terminators.go @@ -47,7 +47,11 @@ func (b *Builder) ExitStatement(ctx *parser.StatementContext) { if last[0].GetTokenType() == parser.MDLParserSLASH { slash, last = last[0], last[1:] } - if len(last) > 0 && last[0].GetTokenType() != parser.MDLParserSEMICOLON && !(b.session && isLastStatement(ctx)) { + // A line that already has a syntax error was cut short by error recovery: + // `drop microflow M.F if exists;` ends the statement at `F` and drops + // `if exists;`, so "no terminating `;`" would blame a `;` that is there. + if len(last) > 0 && last[0].GetTokenType() != parser.MDLParserSEMICOLON && !(b.session && isLastStatement(ctx)) && + !b.syntaxErrorLines[last[0].GetLine()] { if b.gate(semicolonRequired, ctx) { b.addError(fmt.Errorf("line %d: the statement ending at %q has no terminating `;`: "+ "under %s every statement ends with `;`", last[0].GetLine(), last[0].GetText(), b.langVersion)) From aa86b124c58558d2200a8a4493f826229565963f Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 21:18:06 +0000 Subject: [PATCH 07/13] test(roundtrip): strike two snippets the layout-grid appearance fix repaired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit integration (roundtrip) failed with "no longer breaks getput — strike it from knownFailures" for WorkflowCommons.Snip_UserTask_NameColumnWithIcon and Snip_WorkflowJumpToDetails. Both pass getput at a1f04631 and still fail at its parent 4b1cfde3: carrying layout-grid row/column appearance and alignment through describe -> exec is what repaired them. The allowlist may only shrink. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- mdl/roundtrip/testapp_allowlist_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/mdl/roundtrip/testapp_allowlist_test.go b/mdl/roundtrip/testapp_allowlist_test.go index 939b2c370c..9a592ad04b 100644 --- a/mdl/roundtrip/testapp_allowlist_test.go +++ b/mdl/roundtrip/testapp_allowlist_test.go @@ -211,7 +211,6 @@ var testAppKnownFailures = map[string]knownFailure{ "snippet WorkflowCommons.Snip_UserTask_DueDate_Warning": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp (#743); putget passes since a template parameter written as an expression is stored as one (#823 option C)"}, "snippet WorkflowCommons.Snip_UserTask_Header": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_UserTask_NameColumn": {laws: []law{lawGetPut, lawPutGet}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); an association-path template parameter read from a snippet parameter loses its AttributeRef and SourceVariable (#721)"}, - "snippet WorkflowCommons.Snip_UserTask_NameColumnWithIcon": {laws: []law{lawGetPut}, issue: "#721 #826", why: "executes since describe prints snippet call Params (#826); an image's attribute-value visibility is described twice and written as none (#721 C)"}, "snippet WorkflowCommons.Snip_UserTask_NotificationArea": {laws: []law{lawGetPut, lawPutGet}, issue: "#721", why: "snippet: breaks getput and putget on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_UserTask_UnassignedMessage": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowActivityRecord_ActivityIcon": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, @@ -225,7 +224,6 @@ var testAppKnownFailures = map[string]knownFailure{ "snippet WorkflowCommons.Snip_WorkflowDefinition_AuditTrail": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowDefinition_Header": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes; executes since a widget bound to a snippet parameter is described as $Param.Attr — getput not yet triaged"}, "snippet WorkflowCommons.Snip_WorkflowEndedUserTask_Header": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, - "snippet WorkflowCommons.Snip_WorkflowJumpToDetails": {laws: []law{lawGetPut}, issue: "#721", why: "snippet: breaks getput and putget on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowSubProcess_Details": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowSubProcess_IncompatibleWarning": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, "snippet WorkflowCommons.Snip_WorkflowSubProcess_State": {laws: []law{lawExec}, issue: "#721", why: "snippet: breaks exec on TestApp, measured when it joined the harness (#743); not yet triaged into #721's classes"}, From 58565b54431aa9e41dcf1cd4b57595d70aeec437 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 21:24:46 +0000 Subject: [PATCH 08/13] fix(visitor): hint the quoted last segment for a hyphenated icon name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Icon: Atlas_Core.Atlas.add-circle` failed with an error pointing at a dot (or `no viable alternative` on a widget), and quoting the whole name read as a stray string; neither said what to write, and it cost the ChipCoV6 build two retries. The grammar is right — an icon is a qualified name and `add-circle` is not an identifier — so the fix is a source-line hint naming `Atlas_Core.Atlas."add-circle"`, for menu items and widget icons, unquoted or quoted whole. Finding: .claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-hyphenated-icon-name-no-hint.json Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- ...26-10-09-hyphenated-icon-name-no-hint.json | 1 + mdl/visitor/hyphenated_icon_hint_test.go | 52 +++++++++++++++++++ mdl/visitor/visitor.go | 16 ++++++ 3 files changed, 69 insertions(+) create mode 100644 .claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-hyphenated-icon-name-no-hint.json create mode 100644 mdl/visitor/hyphenated_icon_hint_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-hyphenated-icon-name-no-hint.json b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-hyphenated-icon-name-no-hint.json new file mode 100644 index 0000000000..fd47cbd617 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-visitor/2026-10-09-hyphenated-icon-name-no-hint.json @@ -0,0 +1 @@ +{"area":"mdl/visitor","date":"2026-10-09","symptom":"`Icon: Atlas_Core.Atlas.add-circle` failed with `extraneous input '.' expecting {',', ')'}` (pointing at a dot) or `no viable alternative` on a widget, and `Icon: 'Atlas_Core.Atlas.add-circle'` with `mismatched input ... expecting the start of a statement`; neither said that the fix is `Atlas_Core.Atlas.\"add-circle\"` — two retries in ChipCoV6's build","cause":"an icon is a qualified name and `add-circle` is not an identifier; the grammar is right to refuse both forms, but the error carried no hint","file":"`mdl/visitor/visitor.go` (`hyphenatedIconRe` in enhanceErrorMessage)","insight":"**When the grammar is right and the mistake is predictable from the source line, the fix is a hint keyed on the line, not a grammar change** — accepting the whole-name string would break \"references are qualified names\" and need its own resolution path. Guard `TestHyphenatedIconNameHint` covers menu item, quoted-whole and widget icon; `TestHyphenatedIconNameHintControls` keeps the quoted segment parsing and keeps the hint off unrelated errors on an icon line","refs":[],"ce":[]} diff --git a/mdl/visitor/hyphenated_icon_hint_test.go b/mdl/visitor/hyphenated_icon_hint_test.go new file mode 100644 index 0000000000..475daff6b0 --- /dev/null +++ b/mdl/visitor/hyphenated_icon_hint_test.go @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "strings" + "testing" +) + +// TestHyphenatedIconNameHint covers the ChipCoV6 finding: an Atlas icon whose +// name has a hyphen must quote that last segment, `Atlas_Core.Atlas."add-circle"`. +// Unquoted, the parse error pointed at a dot; quoting the whole name read as a +// stray string. Neither said what to write, and it cost two retries. +func TestHyphenatedIconNameHint(t *testing.T) { + const want = "Atlas_Core.Atlas.\"add-circle\"" + for _, src := range []string{ + "mdl 1;\ncreate or modify navigation Responsive\n home page M.Home\n{\n" + + " menu item 'Report' ( OnClick: show page M.P, Icon: Atlas_Core.Atlas.add-circle )\n};", + "mdl 1;\ncreate or modify navigation Responsive\n home page M.Home\n{\n" + + " menu item 'Report' ( OnClick: show page M.P, Icon: 'Atlas_Core.Atlas.add-circle' )\n};", + "mdl 1;\ncreate page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) {\n" + + " actionbutton b (Caption: 'Add', Icon: Atlas_Core.Atlas.add-circle)\n};", + } { + _, errs := Build(src) + if len(errs) == 0 { + t.Fatalf("expected a syntax error for:\n%s", src) + } + joined := errsText(errs) + if !strings.Contains(joined, want) { + t.Errorf("no quoted-segment hint naming %s in:\n%s", want, joined) + } + if n := strings.Count(joined, "quote the icon name"); n > 1 { + t.Errorf("hint repeated %d times for one mistake:\n%s", n, joined) + } + } +} + +// The control: the quoted segment parses, and an unrelated error on a line +// with an icon gets no icon hint. +func TestHyphenatedIconNameHintControls(t *testing.T) { + ok := "mdl 1;\ncreate page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) {\n" + + " actionbutton b (Caption: 'Add', Icon: Atlas_Core.Atlas.\"add-circle\")\n};" + if _, errs := Build(ok); len(errs) != 0 { + t.Errorf("quoted segment refused: %v", errs) + } + bad := "mdl 1;\ncreate page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default) {\n" + + " actionbutton b (Caption: 'Add' Icon: Atlas_Core.Atlas.home)\n};" + _, errs := Build(bad) + if strings.Contains(errsText(errs), "quote the icon name") { + t.Errorf("icon hint on an error that is not about the icon name:\n%s", errsText(errs)) + } +} diff --git a/mdl/visitor/visitor.go b/mdl/visitor/visitor.go index 2ad405417b..8cf8fc8e9f 100644 --- a/mdl/visitor/visitor.go +++ b/mdl/visitor/visitor.go @@ -136,6 +136,17 @@ func enhanceErrorMessage(msg, offendingLine string) string { " drop microflow if exists M.F; (correct)\n"+ " drop microflow M.F if exists; (SQL order, not MDL)", msg) } + // An icon name with a hyphen (`add-circle`): it is not an identifier, so + // the last segment is quoted — and only that segment, since an icon is a + // qualified name, not a string. Unquoted the error points at a dot; quoted + // whole it reads as a stray string. Neither says what to write. + if m := hyphenatedIconRe.FindStringSubmatch(offendingLine); m != nil { + return fmt.Sprintf("%s\n\n An icon name with a hyphen is not an identifier: quote the icon name's last\n"+ + " segment, and only that segment — the icon is a qualified name, not a string:\n"+ + " Icon: %s.\"%s\" (correct)\n"+ + " Icon: %s.%s (not an identifier)\n"+ + " Icon: '%s.%s' (a string, not a reference)", msg, m[1], m[2], m[1], m[2], m[1], m[2]) + } // Grammar removed as dead (ako/mxcli#756): it parsed, and could never // succeed. The parse error is where the explanation now has to live. if workflowAccessRe.MatchString(offendingLine) { @@ -365,6 +376,11 @@ func enhanceErrorMessage(msg, offendingLine string) string { // the clause after the name rather than before it. var dropIfExistsAfterNameRe = regexp.MustCompile(`(?i)^\s*drop\s+[a-z][a-z ]*?\s+[\w."]+\.[\w."]+\s+if\s+exists\b`) +// hyphenatedIconRe matches an icon reference whose last segment has a hyphen, +// unquoted or quoted as one string: `Icon: Mod.Coll.add-circle` or +// `Icon: 'Mod.Coll.add-circle'`. Group 1 is the collection, group 2 the name. +var hyphenatedIconRe = regexp.MustCompile(`(?i)\bicon\s*:?\s*'?([A-Za-z_]\w*\.[A-Za-z_]\w*)\.([A-Za-z_]\w*(?:-\w+)+)'?`) + var workflowAccessRe = regexp.MustCompile(`(?i)^\s*(grant|revoke)\s+execute\s+on\s+workflow\b`) // addMissingAttributeRe matches `add :` on a source line — the shape of an From edb7ef10645ce2b7156372be57b37b269e02f052 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 08:45:37 +0000 Subject: [PATCH 09/13] feat(pages): write a page variable's default as a bare expression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A page or snippet variable's default is a Mendix expression, but MDL took it in a string whose content was the expression: `$show: Boolean = 'true'`. It is now written bare (R5) — `= true`, `= if (3 < 4) then true else false`, `= 'Price' + ' list'`, `= Module.Enum.Value` — in create page, create snippet and `alter page … add variables`. mdl 1 is frozen, so the string form keeps its meaning under every language version and is the deprecated alias MDL-DEPR086 (refused from mdl 2), with the `fmt --upgrade` rewrite. A default that is itself a string (`'''abc'''`) or empty has no bare spelling yet — the bare `'abc'` is the alias — and is not reported. describe writes the bare form when it reads back as the same default. The example scripts were converted with fmt --upgrade itself; docs, syntax help, skills and the generated migration table follow. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01P65SqmwwvbWdJVRwhYiMQw --- .claude/skills/mendix/alter-page/SKILL.md | 4 +- .claude/skills/mendix/create-page/SKILL.md | 2 +- CHANGELOG.md | 1 + cmd/mxcli/syntax/features_page.go | 8 +- docs-site/src/appendixes/quick-reference.md | 4 +- docs-site/src/examples/alter-page.md | 2 +- docs-site/src/language/alter-page.md | 2 +- docs-site/src/language/page-structure.md | 4 +- docs-site/src/language/pages.md | 2 +- docs-site/src/language/versions.md | 3 +- docs-site/src/reference/page/alter-page.md | 4 +- docs-site/src/reference/page/create-page.md | 4 +- docs/01-project/MDL_QUICK_REFERENCE.md | 4 +- .../datagrid-column-visible-expression.mdl | 2 +- .../bug-tests/page-variable-bindings.mdl | 4 +- .../doctype-tests/03-page-examples.mdl | 6 +- .../doctype-tests/29-datagrid-examples.mdl | 2 +- .../doctype-tests/33-alter-page-examples.mdl | 4 +- mdl/ast/ast_alter_page.go | 2 +- mdl/deprecation/deprecation.go | 19 +++ mdl/deprecation/topics.go | 1 + mdl/executor/cmd_pages_builder.go | 2 +- mdl/executor/cmd_pages_describe.go | 14 ++- .../cmd_pages_input_binding_context.go | 2 +- .../describe_page_variable_default_test.go | 33 ++++++ mdl/grammar/domains/MDLPage.g4 | 4 +- mdl/upgrade/page_variable_default_test.go | 34 ++++++ mdl/visitor/visitor_page_v3.go | 8 +- .../visitor_page_variable_default_test.go | 110 ++++++++++++++++++ mdl/visitor/visitor_variable_default.go | 83 +++++++++++++ 30 files changed, 337 insertions(+), 37 deletions(-) create mode 100644 mdl/executor/describe_page_variable_default_test.go create mode 100644 mdl/upgrade/page_variable_default_test.go create mode 100644 mdl/visitor/visitor_page_variable_default_test.go create mode 100644 mdl/visitor/visitor_variable_default.go diff --git a/.claude/skills/mendix/alter-page/SKILL.md b/.claude/skills/mendix/alter-page/SKILL.md index 01d1d60f85..c352ff1a5c 100644 --- a/.claude/skills/mendix/alter-page/SKILL.md +++ b/.claude/skills/mendix/alter-page/SKILL.md @@ -370,7 +370,7 @@ The older dotted form `gridName.columnName` still works; it matches a name mxcli ### ADD Variables - Add a Page Variable ```sql -add variables $showStockColumn: boolean = 'true' +add variables $showStockColumn: boolean = true ``` Adds a new page variable (`Forms$LocalVariable`) to the page/snippet. DataType can be `boolean`, `string`, `integer`, `decimal`, `datetime`, or an entity type. Default value is a Mendix expression in single quotes. @@ -451,7 +451,7 @@ alter page MyModule.Customer_Edit { ```sql mdl 1; alter page MyModule.ProductOverview { - add variables $showStockColumn: boolean = 'if (3 < 4) then true else false' + add variables $showStockColumn: boolean = if (3 < 4) then true else false }; ``` diff --git a/.claude/skills/mendix/create-page/SKILL.md b/.claude/skills/mendix/create-page/SKILL.md index 2e11cd06d1..70611b0969 100644 --- a/.claude/skills/mendix/create-page/SKILL.md +++ b/.claude/skills/mendix/create-page/SKILL.md @@ -30,7 +30,7 @@ Guide for writing CREATE PAGE statements in Mendix Definition Language (MDL). create [or replace] page Module.PageName ( [params: ( $ParamName: Module.EntityType | PrimitiveType, ... ),] - [variables: ( $varName: DataType = 'defaultExpression', ... ),] + [variables: ( $varName: DataType = , ... ),] -- bare: `= true`, not `= 'true'` title: 'Page Title', layout: Module.LayoutName, [url: 'page-url',] diff --git a/CHANGELOG.md b/CHANGELOG.md index 68988b1d9a..57505d5b93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed +- **A page or snippet variable's default is written as a bare expression** — `Variables: ( $show: Boolean = true )`, `= if (3 < 4) then true else false`, `= 'Price' + ' list'`, `= Module.Enum.Value`, and the same in `alter page … add variables`. The old spelling, a string whose content is the expression (`= 'true'`), keeps its meaning under every language version and warns as **MDL-DEPR086** (refused from `mdl 2`). `fmt --upgrade` rewrites it, and `describe` prints the bare form. A default that is itself a string (`$s: String = '''abc'''`) or is empty keeps its current spelling and is not reported, since a bare `'abc'` is the old spelling. - **A published OData service's authentication is a property** — `create published odata service M.S ( …, Authentication: (basic, session, microflow M.Authenticate) )`, in the order written, replaces the trailing `authentication basic, session` clause (R9). The property can also say `Authentication: none`, which the clause could not, and `alter published odata service M.S set ( Authentication: … )` now changes authentication on an existing service — before, it could only be restated with the whole service. Left out, `create or modify` and `alter` keep the stored setting, as before. `describe` prints the property. **Migrating a script:** nothing breaks — the clause (**MDL-DEPR139**) still parses and builds the same service, `check` / `exec` warn, and `mxcli fmt --upgrade` moves it into the list (a clause naming a method MDL has no keyword for, e.g. `authentication Custom`, is reported and left alone). A stored setting MDL cannot state — a microflow stored without the Microflow method, or the reverse — is now a comment in `describe` rather than printed as `Microflow M.F`, which used to add the method when the output was executed. Executing the `describe` output of each of ako/TestApp's three Studio Pro OData services writes nothing. - **`call rest service` takes its settings as one property list** (ADR-0013) — the activity's dialog settings go in one `( Key: value, … )` list after the URL, keyed as the consumed REST service names the same concepts: `$Html = call rest service get 'https://example.com' (Headers: ('Accept': 'text/html'), Authentication: basic (Username: $User, Password: $Password), Timeout: 300) returns String;`. `Body:` is `template '…' [with ({1} = …)]`, `mapping M.EMM from $Var`, `binary ` or an expression. The method, URL, `returns …` and `on error …` stay words. An unknown or repeated key, or a value of the wrong shape, is an error. `describe` writes this form. **Migrating a script:** nothing breaks — the clauses `header 'N' = v`, `auth basic $u password $p`, `body …` and `timeout n` (**MDL-DEPR720**) still parse and store the same activity, `check` / `exec` warn, and `mxcli fmt --upgrade` rewrites them; a statement cannot mix the two forms. ADR-0013 makes this the rule for every new microflow activity and every activity with several settings. - **`run --local --page-check` signs in with `--screenshot-user` without `--screenshot`, and checks all pages in one browser** — the sign-in only ran when `--screenshot` was also given, so `--page-check --screenshot-user U` reported every secured page as the login page. The verdict also no longer counts a list view's "No items found" placeholder or a data grid's header as rows, ignores the demo-user switcher's "Select user" heading, and reports a failed same-origin request (`HTTP 560 POST /xas/`) instead of the duplicate "Failed to load resource" console line. The login script used by `--screenshot-user` falls back to `/login.html` when the app root does not show the sign-in form, and finds Playwright the way the page check does (no `playwright` CLI on `PATH` needed). diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 7451b84a4e..5f22061a97 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -12,7 +12,7 @@ func init() { "page", "pages", "form", "UI", "user interface", "widget", "layout", "screen", }, - Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Page Title',\n Layout: Module.LayoutName\n [, Params: ( $Param: Module.Entity )]\n [, Url: 'page-url']\n [, Variables: ( $var: Boolean = 'true' )]\n [, PopupWidth: 800, PopupHeight: 480, PopupResizable: true]\n [, PopupCloseAction: cancelButton1]\n [, Class: 'css-class', Style: 'css: rule']\n )\n {\n -- widgets\n }", + Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Page Title',\n Layout: Module.LayoutName\n [, Params: ( $Param: Module.Entity )]\n [, Url: 'page-url']\n [, Variables: ( $var: Boolean = true )] -- the default is an expression, written bare\n [, PopupWidth: 800, PopupHeight: 480, PopupResizable: true]\n [, PopupCloseAction: cancelButton1]\n [, Class: 'css-class', Style: 'css: rule']\n )\n {\n -- widgets\n }", Example: "CREATE PAGE MyModule.EditCustomer\n (\n Params: ( $Customer: MyModule.Customer ),\n Title: 'Edit Customer',\n Layout: Atlas_Core.PopupLayout,\n Class: 'container-fluid'\n )\n {\n DATAVIEW dvCustomer (DataSource: $Customer) {\n TEXTBOX txtName (Label: 'Name', Attribute: Name)\n FOOTER {\n ACTIONBUTTON btnSave (Caption: 'Save', Action: SAVE CHANGES, ButtonStyle: Primary)\n ACTIONBUTTON btnCancel (Caption: 'Cancel', Action: CANCEL CHANGES)\n }\n }\n };", SeeAlso: []string{"page.create", "page.widgets", "page.alter", "snippet", "rename"}, }) @@ -24,8 +24,8 @@ func init() { "create page", "new page", "page parameters", "page variables", "layout", "url", "folder", }, - Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Title',\n Layout: Module.Layout\n [, Params: ( $P: Module.Entity, $Qty: Integer )]\n [, Url: 'page-url']\n [, Variables: ( $showStock: Boolean = 'true' )]\n )\n { }", - Example: "CREATE PAGE Module.Products\n (\n Title: 'Products',\n Layout: Atlas_Core.Atlas_Default,\n Url: 'products',\n Variables: ( $showStock: Boolean = 'true' )\n )\n {\n DATAGRID gridProducts (DataSource: DATABASE Module.Product) {\n COLUMN (Attribute: Name, Caption: 'Name')\n }\n };", + Syntax: "CREATE PAGE Module.Name [FOLDER 'FolderPath']\n (\n Title: 'Title',\n Layout: Module.Layout\n [, Params: ( $P: Module.Entity, $Qty: Integer )]\n [, Url: 'page-url']\n [, Variables: ( $showStock: Boolean = true )]\n )\n { }", + Example: "CREATE PAGE Module.Products\n (\n Title: 'Products',\n Layout: Atlas_Core.Atlas_Default,\n Url: 'products',\n Variables: ( $showStock: Boolean = true )\n )\n {\n DATAGRID gridProducts (DataSource: DATABASE Module.Product) {\n COLUMN (Attribute: Name, Caption: 'Name')\n }\n };", SeeAlso: []string{"page", "page.widgets", "page.datasource"}, }) @@ -435,7 +435,7 @@ LIST IMPACT OF htmlelement; // (a PAGE parameter may be primitive; a snippet parameter may not), and // the reporter of mendixlabs/mxcli#1028 reached the bug by following // this line. A primitive is now refused as MDL087. - Syntax: "CREATE SNIPPET Module.Name [FOLDER 'Snippets/Common']\n [( Params: ( $P: Module.Entity ) )] -- entities only; a primitive is CE0046\n [( Variables: ( $isEditable: Boolean = 'true' ) )]\n {\n -- widgets\n }\n\n-- To parameterise a snippet on a primitive, keep the primitive on the\n-- calling PAGE and pass an object, or read the value off an entity member.", + Syntax: "CREATE SNIPPET Module.Name [FOLDER 'Snippets/Common']\n [( Params: ( $P: Module.Entity ) )] -- entities only; a primitive is CE0046\n [( Variables: ( $isEditable: Boolean = true ) )]\n {\n -- widgets\n }\n\n-- To parameterise a snippet on a primitive, keep the primitive on the\n-- calling PAGE and pass an object, or read the value off an entity member.", Example: "CREATE SNIPPET MyModule.NavigationMenu\n{\n NAVIGATIONLIST navMenu {\n ITEM itemCustomers (Action: SHOW PAGE MyModule.CustomerOverview) {\n DYNAMICTEXT txtCustomers (Content: 'Customers')\n }\n }\n};", SeeAlso: []string{"snippet", "snippet.alter", "page.widgets"}, }) diff --git a/docs-site/src/appendixes/quick-reference.md b/docs-site/src/appendixes/quick-reference.md index bb6a3328f1..a7db06bba3 100644 --- a/docs-site/src/appendixes/quick-reference.md +++ b/docs-site/src/appendixes/quick-reference.md @@ -355,7 +355,7 @@ MDL uses explicit property declarations for pages: | Element | Syntax | Example | |---------|-----------|---------| | Page properties | `(Key: value, ...)` | `(Title: 'Edit', Layout: Atlas_Core.Atlas_Default)` | -| Page variables | `Variables: ( $name: Type = 'expr' )` | `Variables: ( $show: Boolean = 'true' )` | +| Page variables | `Variables: ( $name: Type = )` | `Variables: ( $show: Boolean = true )` | | Widget name | Required after type | `TEXTBOX txtName (...)` | | Attribute binding | `Attribute: AttrName` | `TEXTBOX txt (Label: 'Name', Attribute: Name)` | | Variable binding | `DataSource: $Var` | `DATAVIEW dv (DataSource: $Product) { ... }` | @@ -436,7 +436,7 @@ Modify an existing page or snippet's widget tree in-place without full `CREATE O | Drop widgets | `DROP name1, name2` | Remove widgets by name | | Replace widget | `REPLACE widgetName WITH { widgets }` | Replace widget subtree | | Pluggable prop | `SET ('showLabel': false) ON cbStatus` | Quoted name for pluggable widgets | -| Add variable | `ADD Variables $name: Type = 'expr'` | Add a page variable | +| Add variable | `ADD Variables $name: Type = ` | Add a page variable | | Drop variable | `DROP Variables $name` | Remove a page variable | | Add parameter | `ADD Parameters $name: Type` | Add a page/snippet parameter (entity or primitive; snippet: entity only). A page with a `Url` needs a `{name}` segment — `SET (Url: …)` in the same statement | | Drop parameter | `DROP Parameters $name` | Remove a parameter; refused while the page still uses it | diff --git a/docs-site/src/examples/alter-page.md b/docs-site/src/examples/alter-page.md index 6405a985e9..5acd1e6c8b 100644 --- a/docs-site/src/examples/alter-page.md +++ b/docs-site/src/examples/alter-page.md @@ -71,7 +71,7 @@ ALTER PAGE CRM.Customer_Edit { ```sql ALTER PAGE CRM.ProductOverview { - ADD Variables $showStockColumn: Boolean = 'if (3 < 4) then true else false' + ADD Variables $showStockColumn: Boolean = if (3 < 4) then true else false }; ``` diff --git a/docs-site/src/language/alter-page.md b/docs-site/src/language/alter-page.md index 3890d15c14..85c7303843 100644 --- a/docs-site/src/language/alter-page.md +++ b/docs-site/src/language/alter-page.md @@ -170,7 +170,7 @@ Add or remove page-level variables: mdl 1; -- Add a variable ALTER PAGE Module.EditPage { - ADD Variables $showAdvanced: Boolean = 'false' + ADD Variables $showAdvanced: Boolean = false }; -- Remove a variable diff --git a/docs-site/src/language/page-structure.md b/docs-site/src/language/page-structure.md index 24a4364d7c..0add00cbd2 100644 --- a/docs-site/src/language/page-structure.md +++ b/docs-site/src/language/page-structure.md @@ -10,7 +10,7 @@ CREATE [OR REPLACE] PAGE . [FOLDER ''] [Params: ( $Param: Module.Entity | Type [, ...] ),] Title: '', Layout: <Module.LayoutName> - [, Variables: ( $name: Type = 'expression' [, ...] )] + [, Variables: ( $name: Type = <expression> [, ...] )] ) { <widget-tree> @@ -86,7 +86,7 @@ Page variables store local state (booleans, strings, etc.) that can control widg ( Title: 'Product Detail', Layout: Atlas_Core.Atlas_Default, - Variables: ( $showDetails: Boolean = 'true' ) + Variables: ( $showDetails: Boolean = true ) ) ``` diff --git a/docs-site/src/language/pages.md b/docs-site/src/language/pages.md index c5fbf7d565..446f051472 100644 --- a/docs-site/src/language/pages.md +++ b/docs-site/src/language/pages.md @@ -73,7 +73,7 @@ CREATE PAGE MyModule.Customer_Edit | `Params` | Page parameters (entity objects or primitives) | `Params: ( $Order: Sales.Order, $Qty: Integer )` | | `Title` | Page title shown in the browser/tab | `Title: 'Edit Customer'` | | `Layout` | Layout to use for the page | `Layout: Atlas_Core.PopupLayout` | -| `Variables` | Page-level variables for conditional logic | `Variables: ( $show: Boolean = 'true' )` | +| `Variables` | Page-level variables for conditional logic | `Variables: ( $show: Boolean = true )` | | `Class` | CSS class applied to the page (Forms$Appearance) | `Class: 'container-fluid bg-light'` | | `Style` | Inline CSS style applied to the page | `Style: 'min-height: 100vh'` | diff --git a/docs-site/src/language/versions.md b/docs-site/src/language/versions.md index a66e7c2ad5..065c1fd687 100644 --- a/docs-site/src/language/versions.md +++ b/docs-site/src/language/versions.md @@ -255,7 +255,7 @@ refuse the spelling; until then it only warns. ### Deprecated spellings (`MDL-DEPR*`) -87 old spellings mean exactly what their new form means. They warn with their code under every version before the one in the last column, which refuses them. +88 old spellings mean exactly what their new form means. They warn with their code under every version before the one in the last column, which refuses them. | Code | Old form | New form | Rewritten by `fmt --upgrade` | Refused from | |---|---|---|---|---| @@ -292,6 +292,7 @@ refuse the spelling; until then it only warns. | `MDL-DEPR083` | `Username: $Const` | `Username: @Module.Const` | yes: `$Const` becomes `@<the service's module>.Const` | mdl 2 | | `MDL-DEPR084` | `Key: Module.Const` | `Key: @Module.Const` | yes: `@` before the constant's name | mdl 2 | | `MDL-DEPR085` | `alter settings constant 'Module.Const' …` | `alter settings constant @Module.Const …` | yes: constant's name out of its string, with `@`: `constant 'M.ApiUrl'` becomes `constant @M.ApiUrl` | mdl 2 | +| `MDL-DEPR086` | `Variables: ( $name: Type = '<expression>' ) / add variables $name: Type = '<expression>'` | `Variables: ( $name: Type = <expression> ) / add variables $name: Type = <expression>` | yes: default out of its string: `$show: boolean = 'true'` becomes `$show: boolean = true` | mdl 2 | | `MDL-DEPR090` | `show page\|project security\|security matrix\|structure\|context of …` | `describe page\|app security\|security matrix\|structure\|context of …` | yes: verb as `describe`: `show page X` -> `describe page X`, `show project security` -> `describe app security`; the same for `list` on these forms | mdl 2 | | `MDL-DEPR091` | `alter user role R remove module roles (…)` | `alter user role R drop module roles (…)` | yes: `remove` → `drop` | mdl 2 | | `MDL-DEPR092` | `alter settings language remove '…' / alter settings workflows remove group '…'` | `alter settings language drop '…' / alter settings workflows drop group '…'` | yes: `remove` → `drop` | mdl 2 | diff --git a/docs-site/src/reference/page/alter-page.md b/docs-site/src/reference/page/alter-page.md index a3e8457b01..8eea22a0ea 100644 --- a/docs-site/src/reference/page/alter-page.md +++ b/docs-site/src/reference/page/alter-page.md @@ -53,7 +53,7 @@ DROP widgetName1, widgetName2; REPLACE widgetName WITH { widget_definitions }; -- Add a page variable -ADD Variables $name : type = 'expression'; +ADD Variables $name : type = <expression>; -- Drop a page variable DROP Variables $name; @@ -215,7 +215,7 @@ Add and drop page variables: ```sql mdl 1; ALTER PAGE Sales.Order_Edit { - ADD Variables $showAdvanced : Boolean = 'false'; + ADD Variables $showAdvanced : Boolean = false; }; ALTER PAGE Sales.Order_Edit { diff --git a/docs-site/src/reference/page/create-page.md b/docs-site/src/reference/page/create-page.md index 60b431f1b8..b4a30d5b74 100644 --- a/docs-site/src/reference/page/create-page.md +++ b/docs-site/src/reference/page/create-page.md @@ -8,7 +8,7 @@ CREATE [ OR REPLACE ] PAGE module.Name [ FOLDER 'path' ] [ Params: ( $param : Module.Entity | Type [, ...] ), ] Title: 'title', Layout: Module.LayoutName - [, Variables: ( $name : type = 'expression' [, ...] ) ] + [, Variables: ( $name : type = <expression> [, ...] ) ] ) { widget_tree @@ -371,7 +371,7 @@ CREATE PAGE MyModule.AdvancedForm Params: ( $Item: MyModule.Item ), Title: 'Advanced Form', Layout: Atlas_Core.Atlas_Default, - Variables: ( $showAdvanced: Boolean = 'false' ) + Variables: ( $showAdvanced: Boolean = false ) ) { DATAVIEW dvItem (DataSource: $Item) { diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 9e168ad279..13cc50ff10 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -1474,7 +1474,7 @@ MDL uses explicit property declarations for pages: | Pop-up close button | `PopupCloseAction: <widgetName>` | `(Layout: Atlas_Core.PopupLayout, PopupCloseAction: cancelButton1)` — names a widget on this page. Not carried from the stored document on a rewrite: the statement rebuilds the widget tree, so a carried name could dangle | | DataView read-only style | `ReadOnlyStyle: Inherit\|Control\|Text` | `dataview dv (datasource: $O, ReadOnlyStyle: Text)` — a DataView's own, distinct from a checkbox's. **Control** is Studio Pro's default here, not Inherit | | Page CSS class / style | `Class: 'css-class', Style: 'css: rule'` | `(Title: 'Home', Class: 'container-fluid bg-light', Style: 'min-height: 100vh')` — the page's Appearance | -| Page variables | `variables: ( $name: type = 'expr' )` | `variables: ( $show: boolean = 'true' )` | +| Page variables | `variables: ( $name: type = <expr> )` | `variables: ( $show: boolean = true )` | | Page parameters | `params: ( $name: type, … )` | `params: ( $Order: Shop.Order )` — a map, in `( )`; `params: { … }` is the deprecated spelling (MDL-DEPR123) | | Snippet call arguments | `snippetcall s (snippet: M.S, params: (Param = $var))` | Bound as at every call site, `Param = value` (R4). `params: {$Param: $var}` is deprecated (MDL-DEPR126) | | Text template parameters | `contentparams: ({1} = expr, …)` | Also `captionparams:` and a pluggable widget's `<Name>Params:`. `[…]` is deprecated (MDL-DEPR124) | @@ -1717,7 +1717,7 @@ This is the generic ALTER — `alter <type> Module.Name { set (Key: value) on <t | Set column prop | `set (caption: 'New') on dgGrid column(Attr)` | A DataGrid 2 column by its attribute, or `column('Caption')`; `@n` when two columns match. The older `dgGrid.colName` (a derived name) still works | | Drop attribute | `drop dgGrid column(Attr)` | Remove a DataGrid column | | Insert column | `insert after dgGrid column(Attr) { column (…) }` | Add attribute to DataGrid; a column takes no name | -| Add variable | `add variables $name: type = 'expr'` | Add a page variable | +| Add variable | `add variables $name: type = <expr>` | Add a page variable | | Drop variable | `drop variables $name` | Remove a page variable | | Add parameter | `add parameters $name: type` | Add a page/snippet parameter (entity or primitive; snippet: entity only). A page with a `Url` needs a `{name}` segment — `set (Url: …)` in the same statement | | Drop parameter | `drop parameters $name` | Remove a parameter; refused while the page still uses it | diff --git a/mdl-examples/bug-tests/datagrid-column-visible-expression.mdl b/mdl-examples/bug-tests/datagrid-column-visible-expression.mdl index dabf9414d3..5d2ed81e7b 100644 --- a/mdl-examples/bug-tests/datagrid-column-visible-expression.mdl +++ b/mdl-examples/bug-tests/datagrid-column-visible-expression.mdl @@ -35,7 +35,7 @@ create entity MyFirstModule.ColVisItem ( Name: String(100), Price: Decimal ); create or modify page MyFirstModule.P_ColumnVisible ( Title: 'Column visible', Layout: Atlas_Core.Atlas_Default, - Variables: ( $showPrices: boolean = 'true' ) ) + Variables: ( $showPrices: boolean = true ) ) { datagrid dg (datasource: database MyFirstModule.ColVisItem) { column (attribute: Name, caption: 'Var', Visible: $showPrices) diff --git a/mdl-examples/bug-tests/page-variable-bindings.mdl b/mdl-examples/bug-tests/page-variable-bindings.mdl index 6dd74d90bc..edc9f86716 100644 --- a/mdl-examples/bug-tests/page-variable-bindings.mdl +++ b/mdl-examples/bug-tests/page-variable-bindings.mdl @@ -46,8 +46,8 @@ create or modify page PV.VarPage Layout: Atlas_Core.Atlas_Default, Params: ( $Thing: PV.Thing ), Variables: ( - $Filter: Enumeration(PV.ENUM_Status) = 'PV.ENUM_Status.Open', - $Show: Boolean = 'true', + $Filter: Enumeration(PV.ENUM_Status) = PV.ENUM_Status.Open, + $Show: Boolean = true, $Label: String = '''hello''' ) ) diff --git a/mdl-examples/doctype-tests/03-page-examples.mdl b/mdl-examples/doctype-tests/03-page-examples.mdl index c67244f190..d3404cc1c4 100644 --- a/mdl-examples/doctype-tests/03-page-examples.mdl +++ b/mdl-examples/doctype-tests/03-page-examples.mdl @@ -2227,7 +2227,7 @@ create page PgTest.P033b_DataGrid_ColumnProperties folder 'DataGrid' title: 'DataGrid Column Properties', layout: Atlas_Core.Atlas_Default, url: 'p033b_datagrid_column_properties', - variables: ( $showStockColumn: boolean = 'if (3 < 4) then true else false' ) + variables: ( $showStockColumn: boolean = if (3 < 4) then true else false ) ) { datagrid dgProducts (datasource: database PgTest.Product) { @@ -2506,7 +2506,7 @@ create page PgTest.P041_Placeholders -- Add a boolean page variable for controlling column visibility alter page PgTest.P033b_DataGrid_ColumnProperties { - add variables $showPriceColumn: boolean = 'true'; + add variables $showPriceColumn: boolean = true; }; -- Add a string variable with an escaped string default @@ -2521,7 +2521,7 @@ alter page PgTest.P033b_DataGrid_ColumnProperties { -- Multiple variable operations in a single ALTER statement alter page PgTest.P033b_DataGrid_ColumnProperties { - add variables $pageSize: integer = '20'; + add variables $pageSize: integer = 20; drop variables $showPriceColumn; }; diff --git a/mdl-examples/doctype-tests/29-datagrid-examples.mdl b/mdl-examples/doctype-tests/29-datagrid-examples.mdl index bfedd2e856..15db9d68c3 100644 --- a/mdl-examples/doctype-tests/29-datagrid-examples.mdl +++ b/mdl-examples/doctype-tests/29-datagrid-examples.mdl @@ -614,7 +614,7 @@ create page DgTest.DG15_Variable_Visibility folder 'DataGrid' ( title: 'Products (conditional column)', layout: Atlas_Core.Atlas_Default, url: 'dg15_variable_visibility', - variables: ( $showStock: boolean = 'if (1 < 2) then true else false' ) + variables: ( $showStock: boolean = if (1 < 2) then true else false ) ) { datagrid dg1 (datasource: database DgTest.Product) { column (attribute: Name, caption: 'Name') diff --git a/mdl-examples/doctype-tests/33-alter-page-examples.mdl b/mdl-examples/doctype-tests/33-alter-page-examples.mdl index ce29dc4c0f..731fd2803e 100644 --- a/mdl-examples/doctype-tests/33-alter-page-examples.mdl +++ b/mdl-examples/doctype-tests/33-alter-page-examples.mdl @@ -313,7 +313,7 @@ alter page AlterPg.Product_Overview { -- ============================================================================ alter page AlterPg.Product_Overview { - add variables $showPrice: boolean = 'true' + add variables $showPrice: boolean = true }; alter page AlterPg.Product_Overview { @@ -322,7 +322,7 @@ alter page AlterPg.Product_Overview { -- Both add and drop in a single ALTER block. alter page AlterPg.Product_Overview { - add variables $pageSize: integer = '20'; + add variables $pageSize: integer = 20; drop variables $filterMode }; diff --git a/mdl/ast/ast_alter_page.go b/mdl/ast/ast_alter_page.go index 91dfa4477d..b1f70099f8 100644 --- a/mdl/ast/ast_alter_page.go +++ b/mdl/ast/ast_alter_page.go @@ -133,7 +133,7 @@ type ReplaceWidgetOp struct { func (s *ReplaceWidgetOp) isAlterPageOperation() {} -// AddVariableOp represents: ADD Variables $name: Type = 'default' +// AddVariableOp represents: ADD Variables $name: Type = <default expression> type AddVariableOp struct { Variable PageVariable } diff --git a/mdl/deprecation/deprecation.go b/mdl/deprecation/deprecation.go index b236d8afc1..17d1fce3f8 100644 --- a/mdl/deprecation/deprecation.go +++ b/mdl/deprecation/deprecation.go @@ -270,6 +270,9 @@ const ( // QuotedSettingsConstant is `alter settings [drop] constant 'Module.Const'`: // a constant named in a string. QuotedSettingsConstant = "MDL-DEPR085" + // QuotedVariableDefault is a page or snippet variable's default written in + // a string: `$show: boolean = 'true'`. + QuotedVariableDefault = "MDL-DEPR086" // Codes 070-079 are R2's integration documents (ako/mxcli#754): properties // in ( ), declarative children in { }. @@ -991,6 +994,22 @@ var r5Entries = []Entry{ Example: "alter settings constant 'M.ApiUrl' value 'https://test.example.com' in configuration 'Default';", CanonicalExample: "alter settings constant @M.ApiUrl value 'https://test.example.com' in configuration 'Default';", }, + { + Code: QuotedVariableDefault, + Old: "Variables: ( $name: Type = '<expression>' ) / add variables $name: Type = '<expression>'", + Canonical: "Variables: ( $name: Type = <expression> ) / add variables $name: Type = <expression>", + Rewrite: Rewrite{Structural: "default out of its string: `$show: boolean = 'true'` becomes `$show: boolean = true`"}, + RemovedIn: 2, + Note: "A variable's default is an expression, written bare like every other (R5). The string form keeps " + + "its meaning — its content is the expression — under every language version, so it is an alias. " + + "A default that is itself a string (`$s: string = '''abc'''`) or empty has no other spelling yet and " + + "is not reported; one whose content would not read back as the same bare expression is left in " + + "place and reported by fmt --upgrade.", + Example: "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, " + + "Variables: ( $show: boolean = 'if (3 < 4) then true else false' )) { };", + CanonicalExample: "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, " + + "Variables: ( $show: boolean = if (3 < 4) then true else false )) { };", + }, } // r2Entries are R2's integration-document brackets (ako/mxcli#754). diff --git a/mdl/deprecation/topics.go b/mdl/deprecation/topics.go index 62f32823ed..e48599ba2e 100644 --- a/mdl/deprecation/topics.go +++ b/mdl/deprecation/topics.go @@ -75,6 +75,7 @@ var topics = map[string][]string{ "MDL-DEPR083": {"rest.consumed"}, "MDL-DEPR084": {"agents.model"}, "MDL-DEPR085": {"settings.alter", "domain-model.constant"}, + "MDL-DEPR086": {"page.create", "snippet.create"}, "MDL-DEPR070": {"rest.consumed"}, "MDL-DEPR071": {"agents.agent", "agents.mcp-service", "agents.knowledge-base"}, "MDL-DEPR072": {"image-collection"}, diff --git a/mdl/executor/cmd_pages_builder.go b/mdl/executor/cmd_pages_builder.go index 363fa5fb62..82cd0e99fe 100644 --- a/mdl/executor/cmd_pages_builder.go +++ b/mdl/executor/cmd_pages_builder.go @@ -95,7 +95,7 @@ type pageBuilder struct { // reference as a last line, ALTER included (canon.BareAttributeRefError). tolerateDanglingRefs bool - // Local page/snippet variables (Variables: ( $name: Type = 'default' )). + // Local page/snippet variables (Variables: ( $name: Type = <default expression> )). // Used to distinguish a $localVar reference from a page parameter when // resolving TextTemplate parameters — local variables must be stored as // Forms$PageVariable.LocalVariable in BSON, not as a literal Expression. diff --git a/mdl/executor/cmd_pages_describe.go b/mdl/executor/cmd_pages_describe.go index ac22d188e0..216518a121 100644 --- a/mdl/executor/cmd_pages_describe.go +++ b/mdl/executor/cmd_pages_describe.go @@ -9,6 +9,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/visitor" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/pages" @@ -155,7 +156,7 @@ func describePage(ctx *ExecContext, name ast.QualifiedName) error { varTypeName = pageVariableMDLType(vtType, enumQN) } } - varParts = append(varParts, fmt.Sprintf("$%s: %s = %s", varName, varTypeName, mdlQuote(ctx, defaultVal))) + varParts = append(varParts, pageVariableMDL(ctx, varName, varTypeName, defaultVal)) } props = append(props, fmt.Sprintf("Variables: ( %s )", strings.Join(varParts, ", "))) } @@ -1109,3 +1110,14 @@ func primitiveParamTypeMDL(bsonType string) string { return bsonType } } + +// pageVariableMDL writes one page variable, its default as the bare +// expression it stores (R5). A default with no bare spelling — a string, the +// empty default, text that would read back differently — is written in a +// string whose content is the expression (MDL-DEPR086's form), as before. +func pageVariableMDL(ctx *ExecContext, name, typeName, defaultVal string) string { + if visitor.VariableDefaultReadsBack(defaultVal) { + return fmt.Sprintf("$%s: %s = %s", name, typeName, defaultVal) + } + return fmt.Sprintf("$%s: %s = %s", name, typeName, mdlQuote(ctx, defaultVal)) +} diff --git a/mdl/executor/cmd_pages_input_binding_context.go b/mdl/executor/cmd_pages_input_binding_context.go index 3c727fb5fc..552f365989 100644 --- a/mdl/executor/cmd_pages_input_binding_context.go +++ b/mdl/executor/cmd_pages_input_binding_context.go @@ -231,7 +231,7 @@ func validatePageVariableBindings(widgets []*ast.WidgetV3, variables []ast.PageV RuleID: "MDL-WIDGET34", Severity: linter.SeverityError, Message: locationPrefix + ": " + msg, - Suggestion: "Bind an input to a page variable by declaring it: `Variables: ( $name: Boolean = 'true' )`, then `Attribute: $name`.", + Suggestion: "Bind an input to a page variable by declaring it: `Variables: ( $name: Boolean = true )`, then `Attribute: $name`.", }) } } diff --git a/mdl/executor/describe_page_variable_default_test.go b/mdl/executor/describe_page_variable_default_test.go new file mode 100644 index 0000000000..93ecf52cdc --- /dev/null +++ b/mdl/executor/describe_page_variable_default_test.go @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" +) + +// describe writes a variable's default bare (R5), and re-executing that +// output stores the same default. A default that is itself a string, or +// empty, has no bare spelling yet (MDL-DEPR086) and keeps the string form. +func TestDescribePageVariableDefault_RoundTrip(t *testing.T) { + for _, tc := range []struct{ stored, want string }{ + {"true", "$v: Boolean = true"}, + {"if (3 < 4) then true else false", "$v: Boolean = if (3 < 4) then true else false"}, + {"[%CurrentDateTime%]", "$v: Boolean = [%CurrentDateTime%]"}, + {"'abc'", "$v: Boolean = '''abc'''"}, + {"", "$v: Boolean = ''"}, + } { + got := pageVariableMDL(&ExecContext{Output: &bytes.Buffer{}}, "v", "Boolean", tc.stored) + if got != tc.want { + t.Errorf("stored %q: describe wrote %q, want %q", tc.stored, got, tc.want) + } + prog := parseMDL(t, "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Variables: ( "+got+" )) { };") + vars := prog.Statements[0].(*ast.CreatePageStmtV3).Variables + if len(vars) != 1 || vars[0].DefaultValue != tc.stored { + t.Errorf("re-exec of %q stores %+v, want default %q", got, vars, tc.stored) + } + } +} diff --git a/mdl/grammar/domains/MDLPage.g4 b/mdl/grammar/domains/MDLPage.g4 index 4cf836b90c..e0decc4645 100644 --- a/mdl/grammar/domains/MDLPage.g4 +++ b/mdl/grammar/domains/MDLPage.g4 @@ -74,8 +74,10 @@ variableDeclarationList : variableDeclaration (COMMA variableDeclaration)* ; +// A variable's default is an expression, written bare: `$show: Boolean = true`. +// A string whose content is the expression is the old spelling (MDL-DEPR086). variableDeclaration - : VARIABLE COLON dataType EQUALS STRING_LITERAL // $varName: Boolean = 'expression' + : VARIABLE COLON dataType EQUALS (STRING_LITERAL /* @alias MDL-DEPR086 */ | expression) ; // A sort column. The name may navigate associations, one `/` per hop, with the diff --git a/mdl/upgrade/page_variable_default_test.go b/mdl/upgrade/page_variable_default_test.go new file mode 100644 index 0000000000..f6340b79a2 --- /dev/null +++ b/mdl/upgrade/page_variable_default_test.go @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: Apache-2.0 + +package upgrade + +import "testing" + +// A page variable's default written in a string (MDL-DEPR086) is rewritten to +// the bare expression; a default that is itself a string stays as it is. +func TestUpgrade_PageVariableDefaultsBare(t *testing.T) { + for _, tc := range []struct{ name, src, want string }{ + { + "create page", + "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default,\n" + + " Variables: ( $show: boolean = 'true', $n: integer = '20', $s: string = '''x''', $c: boolean = 'if (3 < 4) then true else false' )) { };", + "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default,\n" + + " Variables: ( $show: boolean = true, $n: integer = 20, $s: string = '''x''', $c: boolean = if (3 < 4) then true else false )) { };", + }, + { + "alter page add variables", + "alter page M.P { add variables $show: boolean = 'false' };", + "alter page M.P { add variables $show: boolean = false };", + }, + } { + t.Run(tc.name, func(t *testing.T) { + res := mustUpgrade(t, tc.src, Options{}) + if res.Source != tc.want { + t.Errorf("got:\n%s\nwant:\n%s", res.Source, tc.want) + } + if len(res.Unrewritten) != 0 { + t.Errorf("Unrewritten = %+v", res.Unrewritten) + } + }) + } +} diff --git a/mdl/visitor/visitor_page_v3.go b/mdl/visitor/visitor_page_v3.go index 8587bae98d..42cd924526 100644 --- a/mdl/visitor/visitor_page_v3.go +++ b/mdl/visitor/visitor_page_v3.go @@ -102,7 +102,7 @@ func (b *Builder) parsePageHeaderV3(ctx parser.IPageHeaderV3Context, stmt *ast.C stmt.Parameters = buildPageParameters(paramList) } } else if prop.VARIABLES_KW() != nil { - // Variables: ( $showStock: Boolean = 'true', ... ) + // Variables: ( $showStock: Boolean = true, ... ) if varList := prop.VariableDeclarationList(); varList != nil { stmt.Variables = buildVariableDeclarations(varList) } @@ -277,7 +277,7 @@ func (b *Builder) parseSnippetHeaderV3(ctx parser.ISnippetHeaderV3Context, stmt stmt.Parameters = buildPageParameters(paramList) } } else if prop.VARIABLES_KW() != nil { - // Variables: ( $showStock: Boolean = 'true', ... ) + // Variables: ( $showStock: Boolean = true, ... ) if varList := prop.VariableDeclarationList(); varList != nil { stmt.Variables = buildVariableDeclarations(varList) } @@ -332,8 +332,12 @@ func buildSingleVariableDeclaration(vdCtx *parser.VariableDeclarationContext) as v.DataType = dt.GetText() } + // The default is an expression: bare, or — the old spelling, MDL-DEPR086 + // — a string whose content is the expression (visitor_variable_default.go). if str := vdCtx.STRING_LITERAL(); str != nil { v.DefaultValue = unquoteStringLit(str) + } else if e := vdCtx.Expression(); e != nil { + v.DefaultValue = bareArgumentText(e) } return v diff --git a/mdl/visitor/visitor_page_variable_default_test.go b/mdl/visitor/visitor_page_variable_default_test.go new file mode 100644 index 0000000000..1b20596f50 --- /dev/null +++ b/mdl/visitor/visitor_page_variable_default_test.go @@ -0,0 +1,110 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "fmt" + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" +) + +// A page or snippet variable's default is a Mendix expression. It was written +// in a string whose CONTENT is the expression — `$show: boolean = 'true'`, +// `= 'if (3 < 4) then true else false'` — and is now written bare (R5). The +// string form keeps its meaning under every language version (mdl 1 is +// frozen) and is the deprecated alias MDL-DEPR086. + +func pageVariables(t *testing.T, src string) ([]ast.PageVariable, *ast.Program) { + t.Helper() + prog := mustBuild(t, src) + for _, s := range prog.Statements { + switch st := s.(type) { + case *ast.CreatePageStmtV3: + return st.Variables, prog + case *ast.CreateSnippetStmtV3: + return st.Variables, prog + case *ast.AlterPageStmt: + for _, op := range st.Operations { + if add, ok := op.(*ast.AddVariableOp); ok { + return []ast.PageVariable{add.Variable}, prog + } + } + } + } + t.Fatalf("no variables in %q", src) + return nil, nil +} + +func TestPageVariableDefault_BareExpression(t *testing.T) { + const page = "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Variables: ( %s )) { };" + for _, tc := range []struct{ decl, want string }{ + {"$show: boolean = true", "true"}, + {"$show: boolean = if 3 < 4 then true else false", "if 3 < 4 then true else false"}, + {"$n: integer = 20", "20"}, + {"$when: datetime = [%CurrentDateTime%]", "[%CurrentDateTime%]"}, + {"$name: string = 'a' + 'b'", "'a' + 'b'"}, + } { + vars, prog := pageVariables(t, fmt.Sprintf(page, tc.decl)) + if len(vars) != 1 || vars[0].DefaultValue != tc.want { + t.Errorf("%s: stored %+v, want default %q", tc.decl, vars, tc.want) + } + if got := deprecationCodes(prog); len(got) != 0 { + t.Errorf("%s: recorded %v, want none", tc.decl, got) + } + } + // Several, the list's commas not taken for the expression's. + vars, _ := pageVariables(t, fmt.Sprintf(page, "$a: boolean = not(false), $b: integer = max(1, 2), $c: boolean = $a")) + want := []string{"not(false)", "max(1, 2)", "$a"} + var got []string + for _, v := range vars { + got = append(got, v.DefaultValue) + } + if !reflect.DeepEqual(got, want) { + t.Errorf("defaults %v, want %v", got, want) + } +} + +func TestPageVariableDefault_SnippetAndAlter(t *testing.T) { + vars, _ := pageVariables(t, "create snippet M.S (Variables: ( $show: boolean = false )) { };") + if len(vars) != 1 || vars[0].DefaultValue != "false" { + t.Errorf("snippet: %+v", vars) + } + vars, _ = pageVariables(t, "alter page M.P { add variables $show: boolean = 1 < 2 };") + if len(vars) != 1 || vars[0].DefaultValue != "1 < 2" { + t.Errorf("alter add variables: %+v", vars) + } +} + +// The string form keeps its meaning — the content is the expression — and is +// reported, except where it has no bare spelling: a default that is itself a +// string (`”'abc”'`, whose bare `'abc'` is this very alias) or empty. +func TestPageVariableDefault_StringFormIsTheDeprecatedAlias(t *testing.T) { + const page = "create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Variables: ( %s )) { };" + for _, tc := range []struct { + decl, want string + reported bool + }{ + {"$show: boolean = 'true'", "true", true}, + {"$show: boolean = 'if (3 < 4) then true else false'", "if (3 < 4) then true else false", true}, + {"$n: integer = '20'", "20", true}, + {"$s: string = '''abc'''", "'abc'", false}, + {"$s: string = ''", "", false}, + } { + for _, header := range []string{"", "mdl 1;\n"} { + vars, prog := pageVariables(t, header+fmt.Sprintf(page, tc.decl)) + if len(vars) != 1 || vars[0].DefaultValue != tc.want { + t.Errorf("%q%s: stored %+v, want default %q", header, tc.decl, vars, tc.want) + } + codes := deprecationCodes(prog) + if tc.reported && !reflect.DeepEqual(codes, []string{deprecation.QuotedVariableDefault}) { + t.Errorf("%q%s: recorded %v, want [%s]", header, tc.decl, codes, deprecation.QuotedVariableDefault) + } + if !tc.reported && len(codes) != 0 { + t.Errorf("%q%s: recorded %v, want none", header, tc.decl, codes) + } + } + } +} diff --git a/mdl/visitor/visitor_variable_default.go b/mdl/visitor/visitor_variable_default.go new file mode 100644 index 0000000000..5a8b845f15 --- /dev/null +++ b/mdl/visitor/visitor_variable_default.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "github.com/antlr4-go/antlr/v4" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/deprecation" + "github.com/mendixlabs/mxcli/mdl/grammar/parser" +) + +// R5 (ADR-0010): a page or snippet variable's default is an expression, and +// an expression is written bare. It took a string whose CONTENT was the +// expression — `$show: boolean = 'true'` — which stays the deprecated alias +// MDL-DEPR086 and keeps that meaning under every language version: mdl 1 is +// frozen, so making the string a Mendix string waits for mdl 2. +// +// The one default that cannot be written bare yet is a string: the bare +// `'abc'` is this very alias, so `$s: string = '''abc'''` and the empty `''` +// stay as they are and are not reported. + +// ExitVariableDeclaration records MDL-DEPR086 on a default written in a +// string, with the rewrite that takes it out. +func (b *Builder) ExitVariableDeclaration(ctx *parser.VariableDeclarationContext) { + lit := ctx.STRING_LITERAL() + if lit == nil { + return + } + content := unquoteStringLit(lit) + if content == "" || isLoneStringLiteral(content) { + return + } + b.recordDeprecation(deprecation.QuotedVariableDefault, lit.GetSymbol(), "page variable default") + fix, why := variableDefaultFix(lit, content) + b.fixLastDeprecation(deprecation.QuotedVariableDefault, fix, why) +} + +// variableDefaultFix replaces the string by its content, when that content +// reads back bare as the same default. +func variableDefaultFix(lit antlr.TerminalNode, content string) (*ast.Fix, string) { + if holdsInterpretedEscape(lit) { + return nil, "the default's string holds a backslash escape; write the expression bare by hand" + } + if !BareExpression(content) || !VariableDefaultReadsBack(content) { + return nil, "the string " + lit.GetText() + " does not read back as the same bare expression; write it bare by hand" + } + t := lit.GetSymbol() + text := content + // A string needs no space to part it from its neighbours; a bare + // expression does: `='true'`. + if is := t.GetInputStream(); is != nil { + if t.GetStart() > 0 && gluesToWord(is.GetText(t.GetStart()-1, t.GetStart()-1)) { + text = " " + text + } + if t.GetStop()+1 < is.Size() && gluesToWord(is.GetText(t.GetStop()+1, t.GetStop()+1)) { + text += " " + } + } + return &ast.Fix{Edits: []ast.TextEdit{{Start: t.GetStart(), Stop: t.GetStop() + 1, Text: text}}}, "" +} + +// VariableDefaultReadsBack reports whether `$v: Boolean = <expr>` stores +// exactly expr as the default. describe asks it before writing a default bare. +func VariableDefaultReadsBack(expr string) bool { + if expr == "" || isLoneStringLiteral(expr) { + return false + } + ctx, ok := parseRule("$v: Boolean = "+expr, func(p *parser.MDLParser) antlr.ParserRuleContext { + return p.VariableDeclaration() + }) + if !ok { + return false + } + vd := ctx.(*parser.VariableDeclarationContext) + return vd.STRING_LITERAL() == nil && buildSingleVariableDeclaration(vd).DefaultValue == expr +} + +// isLoneStringLiteral reports whether s is exactly one string literal. +func isLoneStringLiteral(s string) bool { + ctx, ok := parseRule(s, func(p *parser.MDLParser) antlr.ParserRuleContext { return p.Expression() }) + return ok && loneLiteralToken(ctx) != nil +} From 5debe654b7cbcb0d3df22947e42ec31ff3cde001 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Thu, 8 Oct 2026 08:55:59 +0000 Subject: [PATCH 10/13] test: write page variable defaults bare in the R2 header-map fixtures The R2 brace/paren tests used `$show: Boolean = 'true'` as their canonical form, which MDL-DEPR086 now reports, so each case recorded two deprecations instead of the one it exercises, and describe's expected output was the quoted form. The defaults are written bare. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P65SqmwwvbWdJVRwhYiMQw --- mdl/executor/r2_rest_describe_test.go | 2 +- mdl/upgrade/r2_rest_test.go | 4 ++-- mdl/visitor/r2_rest_test.go | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/mdl/executor/r2_rest_describe_test.go b/mdl/executor/r2_rest_describe_test.go index 92b0d4bf38..206564570b 100644 --- a/mdl/executor/r2_rest_describe_test.go +++ b/mdl/executor/r2_rest_describe_test.go @@ -168,5 +168,5 @@ func TestDescribePageHeaderMaps_InParens(t *testing.T) { } ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) assertNoError(t, describePage(ctx, ast.QualifiedName{Module: "M", Name: "Edit"})) - assertCanonicalDescribe(t, buf.String(), "Params: ( $Order: M.Order )", "Variables: ( $show: Boolean = 'true' )") + assertCanonicalDescribe(t, buf.String(), "Params: ( $Order: M.Order )", "Variables: ( $show: Boolean = true )") } diff --git a/mdl/upgrade/r2_rest_test.go b/mdl/upgrade/r2_rest_test.go index 06daffedda..e7b48a93da 100644 --- a/mdl/upgrade/r2_rest_test.go +++ b/mdl/upgrade/r2_rest_test.go @@ -23,7 +23,7 @@ func TestUpgrade_R2NavigationMapsAndDatabaseConnection(t *testing.T) { ); ); CREATE MENU M.Side (MENU ITEM 'Plain';); -create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: { $Asset: M.Asset }, Variables: { $n: Integer = '1' }) { +create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: { $Asset: M.Asset }, Variables: { $n: Integer = 1 }) { dynamictext t (Content: 'Hi {1}', ContentParams: [{1} = Name]) container c (DesignProperties: ['Spacing': ['margin-top': 'Large'], 'Full width': on]) snippetcall s (Snippet: M.S, Params: {$Asset: $Asset}) @@ -59,7 +59,7 @@ end; } }; CREATE MENU M.Side {MENU ITEM 'Plain'}; -create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: ( $Asset: M.Asset ), Variables: ( $n: Integer = '1' )) { +create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: ( $Asset: M.Asset ), Variables: ( $n: Integer = 1 )) { dynamictext t (Content: 'Hi {1}', ContentParams: ({1} = Name)) container c (DesignProperties: ('Spacing': ('margin-top': 'Large'), 'Full width': on)) snippetcall s (Snippet: M.S, Params: (Asset = $Asset)) diff --git a/mdl/visitor/r2_rest_test.go b/mdl/visitor/r2_rest_test.go index e67f37511b..aa8a760559 100644 --- a/mdl/visitor/r2_rest_test.go +++ b/mdl/visitor/r2_rest_test.go @@ -93,16 +93,16 @@ var r2RestCases = []r2Case{ code: deprecation.HeaderMapBraces, old: `create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: { $Order: M.Order, $Qty: Integer }, - Variables: { $show: Boolean = 'true' }) { };`, + Variables: { $show: Boolean = true }) { };`, canonical: `create page M.P (Title: 'P', Layout: Atlas_Core.Atlas_Default, Params: ( $Order: M.Order, $Qty: Integer ), - Variables: ( $show: Boolean = 'true', )) { };`, + Variables: ( $show: Boolean = true, )) { };`, }, { name: "snippet header maps", code: deprecation.HeaderMapBraces, - old: `create snippet M.S (Params: { $Customer: M.Customer }, Variables: {$x: Integer = '1'}) { };`, - canonical: `create snippet M.S (Params: ( $Customer: M.Customer ), Variables: ($x: Integer = '1')) { };`, + old: `create snippet M.S (Params: { $Customer: M.Customer }, Variables: {$x: Integer = 1}) { };`, + canonical: `create snippet M.S (Params: ( $Customer: M.Customer ), Variables: ($x: Integer = 1)) { };`, }, { name: "template parameters", From 3fd9c0723e428260e1ee8efad7aed5cdb5e0f17d Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Fri, 9 Oct 2026 21:32:01 +0000 Subject: [PATCH 11/13] docs: write the ALTER PAGE add-variables example's default bare The generic ALTER PAGE syntax help and the grammar comment, both changed on main since, still showed `$show: Boolean = 'true'` (MDL-DEPR086). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P65SqmwwvbWdJVRwhYiMQw --- cmd/mxcli/syntax/features_page.go | 2 +- mdl/grammar/MDLParser.g4 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/mxcli/syntax/features_page.go b/cmd/mxcli/syntax/features_page.go index 5f22061a97..5e4fdfac14 100644 --- a/cmd/mxcli/syntax/features_page.go +++ b/cmd/mxcli/syntax/features_page.go @@ -304,7 +304,7 @@ LIST IMPACT OF htmlelement; "drop template", "insert template", "list view template", "add parameter", "drop parameter", "page parameter", "add parameters", }, - Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for <type>` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { <widgets> };\n INSERT BEFORE widgetName { <widgets> };\n INSERT INTO containerName { <widgets> };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { <widgets> };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = 'true';\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", + Syntax: "ALTER PAGE Module.Name { -- the generic ALTER: set / insert / replace / drop\n SET (property: value) ON widgetName; -- widget property names: any casing\n SET ('Row size': 'Small') ON lvOrders; -- an Atlas DESIGN property of that widget's\n -- type; quoted and case-sensitive.\n -- `list design properties for <type>` lists\n -- them. ON/OFF for a toggle, where OFF\n -- REMOVES the entry.\n -- A multi-select ('Hide on') or compound\n -- ('Spacing') one needs the inline\n -- DesignProperties: (...) form, because a\n -- SET assignment carries one value.\n SET (Action: CALL MICROFLOW Module.MF) ON btnSave; -- any CREATE PAGE action form\n SET ('createFileAction': CALL MICROFLOW Module.MF) ON fileUploader1;\n -- a pluggable widget's NAMED action slot,\n -- by the widget's own key; refused on a\n -- key that is not action-typed\n SET (DataSource: $Param) ON dvOrder; -- parameter/microflow/nanoflow/selection;\n -- DATABASE and association are REPLACE-only,\n -- and a data view takes no database source\n SET (prop1: val1, prop2: val2) ON widgetName;\n SET (Title: 'New Title'); -- page-level (case-sensitive): no ON\n SET (Documentation: 'What this page is for.');\n SET (Class: 'css-class'); -- page-level CSS class / style\n SET (Style: 'css: rule');\n SET (PopupWidth: 800, PopupHeight: 480, PopupResizable: true); -- page-level pop-up\n INSERT AFTER widgetName { <widgets> };\n INSERT BEFORE widgetName { <widgets> };\n INSERT INTO containerName { <widgets> };\n DROP name1, name2;\n DROP TEMPLATE FOR Module.Specialization IN listViewName;\n REPLACE widgetName WITH { <widgets> };\n ADD PARAMETERS $Order: Module.Order; -- or a primitive: $Count: Integer.\n -- A page with a Url needs a {Order}\n -- segment: SET (Url: '…/{Order}') in the\n -- same statement (CE5601 otherwise).\n -- A snippet takes entities only (MDL087).\n DROP PARAMETERS $Order; -- refused while a data source or an\n -- expression on the page still uses it\n ADD VARIABLES $show: Boolean = true;\n DROP VARIABLES $show;\n};\n-- A target is a widget name, or a DataGrid 2 column by what it shows:\n-- grid column(Attr), grid column('Caption'), @n when two match. The old spellings `SET p = v`,\n-- `SET p: v` (no parentheses) and `DROP WIDGET a` still run and warn\n-- (MDL-DEPR101..103).\n\n-- The BULK form: one design property on every widget of a TYPE.\nALTER PAGES [IN Module]\n SET 'Compact' = ON, 'Striped' = ON\n WHERE WIDGETTYPE = datagrid -- the MDL keyword, which resolves to\n -- exactly one widget id. A full id in\n -- quotes works too. NOT a name: a widget\n -- name is unique only within its page.\n [DRY RUN]; -- run this FIRST. It reports the matches\n -- against a discardable copy and writes\n -- nothing.", Example: "ALTER PAGE Module.EditPage {\n SET (Caption: 'Save & Close', ButtonStyle: Success) ON btnSave;\n INSERT AFTER txtName {\n TEXTBOX txtMiddleName (Label: 'Middle Name', Attribute: MiddleName)\n };\n DROP txtUnused;\n};", SeeAlso: []string{"page.create", "page.show", "snippet.alter"}, }) diff --git a/mdl/grammar/MDLParser.g4 b/mdl/grammar/MDLParser.g4 index 762b6910f6..d5b38a6ee6 100644 --- a/mdl/grammar/MDLParser.g4 +++ b/mdl/grammar/MDLParser.g4 @@ -568,7 +568,7 @@ alterPageDropTemplate ; alterPageAddVariable - : ADD VARIABLES_KW variableDeclaration // ADD Variables $show: Boolean = 'true' + : ADD VARIABLES_KW variableDeclaration // ADD Variables $show: Boolean = true ; alterPageDropVariable From dc4af4b8660ce39983adada1831195a741132a04 Mon Sep 17 00:00:00 2001 From: Claude <noreply@anthropic.com> Date: Fri, 9 Oct 2026 21:34:35 +0000 Subject: [PATCH 12/13] feat(lint): MPR007 flags a user role that cannot open its home page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user role none of whose module roles is allowed on the home page it lands on passes `mxcli lint` and `check --references`, then fails mxbuild with one CE2729 per widget on that page ("No read access to attribute ... for user role 'X' (with no roles defined in module 'M')") — none of which names the cause. ChipCoV6 hit it with the template's generic `User` role left in place after the Responsive home page moved to a new module (FINDINGS.md: 10x CE2729 in docker check). MPR007 only checked that a navigation page has some allowed role (CE0557). It now also resolves, per navigation profile and user role, the effective home page (the role-based entry for that role, else the profile default) and warns when none of the role's module roles is allowed. A microflow home page is checked against the microflow's allowed module roles. Skipped at security level Off; a page with no allowed roles at all is left to the existing CE0557 report. The guest role is an ordinary user role in the list, so it is covered without special-casing; nothing is exempted by name. Verified on a copy of ChipCoV6 with `create user role TmpUser (ModuleRoles: (System.User, Administration.User))`: mxbuild 11.15.0 reports 10x CE2729, lint now reports one MPR007 warning naming TmpUser, Responsive and FieldService.Home_Dashboard; the unmodified project (Administrator, FabUser, ServiceCoordinator, FieldEngineer) and Ledger report no new MPR007. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012PkPkaYM12u8yqMKNmzBgT --- .claude/commands/mendix/lint.md | 2 +- ...cannot-open-home-page-ce2729-unlinted.json | 1 + .claude/skills/mendix/assess-quality/SKILL.md | 1 + cmd/mxcli/cmd_lint.go | 2 +- mdl/linter/rules/page_navigation_security.go | 212 +++++++++++++++--- .../rules/page_navigation_security_test.go | 131 +++++++++++ 6 files changed, 310 insertions(+), 39 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-other/2026-10-09-user-role-cannot-open-home-page-ce2729-unlinted.json diff --git a/.claude/commands/mendix/lint.md b/.claude/commands/mendix/lint.md index e72602c378..3c4bec2cb2 100644 --- a/.claude/commands/mendix/lint.md +++ b/.claude/commands/mendix/lint.md @@ -39,7 +39,7 @@ mxcli lint -p app.mpr --exclude System --exclude Administration | MPR004 | quality | ValidationFeedback - Validation feedback with empty message | | MPR005 | quality | ImageSource - IMAGE widgets with no source configured | | MPR006 | quality | EmptyContainer - Empty layout containers | -| MPR007 | security | PageNavigationSecurity - Navigation pages need allowed roles (CE0557) | +| MPR007 | security | PageNavigationSecurity - Navigation pages need allowed roles (CE0557); every user role must be able to open its home page (else CE2729 per widget) | | MPR012 | correctness | LegacyImageWidget - staticimage/dynamicimage are unsupported by the React client (CE0582) | | SEC001 | security | NoEntityAccessRules - Persistent entities need access rules | | SEC002 | security | WeakPasswordPolicy - Password minimum length should be 8+ | diff --git a/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-user-role-cannot-open-home-page-ce2729-unlinted.json b/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-user-role-cannot-open-home-page-ce2729-unlinted.json new file mode 100644 index 0000000000..d18d794c00 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-other/2026-10-09-user-role-cannot-open-home-page-ce2729-unlinted.json @@ -0,0 +1 @@ +{"area": "mdl/linter", "date": "2026-10-09", "symptom": "`mxcli docker check` fails with ~10× CE2729 \"No read access to attribute … for user role 'X' (with no roles defined in module 'M')\" on the widgets of the navigation home page, while `mxcli lint` and `check --references` are clean — a template user role (e.g. `User`) left in place after the home page moved to a page in a module it has no role in (ChipCoV6 FINDINGS)", "cause": "MPR007 only checked that a navigation page has *some* allowed role (CE0557). mxbuild validates the home page per user role, but nothing checked that each user role can open the home page it lands on, so the CE2729s name widgets and attributes and never the cause", "file": "`mdl/linter/rules/page_navigation_security.go` (`checkHomePagePerUserRole`)", "insight": "MPR007 now resolves each user role's effective home page per profile (role-based entry, else default; microflow home pages against the microflow's allowed roles) and warns when none of the role's module roles is allowed. Skipped at security level Off; a page with no roles at all stays CE0557's report. Repro: `create user role TmpUser (ModuleRoles: (System.User, Administration.User))` on a copy of ChipCoV6 — base lint is silent, mxbuild 11.15.0 reports 10× CE2729. A per-widget CE flood usually has one per-role cause upstream; lint it there"} diff --git a/.claude/skills/mendix/assess-quality/SKILL.md b/.claude/skills/mendix/assess-quality/SKILL.md index d070f81a75..3fc2ed3e69 100644 --- a/.claude/skills/mendix/assess-quality/SKILL.md +++ b/.claude/skills/mendix/assess-quality/SKILL.md @@ -152,6 +152,7 @@ After reviewing automated results, assess the following areas manually. The guid |-----------|------|----------| | 1:1 mapping between module roles and user roles | CONV008 | High | | Pages in navigation must have allowed roles | MPR007 | High | +| Every user role can open its home page (else CE2729) | MPR007 | High | | No guest/anonymous access to sensitive data | SEC004 | Critical | | Strict security mode enabled | SEC005 | High | | No demo users in production | SEC003 | Critical | diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go index 4a62794adc..a74b6a963a 100644 --- a/cmd/mxcli/cmd_lint.go +++ b/cmd/mxcli/cmd_lint.go @@ -28,7 +28,7 @@ Built-in rules check for: - Empty validation feedback (MPR004) - validation feedback with empty message - Unconfigured images (MPR005) - IMAGE widgets with no source configured - Empty containers (MPR006) - layout containers with no children - - Navigation page security (MPR007) - pages in navigation need allowed roles + - Navigation page security (MPR007) - pages in navigation need allowed roles, and every user role must be able to open its home page - Gallery selection listener (MPR009) - DataView 'DataSource: selection X' needs gallery 'ItemSelectionMode: toggle' (Studio Pro CE3637) - Entity access rules (SEC001) - persistent entities need access rules - Password policy (SEC002) - password minimum length should be 8+ diff --git a/mdl/linter/rules/page_navigation_security.go b/mdl/linter/rules/page_navigation_security.go index c3e454b80b..9684229b2c 100644 --- a/mdl/linter/rules/page_navigation_security.go +++ b/mdl/linter/rules/page_navigation_security.go @@ -8,10 +8,20 @@ import ( "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/security" ) // PageNavigationSecurityRule checks that pages used in navigation have at least // one allowed module role. Studio Pro reports this as CE0557 but mx check does not. +// +// It also checks, per navigation profile, that every user role can open the +// home page it lands on: its role-based home page if it has one, otherwise the +// profile's default. A user role none of whose module roles is allowed on that +// page passes `check --references` and reaches mxbuild, which then reports +// CE2729 for every widget on the page ("No read access to attribute … for user +// role 'X' (with no roles defined in module …)") — ten of them for one leftover +// template role in ChipCoV6, and nothing that names the actual cause. type PageNavigationSecurityRule struct{} // NewPageNavigationSecurityRule creates a new page navigation security rule. @@ -25,7 +35,7 @@ func (r *PageNavigationSecurityRule) Category() string { return func (r *PageNavigationSecurityRule) DefaultSeverity() linter.Severity { return linter.SeverityWarning } func (r *PageNavigationSecurityRule) Description() string { - return "Checks that pages used in navigation have at least one allowed role (CE0557)" + return "Checks that pages used in navigation have at least one allowed role (CE0557), and that every user role can open its home page (else CE2729 per widget)" } // navUsage describes how a page is used in navigation. @@ -80,12 +90,8 @@ func (r *PageNavigationSecurityRule) Check(ctx *linter.LintContext) []linter.Vio collectMenuPages(profile.MenuItems, pName, navPages) } - if len(navPages) == 0 { - return nil - } - - // Build map of qualified page name → AllowedRoles count - pageRoleCounts := buildPageRoleCountMap(reader) + // Build map of qualified page name → allowed module roles + pageRoles := buildPageRoleMap(reader) var violations []linter.Violation for pageName, usages := range navPages { @@ -94,12 +100,12 @@ func (r *PageNavigationSecurityRule) Check(ctx *linter.LintContext) []linter.Vio continue } - roleCount, found := pageRoleCounts[pageName] + roles, found := pageRoles[pageName] if !found { continue // page not found in project (may be from marketplace module) } - if roleCount > 0 { + if len(roles) > 0 { continue // has at least one allowed role } @@ -118,9 +124,119 @@ func (r *PageNavigationSecurityRule) Check(ctx *linter.LintContext) []linter.Vio }) } + return append(violations, r.checkHomePagePerUserRole(ctx, nav, pageRoles)...) +} + +// checkHomePagePerUserRole reports each (profile, user role) whose effective +// home page none of the user role's module roles may open. A page with no +// allowed roles at all is left to the CE0557 check above, which already names +// it; a microflow home page is checked against its allowed module roles. +// Security off disables every role check, so nothing is reported then. +func (r *PageNavigationSecurityRule) checkHomePagePerUserRole(ctx *linter.LintContext, nav *types.NavigationDocument, pageRoles map[string][]string) []linter.Violation { + reader := ctx.Reader() + ps, err := reader.GetProjectSecurity() + if err != nil || ps == nil || ps.SecurityLevel == security.SecurityLevelOff || len(ps.UserRoles) == 0 { + return nil + } + + var mfRoles map[string][]string // loaded only if some home page is a microflow + + var violations []linter.Violation + for _, profile := range nav.Profiles { + pName := profile.Kind + if pName == "" { + pName = profile.Name + } + for _, ur := range ps.UserRoles { + target, isMicroflow, viaRole := effectiveHomePage(profile, ur.Name) + if target == "" || ctx.IsExcluded(moduleFromQualified(target)) { + continue + } + var allowed []string + var found bool + if isMicroflow { + if mfRoles == nil { + mfRoles = buildMicroflowRoleMap(reader) + } + allowed, found = mfRoles[target] + } else { + allowed, found = pageRoles[target] + } + if !found || (!isMicroflow && len(allowed) == 0) { + continue // not in the project, or already reported as CE0557 + } + if sharesRole(ur.ModuleRoles, allowed) { + continue + } + violations = append(violations, homePageViolation(r, ur, pName, target, isMicroflow, viaRole)) + } + } return violations } +// effectiveHomePage returns the home page (or microflow) a user role lands on +// in a profile: its role-based entry if there is one, else the default. +func effectiveHomePage(profile *types.NavigationProfile, userRole string) (target string, isMicroflow, viaRole bool) { + for _, rbh := range profile.RoleBasedHomePages { + if rbh.UserRole != userRole { + continue + } + if rbh.Page != "" { + return rbh.Page, false, true + } + if rbh.Microflow != "" { + return rbh.Microflow, true, true + } + } + if profile.HomePage == nil { + return "", false, false + } + if profile.HomePage.Page != "" { + return profile.HomePage.Page, false, false + } + return profile.HomePage.Microflow, profile.HomePage.Microflow != "", false +} + +func homePageViolation(r *PageNavigationSecurityRule, ur *security.UserRole, profile, target string, isMicroflow, viaRole bool) linter.Violation { + kind, docType, verb, grant := "page", "page", "open", "GRANT VIEW ON PAGE" + if isMicroflow { + kind, docType, verb, grant = "microflow", "microflow", "run", "GRANT EXECUTE ON MICROFLOW" + } + which := "default home " + kind + if viaRole { + which = "role-based home " + kind + } + moduleName := moduleFromQualified(target) + suggestion := fmt.Sprintf("%s %s TO %s.<RoleName> for a module role of user role '%s'", grant, target, moduleName, ur.Name) + if !viaRole { + suggestion += fmt.Sprintf(", or give '%s' a role-based home page it can %s in %s navigation", ur.Name, verb, profile) + } + return linter.Violation{ + RuleID: r.ID(), + Severity: r.DefaultSeverity(), + Message: fmt.Sprintf("User role '%s' cannot %s %s '%s' in %s navigation: none of its module roles is allowed (mxbuild reports CE2729 for each widget on it)", + ur.Name, verb, which, target, profile), + Location: linter.Location{ + Module: moduleName, + DocumentType: docType, + DocumentName: docNameFromQualified(target), + }, + Suggestion: suggestion, + } +} + +// sharesRole reports whether any of a user role's module roles is allowed. +func sharesRole(userModuleRoles, allowed []string) bool { + for _, a := range allowed { + for _, m := range userModuleRoles { + if a == m { + return true + } + } + } + return false +} + // collectMenuPages recursively collects pages from navigation menu items. func collectMenuPages(items []*types.NavMenuItem, profileName string, navPages map[string][]navUsage) { for _, item := range items { @@ -132,36 +248,69 @@ func collectMenuPages(items []*types.NavMenuItem, profileName string, navPages m } } -// buildPageRoleCountMap builds a map of qualified page name → number of allowed roles. -func buildPageRoleCountMap(reader linter.LintReader) map[string]int { - result := make(map[string]int) - +// buildPageRoleMap builds a map of qualified page name → allowed module roles. +func buildPageRoleMap(reader linter.LintReader) map[string][]string { + result := make(map[string][]string) pages, err := reader.ListPages() if err != nil { return result } + resolveModule := moduleResolver(reader) + for _, pg := range pages { + moduleName := resolveModule(string(pg.ContainerID)) + if moduleName == "" { + continue + } + result[moduleName+"."+pg.Name] = idsToStrings(pg.AllowedRoles) + } + return result +} - // Build hierarchy to resolve container → module - modules, err := reader.ListModules() +// buildMicroflowRoleMap builds a map of qualified microflow name → allowed module roles. +func buildMicroflowRoleMap(reader linter.LintReader) map[string][]string { + result := make(map[string][]string) + mfs, err := reader.ListMicroflows() if err != nil { return result } - folders, err := reader.ListFolders() - if err != nil { - return result + resolveModule := moduleResolver(reader) + for _, mf := range mfs { + moduleName := resolveModule(string(mf.ContainerID)) + if moduleName == "" { + continue + } + result[moduleName+"."+mf.Name] = idsToStrings(mf.AllowedModuleRoles) } + return result +} - // Build container → module name map - moduleByID := make(map[string]string) // module ID → module name - for _, m := range modules { - moduleByID[string(m.ID)] = m.Name +// idsToStrings converts allowed-role references, which the reader fills with +// qualified module role names, to plain strings. Never nil, so a document with +// no allowed roles is distinguishable from one that was not found. +func idsToStrings(ids []model.ID) []string { + out := make([]string, 0, len(ids)) + for _, id := range ids { + out = append(out, string(id)) } + return out +} + +// moduleResolver returns a function resolving a container ID (module or +// folder) to its module name, or "" when it cannot. +func moduleResolver(reader linter.LintReader) func(string) string { + moduleByID := make(map[string]string) // module ID → module name folderParent := make(map[string]string) // folder ID → parent ID - for _, f := range folders { - folderParent[string(f.ID)] = string(f.ContainerID) + if modules, err := reader.ListModules(); err == nil { + for _, m := range modules { + moduleByID[string(m.ID)] = m.Name + } } - - resolveModule := func(containerID string) string { + if folders, err := reader.ListFolders(); err == nil { + for _, f := range folders { + folderParent[string(f.ID)] = string(f.ContainerID) + } + } + return func(containerID string) string { id := containerID for i := 0; i < 20; i++ { // max depth guard if name, ok := moduleByID[id]; ok { @@ -175,17 +324,6 @@ func buildPageRoleCountMap(reader linter.LintReader) map[string]int { } return "" } - - for _, pg := range pages { - moduleName := resolveModule(string(pg.ContainerID)) - if moduleName == "" { - continue - } - qualifiedName := moduleName + "." + pg.Name - result[qualifiedName] = len(pg.AllowedRoles) - } - - return result } // moduleFromQualified extracts module name from "Module.Name". diff --git a/mdl/linter/rules/page_navigation_security_test.go b/mdl/linter/rules/page_navigation_security_test.go index 271c9a969c..0ec8ff6e82 100644 --- a/mdl/linter/rules/page_navigation_security_test.go +++ b/mdl/linter/rules/page_navigation_security_test.go @@ -3,10 +3,16 @@ package rules import ( + "strings" "testing" + "github.com/mendixlabs/mxcli/mdl/catalog" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" + "github.com/mendixlabs/mxcli/sdk/pages" + "github.com/mendixlabs/mxcli/sdk/security" ) func TestCollectMenuPages_Flat(t *testing.T) { @@ -93,3 +99,128 @@ func TestPageNavigationSecurityRule_Metadata(t *testing.T) { t.Errorf("Category = %q, want security", r.Category()) } } + +// navSecurityReader serves the navigation, security, pages and microflows the +// MPR007 home-page check reads; everything else is empty. +type navSecurityReader struct { + nav *types.NavigationDocument + sec *security.ProjectSecurity + pages []*pages.Page + mfs []*microflows.Microflow +} + +func (r navSecurityReader) GetMicroflow(model.ID) (*microflows.Microflow, error) { return nil, nil } +func (r navSecurityReader) ListMicroflows() ([]*microflows.Microflow, error) { return r.mfs, nil } +func (r navSecurityReader) GetProjectSecurity() (*security.ProjectSecurity, error) { + return r.sec, nil +} +func (r navSecurityReader) GetProjectSettings() (*model.ProjectSettings, error) { return nil, nil } +func (r navSecurityReader) GetNavigation() (*types.NavigationDocument, error) { return r.nav, nil } +func (r navSecurityReader) ListPages() ([]*pages.Page, error) { return r.pages, nil } +func (r navSecurityReader) ListModules() ([]*model.Module, error) { + return []*model.Module{{BaseElement: model.BaseElement{ID: "mod"}, Name: "App"}}, nil +} +func (r navSecurityReader) ListFolders() ([]*types.FolderInfo, error) { return nil, nil } +func (r navSecurityReader) GetRawUnit(model.ID) (map[string]any, error) { return nil, nil } +func (r navSecurityReader) ListScheduledEvents() ([]*model.ScheduledEvent, error) { + return nil, nil +} + +func navPage(name string, roles ...model.ID) *pages.Page { + return &pages.Page{ContainerID: "mod", Name: name, AllowedRoles: roles} +} + +// homePageFixture: App.Home is the default home page, open to App.User only; +// App.AdminHome is open to App.Admin only. The template-style role "Leftover" +// has only System/Administration module roles — the ChipCoV6 case. +func homePageFixture(rbh ...*types.NavRoleBasedHome) navSecurityReader { + return navSecurityReader{ + nav: &types.NavigationDocument{Profiles: []*types.NavigationProfile{{ + Name: "Responsive", Kind: "Responsive", + HomePage: &types.NavHomePage{Page: "App.Home"}, + RoleBasedHomePages: rbh, + }}}, + sec: &security.ProjectSecurity{ + SecurityLevel: security.SecurityLevelProduction, + UserRoles: []*security.UserRole{ + {Name: "Member", ModuleRoles: []string{"System.User", "App.User"}}, + {Name: "Leftover", ModuleRoles: []string{"System.User", "Administration.User"}}, + }, + }, + pages: []*pages.Page{navPage("Home", "App.User"), navPage("AdminHome", "App.Admin"), navPage("LeftoverHome", "Administration.User")}, + } +} + +func homePageViolations(t *testing.T, reader navSecurityReader) []linter.Violation { + t.Helper() + cat, err := catalog.New() + if err != nil { + t.Fatal(err) + } + defer cat.Close() + return NewPageNavigationSecurityRule().Check(linter.NewLintContext(cat, reader)) +} + +// A user role none of whose module roles may open the default home page is +// reported, naming role, profile and page (ChipCoV6: 10× CE2729 in mxbuild, +// silent in lint and check --references). +func TestPageNavigationSecurity_UserRoleCannotOpenHomePage(t *testing.T) { + vs := homePageViolations(t, homePageFixture()) + if len(vs) != 1 { + t.Fatalf("want 1 violation for Leftover, got %d: %+v", len(vs), vs) + } + msg := vs[0].Message + for _, want := range []string{"'Leftover'", "'App.Home'", "Responsive", "CE2729"} { + if !strings.Contains(msg, want) { + t.Errorf("message %q lacks %s", msg, want) + } + } + if !strings.Contains(vs[0].Suggestion, "GRANT VIEW ON PAGE App.Home") || !strings.Contains(vs[0].Suggestion, "role-based home page") { + t.Errorf("suggestion %q should offer both fixes", vs[0].Suggestion) + } +} + +// Control: a role-based home page the role can open replaces the inaccessible default. +func TestPageNavigationSecurity_RoleBasedHomePageItCanOpen(t *testing.T) { + vs := homePageViolations(t, homePageFixture(&types.NavRoleBasedHome{UserRole: "Leftover", Page: "App.LeftoverHome"})) + if len(vs) != 0 { + t.Fatalf("want no violations, got %+v", vs) + } +} + +// A role-based home page the role cannot open is reported as such. +func TestPageNavigationSecurity_RoleBasedHomePageItCannotOpen(t *testing.T) { + vs := homePageViolations(t, homePageFixture(&types.NavRoleBasedHome{UserRole: "Leftover", Page: "App.AdminHome"})) + if len(vs) != 1 || !strings.Contains(vs[0].Message, "role-based home page 'App.AdminHome'") { + t.Fatalf("want 1 role-based violation, got %+v", vs) + } +} + +// Control: when every role has access, nothing is reported. +func TestPageNavigationSecurity_AllRolesCanOpenHomePage(t *testing.T) { + r := homePageFixture() + r.sec.UserRoles[1].ModuleRoles = append(r.sec.UserRoles[1].ModuleRoles, "App.User") + if vs := homePageViolations(t, r); len(vs) != 0 { + t.Fatalf("want no violations, got %+v", vs) + } +} + +// Security off checks no roles, so neither does the rule. +func TestPageNavigationSecurity_SecurityOffSkipsRoleCheck(t *testing.T) { + r := homePageFixture() + r.sec.SecurityLevel = security.SecurityLevelOff + if vs := homePageViolations(t, r); len(vs) != 0 { + t.Fatalf("want no violations, got %+v", vs) + } +} + +// A microflow home page is checked against the microflow's allowed roles. +func TestPageNavigationSecurity_MicroflowHomePage(t *testing.T) { + r := homePageFixture() + r.nav.Profiles[0].HomePage = &types.NavHomePage{Microflow: "App.ACT_Home"} + r.mfs = []*microflows.Microflow{{ContainerID: "mod", Name: "ACT_Home", AllowedModuleRoles: []model.ID{"App.User"}}} + vs := homePageViolations(t, r) + if len(vs) != 1 || !strings.Contains(vs[0].Message, "cannot run default home microflow 'App.ACT_Home'") || !strings.Contains(vs[0].Message, "'Leftover'") { + t.Fatalf("want 1 microflow violation for Leftover, got %+v", vs) + } +} From ae8c4ba7704fbf16b23b2ca5e1ac6b27a6c2d1af Mon Sep 17 00:00:00 2001 From: Ako <andrej@koelewijn.net> Date: Fri, 9 Oct 2026 21:35:59 +0000 Subject: [PATCH 13/13] chore(devcontainer): build with go.mod's Go toolchain (1.27.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The devcontainer was on go:dev-1.26-bookworm while go.mod and CI moved to toolchain go1.27.2 (#1064) for five stdlib security fixes (GO-2026-6603..6608, fixed in 1.26.9 / 1.27.2). The image sets GOTOOLCHAIN=local, so local builds ignored the pin and used the image's Go: 1.26.4 here. Bumping the tag alone is not enough: dev-1.27-bookworm ships 1.27.1, which is still affected, and the image has no patch-level tags. So also set GOTOOLCHAIN=auto, letting go.mod's toolchain line — the same pin CI uses — select the version, now and on future bumps. Verified by building this Dockerfile and running `go version` against the repo's go.mod: this image GOTOOLCHAIN=auto go1.27.2 (downloaded) base image (control) GOTOOLCHAIN=local go1.27.1 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --- .devcontainer/Dockerfile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 702fe6e1ff..46b3334927 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -1,5 +1,11 @@ # Mendix Model SDK Go - Development Container -FROM mcr.microsoft.com/devcontainers/go:dev-1.26-bookworm +FROM mcr.microsoft.com/devcontainers/go:dev-1.27-bookworm + +# The base image sets GOTOOLCHAIN=local, which ignores go.mod's `toolchain` +# line and builds with whatever Go the image ships. Its tags track minor +# versions only (dev-1.27 was 1.27.1 when go.mod moved to 1.27.2 for security +# fixes), so `auto` lets go.mod — the same pin CI uses — choose the toolchain. +ENV GOTOOLCHAIN=auto RUN rm -f /etc/apt/sources.list.d/yarn.list