diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index ee17cd08d..7d17f5a0b 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -165,6 +165,8 @@ to the symptom table below, so the next similar issue costs fewer reads. | No way to change an existing **enumeration value's caption** in place — `alter enumeration` had only `ADD`/`RENAME`/`DROP VALUE` + `SET COMMENT`; `RENAME VALUE X TO Y` changes the *name*, not the caption. The only route was drop + recreate, which fails while the enum is referenced by an attribute | Missing grammar action + AST op + executor case — a full-stack gap, not a code bug | `mdl/grammar/domains/MDLDomainModel.g4` (`alterEnumerationAction`) → `mdl/ast/ast_enumeration.go` (`AlterEnumOp`) → `mdl/visitor/visitor_enumeration.go` (`ExitAlterEnumerationAction`) → `mdl/executor/cmd_enumerations.go` (`execAlterEnumeration`) | Add `MODIFY VALUE IDENTIFIER CAPTION STRING_LITERAL` (reuses existing `MODIFY`/`CAPTION` tokens, so no lexer change); add `AlterEnumModifyCaption` op reusing the `Caption` field; executor finds the value by name and replaces only the `en_US` translation (preserves the value's ID + other locales). Re-captions in place, so it works while referenced (mxbuild 0 errors) — no drop needed. Value names in `alter` must be plain identifiers (a reserved-word-named value like `Created` can't be targeted — pre-existing, shared by ADD/RENAME/DROP). Tests: `TestAlterEnumeration_ModifyValueCaption` (visitor) + `TestAlterEnumeration_ModifyValueCaption_Mock` (executor); example in `01-domain-model-examples.mdl` | | A page with a native **LISTVIEW** over a `database from` (XPath) datasource passes `mx check` but **crashes the browser client at runtime** and redirects to login: `TypeError: Cannot read properties of undefined (reading 'length')` at `processResult` → `retrieveByXPath`. Pluggable Gallery/DataGrid2 database sources are fine | The serialized `Forms$ListViewXPathSource` omitted the arrays the client reads `.length` of. **Codec (default):** `Forms$ListViewSearch` emitted without its `SearchRefs` list — the encoder drops an empty, never-`Set` PartList unless the `$Type` is in `RegisterTypeDefaults` (GridSortBar.SortItems was registered; ListViewSearch was not). **Legacy:** wrote a bogus `Forms$ListViewSort` + a `Paths` key (renamed `SearchRefs` in 7.11.0) and no `Forms$GridSortBar`. NOT a `SortItems` marker issue — an empty `[2]` compiles fine when search is off; the crash is the absent `SearchRefs`. Diagnose by building a Deploy target and reading the compiled `deployment/web/pages/.js` (the client model) + `mxcli bson dump --type page` on the source | `mdl/backend/modelsdk/widget_write.go` (`init` RegisterTypeDefaults) + `sdk/mpr/writer_widgets_display.go` (`serializeListViewDataSource`, `emptyListViewXPathSource`) | Codec: `RegisterTypeDefaults("Forms$ListViewSearch", {MandatoryLists: []string{"SearchRefs"}})` (emits empty marker-3 list). Legacy: emit `Forms$GridSortBar`/`SortItems` (mirror `SerializeCustomWidgetDataSource`) + `Forms$ListViewSearch`/`SearchRefs` + `ForceFullObjects`; drop the bogus `Sort`/`Paths`. Tests: `TestListViewSourceToGen_SearchRefsEmitted` (codec, encode-level), `TestSerializeListViewDataSource_Database` + `TestEmptyListViewXPathSource_Shape` (legacy). Repro `mdl-examples/bug-tests/listview-database-source-searchrefs.mdl` | | Editing `themesource/**/main.scss` (or `theme/web/main.scss`) while `mxcli run --local` serves on `:8080` keeps showing **old styles** — reads exactly like a stale compiled-CSS cache. `rm -rf theme-cache/ .mendix-cache/ deployment/` "fixes" it only because a restart came with it | THREE distinct causes, none a CSS cache: (1) **no `--watch` = no watcher at all** — `mxbuild --serve` only rebuilds on a `/build` request (startup, or a watch tick), so a save changes nothing; (2) **stale process silently adopted** — a leftover serve/runtime on the ports answers the startup readiness probes (`waitReady`/`waitAdminReady` only check "port answers"), so a new run attaches to the OLD process and its own child is torn down by `defer`; a backgrounded `run --local` whose wrapping shell exited non-zero dies while its serve+runtime keep serving; (3) the theme source was **watched by nothing** — the `--watch` signal was model-only (`.mpr`+`mprcontents/`). The incremental theme step itself is FINE: one `/build` after an scss **content** edit does rewrite `theme-cache/web/theme.compiled.css` (verified), so there is no cache to clear | `cmd/mxcli/docker/runlocal.go` — `checkTargetPortsFree` (guard), `themeSourceMTime`/`sourceMTime` (watch signal), `watchAndApply` (generation log) | (2) Refuse to boot when `:8080/:8090/:6543` already answer, with an actionable message (never auto-kill — user's call). (3) Add `theme/`+`themesource/` scss/css/js to the `--watch` mtime-poll signal (`sourceMTime` = max(model, theme)); poll-based so it's container-safe (unlike the rollup chokidar/inotify web-client watcher). Log a build-generation counter (`build #2`) so "did it take?" is answerable. Docs: `docs-site/src/tools/run-local.md` + skill `run-local.md` — "SCSS needs a rebuild (`--watch` or clean restart), never a cache-clear; kill the old serve/runtime first". Tests `TestThemeSourceMTime_WatchesThemeAndThemesource`, `TestCheckTargetPortsFree` | +| `alter entity M.E add attribute X: autonumber;` (no seed) or `... add attribute Created: AutoCreatedDate;` (renamed AutoX) passes `mxcli check` "Check passed!" but fails the build (CE7247) / silently discards the name — while the SAME attribute in `create entity` is correctly flagged (MDL023 / MDL022) | The per-attribute checks (MDL021/022/023) only ran on `CreateEntityStmt`; the ALTER ENTITY ADD ATTRIBUTE path had no validation at all, so an attribute added later escaped every rule | `mdl/executor/cmd_enumerations.go` (`ValidateAlterEntity`, `validateEntityAttribute`) wired from `cmd/mxcli/cmd_check.go` + `cmd/mxcli/lsp_diagnostics.go` | Extract the CREATE loop body into `validateEntityAttribute(attr, persistent, entityName)`; add `ValidateAlterEntity(stmt)` that runs it on `AlterEntityAddAttribute`. The entity kind isn't known from ALTER, so the persistent-only MDL020 is skipped; the kind-independent MDL021/022/023 all run. Bug-test `mdl-examples/bug-tests/f6-autonumber-seed-alter.fail.mdl`. Findings #6 (alter path) | +| `create or modify entity M.E ( )` on an entity that already has more attributes **silently drops** every attribute not re-listed (36→2 attrs seen in practice), then widgets/microflows still bound to them fail the build with CE1613 — and the "already exists" error that leads users here recommends the destructive `create or modify` for a partial edit | `create or modify` rebuilds the entity from the statement alone and REPLACEs the stored one, so any omitted attribute is deleted with no warning; the `NewAlreadyExistsMsg` hint pointed at `create or modify` without distinguishing "replace whole" from "add one" | `mdl/executor/cmd_entities.go` (`droppedEntityMembers`, the warn block before `UpdateEntity`, and the `execCreateEntity` already-exists message) | Warn-only (non-blocking, the user asked to modify): before `UpdateEntity`, diff existing vs replacement members (`droppedEntityMembers` — named attrs case-insensitive + the four audit flags) and print what's dropped + point at `alter entity … add attribute` for incremental edits. Fix the already-exists message to recommend `alter entity` for a member change and reserve `create or modify` for a full replace. Bug-test `mdl-examples/bug-tests/f24-create-or-modify-dataloss.mdl`. Findings #24 | --- diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go index d85808e7c..5c24a0704 100644 --- a/cmd/mxcli/cmd_check.go +++ b/cmd/mxcli/cmd_check.go @@ -113,6 +113,10 @@ Examples: if entityStmt, ok := stmt.(*ast.CreateEntityStmt); ok { violations = append(violations, executor.ValidateEntity(entityStmt)...) } + // Apply the same per-attribute checks to ALTER ENTITY ADD ATTRIBUTE + if alterStmt, ok := stmt.(*ast.AlterEntityStmt); ok { + violations = append(violations, executor.ValidateAlterEntity(alterStmt)...) + } // Check microflow body for common issues if mfStmt, ok := stmt.(*ast.CreateMicroflowStmt); ok { violations = append(violations, executor.ValidateMicroflow(mfStmt)...) diff --git a/cmd/mxcli/lsp_diagnostics.go b/cmd/mxcli/lsp_diagnostics.go index 0acc7d9de..73ea14580 100644 --- a/cmd/mxcli/lsp_diagnostics.go +++ b/cmd/mxcli/lsp_diagnostics.go @@ -272,6 +272,9 @@ func (s *mdlServer) runSemanticValidation(text string) []protocol.Diagnostic { if entityStmt, ok := stmt.(*ast.CreateEntityStmt); ok { violations = append(violations, executor.ValidateEntity(entityStmt)...) } + if alterStmt, ok := stmt.(*ast.AlterEntityStmt); ok { + violations = append(violations, executor.ValidateAlterEntity(alterStmt)...) + } if mfStmt, ok := stmt.(*ast.CreateMicroflowStmt); ok { violations = append(violations, executor.ValidateMicroflow(mfStmt)...) } diff --git a/mdl-examples/bug-tests/f24-create-or-modify-dataloss.mdl b/mdl-examples/bug-tests/f24-create-or-modify-dataloss.mdl new file mode 100644 index 000000000..a23cd4c89 --- /dev/null +++ b/mdl-examples/bug-tests/f24-create-or-modify-dataloss.mdl @@ -0,0 +1,18 @@ +-- Findings #24: `create or modify entity` rebuilds the entity from the statement +-- alone and REPLACES the stored one, so any existing attribute the statement +-- omits is silently dropped (later surfacing as CE1613 on widgets still bound to +-- it). This is now non-blocking but WARNED: running the second statement against +-- a project where Sudoku.Game already has Solution/Difficulty prints a data-loss +-- warning listing the dropped members, and points at ALTER ENTITY for safe, +-- incremental edits. +-- +-- Syntactically valid (this file is a syntax example, not a negative test); the +-- warning only appears when executed against a project that has the fuller entity. + +-- Safe incremental change — edits only the named member, leaves the rest intact: +alter entity Sudoku.Game add attribute Score: integer; + +-- Destructive replace — keep ONLY if you intend to drop everything not listed: +create or modify entity Sudoku.Game ( + Puzzle: string +); diff --git a/mdl-examples/bug-tests/f6-autonumber-seed-alter.fail.mdl b/mdl-examples/bug-tests/f6-autonumber-seed-alter.fail.mdl new file mode 100644 index 000000000..cfb85a174 --- /dev/null +++ b/mdl-examples/bug-tests/f6-autonumber-seed-alter.fail.mdl @@ -0,0 +1,6 @@ +-- Findings #6 (alter path): the seedless-autonumber check (MDL023) also applies +-- when the attribute is added later via ALTER ENTITY, not only at CREATE. Before +-- this fix, `alter entity ... add attribute X: autonumber;` returned "Check +-- passed!" and then failed the build with CE7247 "Value cannot be empty". +-- Negative test: `mxcli check` MUST fail here. +alter entity Bug6.Game add attribute PuzzleNo: autonumber; diff --git a/mdl/executor/bugfix_test.go b/mdl/executor/bugfix_test.go index c2b4dc37b..b20ad422e 100644 --- a/mdl/executor/bugfix_test.go +++ b/mdl/executor/bugfix_test.go @@ -9,8 +9,11 @@ import ( "testing" "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" "github.com/mendixlabs/mxcli/mdl/linter" "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" ) // TestValidateDuplicateVariableDeclareRetrieve verifies that DECLARE followed by @@ -189,6 +192,147 @@ func TestValidateEntityAutoMemberRename(t *testing.T) { } } +// TestValidateAlterEntityAddAttribute verifies that the same per-attribute +// checks applied on CREATE (MDL022 AutoX rename, MDL023 autonumber seed, MDL021 +// reserved words) also run on the ALTER ENTITY ADD ATTRIBUTE path. Previously an +// attribute added via ALTER escaped these checks entirely (findings #6, half-fix). +func TestValidateAlterEntityAddAttribute(t *testing.T) { + build := func(src string) []linter.Violation { + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse error: %v", errs[0]) + } + return ValidateAlterEntity(prog.Statements[0].(*ast.AlterEntityStmt)) + } + hasMDL := func(vs []linter.Violation, id string) bool { + for _, v := range vs { + if v.RuleID == id { + return true + } + } + return false + } + + // MDL023: seedless autonumber added via ALTER. + if !hasMDL(build(`alter entity M.G add attribute PuzzleNo: autonumber;`), "MDL023") { + t.Error("expected MDL023 for a seedless autonumber added via ALTER") + } + if hasMDL(build(`alter entity M.G add attribute PuzzleNo: autonumber default 1;`), "MDL023") { + t.Error("did not expect MDL023 when the added autonumber has a seed") + } + // MDL022: AutoX pseudo-type added under a non-canonical name. + if !hasMDL(build(`alter entity M.G add attribute StartedAt: autocreateddate;`), "MDL022") { + t.Error("expected MDL022 for a renamed autocreateddate added via ALTER") + } + if hasMDL(build(`alter entity M.G add attribute CreatedDate: autocreateddate;`), "MDL022") { + t.Error("did not expect MDL022 when the added AutoX name matches the system member") + } + // MDL021: reserved word added via ALTER (kind-independent). + if !hasMDL(build(`alter entity M.G add attribute Type: string;`), "MDL021") { + t.Error("expected MDL021 for a reserved word added via ALTER") + } + // A plain, valid attribute added via ALTER produces no violations. + if vs := build(`alter entity M.G add attribute Score: integer;`); len(vs) != 0 { + t.Errorf("expected no violations for a plain added attribute, got %v", vs) + } + // Non-ADD alter operations are ignored. + if vs := build(`alter entity M.G drop attribute Score;`); len(vs) != 0 { + t.Errorf("expected no violations for a DROP ATTRIBUTE op, got %v", vs) + } +} + +// TestDroppedEntityMembers verifies the helper that lists members a CREATE OR +// MODIFY replace would delete (findings #24). +func TestDroppedEntityMembers(t *testing.T) { + existing := &domainmodel.Entity{ + Name: "Game", + Attributes: []*domainmodel.Attribute{ + {Name: "Puzzle"}, {Name: "Solution"}, {Name: "Difficulty"}, + }, + HasCreatedDate: true, + HasOwner: true, + } + replacement := &domainmodel.Entity{ + Name: "Game", + Attributes: []*domainmodel.Attribute{ + {Name: "puzzle"}, // case-insensitive match — kept + {Name: "Score"}, // new — not a drop + }, + HasOwner: true, // kept + // HasCreatedDate now false — dropped + } + dropped := droppedEntityMembers(existing, replacement) + got := strings.Join(dropped, ",") + // Solution and Difficulty are dropped; Puzzle survives (case-insensitive); + // createdDate audit field is dropped; owner survives. + for _, want := range []string{"Solution", "Difficulty", "createdDate (system field)"} { + if !strings.Contains(got, want) { + t.Errorf("expected dropped members to include %q, got %q", want, got) + } + } + for _, unwanted := range []string{"Puzzle", "puzzle", "Score", "owner"} { + if strings.Contains(got, unwanted) { + t.Errorf("did not expect %q among dropped members, got %q", unwanted, got) + } + } +} + +// TestCreateOrModifyEntity_WarnsOnDrop verifies that CREATE OR MODIFY ENTITY +// still applies (non-blocking) but prints a data-loss warning listing the +// attributes it removes (findings #24). +func TestCreateOrModifyEntity_WarnsOnDrop(t *testing.T) { + mod := mkModule("Sudoku") + existing := &domainmodel.Entity{ + BaseElement: model.BaseElement{ID: nextID("ent")}, + ContainerID: nextID("dm"), + Name: "Game", + Persistable: true, + Attributes: []*domainmodel.Attribute{ + {Name: "Puzzle"}, {Name: "Solution"}, {Name: "Difficulty"}, + }, + } + dm := &domainmodel.DomainModel{ + BaseElement: model.BaseElement{ID: nextID("dm")}, + ContainerID: mod.ID, + Entities: []*domainmodel.Entity{existing}, + } + h := mkHierarchy(mod) + withContainer(h, dm.ID, mod.ID) + withContainer(h, existing.ContainerID, dm.ID) + + updateCalled := false + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dm, nil }, + UpdateEntityFunc: func(dmID model.ID, e *domainmodel.Entity) error { + updateCalled = true + return nil + }, + } + + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + // Redeclare only one of the three existing attributes. + err := execCreateEntity(ctx, &ast.CreateEntityStmt{ + Name: ast.QualifiedName{Module: "Sudoku", Name: "Game"}, + Kind: ast.EntityPersistent, + CreateOrModify: true, + Attributes: []ast.Attribute{ + {Name: "Puzzle", Type: ast.DataType{Kind: ast.TypeString}}, + }, + }) + assertNoError(t, err) + if !updateCalled { + t.Fatal("UpdateEntity was not called — modify should still apply (non-blocking)") + } + out := buf.String() + assertContainsStr(t, out, "drops 2 existing member(s)") + assertContainsStr(t, out, "Solution") + assertContainsStr(t, out, "Difficulty") + assertContainsStr(t, out, "alter entity Sudoku.Game add attribute") +} + // TestValidateEntityNPEReservedWords verifies that non-persistent entities // reject runtime-reserved attribute names (CE7247). Issue #552. // diff --git a/mdl/executor/cmd_entities.go b/mdl/executor/cmd_entities.go index aefbfdf92..6bf8979db 100644 --- a/mdl/executor/cmd_entities.go +++ b/mdl/executor/cmd_entities.go @@ -71,9 +71,17 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { } } - // If entity exists and not using CREATE OR MODIFY, return error + // If entity exists and not using CREATE OR MODIFY, return error. Note the + // suggested fixes: `alter entity` for an incremental change (safe — it edits + // the members it names and leaves the rest alone), and `create or modify` only + // when the intent is to replace the whole definition — that path rebuilds the + // entity from the statement and DROPS any attribute the statement omits, so it + // is destructive if used for a partial update. (findings #24) if existingEntity != nil && !s.CreateOrModify { - return mdlerrors.NewAlreadyExistsMsg("entity", s.Name.Module+"."+s.Name.Name, fmt.Sprintf("entity already exists: %s.%s (use create or modify to update)", s.Name.Module, s.Name.Name)) + return mdlerrors.NewAlreadyExistsMsg("entity", s.Name.Module+"."+s.Name.Name, + fmt.Sprintf("entity already exists: %s.%s — to add or change a member use 'alter entity %s.%s add attribute ...' (leaves the rest intact); "+ + "use 'create or modify entity' only to replace the whole definition (it drops any attribute this statement omits)", + s.Name.Module, s.Name.Name, s.Name.Module, s.Name.Name)) } // Calculate position @@ -307,6 +315,22 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { return mdlerrors.NewBackend("delete orphaned view entity source document", err) } } + // Warn (non-blocking) about members this CREATE OR MODIFY drops. The + // path rebuilds the entity from the statement alone and replaces the + // stored one, so any existing attribute the statement omits is silently + // deleted — which, when a widget or microflow still binds it, surfaces + // much later as CE1613. The user asked to modify, so we still apply it, + // but list what is being removed so accidental data loss is visible + // rather than silent. (findings #24) + if dropped := droppedEntityMembers(existingEntity, entity); len(dropped) > 0 { + fmt.Fprintf(ctx.Output, + "⚠ create or modify entity %s drops %d existing member(s) not listed in this statement: %s\n", + s.Name, len(dropped), strings.Join(dropped, ", ")) + fmt.Fprintf(ctx.Output, + " They are removed from the entity; anything still bound to them (widgets, microflows) will fail the build with CE1613.\n") + fmt.Fprintf(ctx.Output, + " To add attributes without disturbing the rest, use: alter entity %s add attribute : ;\n", s.Name) + } // Update existing entity entity.ID = existingEntity.ID if err := ctx.Backend.UpdateEntity(dm.ID, entity); err != nil { @@ -331,6 +355,39 @@ func execCreateEntity(ctx *ExecContext, s *ast.CreateEntityStmt) error { return nil } +// droppedEntityMembers reports the members present on existing but absent from +// replacement — i.e. what a CREATE OR MODIFY replace would delete. Named +// attributes are compared case-insensitively; the four audit system fields are +// reported when their flag is on in existing but off in replacement. Used to +// surface accidental data loss (findings #24). +func droppedEntityMembers(existing, replacement *domainmodel.Entity) []string { + keep := make(map[string]bool, len(replacement.Attributes)) + for _, a := range replacement.Attributes { + keep[strings.ToLower(a.Name)] = true + } + var dropped []string + for _, a := range existing.Attributes { + if !keep[strings.ToLower(a.Name)] { + dropped = append(dropped, a.Name) + } + } + // Audit system fields that were enabled and are no longer requested are also + // removed by the replace. + if existing.HasOwner && !replacement.HasOwner { + dropped = append(dropped, "owner (system field)") + } + if existing.HasChangedBy && !replacement.HasChangedBy { + dropped = append(dropped, "changedBy (system field)") + } + if existing.HasCreatedDate && !replacement.HasCreatedDate { + dropped = append(dropped, "createdDate (system field)") + } + if existing.HasChangedDate && !replacement.HasChangedDate { + dropped = append(dropped, "changedDate (system field)") + } + return dropped +} + // execCreateViewEntity handles CREATE VIEW ENTITY statements. func execCreateViewEntity(ctx *ExecContext, s *ast.CreateViewEntityStmt) error { if !ctx.Connected() { diff --git a/mdl/executor/cmd_enumerations.go b/mdl/executor/cmd_enumerations.go index f93fc1154..73050fa7c 100644 --- a/mdl/executor/cmd_enumerations.go +++ b/mdl/executor/cmd_enumerations.go @@ -446,80 +446,105 @@ var autoMemberNames = map[ast.DataTypeKind]string{ // Mendix rejects these names at runtime regardless of persistence. func ValidateEntity(stmt *ast.CreateEntityStmt) []linter.Violation { var violations []linter.Violation + persistent := stmt.Kind == ast.EntityPersistent + entityName := stmt.Name.String() for _, attr := range stmt.Attributes { - // AutoX pseudo-types ARE the system attributes. The declared identifier is - // discarded — the field always materializes under its fixed system member - // name — so warn (MDL022) when the two differ, since the write silently - // renames it and the resulting member can't be bound as a regular widget - // attribute (a widget binding it fails the build with CE1613). (findings #7) - if canon, ok := autoMemberNames[attr.Type.Kind]; ok { - if !strings.EqualFold(attr.Name, canon) { - violations = append(violations, linter.Violation{ - RuleID: "MDL022", - Severity: linter.SeverityWarning, - Message: fmt.Sprintf( - "attribute '%s: %s' is renamed to the fixed system member '%s' on write — "+ - "the declared name is discarded", - attr.Name, attr.Type.Kind, canon), - Location: linter.Location{DocumentType: "entity", DocumentName: stmt.Name.String()}, - Suggestion: fmt.Sprintf( - "Declare it as '%s: %s' to match. This is a Mendix system member and cannot be "+ - "bound in a widget; to store a value a widget can show, use a plain attribute "+ - "(e.g. '%s: DateTime') and set it yourself.", - canon, attr.Type.Kind, attr.Name), - }) - } - continue - } - // autonumber needs a seed, or the build fails CE7247 "Value cannot be - // empty". mxcli check accepted it silently before. (findings #6) - if attr.Type.Kind == ast.TypeAutoNumber && !attr.HasDefault { - violations = append(violations, linter.Violation{ - RuleID: "MDL023", - Severity: linter.SeverityError, - Message: fmt.Sprintf( - "autonumber attribute '%s' has no seed — the build fails CE7247 \"Value cannot be empty\"", - attr.Name), - Location: linter.Location{DocumentType: "entity", DocumentName: stmt.Name.String()}, - Suggestion: fmt.Sprintf("Give it a start value: '%s: autonumber default 1'", attr.Name), - }) - continue - } - lower := strings.ToLower(attr.Name) - // MDL020: system-attribute conflict — only meaningful on persistent entities, - // where the suggested fix is to use the AutoX pseudo-type. - if stmt.Kind == ast.EntityPersistent && mendixSystemAttributeNames[lower] { - violations = append(violations, linter.Violation{ - RuleID: "MDL020", - Severity: linter.SeverityError, - Message: fmt.Sprintf( - "attribute '%s' conflicts with a Mendix system attribute name. "+ - "Mendix automatically manages '%s' on persistent entities", - attr.Name, attr.Name), - Location: linter.Location{ - DocumentType: "entity", - DocumentName: stmt.Name.String(), - }, - Suggestion: fmt.Sprintf("To use the Mendix built-in audit field, declare it with the pseudo-type: '%s: Auto%s'. To store an unrelated date, choose a different name (e.g., 'EntryDate', 'RecordDate')", attr.Name, attr.Name), - }) - continue - } - // MDL021: CE7247 runtime reserved words — apply to all entity kinds - // including non-persistent and view entities. - if mendixReservedWords[lower] { + violations = append(violations, validateEntityAttribute(attr, persistent, entityName)...) + } + return violations +} + +// ValidateAlterEntity applies the same per-attribute name/seed checks to the +// ALTER ENTITY … ADD ATTRIBUTE path that ValidateEntity applies on CREATE, so +// an attribute added later is held to the same rules as one declared up front +// (findings #6). The entity's persistence kind is not known from the ALTER +// statement alone, so the persistent-only MDL020 system-attribute check is +// skipped here; the kind-independent checks (MDL021 reserved words, MDL022 AutoX +// rename, MDL023 autonumber seed) all still run. +func ValidateAlterEntity(stmt *ast.AlterEntityStmt) []linter.Violation { + if stmt.Operation != ast.AlterEntityAddAttribute || stmt.Attribute == nil { + return nil + } + return validateEntityAttribute(*stmt.Attribute, false, stmt.Name.String()) +} + +// validateEntityAttribute runs the reserved-name / AutoX / autonumber-seed checks +// for a single attribute. persistent enables the MDL020 system-attribute check, +// which is only meaningful when the entity is known to be persistent. +func validateEntityAttribute(attr ast.Attribute, persistent bool, entityName string) []linter.Violation { + var violations []linter.Violation + // AutoX pseudo-types ARE the system attributes. The declared identifier is + // discarded — the field always materializes under its fixed system member + // name — so warn (MDL022) when the two differ, since the write silently + // renames it and the resulting member can't be bound as a regular widget + // attribute (a widget binding it fails the build with CE1613). (findings #7) + if canon, ok := autoMemberNames[attr.Type.Kind]; ok { + if !strings.EqualFold(attr.Name, canon) { violations = append(violations, linter.Violation{ - RuleID: "MDL021", - Severity: linter.SeverityError, + RuleID: "MDL022", + Severity: linter.SeverityWarning, Message: fmt.Sprintf( - "attribute '%s' is a reserved word (CE7247)", - attr.Name), - Location: linter.Location{ - DocumentType: "entity", - DocumentName: stmt.Name.String(), - }, - Suggestion: fmt.Sprintf("Rename to a non-reserved name (e.g., '%sValue' or '%sField')", attr.Name, attr.Name), + "attribute '%s: %s' is renamed to the fixed system member '%s' on write — "+ + "the declared name is discarded", + attr.Name, attr.Type.Kind, canon), + Location: linter.Location{DocumentType: "entity", DocumentName: entityName}, + Suggestion: fmt.Sprintf( + "Declare it as '%s: %s' to match. This is a Mendix system member and cannot be "+ + "bound in a widget; to store a value a widget can show, use a plain attribute "+ + "(e.g. '%s: DateTime') and set it yourself.", + canon, attr.Type.Kind, attr.Name), }) } + return violations + } + // autonumber needs a seed, or the build fails CE7247 "Value cannot be + // empty". mxcli check accepted it silently before. (findings #6) + if attr.Type.Kind == ast.TypeAutoNumber && !attr.HasDefault { + violations = append(violations, linter.Violation{ + RuleID: "MDL023", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "autonumber attribute '%s' has no seed — the build fails CE7247 \"Value cannot be empty\"", + attr.Name), + Location: linter.Location{DocumentType: "entity", DocumentName: entityName}, + Suggestion: fmt.Sprintf("Give it a start value: '%s: autonumber default 1'", attr.Name), + }) + return violations + } + lower := strings.ToLower(attr.Name) + // MDL020: system-attribute conflict — only meaningful on persistent entities, + // where the suggested fix is to use the AutoX pseudo-type. + if persistent && mendixSystemAttributeNames[lower] { + violations = append(violations, linter.Violation{ + RuleID: "MDL020", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "attribute '%s' conflicts with a Mendix system attribute name. "+ + "Mendix automatically manages '%s' on persistent entities", + attr.Name, attr.Name), + Location: linter.Location{ + DocumentType: "entity", + DocumentName: entityName, + }, + Suggestion: fmt.Sprintf("To use the Mendix built-in audit field, declare it with the pseudo-type: '%s: Auto%s'. To store an unrelated date, choose a different name (e.g., 'EntryDate', 'RecordDate')", attr.Name, attr.Name), + }) + return violations + } + // MDL021: CE7247 runtime reserved words — apply to all entity kinds + // including non-persistent and view entities. + if mendixReservedWords[lower] { + violations = append(violations, linter.Violation{ + RuleID: "MDL021", + Severity: linter.SeverityError, + Message: fmt.Sprintf( + "attribute '%s' is a reserved word (CE7247)", + attr.Name), + Location: linter.Location{ + DocumentType: "entity", + DocumentName: entityName, + }, + Suggestion: fmt.Sprintf("Rename to a non-reserved name (e.g., '%sValue' or '%sField')", attr.Name, attr.Name), + }) } return violations }