From 8a70726b3a09b77dab1915419f42c03c6008eb96 Mon Sep 17 00:00:00 2001 From: stagknee <241821830+stagknee@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:01:13 -0500 Subject: [PATCH] fix: accept a bare enum name as a return type in exec, as check does A bare `returns Module.Name` is recorded by the parser as an entity reference. exec retried it as an enumeration for parameters but not for the return types of a microflow, a nanoflow or a rule, so a script that `check -p --references` accepted failed with "entity 'M.E' not found for return type". For a rule, check refused it as well ("a rule must return a Boolean or an enumeration, not Entity"). The three create handlers now resolve a return type through the helper the parameter path uses (entity first, then enumeration), and the rule check applies the same retyping before validateRule. The stored return type equals the one `returns enum M.E` stores. An unknown name now reports "entity or enumeration 'M.X' not found for return type". Co-Authored-By: Claude Sonnet 5.5 --- ...-enum-return-type-check-exec-disagree.json | 1 + CHANGELOG.md | 4 + docs-site/src/language/microflow-structure.md | 11 + .../bug-tests/1562-enum-return-type.mdl | 72 +++++++ mdl/executor/cmd_microflows_build.go | 30 ++- mdl/executor/cmd_microflows_helpers.go | 16 ++ mdl/executor/cmd_rules_create.go | 15 +- mdl/executor/enum_return_type_pedapp_test.go | 189 ++++++++++++++++ mdl/executor/enum_return_type_test.go | 201 ++++++++++++++++++ mdl/executor/validate.go | 14 +- mdl/executor/validate_declared_types.go | 22 +- 11 files changed, 558 insertions(+), 17 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-16-bare-enum-return-type-check-exec-disagree.json create mode 100644 mdl-examples/bug-tests/1562-enum-return-type.mdl create mode 100644 mdl/executor/enum_return_type_pedapp_test.go create mode 100644 mdl/executor/enum_return_type_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-bare-enum-return-type-check-exec-disagree.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-bare-enum-return-type-check-exec-disagree.json new file mode 100644 index 0000000000..57f8723357 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-16-bare-enum-return-type-check-exec-disagree.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-16", "symptom": "`create microflow M.F () returns M.Enum as $V` (also nanoflow and rule) passed `check -p --references` and was then refused by exec with \"entity 'M.Enum' not found for return type\"; for a rule check refused it too, with a misleading \"a rule must return a Boolean or an enumeration, not Entity\". `returns enum M.Enum`, `Enumeration(M.Enum)` and a bare enum name as a PARAMETER type all worked", "cause": "the parser records a bare Module.Name as an entity reference; the create handlers retry it as an enumeration for parameters but their return-type block only looked for an entity, while declaredTypeResolver.check was written against the general rule (entity or enumeration) and validateRule saw the unretyped entity kind", "file": "`mdl/executor/cmd_microflows_build.go` (microflow and nanoflow build), `mdl/executor/cmd_rules_create.go`, `bareEnumerationType` in `cmd_microflows_helpers.go`, `declaredTypeResolver.bareEnumeration` and the rule case in `validate.go`", "insight": "A fallback added for one declaration site (parameters) but not its sibling (return types) is a check/exec disagreement, because the checker was written against the general rule, not the site. When a handler resolves a name in one place, grep for the same lookup at every sibling site before trusting a green `check -p`; and a check-time validator that reads the AST kind (validateRule: Boolean or enumeration) needs the same retyping exec applies, or it refuses what exec now accepts. Settled by a PedApp assertAgree test per kind plus a stored-BSON comparison against the `enum` spelling, and mx check 11.12.5 at 0 errors. The same ambiguity is still open for a Java action's bare enum parameter and return type, which stores an entity type (CE1613)", "refs": ["mendixlabs/mxcli#1562"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 0798c1db7b..fbc14dc092 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed + +- **A bare `returns Module.Name` that names an enumeration is accepted by `exec`, as `check` already said** (mendixlabs/mxcli#1562) — `create microflow M.F () returns M.Variant as $V` (and the nanoflow and rule forms) passed `check -p --references` and was then refused by `exec` with "entity 'M.Variant' not found for return type". The parser cannot tell an entity from an enumeration for a bare qualified name; `exec` resolved that for parameters (the entity first, then an enumeration) but not for return types. All three kinds now do, and store exactly what `returns enum M.Variant` stores. A rule's `check` refused the bare form too, with a misleading "a rule must return a Boolean or an enumeration, not Entity". A name that is both an entity and an enumeration stays the entity, as for a parameter, and a name that is neither now says "entity or enumeration" in `check` and in `exec`, for return types and parameters alike, where it used to say only "entity". `returns enum M.Variant` and `Enumeration(M.Variant)` always worked and are what `describe` prints. + ## [0.27.0] - 2026-10-15 Headline: **`exec` is all or nothing: when a statement fails at write time, the whole run is rolled back, so a script never leaves a project half-changed.** Alongside it: a wave of `describe` → `check` → `exec` fixes from round-trip runs over real apps (the Siemens FSM template, the GenAI showcase, Marketplace modules), Mendix 9 projects can take microflows, nanoflows and pluggable widgets again, `check -p` resolves many more of the references `exec` and the build would refuse, the classic `filemanager` widget and role-based `Visible: roles (…)` are in MDL, and the companion draws a module's domain model. The MongoDB driver is upgraded for the October 2026 BSON advisories. diff --git a/docs-site/src/language/microflow-structure.md b/docs-site/src/language/microflow-structure.md index c3948bd197..a4aa0d481b 100644 --- a/docs-site/src/language/microflow-structure.md +++ b/docs-site/src/language/microflow-structure.md @@ -117,6 +117,17 @@ RETURN $Order; RETURN $FilteredOrders; ``` +### Returning an Enumeration + +Declare the type in the signature with `returns enum Module.Name`; `returns Enumeration(Module.Name)` is the same. A bare `returns Module.Name` is also accepted when no entity has that name and an enumeration does. If an entity and an enumeration share the name, the entity wins, so write `enum` when a name could be either. Nanoflows and rules take the same spellings, and a rule must return a Boolean or an enumeration. + +```sql +CREATE MICROFLOW Shop.SUB_Pick () RETURNS enum Shop.Variant AS $Variant +BEGIN + RETURN Shop.Variant.Original; +END; +``` + ### Returning Nothing If the microflow returns nothing (void), you can omit the `RETURN` or use: diff --git a/mdl-examples/bug-tests/1562-enum-return-type.mdl b/mdl-examples/bug-tests/1562-enum-return-type.mdl new file mode 100644 index 0000000000..31414e251f --- /dev/null +++ b/mdl-examples/bug-tests/1562-enum-return-type.mdl @@ -0,0 +1,72 @@ +mdl 1; +-- ============================================================================ +-- A bare `returns Module.Name` that names an enumeration +-- ============================================================================ +-- +-- POSITIVE TEST: `mxcli check` (with and without -p --references) and `exec` +-- MUST accept it. +-- +-- The parser cannot tell an entity from an enumeration for a bare qualified +-- name, so it records an entity reference. `exec` resolved that for PARAMETERS +-- (entity first, then enumeration) but not for RETURN TYPES, while `check` +-- resolved both the same way: a script `check` passed failed at exec with +-- "entity 'Shop.Variant' not found for return type". For a rule, check refused +-- it too, with a misleading "a rule must return a Boolean or an enumeration, +-- not Entity". +-- +-- A microflow, a nanoflow and a rule now accept the bare name, and store exactly +-- what `returns enum Module.Name` stores. A name that is both an entity and an +-- enumeration stays the entity, as it does for a parameter. +-- +-- Measured on mxbuild 11.12.5: this script execs and the app checks at 0 errors. + +CREATE MODULE EnumReturn; + +CREATE OR MODIFY ENUMERATION EnumReturn.Variant ( + Original 'Original', + Released 'Released', + None 'None' +); + +CREATE OR MODIFY PERSISTENT ENTITY EnumReturn.Customer ( + "Name": String(50) +); + +-- The bare name, as a parameter (already worked) and as a return type. +CREATE OR MODIFY MICROFLOW EnumReturn.SUB_Pick ($Current: EnumReturn.Variant) +RETURNS EnumReturn.Variant AS $Variant +BEGIN + RETURN EnumReturn.Variant.Original; +END; + +CREATE OR MODIFY NANOFLOW EnumReturn.NF_Pick () +RETURNS EnumReturn.Variant AS $Variant +BEGIN + RETURN EnumReturn.Variant.Released; +END; + +CREATE OR MODIFY RULE EnumReturn.RL_Pick ($Customer: EnumReturn.Customer) +RETURNS EnumReturn.Variant AS $Variant +BEGIN + RETURN EnumReturn.Variant.None; +END; + +-- The explicit spellings, which always worked, store the same thing. +CREATE OR MODIFY MICROFLOW EnumReturn.SUB_PickExplicit () +RETURNS enum EnumReturn.Variant AS $Variant +BEGIN + RETURN EnumReturn.Variant.Original; +END; + +CREATE OR MODIFY MICROFLOW EnumReturn.SUB_PickFunction () +RETURNS Enumeration(EnumReturn.Variant) AS $Variant +BEGIN + RETURN EnumReturn.Variant.Original; +END; + +-- A bare entity name is still an entity. +CREATE OR MODIFY MICROFLOW EnumReturn.SUB_PickCustomer () +RETURNS EnumReturn.Customer AS $Customer +BEGIN + RETURN empty; +END; diff --git a/mdl/executor/cmd_microflows_build.go b/mdl/executor/cmd_microflows_build.go index dd1d3e9bca..7f86748f57 100644 --- a/mdl/executor/cmd_microflows_build.go +++ b/mdl/executor/cmd_microflows_build.go @@ -326,12 +326,12 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b if entityID == "" { // Bare qualified name in microflow context is treated as TypeEntity by the // visitor, but it may actually be an enumeration. Try enum lookup before failing. - if found := findEnumeration(ctx, p.Type.EntityRef.Module, p.Type.EntityRef.Name); found != nil { - s.Parameters[i].Type = ast.DataType{Kind: ast.TypeEnumeration, EnumRef: p.Type.EntityRef} + if retyped, ok := bareEnumerationType(ctx, p.Type); ok { + s.Parameters[i].Type = retyped p = s.Parameters[i] } else { return nil, mdlerrors.NewNotFoundMsg("entity", p.Type.EntityRef.Module+"."+p.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) + fmt.Sprintf("entity or enumeration '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) } } } @@ -362,8 +362,13 @@ func buildMicroflowFromStmt(ctx *ExecContext, s *ast.CreateMicroflowStmt, opts b if s.ReturnType.Type.EntityRef != nil && !isBuiltinModuleEntity(s.ReturnType.Type.EntityRef.Module) { entityID := entityResolver(*s.ReturnType.Type.EntityRef) if entityID == "" { - return nil, mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + // A bare qualified name may be an enumeration, as for parameters above. + if retyped, ok := bareEnumerationType(ctx, s.ReturnType.Type); ok && s.ReturnType.Type.Kind == ast.TypeEntity { + s.ReturnType.Type = retyped + } else { + return nil, mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, + fmt.Sprintf("entity or enumeration '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + } } } // Validate enumeration references for return type @@ -625,12 +630,12 @@ func buildNanoflowFromStmt(ctx *ExecContext, s *ast.CreateNanoflowStmt, opts bui if entityID == "" { // Bare qualified name in microflow context is treated as TypeEntity by the // visitor, but it may actually be an enumeration. Try enum lookup before failing. - if found := findEnumeration(ctx, p.Type.EntityRef.Module, p.Type.EntityRef.Name); found != nil { - s.Parameters[i].Type = ast.DataType{Kind: ast.TypeEnumeration, EnumRef: p.Type.EntityRef} + if retyped, ok := bareEnumerationType(ctx, p.Type); ok { + s.Parameters[i].Type = retyped p = s.Parameters[i] } else { return nil, mdlerrors.NewNotFoundMsg("entity", p.Type.EntityRef.Module+"."+p.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) + fmt.Sprintf("entity or enumeration '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) } } } @@ -657,8 +662,13 @@ func buildNanoflowFromStmt(ctx *ExecContext, s *ast.CreateNanoflowStmt, opts bui if s.ReturnType.Type.EntityRef != nil && !isBuiltinModuleEntity(s.ReturnType.Type.EntityRef.Module) { entityID := entityResolver(*s.ReturnType.Type.EntityRef) if entityID == "" { - return nil, mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + // A bare qualified name may be an enumeration, as for parameters above. + if retyped, ok := bareEnumerationType(ctx, s.ReturnType.Type); ok && s.ReturnType.Type.Kind == ast.TypeEntity { + s.ReturnType.Type = retyped + } else { + return nil, mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, + fmt.Sprintf("entity or enumeration '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + } } } if s.ReturnType.Type.Kind == ast.TypeEnumeration && s.ReturnType.Type.EnumRef != nil { diff --git a/mdl/executor/cmd_microflows_helpers.go b/mdl/executor/cmd_microflows_helpers.go index d3caaefe9e..655015c9e6 100644 --- a/mdl/executor/cmd_microflows_helpers.go +++ b/mdl/executor/cmd_microflows_helpers.go @@ -70,6 +70,22 @@ func convertASTToMicroflowDataType(dt ast.DataType, entityResolver func(ast.Qual } } +// bareEnumerationType resolves the entity/enumeration ambiguity of a declared +// type. The parser cannot tell an entity from an enumeration for a bare +// `Module.Name` and always records an entity reference, so a caller that has +// found no entity of that name asks here whether an enumeration has it. It +// returns dt retyped as that enumeration, or false when none does. +// +// Parameters and return types of microflows, nanoflows and rules all resolve +// through this, so the three kinds accept the same spellings; `check` makes the +// same decision for a project that is not yet written in declaredTypeResolver. +func bareEnumerationType(ctx *ExecContext, dt ast.DataType) (ast.DataType, bool) { + if dt.EntityRef == nil || findEnumeration(ctx, dt.EntityRef.Module, dt.EntityRef.Name) == nil { + return dt, false + } + return ast.DataType{Kind: ast.TypeEnumeration, EnumRef: dt.EntityRef}, true +} + // The Mendix expression renderer lives in mdl/mendixexpr, where the visitor // can reach it too (fmt --upgrade, ako/mxcli#804). These names are the // executor's handles on it. diff --git a/mdl/executor/cmd_rules_create.go b/mdl/executor/cmd_rules_create.go index 1e0f254f45..d7af12b895 100644 --- a/mdl/executor/cmd_rules_create.go +++ b/mdl/executor/cmd_rules_create.go @@ -168,12 +168,12 @@ func execCreateRule(ctx *ExecContext, s *ast.CreateRuleStmt) error { if entityID == "" { // Bare qualified name in microflow context is treated as TypeEntity by the // visitor, but it may actually be an enumeration. Try enum lookup before failing. - if found := findEnumeration(ctx, p.Type.EntityRef.Module, p.Type.EntityRef.Name); found != nil { - s.Parameters[i].Type = ast.DataType{Kind: ast.TypeEnumeration, EnumRef: p.Type.EntityRef} + if retyped, ok := bareEnumerationType(ctx, p.Type); ok { + s.Parameters[i].Type = retyped p = s.Parameters[i] } else { return mdlerrors.NewNotFoundMsg("entity", p.Type.EntityRef.Module+"."+p.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) + fmt.Sprintf("entity or enumeration '%s.%s' not found for parameter '%s'", p.Type.EntityRef.Module, p.Type.EntityRef.Name, p.Name)) } } } @@ -200,8 +200,13 @@ func execCreateRule(ctx *ExecContext, s *ast.CreateRuleStmt) error { if s.ReturnType.Type.EntityRef != nil && !isBuiltinModuleEntity(s.ReturnType.Type.EntityRef.Module) { entityID := entityResolver(*s.ReturnType.Type.EntityRef) if entityID == "" { - return mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, - fmt.Sprintf("entity '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + // A bare qualified name may be an enumeration, as for parameters above. + if retyped, ok := bareEnumerationType(ctx, s.ReturnType.Type); ok && s.ReturnType.Type.Kind == ast.TypeEntity { + s.ReturnType.Type = retyped + } else { + return mdlerrors.NewNotFoundMsg("entity", s.ReturnType.Type.EntityRef.Module+"."+s.ReturnType.Type.EntityRef.Name, + fmt.Sprintf("entity or enumeration '%s.%s' not found for return type", s.ReturnType.Type.EntityRef.Module, s.ReturnType.Type.EntityRef.Name)) + } } } if s.ReturnType.Type.Kind == ast.TypeEnumeration && s.ReturnType.Type.EnumRef != nil { diff --git a/mdl/executor/enum_return_type_pedapp_test.go b/mdl/executor/enum_return_type_pedapp_test.go new file mode 100644 index 0000000000..7ffcc50f1c --- /dev/null +++ b/mdl/executor/enum_return_type_pedapp_test.go @@ -0,0 +1,189 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "context" + "fmt" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/bson/primitive" + + "github.com/mendixlabs/mxcli/model" +) + +// check -p and exec must agree on a bare `Module.Name` return type that names +// an enumeration: check resolves the name against entities AND enumerations, +// so exec has to as well. Run on the Studio Pro-authored PedApp fixture, with a +// control that stores the explicit `enum Module.Name` spelling. + +const enumReturnSetup = "mdl 1;\ncreate enumeration MyFirstModule.Variant ( Original 'Original', Released 'Released', None 'None' );" + +// enumReturnDecl is one flow declaration returning `spelling`, named name. +func enumReturnDecl(kind, name, spelling string) string { + switch kind { + case "microflow": + return fmt.Sprintf("create or modify microflow MyFirstModule.%s () returns %s as $Variant begin return MyFirstModule.Variant.Original; end;", name, spelling) + case "nanoflow": + return fmt.Sprintf("create or modify nanoflow MyFirstModule.%s () returns %s as $Variant begin return MyFirstModule.Variant.Original; end;", name, spelling) + default: + return fmt.Sprintf("create or modify rule MyFirstModule.%s ($c: MyFirstModule.Variant) returns %s as $Variant begin return MyFirstModule.Variant.Original; end;", name, spelling) + } +} + +func TestCheckExecAgree_BareEnumerationReturnType(t *testing.T) { + for _, kind := range []string{"microflow", "nanoflow", "rule"} { + t.Run(kind, func(t *testing.T) { + exec, out, dir := openPedAppCopy(t) + if err := agreeExec(t, exec, enumReturnSetup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + // Control: the explicit spellings both accept. + assertAgree(t, exec, dir, "mdl 1;\n"+enumReturnDecl(kind, "Ctl", "enum MyFirstModule.Variant"), "") + assertAgree(t, exec, dir, "mdl 1;\n"+enumReturnDecl(kind, "Ctl2", "Enumeration(MyFirstModule.Variant)"), "") + // The bare name: check and exec both accept. + assertAgree(t, exec, dir, "mdl 1;\n"+enumReturnDecl(kind, "Bare", "MyFirstModule.Variant"), "") + // A name that is neither an entity nor an enumeration: both refuse. + nope := "mdl 1;\n" + enumReturnDecl(kind, "NopeFlow", "MyFirstModule.Nope") + assertAgree(t, exec, dir, nope, "MyFirstModule.Nope") + // Both say the name may have meant either kind. + if got := strings.Join(agreeCheck(t, exec, dir, nope), "\n"); !strings.Contains(got, "return type of "+kind+" MyFirstModule.NopeFlow: entity or enumeration MyFirstModule.Nope does not exist") { + t.Errorf("check must name both kinds, reported:\n%s", got) + } + if err := agreeExec(t, exec, nope); err == nil || !strings.Contains(err.Error(), "entity or enumeration 'MyFirstModule.Nope' not found for return type") { + t.Errorf("exec must name both kinds, got: %v", err) + } + }) + } +} + +// A rule that returns an entity is still refused by both: the fallback widens +// what a bare name can mean, not what a rule may return. +func TestCheckExecAgree_RuleReturningEntity(t *testing.T) { + exec, out, dir := openPedAppCopy(t) + setup := enumReturnSetup + "\ncreate persistent entity MyFirstModule.Thing (Name: String(10));" + if err := agreeExec(t, exec, setup); err != nil { + t.Fatalf("setup: %v\n%s", err, out.String()) + } + assertAgree(t, exec, dir, "mdl 1;\n"+enumReturnDecl("rule", "Bad", "MyFirstModule.Thing"), + "a rule must return a Boolean or an enumeration, not Entity") +} + +// The same script in one run: the enumeration is created by the script itself. +func TestCheckExecAgree_BareEnumerationReturnType_SameScript(t *testing.T) { + for _, kind := range []string{"microflow", "nanoflow", "rule"} { + t.Run(kind, func(t *testing.T) { + exec, _, dir := openPedAppCopy(t) + assertAgree(t, exec, dir, enumReturnSetup+"\n"+enumReturnDecl(kind, "Bare", "MyFirstModule.Variant"), "") + }) + } +} + +// What the bare spelling stores equals what `enum X` stores, byte for byte +// apart from the document's own id and name. +func TestBareEnumerationReturnType_StoredBytesEqualExplicit(t *testing.T) { + for _, kind := range []string{"microflow", "nanoflow", "rule"} { + t.Run(kind, func(t *testing.T) { + exec, out, _ := openPedAppCopy(t) + script := enumReturnSetup + "\n" + + enumReturnDecl(kind, "Explicit", "enum MyFirstModule.Variant") + "\n" + + enumReturnDecl(kind, "Function", "Enumeration(MyFirstModule.Variant)") + "\n" + + enumReturnDecl(kind, "Bare", "MyFirstModule.Variant") + if err := agreeExec(t, exec, script); err != nil { + t.Fatalf("exec: %v\n%s", err, out.String()) + } + explicit := normalisedFlowBytes(t, exec, kind, "Explicit") + for _, name := range []string{"Function", "Bare"} { + if got := normalisedFlowBytes(t, exec, kind, name); got != explicit { + t.Errorf("%s stores something other than the explicit `enum` spelling:\n got: %s\nwant: %s", name, got, explicit) + } + } + }) + } +} + +// normalisedFlowBytes returns the stored unit of MyFirstModule. as +// extended JSON with the identities removed: every binary value (the $ID, the +// flow pointers, the StableId are all GUIDs) and the document's own name. +func normalisedFlowBytes(t *testing.T, exec *Executor, kind, name string) string { + t.Helper() + ctx := exec.newExecContext(context.Background()) + var id model.ID + switch kind { + case "microflow": + all, err := ctx.Backend.ListMicroflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name { + id = m.ID + } + } + case "nanoflow": + all, err := ctx.Backend.ListNanoflows() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name { + id = m.ID + } + } + default: + all, err := ctx.Backend.ListRules() + if err != nil { + t.Fatal(err) + } + for _, m := range all { + if m.Name == name { + id = m.ID + } + } + } + if id == "" { + t.Fatalf("%s MyFirstModule.%s not found", kind, name) + } + raw, err := ctx.Backend.GetRawUnitBytes(id) + if err != nil { + t.Fatal(err) + } + var d bson.D + if err := bson.Unmarshal(raw, &d); err != nil { + t.Fatal(err) + } + out, err := bson.MarshalExtJSON(stripFlowIdentity(d), false, false) + if err != nil { + t.Fatal(err) + } + return string(out) +} + +// stripFlowIdentity drops every binary value and the top-level Name, recursively. +func stripFlowIdentity(v bson.D) bson.D { + var walk func(x any, top bool) any + walk = func(x any, top bool) any { + switch t := x.(type) { + case bson.D: + var out bson.D + for _, e := range t { + if _, isID := e.Value.(primitive.Binary); isID || (top && e.Key == "Name") { + continue + } + out = append(out, bson.E{Key: e.Key, Value: walk(e.Value, false)}) + } + return out + case bson.A: + out := make(bson.A, len(t)) + for i, e := range t { + out[i] = walk(e, false) + } + return out + default: + return x + } + } + return walk(v, true).(bson.D) +} diff --git a/mdl/executor/enum_return_type_test.go b/mdl/executor/enum_return_type_test.go new file mode 100644 index 0000000000..4de54fc992 --- /dev/null +++ b/mdl/executor/enum_return_type_test.go @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "fmt" + "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/domainmodel" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// A bare `Module.Name` return type is ambiguous: the parser cannot tell an +// entity from an enumeration and records an entity reference. exec already +// resolved that for PARAMETERS (entity first, then enumeration); the return +// types of a microflow, a nanoflow and a rule did not, so a script that +// `check` accepted failed at exec with "entity 'Shop.Variant' not found for +// return type". + +// enumReturnCapture records what the create handlers hand to the backend. +type enumReturnCapture struct { + microflow *microflows.Microflow + nanoflow *microflows.Nanoflow + rule *microflows.Rule +} + +// enumReturnCtx builds a mock project with module Shop, an enumeration +// Shop.Variant, an entity Shop.Customer and, when twin is set, an entity that +// shares the enumeration's name. +func enumReturnCtx(t *testing.T, twin bool) (*ExecContext, *enumReturnCapture) { + t.Helper() + mod := mkModule("Shop") + enum := mkEnumeration(mod.ID, "Variant", "Original", "Released") + ents := []*domainmodel.Entity{mkEntity(mod.ID, "Customer")} + if twin { + ents = append(ents, mkEntity(mod.ID, "Variant")) + } + dm := mkDomainModel(mod.ID, ents...) + h := mkHierarchy(mod) + withContainer(h, enum.ContainerID, mod.ID) + + got := &enumReturnCapture{} + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil }, + ListEnumerationsFunc: func() ([]*model.Enumeration, error) { return []*model.Enumeration{enum}, nil }, + ListDomainModelsFunc: func() ([]*domainmodel.DomainModel, error) { return []*domainmodel.DomainModel{dm}, nil }, + ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return nil, nil }, + ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { return nil, nil }, + ListRulesFunc: func() ([]*microflows.Rule, error) { return nil, nil }, + CreateMicroflowFunc: func(mf *microflows.Microflow) error { got.microflow = mf; return nil }, + CreateNanoflowFunc: func(nf *microflows.Nanoflow) error { got.nanoflow = nf; return nil }, + CreateRuleFunc: func(r *microflows.Rule) error { got.rule = r; return nil }, + } + ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h)) + return ctx, got +} + +// enumReturnFlows are the three declaration sites, each with the return type +// spelled as %s. +var enumReturnFlows = map[string]string{ + "microflow": "create or modify microflow Shop.SUB_Pick () returns %s as $Variant begin return Shop.Variant.Original; end;", + "nanoflow": "create or modify nanoflow Shop.NF_Pick () returns %s as $Variant begin return Shop.Variant.Original; end;", + "rule": "create or modify rule Shop.RL_Pick ($c: Shop.Customer) returns %s as $Variant begin return Shop.Variant.Original; end;", +} + +// execEnumReturn parses and executes one flow declaration. +func execEnumReturn(t *testing.T, ctx *ExecContext, kind, returnSpelling string) error { + t.Helper() + src := "mdl 1;\n" + fmt.Sprintf(enumReturnFlows[kind], returnSpelling) + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatalf("parse: %v\n%s", errs[0], src) + } + switch s := prog.Statements[0].(type) { + case *ast.CreateMicroflowStmt: + return execCreateMicroflow(ctx, s) + case *ast.CreateNanoflowStmt: + return execCreateNanoflow(ctx, s) + case *ast.CreateRuleStmt: + return execCreateRule(ctx, s) + default: + t.Fatalf("unexpected statement %T", s) + return nil + } +} + +func (c *enumReturnCapture) returnType(kind string) microflows.DataType { + switch kind { + case "microflow": + return c.microflow.ReturnType + case "nanoflow": + return c.nanoflow.ReturnType + default: + return c.rule.ReturnType + } +} + +func TestBareEnumReturnType_StoredAsEnumeration(t *testing.T) { + for kind := range enumReturnFlows { + t.Run(kind, func(t *testing.T) { + ctx, got := enumReturnCtx(t, false) + if err := execEnumReturn(t, ctx, kind, "Shop.Variant"); err != nil { + t.Fatalf("bare enumeration return type must be accepted: %v", err) + } + et, ok := got.returnType(kind).(*microflows.EnumerationType) + if !ok { + t.Fatalf("return type = %T, want *EnumerationType", got.returnType(kind)) + } + if et.EnumerationQualifiedName != "Shop.Variant" { + t.Errorf("enumeration = %q, want Shop.Variant", et.EnumerationQualifiedName) + } + }) + } +} + +// The bare spelling must store exactly what the explicit spellings store. +func TestBareEnumReturnType_EqualsExplicitSpellings(t *testing.T) { + for kind := range enumReturnFlows { + t.Run(kind, func(t *testing.T) { + var stored []*microflows.EnumerationType + for _, spelling := range []string{"enum Shop.Variant", "Enumeration(Shop.Variant)", "Shop.Variant"} { + ctx, got := enumReturnCtx(t, false) + if err := execEnumReturn(t, ctx, kind, spelling); err != nil { + t.Fatalf("%q: %v", spelling, err) + } + et, ok := got.returnType(kind).(*microflows.EnumerationType) + if !ok { + t.Fatalf("%q: return type = %T, want *EnumerationType", spelling, got.returnType(kind)) + } + stored = append(stored, et) + } + for i := 1; i < len(stored); i++ { + if stored[i].EnumerationQualifiedName != stored[0].EnumerationQualifiedName { + t.Errorf("spelling %d stored %+v, explicit spelling stored %+v", i, stored[i], stored[0]) + } + } + }) + } +} + +// An unknown name names both candidates, so the user is not told to look for an +// entity when they may have meant an enumeration. +func TestBareEnumReturnType_UnknownName(t *testing.T) { + for kind := range enumReturnFlows { + t.Run(kind, func(t *testing.T) { + ctx, _ := enumReturnCtx(t, false) + err := execEnumReturn(t, ctx, kind, "Shop.Nope") + assertError(t, err) + assertContainsStr(t, err.Error(), "entity or enumeration 'Shop.Nope' not found for return type") + }) + } +} + +// A bare name that is an entity stays an entity, and wins when an entity and an +// enumeration share the name — the order the parameter path already has. +func TestBareEnumReturnType_EntityStaysEntity(t *testing.T) { + for _, kind := range []string{"microflow", "nanoflow"} { + t.Run(kind, func(t *testing.T) { + // Shop.Customer is an entity only. + ctx, got := enumReturnCtx(t, false) + if err := execEnumReturn(t, ctx, kind, "Shop.Customer"); err != nil { + t.Fatalf("entity return type: %v", err) + } + ot, ok := got.returnType(kind).(*microflows.ObjectType) + if !ok || ot.EntityQualifiedName != "Shop.Customer" { + t.Fatalf("return type = %#v, want ObjectType Shop.Customer", got.returnType(kind)) + } + + // Shop.Variant is both an entity and an enumeration: the entity wins. + ctx, got = enumReturnCtx(t, true) + if err := execEnumReturn(t, ctx, kind, "Shop.Variant"); err != nil { + t.Fatalf("twin return type: %v", err) + } + if ot, ok := got.returnType(kind).(*microflows.ObjectType); !ok || ot.EntityQualifiedName != "Shop.Variant" { + t.Errorf("return type = %#v, want ObjectType Shop.Variant (entity wins)", got.returnType(kind)) + } + }) + } +} + +// The parameter path is the model for the return path: the same bare name must +// resolve the same way in either position. +func TestBareEnumParameter_StillResolves(t *testing.T) { + ctx, got := enumReturnCtx(t, false) + src := "mdl 1;\ncreate or modify microflow Shop.SUB_Take ($v: Shop.Variant) begin return; end;" + prog, errs := visitor.Build(src) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + if err := execCreateMicroflow(ctx, prog.Statements[0].(*ast.CreateMicroflowStmt)); err != nil { + t.Fatal(err) + } + if _, ok := got.microflow.Parameters[0].Type.(*microflows.EnumerationType); !ok { + t.Fatalf("parameter type = %T, want *EnumerationType", got.microflow.Parameters[0].Type) + } +} diff --git a/mdl/executor/validate.go b/mdl/executor/validate.go index 5f98e2162b..319f86356f 100644 --- a/mdl/executor/validate.go +++ b/mdl/executor/validate.go @@ -773,9 +773,21 @@ func validateWithContext(ctx *ExecContext, stmt ast.Statement, sc *scriptContext return mdlerrors.NewNotFound("module", s.Name.Module) } } + // A bare `returns Module.Name` that names an enumeration is one: exec + // retypes it the same way before it validates the rule. One that names + // nothing is reported as that, not as "not a Boolean or an enumeration". + ruleReturn := s.ReturnType + if ruleReturn != nil && ctx.Connected() { + r := newDeclaredTypeResolver(ctx, sc) + if retyped, ok := r.bareEnumeration(ruleReturn.Type); ok { + ruleReturn = &ast.MicroflowReturnType{Type: retyped, Variable: ruleReturn.Variable} + } else if msg := r.check(ruleReturn.Type, "return type of rule "+s.Name.String()); msg != "" { + return mdlerrors.NewValidationf("rule '%s' has reference errors:\n - %s", s.Name.String(), msg) + } + } // The same validateRule the executor calls, so `check` and `exec` cannot // disagree about what a rule may contain. - if errMsg := validateRule(s.Name.String(), s.Body, s.ReturnType); errMsg != "" { + if errMsg := validateRule(s.Name.String(), s.Body, ruleReturn); errMsg != "" { return mdlerrors.NewValidationf("%s", strings.TrimRight(errMsg, "\n")) } if validationErrors := ValidateRuleBody(s); len(validationErrors) > 0 { diff --git a/mdl/executor/validate_declared_types.go b/mdl/executor/validate_declared_types.go index 9640e0153e..01f48d1301 100644 --- a/mdl/executor/validate_declared_types.go +++ b/mdl/executor/validate_declared_types.go @@ -91,13 +91,33 @@ func (r *declaredTypeResolver) check(dt ast.DataType, where string) string { if r.entityExists(*qn) || r.enumExists(*qn) { return "" } - msg := fmt.Sprintf("%s: entity %s does not exist", where, qn.String()) + // A bare scalar name may have meant an enumeration, so the message names both. + kind := "entity" + if dt.Kind == ast.TypeEntity { + kind = "entity or enumeration" + } + msg := fmt.Sprintf("%s: %s %s does not exist", where, kind, qn.String()) if hint := entityNameHint(*qn); hint != "" { msg += ". " + hint } return msg } +// bareEnumeration is the check-side twin of bareEnumerationType: a bare +// `Module.Name` that names no entity but names an enumeration, as exec's +// create handlers read it. It returns dt retyped as that enumeration, or false +// when dt is anything else — including a name both an entity and an +// enumeration have, which exec keeps as the entity. +func (r *declaredTypeResolver) bareEnumeration(dt ast.DataType) (ast.DataType, bool) { + if dt.Kind != ast.TypeEntity || dt.EntityRef == nil || isBuiltinModuleEntity(dt.EntityRef.Module) { + return dt, false + } + if r.entityExists(*dt.EntityRef) || !r.enumExists(*dt.EntityRef) { + return dt, false + } + return ast.DataType{Kind: ast.TypeEnumeration, EnumRef: dt.EntityRef}, true +} + // flowSignatureErrors resolves a microflow's, nanoflow's or rule's parameter // and return types. func flowSignatureErrors(ctx *ExecContext, sc *scriptContext, kind string, name ast.QualifiedName,