diff --git a/CHANGELOG.md b/CHANGELOG.md index a11007213..d15b9f275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,49 @@ here. The format follows [Keep a Changelog](https://keepachangelog.com/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) (pre-1.0; MINOR bumps may introduce breaking changes with notice). +## [Unreleased] + +Shared-enum cross-package hardening (**#246** + its sibling **#259**). When cut this releases +as a coordinated PATCH — the loader change (#246) lands in all five ports, the Kotlin codegen +changes (#246 Bug 1, #259) land on Maven Central; no metadata vocabulary changes, byte-identical +output for any model that doesn't hit the specific cross-package/two-hop enum shapes below. + +- **#246 — a `field.enum` may now be shared across packages, and a conflicting redeclaration is + rejected instead of silently dropped.** Two independent fixes: + - **Kotlin codegen (Bug 1).** The Exposed table generator dropped the cross-package import for a + shared `field.enum`: when two entities in different packages `extends` one abstract enum + declared in a common package, the generated `Table` referenced the shared enum by its + simple name but never imported it (`Unresolved reference`). It now emits the cross-package + import for enum columns on both the vanilla and TPH-fold paths, mirroring the existing + FK-import machinery. Same-package models are byte-identical (an enum in the table's own + package adds no import). Gated by a new `enum-xpkg` fixture + a `KotlinCompilation` compile-gate. + - **Cross-port loader error `ERR_ENUM_EXTENDS_VALUES_CONFLICT`.** A `field.enum` that both + `extends` a shared package-level abstract enum **and** declares its own `@values` now fails to + load (was silently dropped — one shared enum type has one member set, so the own `@values` + would be discarded by the shared-enum codegen collapse). Enforced identically in all four + loaders (TypeScript, Python, Java, C#; Kotlin inherits the JVM loader), gated by a shared + conformance fixture with an exact cross-port error `jsonPath`. Extending a **concrete** + (non-shared) enum with your own `@values` is still legal. +- **#259 — a `field.enum` inheriting `@values` through TWO `extends` hops now generates correctly + (Kotlin codegen; sibling of #246).** A projection field extending an entity field, where that + entity field itself extends a shared abstract enum, generated **no** per-projection enum at all + — its type collapsed onto the shared enum because the collapse decision inspected the top-most + super rather than the immediate one, so every consumer of that column failed to resolve the + absent per-projection type. The collapse now keys on the **immediate** super: a field whose + direct `extends` target is a package-level abstract enum collapses onto the shared type; a field + whose direct super is a concrete entity/projection field gets its own `` enum, + populated with the values it inherits (resolved across any number of hops). Byte-identical for + one-hop projections, shared-enum (FR-019) collapse, and entity-extends-shared. Also hardens the + enum emitter: a `field.enum` that resolves to no `@values` now fails loudly at generate-time + instead of silently emitting nothing (the exact silent no-emit #259 reported). Gated by a + two-hop `KotlinCompilation` compile-gate + a depth-2 cross-port conformance fixture. + +The C# materialized-enum cross-namespace sibling and the enum-primary-key-as-`String` issue are +documented as out-of-scope in the design spec +(`docs/superpowers/specs/2026-07-31-shared-enum-cross-package-design.md`); a follow-up is the +Kotlin `enumTypeName` collapse gaining the `isAbstract` leg the other ports already carry (so a +root-level *concrete* enum extended with own `@values` gets a per-field enum on every port). + ## [0.20.10] — 2026-08-02 **Coordinated PATCH** — npm `0.20.10` · PyPI `0.19.9` · NuGet `0.19.7` · Maven Central `7.11.7`. diff --git a/CLAUDE.md b/CLAUDE.md index 2f7de3d8f..eff732aa9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,7 +30,7 @@ _Last refreshed 2026-08-02._ - **Kotlin** — `codegen-kotlin` (KotlinPoet on JVM): entity + Exposed table + Spring controller + payload + relations + filter allowlist + validator + stored-proc + output-parser generators. `integration-tests-kotlin` runs the persistence-conformance corpus through Exposed against Testcontainers Postgres. **Cross-port conformance corpora** (every port runs the shared corpus): -- Metamodel: `fixtures/conformance/` (255 fixtures; 19 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. +- Metamodel: `fixtures/conformance/` (258 fixtures; 19 shared corpora in total — per-corpus counts + the corpus x port matrix live in `docs/CONFORMANCE.md`). TS / C# / Java / Python all green. - Render: `fixtures/render-conformance/`. TS / C# / Java / Kotlin / Python byte-identical. - Persistence: `fixtures/persistence-conformance/`. **Query** scenarios run on every port (TS / C# / Java / Kotlin / Python), each provisioning its test DB by executing the committed, TS-produced `canonical/schema.postgres.sql` (Postgres only — Derby dropped for the cross-port query corpus, ADR-0015). The **migration** scenarios are exercised by **TS only** (TS owns schema migrations). **The corpus now gates WRITES, not just reads (SP-H):** an `op: roundtrip` scenario type INSERTs through each port's runtime/ORM write codec (NOT raw SQL), reads the row back, and asserts the wire-normalized value. The `AllTypes` entity (`roundtrip-all-types.yaml`) carries one field of **every** persistable `field.*` subtype — string/int/long/double/float/decimal/boolean/date/time/timestamp(+tz)/currency/enum/uuid/object — plus an **array-of-VO** `field.object @isArray @storage:jsonb` column (`labels`, written as 2-element / empty-`[]` / single-element arrays across the three rows) — so every subtype write+read (incl. the array-of-value-object jsonb codec) round-trips through every port against Testcontainers PG. (`field.byte`/`field.short`/`field.class` were cut as non-functional registration-only stubs — the matrix tracks only genuinely-supported subtypes; see `fixtures/registry-conformance/README.md` → "Per-subtype write-round-trip matrix".) - API-contract: `fixtures/api-contract-conformance/`. TS / C# / Java / Kotlin / Python all green — each port runs **two lanes**: a hand-rolled reference server AND its **generated** API artifact booted over HTTP (the deployed controller/routes; TS+C# full-stack vs Testcontainers PG, Java/Kotlin/Python generated controller + in-memory repo behind the consumer seam). The generated fan-out found 10 real deployment bugs golden snapshots missed. diff --git a/README.md b/README.md index a6eed892a..e7284f2cd 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,7 @@ metaobjects/ ├── CLAUDE.md # project instructions for Claude ├── spec/ # canonical metamodel docs, ADRs, roadmap ├── fixtures/ # 19 cross-language conformance corpora — the oracle -│ ├── conformance/ # metamodel (loader + serializer + navigation), 249 fixtures +│ ├── conformance/ # metamodel (loader + serializer + navigation), 258 fixtures │ ├── yaml-conformance/ # YAML authoring desugar │ ├── render-conformance/ # FR-004 byte-identical render oracle │ ├── verify-conformance/ # FR-004 template-drift gate diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 5f076a32f..7248d70ab 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -25,7 +25,7 @@ regenerate with `ls -d fixtures//*/ | wc -l`. | Corpus | Fixtures | TS | Java | Kotlin | C# | Python | |---|---|---|---|---|---|---| -| [`fixtures/conformance/`](../fixtures/conformance/) (metamodel) | 255 | ✓ | ✓ | inherits via `metadata-ktx` | ✓ | ✓ | +| [`fixtures/conformance/`](../fixtures/conformance/) (metamodel) | 258 | ✓ | ✓ | inherits via `metadata-ktx` | ✓ | ✓ | | [`fixtures/yaml-conformance/`](../fixtures/yaml-conformance/) | 15 | 15 / 15 | 14 / 15 (1 ledgered: `yaml-quoted-leading-zero` — Java pipeline strips quotes off `"007"`) | inherits via Java | 14 / 15 (1 ledgered: `error-yaml-coerced-hex-in-string` — YamlDotNet doesn't coerce `0xFF`) | 15 / 15 | | [`fixtures/verify-conformance/`](../fixtures/verify-conformance/) | 31 | ✓ | ✓ | inherits via Java | ✓ | ✓ | | [`fixtures/verify-strict-conformance/`](../fixtures/verify-strict-conformance/) | 1 | ✓ | — | — | — | ✓ | @@ -69,7 +69,7 @@ unit-test runners (`bun test`, `dotnet test`, `pytest`, `mvn test`) pull Docker. ## Fixture-to-doc mapping -### `fixtures/conformance/` — metamodel loader + canonical serializer (255) +### `fixtures/conformance/` — metamodel loader + canonical serializer (258) | Fixture prefix | Feature doc | |---|---| diff --git a/docs/features/abstracts-and-inheritance.md b/docs/features/abstracts-and-inheritance.md index 71b74cffb..395961335 100644 --- a/docs/features/abstracts-and-inheritance.md +++ b/docs/features/abstracts-and-inheritance.md @@ -390,6 +390,9 @@ inheritance chain. — `ERR_UNRESOLVED_SUPER` when the reference doesn't resolve - [`enum-abstract-extends`](../../fixtures/conformance/enum-abstract-extends/) — abstracts on enum fields specifically +- [`enum-extends-two-hop-projection`](../../fixtures/conformance/enum-extends-two-hop-projection/) + — inherited enum members resolve through two `extends` hops (projection → entity + → shared abstract enum) ## See also diff --git a/docs/features/field-types.md b/docs/features/field-types.md index bf4c6790f..b331d186c 100644 --- a/docs/features/field-types.md +++ b/docs/features/field-types.md @@ -66,11 +66,42 @@ must be a non-empty set of unique members matching `^[A-Za-z_][A-Za-z0-9_]*$`. ``` The loader enforces members own-only and emits `ERR_BAD_ATTR_VALUE` on a bad -member or `ERR_MISSING_REQUIRED_ATTR` on missing `@values`. Reuse via an abstract -`field.enum` + `extends`. Int-backed enums, display labels, and native Postgres -`ENUM` types are deferred (see +member or `ERR_MISSING_REQUIRED_ATTR` on missing `@values`. Int-backed enums, +display labels, and native Postgres `ENUM` types are deferred (see [enum-datatype-design.md](../superpowers/specs/2026-05-23-enum-datatype-design.md)). +### Sharing one enum — abstract `field.enum` + `extends` + +Reuse a constraint set across entities by declaring one **abstract** `field.enum` +(with the `@values`) and having each concrete field `extends` it — including +**across packages** (declare the shared enum in a common package and reference it +by FQN). Every field that extends the same abstract enum collapses onto one +generated enum type, so the members stay in exactly one place: + +```jsonc +// meta.common.json — package acme::common +{ "field.enum": { "name": "RecordStatus", "abstract": true, + "@values": ["DRAFT", "ACTIVE", "CLOSED"] } } + +// meta.orders.json — package acme::orders +{ "field.enum": { "name": "status", "extends": "acme::common::RecordStatus" } } +``` + +Two rules the loader/codegen enforce: + +- **A field extending a shared abstract enum must NOT declare its own `@values`.** + A shared enum is one type with one member set, so an own `@values` on the + extending field would be silently dropped by the shared-enum collapse — it is a + load error, `ERR_ENUM_EXTENDS_VALUES_CONFLICT`. Remove the own `@values` to + inherit the shared set. If you need a **different** member set, declare a + separate `field.enum` with its own `@values` rather than extending the shared + one — extending is for *reuse* of one member set, not for overriding it. +- **Inherited members resolve through any number of `extends` hops.** A projection + field that extends an entity field which itself extends a shared abstract enum + gets the shared members transitively — the projection still materializes its own + per-projection enum type, populated from the inherited set (a projection is + self-contained: its enum lives in the projection's own package). + ## Embedded value objects — `field.object` + `@storage` `field.object` declares an embedded structured value backed by another `object` @@ -236,6 +267,8 @@ The following conformance fixtures gate this feature's behavior across ports: - [`fixtures/conformance/enum-inline/`](../../fixtures/conformance/enum-inline/) — inline `field.enum @values` - [`fixtures/conformance/enum-array/`](../../fixtures/conformance/enum-array/) — array-of-enum field - [`fixtures/conformance/enum-abstract-extends/`](../../fixtures/conformance/enum-abstract-extends/) — reuse via abstract `field.enum` + `extends` +- [`fixtures/conformance/enum-extends-two-hop-projection/`](../../fixtures/conformance/enum-extends-two-hop-projection/) — inherited members resolve through two `extends` hops (projection → entity → shared) +- [`fixtures/conformance/error-enum-extends-values-conflict/`](../../fixtures/conformance/error-enum-extends-values-conflict/) — `ERR_ENUM_EXTENDS_VALUES_CONFLICT` when extending a shared abstract enum and also declaring own `@values` - [`fixtures/conformance/error-enum-missing-values/`](../../fixtures/conformance/error-enum-missing-values/) — `ERR_MISSING_REQUIRED_ATTR` when `@values` absent - [`fixtures/conformance/error-enum-empty-values/`](../../fixtures/conformance/error-enum-empty-values/) — empty `@values` rejected - [`fixtures/conformance/error-enum-duplicate-member/`](../../fixtures/conformance/error-enum-duplicate-member/) — duplicate member symbols rejected diff --git a/docs/superpowers/plans/2026-07-31-shared-enum-cross-package.md b/docs/superpowers/plans/2026-07-31-shared-enum-cross-package.md new file mode 100644 index 000000000..5c069e35c --- /dev/null +++ b/docs/superpowers/plans/2026-07-31-shared-enum-cross-package.md @@ -0,0 +1,347 @@ +# Shared enum across packages (#246) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax. + +**Goal:** Fix the Kotlin table-generator dropping a cross-package shared-`field.enum` import (#246 Bug 1), and add a cross-port loader error when a `field.enum` both `extends` a shared package-level abstract enum and declares its own `values` (#246 Bug 2 — today silently dropped). + +**Architecture:** Two independent halves. (A) Kotlin-only: `KotlinExposedTableGenerator` keeps the package-qualified `ClassName` and emits its cross-package import, mirroring the FK-import machinery already in the file. (B) Cross-port loader validation: a new `ERR_ENUM_EXTENDS_VALUES_CONFLICT` enforced in the Java/Python/C#/TypeScript loaders (Kotlin inherits the JVM loader), gated by a shared conformance fixture. + +**Tech Stack:** Kotlin (codegen-kotlin, Exposed, `KotlinCompilation` test harness), Java/Python/C#/TypeScript loaders, the `fixtures/conformance/` corpus. + +**Design spec:** `docs/superpowers/specs/2026-07-31-shared-enum-cross-package-design.md` + +## Global Constraints + +- **Byte-identical for any model without a cross-package shared enum.** The Kotlin enum-import addition fires ONLY when the enum's `ClassName.packageName` is non-empty and ≠ the table's package. The load error fires ONLY on the precise condition (own `values` + `extends` resolves to a shared root-level abstract enum). Over-rejection (firing on a concrete/non-shared super) is a task failure. +- **New error code name:** `ERR_ENUM_EXTENDS_VALUES_CONFLICT` (all ports, verbatim). +- **Cross-port scope:** the load error lands in all four loaders (Java, Python, C#, TypeScript); Kotlin needs NO loader change (inherits the JVM loader). The Kotlin table-import fix is Kotlin-only. +- **ADR-0039 own-accessor discipline:** read the field's own `values` with the own-only accessor already used at each validation site; read the super via the resolving super accessor. +- **Named constants:** reuse each port's existing enum/type constants (`EnumField.ATTR_VALUES`, `TYPE_METADATA`, etc.) — no inlined metamodel strings. +- **Git:** stage explicit paths only, NEVER `git add -A` (untracked `.serena/` must never be committed). Commit to branch `fix/246-shared-enum-cross-package`. +- **Public-repo hygiene:** fixtures use `acme::*` example packages; no private/other-project names, no absolute home paths in committed files or commit messages. +- Each task runs its port's own toolchain FOREGROUND/blocking and must be green before commit. NO `-T` on Maven. + +--- + +### Task 1: Kotlin table-generator cross-package enum import (#246 Bug 1) + +**Files:** +- Modify: `server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt` (enum emit sites ~`:586-589` vanilla, ~`:617-620` TPH-fold; import assembly ~`:467-517`) +- Create fixture: `fixtures/codegen-conformance/enum-xpkg/input/meta.common.json`, `meta.orders.json`, `meta.billing.json` +- Test: `server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableCrossPackageEnumTest.kt` (new) + +**Interfaces:** +- Consumes: `KotlinTypeMapper.enumTypeName(field, entity): ClassName?` (returns the package-qualified enum `ClassName`, walking to the shared abstract super); `KotlinTphPlan.collectSubtypeFields(...)`; the `KotlinCompilation` harness pattern from `KotlinExtractTierCollisionTest.kt` (its private `compile(outDir): KotlinCompilation.Result` helper). +- Produces: nothing consumed by later tasks (Kotlin-only, self-contained). + +- [ ] **Step 1: Write the failing fixture.** Create a two-package (plus a common package) cross-package shared-enum model modeled on `fixtures/codegen-conformance/enum/input/meta.enum.json`: + - `meta.common.json` — package `acme::common`, one root-level `field.enum` `RecordStatus` with `abstract: true`, `@values: ["DRAFT","ACTIVE","CLOSED"]`. + - `meta.orders.json` — package `acme::orders`, `object.entity` `Order` with `source.rdb @table:"orders"`, an `identity.primary`, and a `field.enum` `status` with `extends: "acme::common::RecordStatus"` (no own values). + - `meta.billing.json` — package `acme::billing`, `object.entity` `Invoice` with `source.rdb @table:"invoices"`, an `identity.primary`, and a `field.enum` `status` with `extends: "acme::common::RecordStatus"`. + Validate each parses: `node -e "JSON.parse(require('fs').readFileSync(''))"`. + +- [ ] **Step 2: Write the failing test.** In `KotlinExposedTableCrossPackageEnumTest.kt`, load the `enum-xpkg` fixture directory, run `KotlinEntityGenerator` + `KotlinExposedTableGenerator` + `KotlinEnumEmitter` into a temp `outDir` (follow the generator-invocation + directory-walking pattern in `KotlinExtractTierCollisionTest.kt`), then assert: + +```kotlin +// OrderTable.kt is generated in package acme.orders and references the shared enum: +val orderTable = readGenerated(outDir, "acme/orders/OrderTable.kt") +assertTrue(orderTable.contains("import acme.common.RecordStatus"), + "OrderTable must import the cross-package shared enum") +assertTrue(orderTable.contains("RecordStatus::class"), + "OrderTable still references the enum in enumerationByName") +val invoiceTable = readGenerated(outDir, "acme/billing/InvoiceTable.kt") +assertTrue(invoiceTable.contains("import acme.common.RecordStatus")) +// Compile-gate: the whole generated tree must compile (catches any import variant) +val result = compile(outDir) // reuse the KotlinCompilation helper pattern +assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode, result.messages) +``` + +- [ ] **Step 3: Run the test to verify it fails.** Run: `cd server/java && mvn -pl codegen-kotlin test -Dtest=KotlinExposedTableCrossPackageEnumTest` (NO `-T`). Expected: FAIL — the `import acme.common.RecordStatus` assertion fails and/or `KotlinCompilation` returns `COMPILATION_ERROR` with `Unresolved reference: RecordStatus`. + +- [ ] **Step 4: Implement the fix.** In `KotlinExposedTableGenerator.kt`: + - At the two enum-column emit sites (~`:586`, ~`:617`): retain the full `ClassName` returned by `KotlinTypeMapper.enumTypeName(field, entity)` in a local (e.g. `val enumCn = ... ?: error(...)`); use `enumCn.simpleName` for the `enumerationByName("...", LEN, ${enumCn.simpleName}::class)` token (unchanged output), and collect `enumCn` for import emission. + - In the import-assembly block (~`:467-517`), add a pass over enum-typed fields — iterate `entity.metaFields` filtered to `EnumField`, plus (for the TPH path) `KotlinTphPlan.collectSubtypeFields(...)` — resolving each via `KotlinTypeMapper.enumTypeName(field, entity)`; for each resolved `cn`, add `"${cn.packageName}.${cn.simpleName}"` to the import set **only when** `cn.packageName.isNotEmpty() && cn.packageName != `. + - Emit those imports in the existing `for (imp in ...) append("import $imp\n")` block alongside `crossPackageTableImports`. + +- [ ] **Step 5: Run the new test — expect PASS.** Run: `cd server/java && mvn -pl codegen-kotlin test -Dtest=KotlinExposedTableCrossPackageEnumTest`. Expected: PASS (imports present + `KotlinCompilation.ExitCode.OK`). + +- [ ] **Step 6: Run the full codegen-kotlin suite for byte-identity.** Run: `cd server/java && mvn -pl codegen-kotlin test`. Expected: all existing tests PASS unchanged (same-package enum fixtures — `enumFieldEmitsEnumerationByName`, `KotlinEnumConformanceTest` — emit byte-identical output because same-package enums add no import). + +- [ ] **Step 7: Commit.** + +```bash +git add server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt \ + fixtures/codegen-conformance/enum-xpkg \ + server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableCrossPackageEnumTest.kt +git commit -m "fix(#246): Kotlin table generator emits cross-package shared-enum import" +``` + +--- + +### Task 2: Add `ERR_ENUM_EXTENDS_VALUES_CONFLICT` to the cross-port error ledger + +**Files:** +- Modify: `fixtures/conformance/ERROR-CODES.json` +- Modify: `server/typescript/packages/metadata/src/errors.ts` +- Modify: `server/python/src/metaobjects/errors.py` +- Modify: `server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java` +- Modify: `server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java` +- Modify: `server/csharp/MetaObjects/Errors.cs` +- Test: `server/typescript/packages/metadata/test/errors.test.ts` (existing, must stay green), `server/python/tests/unit/test_errors.py` (existing, must stay green) + +**Interfaces:** +- Produces: the string `ERR_ENUM_EXTENDS_VALUES_CONFLICT` present in every port's error-code ledger; consumed by Tasks 3-7. + +- [ ] **Step 1: Add the JSON ledger entry.** In `fixtures/conformance/ERROR-CODES.json`, add to the `codes` object (match the existing entry format): + +```json +"ERR_ENUM_EXTENDS_VALUES_CONFLICT": "A field.enum both extends a shared package-level abstract enum and declares its own @values. One shared enum type has one member set — the own @values would be silently dropped in codegen. Remove the own @values to inherit the shared set, or extend a concrete (non-shared) enum instead." +``` + +- [ ] **Step 2: Add the TS entry.** In `errors.ts`, add a `//`-commented string literal `"ERR_ENUM_EXTENDS_VALUES_CONFLICT",` to the `ERROR_CODES` array (near the other enum-related codes). + +- [ ] **Step 3: Add the Python entry.** In `errors.py`, add the enum member `ERR_ENUM_EXTENDS_VALUES_CONFLICT = "ERR_ENUM_EXTENDS_VALUES_CONFLICT"` to `class ErrorCode`. + +- [ ] **Step 4: Add the Java entries (BOTH files).** In `ErrorCode.java` add `ERR_ENUM_EXTENDS_VALUES_CONFLICT,` to the enum. In `ErrorMessageConstants.java` add `public static final String ERR_ENUM_EXTENDS_VALUES_CONFLICT = "ERR_ENUM_EXTENDS_VALUES_CONFLICT";`. + +- [ ] **Step 5: Add the C# entry.** In `Errors.cs` add `ERR_ENUM_EXTENDS_VALUES_CONFLICT,` to the `ErrorCode` enum. + +- [ ] **Step 6: Run the ledger-completeness tests.** Run: `cd server/typescript && bun test packages/metadata/test/errors.test.ts` (the exact-bidirectional check — passes only because Steps 1 AND 2 are both done) and `cd server/python && uv run --extra integration pytest tests/unit/test_errors.py`. Expected: PASS. + +- [ ] **Step 7: Commit.** + +```bash +git add fixtures/conformance/ERROR-CODES.json \ + server/typescript/packages/metadata/src/errors.ts \ + server/python/src/metaobjects/errors.py \ + server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java \ + server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java \ + server/csharp/MetaObjects/Errors.cs +git commit -m "feat(#246): register ERR_ENUM_EXTENDS_VALUES_CONFLICT in the cross-port error ledger" +``` + +--- + +### Task 3: TypeScript loader validation + +**Files:** +- Modify: `server/typescript/packages/metadata/src/attr-schema-validate.ts` (Check 4, the `field.enum @values` content block, own-values branch ~`:304-350`, insertion inside the `if (Array.isArray(rawValues)) { … }` at ~`:312`) +- Test: `server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts` (new) + +**Interfaces:** +- Consumes: `node.ownAttrs().get(FIELD_ATTR_VALUES)` (own values, already read here); `node.superData` (resolved super, `meta-data.ts:175`); `sup.isAbstract` (`:39`); `sup.parent` (`:392`); `TYPE_METADATA` constant; `ERROR_CODES`/the error-emitting helper this file already uses. +- Produces: TS loader rejects the conflict with `ERR_ENUM_EXTENDS_VALUES_CONFLICT`. + +- [ ] **Step 1: Write the failing test.** In the new test, build two in-memory metadata roots via the existing loader test helpers (mirror the style in `packages/metadata/test/` enum tests): + - CONFLICT case: package `acme`, root-level `field.enum Status { abstract: true, values: ["A","B"] }` + an `object.entity` with `field.enum status { extends: "acme::Status", values: ["A","B","C"] }`. Assert loading produces an error whose code is `ERR_ENUM_EXTENDS_VALUES_CONFLICT` on the `status` field node. + - LEGAL case: an entity `field.enum` extending a CONCRETE (non-abstract or non-root) enum field AND declaring own values → assert NO `ERR_ENUM_EXTENDS_VALUES_CONFLICT` error. + +- [ ] **Step 2: Run to verify it fails.** Run: `cd server/typescript && bun test packages/metadata/test/enum-extends-values-conflict.test.ts`. Expected: FAIL (no such error produced yet). + +- [ ] **Step 3: Implement.** Inside the own-values branch of Check 4, after the existing content checks, add: + +```ts +const sup = node.superData; +if ( + sup !== undefined && + sup.isAbstract === true && + sup.parent?.type === TYPE_METADATA +) { + // #246: own @values on a field extending a shared package-level abstract enum + // would be silently dropped by the shared-enum codegen collapse. + pushError(ERROR_CODES.ERR_ENUM_EXTENDS_VALUES_CONFLICT, node /* + source loc as the file's other errors do */); +} +``` +(Use this file's existing error-push mechanism and source-location plumbing — match the sibling `field.enum` errors in the same block.) + +- [ ] **Step 4: Run the test — expect PASS.** Run: `cd server/typescript && bun test packages/metadata/test/enum-extends-values-conflict.test.ts`. Expected: PASS (both the conflict and the legal case). + +- [ ] **Step 5: Run the metadata package suite.** Run: `cd server/typescript && bun test packages/metadata`. Expected: all green (no over-rejection of existing enum-extends fixtures, which have no own values). + +- [ ] **Step 6: Commit.** + +```bash +git add server/typescript/packages/metadata/src/attr-schema-validate.ts \ + server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts +git commit -m "feat(#246): TS loader rejects extends-shared-enum with own values" +``` + +--- + +### Task 4: Python loader validation + +**Files:** +- Modify: `server/python/src/metaobjects/loader/validation_passes.py` (`_validate_enum_values`, own-values branch ~`:541-601`; add one import of `TYPE_METADATA` from `metaobjects.shared.base_types`) +- Test: `server/python/tests/loader/test_enum_extends_values_conflict.py` (new) + +**Interfaces:** +- Consumes: `node.attr(FIELD_ATTR_VALUES)` (own values, already read at `:556`); `node.super_data`; `node.is_abstract` (used at `:445`); `node.parent`; `TYPE_METADATA`; `ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT`; the `errors.append(...)` mechanism this pass already uses. +- Produces: Python loader rejects the conflict. + +- [ ] **Step 1: Write the failing test.** Mirror the TS test's two cases (conflict + legal) using the Python loader test helpers (see other tests under `server/python/tests/loader/`). Assert the conflict case yields an error with `code == ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT` and the legal (concrete-super + own values) case does not. + +- [ ] **Step 2: Run to verify it fails.** Run: `cd server/python && uv run --extra integration pytest tests/loader/test_enum_extends_values_conflict.py`. Expected: FAIL. + +- [ ] **Step 3: Implement.** Add `from metaobjects.shared.base_types import TYPE_METADATA` (if not already imported). Inside the own-values branch of `_validate_enum_values`, add: + +```python +sup = node.super_data +if sup is not None and sup.is_abstract and sup.parent is not None and sup.parent.type == TYPE_METADATA: + # #246: own values + extends a shared package-level abstract enum -> silently dropped by codegen collapse + errors.append(make_error(ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT, node)) # match this pass's error helper +``` +(Use the exact error-append helper/signature `_validate_enum_values` already uses for its other enum errors, including source location.) + +- [ ] **Step 4: Run the test — expect PASS.** Run: `cd server/python && uv run --extra integration pytest tests/loader/test_enum_extends_values_conflict.py`. Expected: PASS. + +- [ ] **Step 5: Run the loader + codegen suites.** Run: `cd server/python && uv run --extra integration pytest tests/loader tests/codegen`. Expected: green (no over-rejection). + +- [ ] **Step 6: Commit.** + +```bash +git add server/python/src/metaobjects/loader/validation_passes.py \ + server/python/tests/loader/test_enum_extends_values_conflict.py +git commit -m "feat(#246): Python loader rejects extends-shared-enum with own values" +``` + +--- + +### Task 5: Java loader validation (also covers Kotlin) + +**Files:** +- Modify: `server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java` (`validateEnumNode`, own-values branch `:516-529`, insert before its `return`; reuse the file's own `isAbstract(MetaData)` helper at `:2669-2677`) +- Test: `server/java/metadata/src/test/java/com/metaobjects/loader/EnumExtendsValuesConflictTest.java` (new) + +**Interfaces:** +- Consumes: `node.hasMetaAttr(EnumField.ATTR_VALUES, false)` (own values, already checked); `node.getSuperData()`; the private `isAbstract(MetaData)` helper (`:2669`); `MetaRoot` (imported `:21`); `ErrorMessageConstants.ERR_ENUM_EXTENDS_VALUES_CONFLICT` + `ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT`; the file's existing error-emit call used in `validateEnumNode`. +- Produces: Java (and therefore Kotlin) loader rejects the conflict. + +- [ ] **Step 1: Write the failing test.** In `EnumExtendsValuesConflictTest.java`, load two in-memory models (conflict + legal) via the Java loader test helpers (mirror an existing `ValidationPhase` enum test). Assert the conflict model fails to load with `ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT`, and the legal (concrete-super) model loads without it. + +- [ ] **Step 2: Run to verify it fails.** Run: `cd server/java && mvn -pl metadata test -Dtest=EnumExtendsValuesConflictTest`. Expected: FAIL. + +- [ ] **Step 3: Implement.** Inside the own-values branch of `validateEnumNode`, before the `return`, add: + +```java +MetaData sup = node.getSuperData(); +if (sup != null && isAbstract(sup) && sup.getParent() instanceof MetaRoot) { + // #246: own @values + extends a shared package-level abstract enum -> silently dropped by codegen collapse + addError(node, ErrorMessageConstants.ERR_ENUM_EXTENDS_VALUES_CONFLICT, + ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT /* + message text/loc as sibling errors do */); +} +``` +(Match the exact error-emission call `validateEnumNode` already uses for `ERR_BAD_ATTR_VALUE`, passing both the message-prefix constant and the structured `ErrorCode`.) + +- [ ] **Step 4: Run the test — expect PASS.** Run: `cd server/java && mvn -pl metadata test -Dtest=EnumExtendsValuesConflictTest`. Expected: PASS. + +- [ ] **Step 5: Run the metadata suite.** Run: `cd server/java && mvn -pl metadata test`. Expected: green. + +- [ ] **Step 6: Commit.** + +```bash +git add server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java \ + server/java/metadata/src/test/java/com/metaobjects/loader/EnumExtendsValuesConflictTest.java +git commit -m "feat(#246): Java loader rejects extends-shared-enum with own values (covers Kotlin)" +``` + +--- + +### Task 6: C# loader validation + +**Files:** +- Modify: `server/csharp/MetaObjects/Loader/ValidationPasses.cs` (`WalkEnumValues`, Pass 10, the `members is not null` block `:2287-2325`) +- Test: `server/csharp/MetaObjects.Tests/EnumExtendsValuesConflictTests.cs` (new) + +**Interfaces:** +- Consumes: `field.EnumValues` (own values, already read); core `MetaData.SuperData` (`:209`), `MetaData.IsAbstract` (`:44`), `MetaData.Parent` (`:388`); `BaseTypes.TYPE_METADATA`; `ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT`; the `errors.Add(...)` mechanism this pass uses. +- Produces: C# loader rejects the conflict. + +- [ ] **Step 1: Write the failing test.** In `EnumExtendsValuesConflictTests.cs`, load two models (conflict + legal) via the C# loader test helpers (mirror an existing `ValidationPasses` enum test). Assert the conflict model yields `ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT`, the legal model does not. + +- [ ] **Step 2: Run to verify it fails.** Run: `cd server/csharp && dotnet test MetaObjects.Tests/MetaObjects.Tests.csproj --filter EnumExtendsValuesConflict`. Expected: FAIL. + +- [ ] **Step 3: Implement.** Inside the `members is not null` block of `WalkEnumValues`, after the existing content checks, add: + +```csharp +var sup = node.SuperData; +if (sup is not null && sup.IsAbstract && sup.Parent is {} p && p.Type == BaseTypes.TYPE_METADATA) +{ + // #246: own values + extends a shared package-level abstract enum -> silently dropped by codegen collapse + errors.Add(new MetaError(ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT, node /* + source loc as siblings do */)); +} +``` +(Match the exact `MetaError` construction `WalkEnumValues` already uses for its other enum errors.) + +- [ ] **Step 4: Run the test — expect PASS.** Run: `cd server/csharp && dotnet test MetaObjects.Tests/MetaObjects.Tests.csproj --filter EnumExtendsValuesConflict`. Expected: PASS. + +- [ ] **Step 5: Run the core + conformance test projects.** Run: `cd server/csharp && dotnet test MetaObjects.Tests/MetaObjects.Tests.csproj`. Expected: green. + +- [ ] **Step 6: Commit.** + +```bash +git add server/csharp/MetaObjects/Loader/ValidationPasses.cs \ + server/csharp/MetaObjects.Tests/EnumExtendsValuesConflictTests.cs +git commit -m "feat(#246): C# loader rejects extends-shared-enum with own values" +``` + +--- + +### Task 7: Shared conformance fixtures — cross-port gate + +**Files:** +- Create: `fixtures/conformance/error-enum-extends-values-conflict/input/meta.enums.json` + `expected-errors.json` +- Create: `fixtures/conformance/enum-extends-concrete-with-own-values/input/meta.enums.json` + `expected.json` + +**Interfaces:** +- Consumes: Tasks 2-6 (the code + all four loaders reject the conflict). Auto-discovered by every port's `FixtureDiscovery` — no runner code changes. +- Produces: the cross-port gate proving all ports agree (same code, same jsonPath, negative loads clean). + +- [ ] **Step 1: Author the ERROR fixture.** In `error-enum-extends-values-conflict/`, model `input/meta.enums.json` on `fixtures/conformance/error-enum-duplicate-member/input/meta.enums.json`: a root-level `field.enum` `Status { abstract: true, @values:[...] }` plus an `object.entity` whose `field.enum status` has `extends` the shared `Status` AND its own `@values`. Write `expected-errors.json` mirroring `error-enum-duplicate-member/expected-errors.json`, with `code: "ERR_ENUM_EXTENDS_VALUES_CONFLICT"` and a `source.jsonPath` pointing at the offending `field.enum` node (compute the path the same way the sibling fixture does). + +- [ ] **Step 2: Author the NEGATIVE (legal) fixture.** In `enum-extends-concrete-with-own-values/`, `input/meta.enums.json` = a CONCRETE enum field (e.g. an entity-nested `field.enum` that is NOT abstract-at-root) extended by another `field.enum` that declares its own differing `@values`; add `expected.json` (a happy-path fixture, mirror any existing `fixtures/conformance/enum-*/expected.json`) so it must load clean. Validate both JSON files parse. + +- [ ] **Step 3: Run the conformance suites on all four ports.** Run FOREGROUND: + - `cd server/typescript && bun test packages/conformance` + - `cd server/python && uv run --extra integration pytest tests/conformance` + - `cd server/java && mvn -pl metadata test -Dtest=ConformanceTest` + - `cd server/csharp && dotnet test MetaObjects.Conformance.Tests/MetaObjects.Conformance.Tests.csproj` + Expected: all green — the error fixture's code-set matches in every port (same jsonPath), and the negative fixture loads with zero errors under strict mode. If a port reports a different jsonPath, fix that port's error location (Task 3-6) to target the offending `field.enum` node, then re-run. + +- [ ] **Step 4: Commit.** + +```bash +git add fixtures/conformance/error-enum-extends-values-conflict \ + fixtures/conformance/enum-extends-concrete-with-own-values +git commit -m "test(#246): cross-port conformance fixtures for extends-shared-enum values conflict" +``` + +--- + +### Task 8: Docs + CHANGELOG + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: the enum authoring docs that describe "reuse a constraint set across entities with an abstract `field.enum` + `extends`" (locate via `grep -rl "reuse a constraint set" docs/ spec/` and the enum feature doc under `docs/features/`) + +- [ ] **Step 1: CHANGELOG.** Add a `## [Unreleased]` section (or append to it): (a) `#246` Kotlin table generator now emits the cross-package shared-`field.enum` import (was `Unresolved reference` under a cross-package shared enum) — Kotlin-only, byte-identical for same-package models, gated by a new `enum-xpkg` fixture + a `KotlinCompilation` compile-gate; (b) new cross-port loader error `ERR_ENUM_EXTENDS_VALUES_CONFLICT` — a `field.enum` that both `extends` a shared package-level abstract enum and declares its own `values` now fails to load (was silently dropped) in all five ports, gated by a shared conformance fixture. Note the C# materialized-enum cross-namespace sibling and the enum-PK-as-String issue are tracked separately. + +- [ ] **Step 2: Authoring docs.** In the enum authoring guidance, clarify that (a) sharing one enum across packages via an abstract `field.enum` + `extends` now works cross-package, and (b) a child extending a shared abstract enum must NOT declare its own `values` (it is now a load error — the shared enum is one type with one member set; extend a concrete enum if you need an independent set). + +- [ ] **Step 3: Commit.** + +```bash +git add CHANGELOG.md +git commit -m "docs(#246): CHANGELOG + enum authoring guidance for cross-package shared enums" +``` + +--- + +## Self-Review + +**Spec coverage:** Bug 1 (Kotlin import) → Task 1; new error code → Task 2; the load error in all four loaders → Tasks 3-6 (Kotlin covered by Task 5); cross-port conformance (error + negative) → Task 7; byte-identity → pinned in Task 1 Step 6 (Kotlin) and by the negative fixture + each port's full-suite run; docs → Task 8. Out-of-scope items (C# materialized-enum, enum-PK, Python latent) are explicitly deferred in the spec and not tasked. + +**Type/name consistency:** `ERR_ENUM_EXTENDS_VALUES_CONFLICT` used identically in Tasks 2-7. The detection predicate is the same across ports (own-values-present AND super resolves AND super is abstract AND super's parent is the metadata root), expressed with each port's own accessors (`superData`/`super_data`/`getSuperData()`/`SuperData`; `isAbstract`/`is_abstract`/`isAbstract(...)`/`IsAbstract`; parent-is-root via `instanceof MetaRoot` or `parent.type == TYPE_METADATA`). The Kotlin fix consumes only `KotlinTypeMapper.enumTypeName` (existing). + +**Ordering:** Task 1 (Kotlin, independent) first — the urgent filed bug. Task 2 (ledger) before Tasks 3-6 (each loader references the code). Tasks 3-6 (each port, independently testable via a port-local unit test — no shared fixture yet, so no cross-port breakage). Task 7 (shared fixtures) LAST among the Part-B tasks, once all four loaders reject the conflict, so the shared corpus is green when it lands. Task 8 docs last. + +**Byte-identity guardrail:** every enum-import addition (Task 1) is gated on `packageName != table-package`; the load error (Tasks 3-6) fires only on the precise shared-abstract-root condition and is proven not to over-reject by the negative fixture (Task 7) + each port's full-suite run. diff --git a/docs/superpowers/specs/2026-07-31-shared-enum-cross-package-design.md b/docs/superpowers/specs/2026-07-31-shared-enum-cross-package-design.md new file mode 100644 index 000000000..2071ab4f5 --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-shared-enum-cross-package-design.md @@ -0,0 +1,81 @@ +# Shared enum across packages — Kotlin table-generator import fix (#246) + cross-port `extends`/`values` load error + +**Date:** 2026-07-31 +**Issues:** [#246](https://github.com/metaobjectsdev/metaobjects/issues/246) (primary) + the secondary observation in that issue (`extends` + own `values` silently dropped). +**Status:** design approved; proceeding to plan + implementation. + +## Problem + +The documented reuse mechanism — "reuse a constraint set across entities with an abstract `field.enum` + `extends`" — is broken in two ways when the sharing entities live in **different packages**. Both are instances of the cross-package bare-name bug class (a cross-package reference resolved/named/imported by bare short name instead of package-qualified), the same family #228 closed in the payload/extract tier. This work targets two siblings in the entity/enum tier. + +Reproduction (canonical example packages): + +```yaml +# acme::common +field.enum: { name: RecordStatus, abstract: true, values: ["DRAFT","ACTIVE","CLOSED"] } +# acme::orders → field.enum: { name: status, extends: "acme::common::RecordStatus" } +# acme::billing → field.enum: { name: status, extends: "acme::common::RecordStatus" } +``` + +### Bug 1 — Kotlin `Table` drops the cross-package enum import (reproduces at HEAD) + +`KotlinExposedTableGenerator` is a deliberately string-hand-rolled emitter (its class doc explains it bypasses KotlinPoet because Exposed's `Column` types are inferred). At the two enum-column emit sites — `KotlinExposedTableGenerator.kt:586-589` (vanilla entity) and `:617-620` (TPH subtype-fold) — it does: + +```kotlin +val enumName = KotlinTypeMapper.enumTypeName(field, entity)?.simpleName ?: error(...) +"enumerationByName(\"$colName\", ${KotlinTypeMapper.ENUM_VARCHAR_LEN}, $enumName::class)" +``` + +`enumTypeName()` (`KotlinTypeMapper.kt:175-205`) correctly resolves a **package-qualified `ClassName`** (walking to the shared abstract super at `:192-196`), but the table generator immediately discards the package via `.simpleName`. The file's manual import passes — `columnFunctionImports` (`:467-482`) and `crossPackageTableImports` (`:507-517`) — explicitly skip `EnumField` (`:476`) and only walk FK targets, so no import is ever added for the enum's own `ClassName.packageName`. Result: `OrderTable.kt` (package `acme.orders`) emits bare `RecordStatus::class` with **no `import acme.common.RecordStatus`** → `Unresolved reference`. In the reporter's tree this produced 214 compile errors. + +The **data class** (`KotlinEntityGenerator.resolveElementType`, `:351-377`) gets it right only because it keeps the full `ClassName` and hands it to KotlinPoet, whose `FileSpec.writeTo()` auto-emits imports. The `RepositoryBase` is **not** affected (it uses type-inferred bare property access — the issue text's claim about `RepositoryBase` does not hold). + +### Bug 2 — `extends` + own `values` silently dropped (codegen collapse, not a loader defect) + +The loader intentionally treats an own `@values` as authoritative when present (`ValidationPhase` `validateEnumNode`, own-wins per ADR-0039) — there is no error for declaring both `extends` and own `values`. The silent drop happens in **codegen's shared-enum collapse**: `KotlinTypeMapper.enumTypeName()` names every field extending the same abstract super after that super, and `KotlinEnumEmitter.emitEnumFile()` dedupes by that shared `ClassName` — so only the **first** field processed (in loader iteration order) has its `values` read; a later field's differing own `values` are skipped before they are ever consulted. One shared type cannot satisfy two member sets, so this is a structural conflict, not merely an ordering quirk. It surfaces only when exactly one extending field declares differing own `values` — and it is silent (the reporter's two copies diverged 10 vs 12 members, causing a runtime record-drop). + +## Scope + +**In scope:** +1. Fix Bug 1 — the Kotlin table-generator cross-package enum import (both the vanilla and TPH-fold emit sites). +2. Fix Bug 2 — a new **cross-port load error** when a `field.enum` both `extends` a shared package-level abstract enum and declares its own `values`. + +**Out of scope (flagged, tracked separately — not dropped):** +- **C# materialized shared-enum cross-namespace reference** (`Fr019SharedEnum.cs:118-120` returns the bare name assuming one namespace; under `GenConfig.PackageNamespaces` splitting entities across namespaces it would go unresolved, and it hits the primary entity class). Its own follow-up: it needs a `PackageNamespaces` compile-repro before the exact fix is known, and it is arguably more severe. +- **Enum-typed primary key mistyped as `String`** in `KotlinRepositoryGenerator.primaryKeyParamType` (`:369-374`, uses `kotlinTypeName` not `enumTypeName`). Narrow, different bug class (enum PKs are an edge case). Separate item. +- **Python latent same-premise** (`fr019_shared_enum.py` relative `from .enums import Name`) — correct today because Python emits entity files flat; only a risk if a nested per-package layout is ever added. No change now. +- TypeScript is already correct (`enum-import.ts` computes a real relative module specifier); it is the reference design. No change. +- Java (`codegen-spring`) is immune — no generated typed persistence class (OMDB is runtime); its DTO tier already uses an inline-FQN fallback. +- **Kotlin `enumTypeName` collapse lacks the `isAbstract` leg** (`KotlinTypeMapper.kt`). After #259 the collapse keys on the *immediate* super having no declaring object (root-level), but — unlike the TS/C#/Python `resolveSharedEnumDecl` resolvers, which also require the super to be **abstract** — it does not check `isAbstract`. So a root-level *concrete* enum extended with own `@values` collapses onto the super's name on Kotlin (with a first-wins FQN dedupe that silently drops one member set under multiple extenders), where the other three ports emit an independent per-field enum. Pre-existing (the old `resolveSuperRoot` keying had the same hole); output-changing to fix (renames the collapsed enum for existing consumers), so it is a separate follow-up, not part of the #246/#259 PATCH. Surfaced by the Fable whole-branch review 2026-08-02. + +## Design + +### Part A — Kotlin table-generator import fix + +Surgical fix mirroring the existing FK cross-package import machinery: +1. At `KotlinExposedTableGenerator.kt:586` and `:617`, keep the full `ClassName` from `KotlinTypeMapper.enumTypeName(field, entity)` rather than taking `.simpleName` immediately (still use `simpleName` for the `enumerationByName(... , ::class)` token, but retain the `ClassName` for import collection). +2. In the import-assembly block (`:467-517`), add a pass over enum-typed fields — `entity.metaFields` plus `KotlinTphPlan.collectSubtypeFields` for the TPH path — that, for each resolved enum `ClassName cn`, adds `"${cn.packageName}.${cn.simpleName}"` to the import set **only when `cn.packageName.isNotEmpty() && cn.packageName != `**. Same-package enums add nothing → **byte-identical output for every existing single-package fixture/snapshot**. +3. Emit those imports in the existing `import $imp` loop alongside `crossPackageTableImports`. + +**Alternatives rejected:** a unified all-type-reference import collector (larger, risks byte-identity now) and converting the generator to KotlinPoet (would eliminate the class of bug, but the generator hand-rolls strings for a documented Exposed reason — too big/risky here). Recorded so a future maintainer knows the surgical choice was deliberate. + +### Part B — cross-port `extends` + `values` load error + +A new **loader validation** in the shared loader (Java / Python / C# / TypeScript; Kotlin inherits the JVM loader), emitting a new error code **`ERR_ENUM_EXTENDS_VALUES_CONFLICT`** (name may be refined during implementation to match ledger conventions). + +**Trigger condition (precise):** a `field.enum` node that (a) has an `extends` that resolves to a **shared, package-level abstract enum** — the same "abstract AND a direct child of the metadata root" condition the shared-enum codegen (`ResolveSharedEnumDecl` / `Fr019SharedEnum`) uses to decide collapse — AND (b) declares its own `values` (own-only check, per ADR-0039 own-accessor discipline). + +**Explicitly still legal (must NOT fire):** own `values` on a field whose `extends` targets a **concrete, non-shared** enum (e.g. a projection extending `ActiveNpc.status`). That path already produces an independent per-object type with no collapse and no conflict. + +The code is added to the shared error-code ledger (`packages/metadata/src/errors.ts`, `server/python/src/metaobjects/errors.py`, Java `ErrorCode`, `fixtures/conformance/ERROR-CODES.json`) so all five ports enforce it identically (ADR-0023: new validation, human-agreed, cross-port, conformance-gated). + +## Testing + +- **Bug 1 (Kotlin):** a new **two-package** cross-package-enum fixture (`acme::common` abstract enum + `acme::orders` / `acme::billing` entities extending it, each with `source.rdb`), asserting `import acme.common.RecordStatus` appears in the generated `OrderTable.kt` / `InvoiceTable.kt`, **plus a `KotlinCompilation` compile-gate** (the harness the #228 `KotlinExtractTierCollisionTest` already uses) that compiles the generated table + data class + enum together so the test fails on *any* variant of the missing import, not just a text mismatch. Kotlin currently has no compile-gate for generated table output; this closes that gap. A same-package no-churn assertion pins byte-identity. +- **Bug 2 (load error):** a cross-port loader-error conformance fixture (extends-a-shared-abstract-enum + own `values` → `ERR_ENUM_EXTENDS_VALUES_CONFLICT`) plus a **negative** fixture (extends-a-concrete-enum + own `values` → still valid) so the scoping cannot regress into over-rejection. + +## Cross-language contract + +- The new `ERR_ENUM_EXTENDS_VALUES_CONFLICT` is a cross-port error code (all five ports; conformance-gated). +- Bug 1 is Kotlin-only (the other ports are unaffected or immune, per Scope). +- **Byte-identity guardrail throughout:** nothing changes for a model without a cross-package shared enum. Every enum-import addition is gated on `packageName != table-package`; the load error fires only on the precise shared-abstract-plus-own-values condition. diff --git a/fixtures/codegen-conformance/enum-xpkg/input/meta.billing.json b/fixtures/codegen-conformance/enum-xpkg/input/meta.billing.json new file mode 100644 index 000000000..430460048 --- /dev/null +++ b/fixtures/codegen-conformance/enum-xpkg/input/meta.billing.json @@ -0,0 +1,13 @@ +{ + "metadata.root": { + "package": "acme::billing", + "children": [ + { "object.entity": { "name": "Invoice", "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "@required": true, "extends": "acme::common::RecordStatus" } }, + { "source.rdb": { "@table": "invoices" } }, + { "identity.primary": { "name": "pk", "@fields": ["id"], "@generation": "increment" } } + ] } } + ] + } +} diff --git a/fixtures/codegen-conformance/enum-xpkg/input/meta.common.json b/fixtures/codegen-conformance/enum-xpkg/input/meta.common.json new file mode 100644 index 000000000..060207fac --- /dev/null +++ b/fixtures/codegen-conformance/enum-xpkg/input/meta.common.json @@ -0,0 +1,8 @@ +{ + "metadata.root": { + "package": "acme::common", + "children": [ + { "field.enum": { "name": "RecordStatus", "abstract": true, "@values": ["DRAFT", "ACTIVE", "CLOSED"] } } + ] + } +} diff --git a/fixtures/codegen-conformance/enum-xpkg/input/meta.orders.json b/fixtures/codegen-conformance/enum-xpkg/input/meta.orders.json new file mode 100644 index 000000000..8de7263d2 --- /dev/null +++ b/fixtures/codegen-conformance/enum-xpkg/input/meta.orders.json @@ -0,0 +1,13 @@ +{ + "metadata.root": { + "package": "acme::orders", + "children": [ + { "object.entity": { "name": "Order", "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "@required": true, "extends": "acme::common::RecordStatus" } }, + { "source.rdb": { "@table": "orders" } }, + { "identity.primary": { "name": "pk", "@fields": ["id"], "@generation": "increment" } } + ] } } + ] + } +} diff --git a/fixtures/codegen-conformance/enum-xpkg/input/meta.shipping.json b/fixtures/codegen-conformance/enum-xpkg/input/meta.shipping.json new file mode 100644 index 000000000..95b50dde4 --- /dev/null +++ b/fixtures/codegen-conformance/enum-xpkg/input/meta.shipping.json @@ -0,0 +1,17 @@ +{ + "metadata.root": { + "package": "acme::shipping", + "children": [ + { "object.entity": { "name": "Shipment", "@discriminator": "carrier", "children": [ + { "source.rdb": { "@table": "shipments" } }, + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "carrier", "@values": ["AIR", "GROUND"] } }, + { "identity.primary": { "@fields": "id", "@generation": "increment" } } + ] } }, + { "object.entity": { "name": "AirShipment", "extends": "Shipment", + "@discriminatorValue": "AIR", "children": [ + { "field.enum": { "name": "recordStatus", "@required": true, "extends": "acme::common::RecordStatus" } } + ] } } + ] + } +} diff --git a/fixtures/conformance/ERROR-CODES.json b/fixtures/conformance/ERROR-CODES.json index bc8420748..2440dd1cd 100644 --- a/fixtures/conformance/ERROR-CODES.json +++ b/fixtures/conformance/ERROR-CODES.json @@ -73,6 +73,7 @@ "ERR_UNKNOWN_EXPR_NODE": "#195: an origin.computed @expr tree contains a node whose kind/op/fn is not in the closed expression grammar (field/value refs, comparisons sharing the filter op vocabulary, isNull/isNotNull, and/or/not, coalesce). Fail-closed per ADR-0023; the detail names the offending token.", "ERR_SQL_BODY_WITH_UNMANAGED": "#208: a source.rdb declares both @sql (author-supplied body) and @unmanaged (DDL owned elsewhere). The two markers are the mutually exclusive non-default states of one DDL-ownership axis — contradictory on a single source.", "ERR_SQL_BODY_ON_WRITABLE_KIND": "#208: a source.rdb declares @sql with a writable @kind (\"table\", the default). A hand-written CREATE TABLE would bypass the column-diff machinery; tables are fully modeled or @unmanaged, never opaque-bodied.", - "ERR_ORIGIN_UNDER_SQL_BODY": "#208: a host object whose read source carries @sql also declares an origin.*-bearing (derived) field, or (on object.projection) an @filter (#207) — two sources of truth for the same body (the synthesized derivation/outer-WHERE vs. the author's verbatim SQL). Fail-closed." + "ERR_ORIGIN_UNDER_SQL_BODY": "#208: a host object whose read source carries @sql also declares an origin.*-bearing (derived) field, or (on object.projection) an @filter (#207) — two sources of truth for the same body (the synthesized derivation/outer-WHERE vs. the author's verbatim SQL). Fail-closed.", + "ERR_ENUM_EXTENDS_VALUES_CONFLICT": "A field.enum both extends a shared package-level abstract enum and declares its own @values. One shared enum type has one member set — the own @values would be silently dropped in codegen. Remove the own @values to inherit the shared set, or extend a concrete (non-shared) enum instead." } } diff --git a/fixtures/conformance/enum-extends-concrete-with-own-values/expected.json b/fixtures/conformance/enum-extends-concrete-with-own-values/expected.json new file mode 100644 index 000000000..32a794ef6 --- /dev/null +++ b/fixtures/conformance/enum-extends-concrete-with-own-values/expected.json @@ -0,0 +1,50 @@ +{ + "metadata.root": { + "package": "acme", + "children": [ + { + "field.enum": { + "name": "Status", + "package": "acme", + "@values": [ + "DRAFT", + "PUBLISHED", + "ARCHIVED" + ] + } + }, + { + "object.entity": { + "name": "Order", + "children": [ + { + "field.long": { + "name": "id" + } + }, + { + "field.enum": { + "name": "status", + "extends": "acme::Status", + "@values": [ + "DRAFT", + "PUBLISHED", + "ARCHIVED", + "CANCELLED" + ] + } + }, + { + "identity.primary": { + "name": "id", + "@fields": [ + "id" + ] + } + } + ] + } + } + ] + } +} diff --git a/fixtures/conformance/enum-extends-concrete-with-own-values/input/meta.enums.json b/fixtures/conformance/enum-extends-concrete-with-own-values/input/meta.enums.json new file mode 100644 index 000000000..79818d4da --- /dev/null +++ b/fixtures/conformance/enum-extends-concrete-with-own-values/input/meta.enums.json @@ -0,0 +1,18 @@ +{ + "metadata.root": { + "package": "acme", + "children": [ + { "field.enum": { "name": "Status", "@values": ["DRAFT", "PUBLISHED", "ARCHIVED"] } }, + { + "object.entity": { + "name": "Order", + "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "acme::Status", "@values": ["DRAFT", "PUBLISHED", "ARCHIVED", "CANCELLED"] } }, + { "identity.primary": { "name": "id", "@fields": "id" } } + ] + } + } + ] + } +} diff --git a/fixtures/conformance/enum-extends-two-hop-projection/expected.json b/fixtures/conformance/enum-extends-two-hop-projection/expected.json new file mode 100644 index 000000000..319499c0f --- /dev/null +++ b/fixtures/conformance/enum-extends-two-hop-projection/expected.json @@ -0,0 +1,75 @@ +{ + "metadata.root": { + "package": "demo", + "children": [ + { + "field.enum": { + "name": "SharedStatus", + "package": "demo", + "abstract": true, + "@values": [ + "DRAFT", + "PUBLISHED", + "ARCHIVED" + ] + } + }, + { + "object.entity": { + "name": "Program", + "children": [ + { + "source.rdb": { + "@table": "programs" + } + }, + { + "field.uuid": { + "name": "id" + } + }, + { + "field.enum": { + "name": "status", + "extends": "demo::SharedStatus" + } + }, + { + "identity.primary": { + "name": "id", + "@fields": [ + "id" + ] + } + } + ] + } + }, + { + "object.projection": { + "name": "ProgramView", + "children": [ + { + "field.uuid": { + "name": "id", + "extends": "demo::Program.id" + } + }, + { + "field.enum": { + "name": "status", + "extends": "demo::Program.status" + } + }, + { + "identity.primary": { + "name": "id", + "extends": "demo::Program.id" + } + } + ] + } + } + ] + } +} diff --git a/fixtures/conformance/enum-extends-two-hop-projection/input/meta.enums.json b/fixtures/conformance/enum-extends-two-hop-projection/input/meta.enums.json new file mode 100644 index 000000000..92bc799e0 --- /dev/null +++ b/fixtures/conformance/enum-extends-two-hop-projection/input/meta.enums.json @@ -0,0 +1,29 @@ +{ + "metadata.root": { + "package": "demo", + "children": [ + { "field.enum": { "name": "SharedStatus", "abstract": true, "@values": ["DRAFT", "PUBLISHED", "ARCHIVED"] } }, + { + "object.entity": { + "name": "Program", + "children": [ + { "source.rdb": { "@table": "programs" } }, + { "field.uuid": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "demo::SharedStatus" } }, + { "identity.primary": { "name": "id", "@fields": ["id"] } } + ] + } + }, + { + "object.projection": { + "name": "ProgramView", + "children": [ + { "field.uuid": { "name": "id", "extends": "demo::Program.id" } }, + { "field.enum": { "name": "status", "extends": "demo::Program.status" } }, + { "identity.primary": { "name": "id", "extends": "demo::Program.id" } } + ] + } + } + ] + } +} diff --git a/fixtures/conformance/error-enum-extends-values-conflict/expected-errors.json b/fixtures/conformance/error-enum-extends-values-conflict/expected-errors.json new file mode 100644 index 000000000..bfa5c11ee --- /dev/null +++ b/fixtures/conformance/error-enum-extends-values-conflict/expected-errors.json @@ -0,0 +1,15 @@ +{ + "errors": [ + { + "code": "ERR_ENUM_EXTENDS_VALUES_CONFLICT", + "source": { + "format": "json", + "files": [ + "meta.enums.json" + ], + "jsonPath": "$['metadata.root'].children[1]['object.entity'].children[1]['field.enum']" + } + } + ], + "warnings": [] +} diff --git a/fixtures/conformance/error-enum-extends-values-conflict/input/meta.enums.json b/fixtures/conformance/error-enum-extends-values-conflict/input/meta.enums.json new file mode 100644 index 000000000..0c2b3568d --- /dev/null +++ b/fixtures/conformance/error-enum-extends-values-conflict/input/meta.enums.json @@ -0,0 +1,18 @@ +{ + "metadata.root": { + "package": "acme", + "children": [ + { "field.enum": { "name": "Status", "abstract": true, "@values": ["DRAFT", "PUBLISHED", "ARCHIVED"] } }, + { + "object.entity": { + "name": "Order", + "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "acme::Status", "@values": ["DRAFT", "PUBLISHED", "ARCHIVED", "CANCELLED"] } }, + { "identity.primary": { "name": "id", "@fields": "id" } } + ] + } + } + ] + } +} diff --git a/server/csharp/MetaObjects.Conformance.Tests/EnumExtendsValuesConflictTests.cs b/server/csharp/MetaObjects.Conformance.Tests/EnumExtendsValuesConflictTests.cs new file mode 100644 index 000000000..d900e6927 --- /dev/null +++ b/server/csharp/MetaObjects.Conformance.Tests/EnumExtendsValuesConflictTests.cs @@ -0,0 +1,89 @@ +// #246 — a field.enum that both extends a shared root-level abstract enum AND +// declares its own @values must fail to load with ERR_ENUM_EXTENDS_VALUES_CONFLICT: +// one shared enum type has one member set, so the own @values would be silently +// dropped by codegen's shared-enum collapse. +// +// Mirrors the TS reference test +// (typescript/packages/metadata/test/enum-extends-values-conflict.test.ts) and the +// Java/Python ports (server/java/metadata/.../EnumExtendsValuesConflictTest.java, +// server/python/tests/loader/test_enum_extends_values_conflict.py). +// +// Three cases (the third pins the "root-level" clause of the predicate — dropping +// the `p.Type == BaseTypes.TYPE_METADATA` check would let a non-root abstract super +// go unrejected): +// 1. CONFLICT — extends a root-level (metadata.root child) abstract enum, and also +// declares its own @values. +// 2. LEGAL — extends a CONCRETE (non-abstract) enum, and also declares its own @values. +// 3. LEGAL — extends an ABSTRACT but NON-ROOT enum (declared as a child of an +// object.entity, not the shared package level), and also declares its own @values. + +using MetaObjects; +using MetaObjects.Loader; +using Xunit; + +namespace MetaObjects.Conformance.Tests; + +public class EnumExtendsValuesConflictTests +{ + private static LoadResult LoadInline(string json) => + MetaDataLoader.FromString(json, MetaDataFormat.Json); + + [Fact] + public void Conflict_extends_root_level_abstract_enum_with_own_values() + { + var doc = """ + { "metadata.root": { "package": "acme", "children": [ + { "field.enum": { "name": "Status", "abstract": true, "@values": ["A", "B"] } }, + { "object.entity": { "name": "Order", "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "acme::Status", "@values": ["A", "B", "C"] } }, + { "identity.primary": { "name": "id", "@fields": ["id"] } } + ] } } + ] } } + """; + var result = LoadInline(doc); + + var err = Assert.Single(result.Errors, e => e.Code == ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT); + Assert.Contains("status", err.Message); + } + + [Fact] + public void Legal_extends_concrete_enum_with_own_values() + { + var doc = """ + { "metadata.root": { "package": "acme", "children": [ + { "field.enum": { "name": "Status", "@values": ["A", "B"] } }, + { "object.entity": { "name": "Order", "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "acme::Status", "@values": ["A", "B", "C"] } }, + { "identity.primary": { "name": "id", "@fields": ["id"] } } + ] } } + ] } } + """; + var result = LoadInline(doc); + + Assert.DoesNotContain(result.Errors, e => e.Code == ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT); + } + + [Fact] + public void Legal_extends_non_root_abstract_enum_with_own_values() + { + // Pins the `p.Type == BaseTypes.TYPE_METADATA` clause of the predicate — + // dropping it would incorrectly reject this nested-abstract-super case. + var doc = """ + { "metadata.root": { "package": "acme", "children": [ + { "object.entity": { "name": "Container", "abstract": true, "children": [ + { "field.enum": { "name": "kind", "abstract": true, "@values": ["X", "Y"] } } + ] } }, + { "object.entity": { "name": "Order", "children": [ + { "field.long": { "name": "id" } }, + { "field.enum": { "name": "status", "extends": "acme::Container.kind", "@values": ["X", "Y", "Z"] } }, + { "identity.primary": { "name": "id", "@fields": ["id"] } } + ] } } + ] } } + """; + var result = LoadInline(doc); + + Assert.DoesNotContain(result.Errors, e => e.Code == ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT); + } +} diff --git a/server/csharp/MetaObjects/Errors.cs b/server/csharp/MetaObjects/Errors.cs index 12bce6c0a..0a377283e 100644 --- a/server/csharp/MetaObjects/Errors.cs +++ b/server/csharp/MetaObjects/Errors.cs @@ -173,6 +173,11 @@ public enum ErrorCode ERR_SQL_BODY_WITH_UNMANAGED, ERR_SQL_BODY_ON_WRITABLE_KIND, ERR_ORIGIN_UNDER_SQL_BODY, + // #246 — a field.enum both extends a shared package-level abstract enum and + // declares its own @values. One shared enum type has one member set — the + // own @values would be silently dropped in codegen. Remove the own @values + // to inherit the shared set, or extend a concrete (non-shared) enum instead. + ERR_ENUM_EXTENDS_VALUES_CONFLICT, ERR_UNKNOWN, } diff --git a/server/csharp/MetaObjects/Loader/ValidationPasses.cs b/server/csharp/MetaObjects/Loader/ValidationPasses.cs index f64bb2ba8..90940a32d 100644 --- a/server/csharp/MetaObjects/Loader/ValidationPasses.cs +++ b/server/csharp/MetaObjects/Loader/ValidationPasses.cs @@ -2325,6 +2325,23 @@ private static void WalkEnumValues(MetaData node, List errors) } } } + + // #246: a field.enum extending a shared root-level abstract enum (a metadata.root + // child, not one nested under an object) that ALSO declares its own @values is a + // conflict: one shared enum type has one member set, so codegen's shared-enum + // collapse would silently drop this field's own @values in favor of the shared + // type's. Own-attrs-only (matches Rules 1-3 above): only fires when THIS node + // declares @values itself, not when it merely inherits. + var sup = field.SuperData; + if (sup is not null && sup.IsAbstract && sup.Parent is { } p && p.Type == TYPE_METADATA) + { + errors.Add(new MetaError( + $"field.enum '{field.Name}' extends shared abstract enum '{sup.Name}' AND declares its own " + + $"@{FIELD_ATTR_VALUES} — a shared enum's member set is owned by the shared declaration; " + + $"remove the own @{FIELD_ATTR_VALUES} to inherit it, or extend a non-shared enum instead.", + ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT, + Envelope: field.Source)); + } } // Rule 4 (FR-011): the enum fallback attrs must be one of the field's @values. diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinEnumEmitter.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinEnumEmitter.kt index 27d9534d0..1492ab511 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinEnumEmitter.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinEnumEmitter.kt @@ -41,7 +41,19 @@ internal object KotlinEnumEmitter { val enumClassName = KotlinTypeMapper.enumTypeName(field, owner) ?: return false if (!emittedFqns.add(enumClassName.canonicalName)) return false // dedupe by enum FQN val members = readEnumValues(field) - if (members.isNullOrEmpty()) return false + if (members.isNullOrEmpty()) { + // #259: a field.enum must resolve to a NON-EMPTY @values set — the loader's ValidationPhase + // guarantees it (a root enum owns @values or fails ERR_MISSING_REQUIRED_ATTR; a field with a + // super inherits it, resolved multi-hop). Reaching here with none means a loader gap or a + // genuinely value-less enum; FAIL LOUDLY at generate-time instead of silently emitting no file + // (which surfaces only later as a downstream "unresolved reference" compile error — the exact + // failure #259 reported). + error( + "field.enum '${field.name}' on '${owner.name}' resolved to no @values — cannot emit enum " + + "class ${enumClassName.canonicalName}. A field.enum must declare or inherit a non-empty " + + "@values set." + ) + } val serializable = ClassName("kotlinx.serialization", "Serializable") val enumBuilder = TypeSpec.enumBuilder(enumClassName.simpleName) .addAnnotation(serializable) diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt index 8292e7183..cb2d05460 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableGenerator.kt @@ -16,6 +16,7 @@ import com.metaobjects.relationship.CompositionRelationship import com.metaobjects.relationship.MetaRelationship import com.metaobjects.source.MetaSource import com.metaobjects.source.RdbSource +import com.squareup.kotlinpoet.ClassName import java.io.OutputStream import java.io.PrintWriter import java.nio.file.Files @@ -516,6 +517,31 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase naming) contribute no + // import, so a single-package model's output stays byte-identical. + val crossPackageEnumImports = sortedSetOf().apply { + fun consider(cn: ClassName?) { + if (cn != null && cn.packageName.isNotEmpty() && cn.packageName != pkg) { + add("${cn.packageName}.${cn.simpleName}") + } + } + for (field in entity.metaFields) { + if (field is EnumField) consider(KotlinTypeMapper.enumTypeName(field, entity)) + } + for (field in KotlinTphPlan.collectSubtypeFields(entity, loader)) { + if (field is EnumField) consider(KotlinTypeMapper.enumTypeName(field, entity)) + } + } + val source = buildString { if (pkg.isNotEmpty()) { append("package $pkg\n\n") @@ -530,6 +556,9 @@ open class KotlinExposedTableGenerator : MultiFileDirectGeneratorBase, entity: MetaObject?): ClassName? { if (field !is EnumField) return null - // Shared-enum naming: when this field `extends` an abstract enum super (e.g. two fields - // both `extends: Priority`), ALL such fields collapse onto ONE enum class named for the - // top-most super (`Priority`) so the generated type is shared (and deduped by FQN at - // emission). Walk the super chain to the root. When there is no super, fall back to the - // per-entity `` naming. Mirrors the C#/TS/Python ports. - // Collapse onto a SHARED enum type only for a package-level ABSTRACT enum super (FR-019, - // e.g. two fields `extends: Priority`) — identified by having NO declaring object. A super - // bound to a CONCRETE entity field (e.g. a read-model projection `extends`-ing - // `ActiveNpc.status`) does NOT collapse: it falls through to the per-object naming below, - // so the projection gets its OWN `` enum in its OWN package - // (self-contained — no cross-package reference), populated with the values it inherits via - // `extends` ([KotlinEnumEmitter.readEnumValues] is inheritance-aware). Without this guard - // such a field named the enum from the super's bare short name (`status`), which `splitFqn` - // collapsed to a root-package `Status` — colliding across every entity that has a `status`. - val superRoot = resolveSuperRoot(field) - if (superRoot != null && runCatching { superRoot.declaringObject }.getOrNull() == null) { + // Shared-enum naming: when this field `extends` a package-level ABSTRACT enum super (e.g. + // two fields both `extends: Priority`), ALL such fields collapse onto ONE enum class named + // for the top-most super (`Priority`) so the generated type is shared (and deduped by FQN at + // emission). When there is no super, fall back to the per-entity `` + // naming. Mirrors the C#/TS/Python ports. + // + // The collapse decision keys on the DIRECT (immediate) super, NOT the top-most root: collapse + // onto a SHARED enum type ONLY when the field's IMMEDIATE `extends` target is a package-level + // abstract enum — identified by having NO declaring object. A field whose direct super is a + // CONCRETE entity/projection field (e.g. a read-model projection `extends`-ing `ActiveNpc.status`) + // does NOT collapse — even when that entity field itself `extends` a shared abstract enum (#259, + // the two-hop case): it falls through to the per-object naming below, so the projection gets its + // OWN `` enum in its OWN package (self-contained — no cross-package + // reference), populated with the values it inherits via `extends` ([KotlinEnumEmitter.readEnumValues] + // is inheritance-aware across ANY number of hops). Keying on the top-most root instead (an earlier + // bug) walked PAST the concrete entity super to the shared abstract root and wrongly collapsed the + // projection onto the shared type, so its per-projection enum was never emitted. Without any guard + // at all, a concrete-super field named the enum from the super's bare short name (`status`), which + // `splitFqn` collapsed to a root-package `Status` — colliding across every entity with a `status`. + val immediateSuper = field.superField + if (immediateSuper != null && runCatching { immediateSuper.declaringObject }.getOrNull() == null) { + // The DIRECT super is a package-level abstract enum → collapse onto the shared type. Name it + // for the top-most abstract root so a chain of abstract enums (abstract extends abstract) still + // collapses onto one type (resolveSuperRoot walks the abstract chain; == immediateSuper when + // there is no further super). + val superRoot = resolveSuperRoot(field) ?: immediateSuper val (superPkg, superShort) = PackageMapping.splitFqn(superRoot.name) return ClassName(superPkg, superShort.replaceFirstChar { it.uppercase() }) } diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableCrossPackageEnumTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableCrossPackageEnumTest.kt new file mode 100644 index 000000000..99dbee9a2 --- /dev/null +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinExposedTableCrossPackageEnumTest.kt @@ -0,0 +1,108 @@ +package com.metaobjects.generator.kotlin + +import com.metaobjects.metadata.ktx.loadDirectory +import com.tschuchort.compiletesting.KotlinCompilation +import com.tschuchort.compiletesting.SourceFile +import java.nio.file.Files +import java.nio.file.Path +import kotlin.io.path.isRegularFile +import kotlin.io.path.readText +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * #246 Bug 1 — [KotlinExposedTableGenerator] dropped the cross-package import for a shared + * `field.enum` (FR-019 collapse): the `enumerationByName("...", LEN, ::class)` column + * token used only the enum's simple name (via [KotlinTypeMapper.enumTypeName]?.simpleName), + * discarding the package the shared enum class actually lives in. [KotlinEntityGenerator]'s + * data class never showed the bug because KotlinPoet auto-imports the full `ClassName` it is + * handed; this generator hand-rolls its file body as a string (see class kdoc) and must add the + * import itself. + * + * Loads the shared `fixtures/codegen-conformance/enum-xpkg/input/` fixture: an abstract + * `field.enum RecordStatus` declared in `acme::common`, and two entities in DIFFERENT packages + * (`acme::orders::Order`, `acme::billing::Invoice`) whose `status` field `extends` it — so both + * generated tables reference the ONE shared `acme.common.RecordStatus` enum class. + * + * The fixture also covers the TPH-fold path (FR-017 discriminator): `acme::shipping::Shipment` + * is a discriminator base whose SUBTYPE `AirShipment` declares a subtype-only `recordStatus` + * field that `extends` the SAME cross-package `acme::common::RecordStatus`. The subtype field is + * folded into the base's single table via [KotlinTphPlan.collectSubtypeFields] — a SEPARATE + * code path from the vanilla own-field loop above, with its own import-collection walk, that a + * base-only-enum TPH fixture (every other TPH fixture in this module) never exercises. + */ +@OptIn(org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi::class) +class KotlinExposedTableCrossPackageEnumTest { + + private val corpus: Path = run { + var p: Path? = Path.of(System.getProperty("user.dir")).toAbsolutePath() + while (p != null && !Files.exists(p.resolve("fixtures/codegen-conformance"))) { + p = p.parent + } + assertTrue(p != null, "could not locate fixtures/codegen-conformance from user.dir") + p!!.resolve("fixtures/codegen-conformance") + } + + private fun compile(outDir: Path): KotlinCompilation.Result { + val sources = Files.walk(outDir).filter { it.isRegularFile() }.sorted().toList() + .map { path -> SourceFile.kotlin(path.parent.relativize(path).toString().replace('/', '_'), path.readText()) } + return KotlinCompilation().apply { + this.sources = sources + inheritClassPath = true + messageOutputStream = System.out + }.compile() + } + + private fun readGenerated(outDir: Path, relative: String): String { + val file = outDir.resolve(relative) + assertTrue(Files.exists(file), "expected generated file $file; files=${Files.walk(outDir).toList()}") + return file.readText() + } + + @Test fun `table generator imports a cross-package shared enum`() { + val outDir = Files.createTempDirectory("ktbl-xpkg-enum-") + try { + val loader = loadDirectory("enum-xpkg", corpus.resolve("enum-xpkg/input")) + + for (gen in listOf(KotlinEntityGenerator(), KotlinExposedTableGenerator())) { + gen.setArgs(mapOf("outputDir" to outDir.toString())) + gen.execute(loader) + } + + // The shared enum class itself lands in the SUPER's package (acme.common). + val recordStatus = readGenerated(outDir, "acme/common/RecordStatus.kt") + assertTrue("enum class RecordStatus" in recordStatus, recordStatus) + + // OrderTable.kt is generated in package acme.orders and references the shared enum. + val orderTable = readGenerated(outDir, "acme/orders/OrderTable.kt") + assertTrue(orderTable.contains("import acme.common.RecordStatus"), + "OrderTable must import the cross-package shared enum; saw:\n$orderTable") + assertTrue(orderTable.contains("RecordStatus::class"), + "OrderTable still references the enum in enumerationByName; saw:\n$orderTable") + + val invoiceTable = readGenerated(outDir, "acme/billing/InvoiceTable.kt") + assertTrue(invoiceTable.contains("import acme.common.RecordStatus"), + "InvoiceTable must import the cross-package shared enum; saw:\n$invoiceTable") + assertTrue(invoiceTable.contains("RecordStatus::class"), + "InvoiceTable still references the enum in enumerationByName; saw:\n$invoiceTable") + + // TPH-fold path: the base ShipmentTable.kt (NOT AirShipmentTable — TPH subtypes emit + // no table of their own) must carry the import for the SUBTYPE-only cross-package + // enum folded into its single table. + val shipmentTable = readGenerated(outDir, "acme/shipping/ShipmentTable.kt") + assertTrue(shipmentTable.contains("import acme.common.RecordStatus"), + "TPH-folded ShipmentTable must import the cross-package shared enum; saw:\n$shipmentTable") + assertTrue(shipmentTable.contains("val recordStatus = enumerationByName(\"record_status\", ${KotlinTypeMapper.ENUM_VARCHAR_LEN}, RecordStatus::class).nullable()"), + "TPH-folded status column must reference the shared enum; saw:\n$shipmentTable") + assertTrue(!Files.exists(outDir.resolve("acme/shipping/AirShipmentTable.kt")), + "a TPH subtype must NOT emit its own table") + + // Compile-gate: the whole generated tree must compile (catches any import variant). + val result = compile(outDir) + assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode, result.messages) + } finally { + outDir.toFile().deleteRecursively() + } + } +} diff --git a/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionTwoHopEnumTest.kt b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionTwoHopEnumTest.kt new file mode 100644 index 000000000..8a42d6e6e --- /dev/null +++ b/server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinProjectionTwoHopEnumTest.kt @@ -0,0 +1,110 @@ +package com.metaobjects.generator.kotlin + +import com.metaobjects.metadata.ktx.loadString +import com.tschuchort.compiletesting.KotlinCompilation +import com.tschuchort.compiletesting.SourceFile +import java.nio.file.Files +import java.nio.file.Path +import kotlin.io.path.isRegularFile +import kotlin.io.path.readText +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * #259 — a `field.enum` that inherits its `@values` through TWO `extends` hops + * (projection.status → entity.status → shared-abstract enum) must still generate the + * projection's OWN per-projection enum, populated with the transitively-inherited members. + * + * Sibling of #246 (same shared-enum feature area). The one-hop case + * ([KotlinProjectionExtendsInheritanceTest]) already worked because the entity field carried + * its own inline `@values`. The two-hop case regressed: [KotlinTypeMapper.enumTypeName] + * decided "collapse onto the shared enum" from the TOP-MOST super (its private + * `resolveSuperRoot`), so a projection field walked PAST its concrete entity super to the root + * abstract enum (no declaring object) and wrongly collapsed to the shared type — the + * per-projection enum was never emitted, and every consumer of that column failed to resolve + * the (now-absent) per-projection type. Values themselves were never the problem: + * `getMetaAttr(@values)` is inheritance-aware across multiple hops. + * + * The fix keys the collapse decision on the IMMEDIATE super: only a field whose direct + * `extends` target is a package-level abstract enum (no declaring object) collapses onto the + * shared type. A field whose direct super is a CONCRETE entity/projection field gets its own + * `` enum, populated with the depth-2-inherited members. + */ +@OptIn(org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi::class) +class KotlinProjectionTwoHopEnumTest { + + // SharedStatus (abstract, root) ← Program.status (entity, hop 1) ← ProgramView.status + // (projection, hop 2). ProgramView.status carries NO own @values — they resolve two hops up. + private val fixture = """{ + "metadata.root": { "package": "acme::commerce", "children": [ + { "field.enum": { "name": "SharedStatus", "abstract": true, "@required": true, + "@values": ["DRAFT", "LIVE", "ARCHIVED"] } }, + { "object.entity": { "name": "Program", "children": [ + { "field.string": { "name": "id", "@required": true, "@dbColumnType": "uuid" } }, + { "field.enum": { "name": "status", "extends": "acme::commerce::SharedStatus", "@required": true } }, + { "source.rdb": { "@table": "programs" } }, + { "identity.primary": { "name": "id", "@fields": "id" } } + ] } }, + { "object.projection": { "name": "ProgramView", "children": [ + { "source.rdb": { "@kind": "view", "@view": "v_program" } }, + { "field.string": { "name": "id", "extends": "acme::commerce::Program.id", "@required": true } }, + { "field.enum": { "name": "status", "extends": "acme::commerce::Program.status", "@required": true } }, + { "identity.primary": { "name": "id", "extends": "acme::commerce::Program.id" } } + ] } } + ] } + }""".trimIndent() + + private fun compile(outDir: Path): KotlinCompilation.Result { + val sources = Files.walk(outDir).filter { it.isRegularFile() }.sorted().toList() + .map { path -> SourceFile.kotlin(path.parent.relativize(path).toString().replace('/', '_'), path.readText()) } + return KotlinCompilation().apply { + this.sources = sources + inheritClassPath = true + messageOutputStream = System.out + }.compile() + } + + @Test fun `two-hop projection enum materializes its own per-projection type`() { + val outDir = Files.createTempDirectory("k2hop-enum-") + try { + val loader = loadString("two-hop-enum", fixture) + for (gen in listOf(KotlinEntityGenerator(), KotlinExposedTableGenerator())) { + gen.setArgs(mapOf("outputDir" to outDir.toString())) + gen.execute(loader) + } + + // The shared enum is emitted once (the entity field DOES collapse onto it). + assertTrue(Files.exists(outDir.resolve("acme/commerce/SharedStatus.kt")), + "shared SharedStatus.kt must be emitted; files=${Files.walk(outDir).toList()}") + + // The projection gets its OWN enum, populated with the DEPTH-2-inherited values. + val viewEnum = outDir.resolve("acme/commerce/ProgramViewStatus.kt") + assertTrue(Files.exists(viewEnum), + "#259: projection enum ProgramViewStatus.kt must be materialized (values inherited " + + "through TWO extends hops); files=${Files.walk(outDir).toList()}") + val viewEnumSrc = viewEnum.readText() + for (m in listOf("DRAFT", "LIVE", "ARCHIVED")) + assertTrue(m in viewEnumSrc, "inherited member $m must be present; saw:\n$viewEnumSrc") + + // The projection table references its OWN enum, NOT the collapsed shared one. + val viewTable = outDir.resolve("acme/commerce/ProgramViewTable.kt").readText() + assertTrue("ProgramViewStatus::class" in viewTable, + "projection table must reference its own ProgramViewStatus enum; saw:\n$viewTable") + assertFalse("SharedStatus::class" in viewTable, + "#259: projection table must NOT collapse to the shared SharedStatus enum; saw:\n$viewTable") + + // The ENTITY table still collapses onto the shared enum (byte-identity guardrail). + val programTable = outDir.resolve("acme/commerce/ProgramTable.kt").readText() + assertTrue("SharedStatus::class" in programTable, + "entity table must still reference the shared enum; saw:\n$programTable") + + // Compile-gate: the whole generated tree must resolve. + val result = compile(outDir) + assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode, result.messages) + } finally { + outDir.toFile().deleteRecursively() + } + } +} diff --git a/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java b/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java index 00ee584ca..e2c0ce614 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java +++ b/server/java/metadata/src/main/java/com/metaobjects/ErrorCode.java @@ -412,6 +412,15 @@ public enum ErrorCode { */ ERR_ORIGIN_UNDER_SQL_BODY, + /** + * #246: a {@code field.enum} both extends a shared package-level abstract + * enum and declares its own {@code @values}. One shared enum type has one + * member set — the own {@code @values} would be silently dropped in + * codegen. Remove the own {@code @values} to inherit the shared set, or + * extend a concrete (non-shared) enum instead. + */ + ERR_ENUM_EXTENDS_VALUES_CONFLICT, + /** An internal loader error with no stable error code. */ ERR_UNKNOWN, } diff --git a/server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java b/server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java index af601beea..e5a7dcab0 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java +++ b/server/java/metadata/src/main/java/com/metaobjects/loader/ValidationPhase.java @@ -523,6 +523,24 @@ private static void validateEnumNode(MetaData node) { + " (e.g. [\"DRAFT\",\"PUBLISHED\"])", ErrorCode.ERR_BAD_ATTR_VALUE, node.getSource()); } + // #246: own @values + extends a shared root-level abstract enum is a + // conflict — one shared enum type has one member set, so codegen's + // shared-enum collapse would silently drop this field's own @values in + // favor of the shared type's. Own-attrs-only (matches the check above): + // only fires when THIS node declares @values itself, not when it merely + // inherits. + MetaData sup = node.getSuperData(); + if (sup != null && isAbstract(sup) && sup.getParent() instanceof MetaRoot) { + throw new MetaDataException( + ErrorMessageConstants.ERR_ENUM_EXTENDS_VALUES_CONFLICT + + ": field.enum '" + node.getName() + + "' extends shared abstract enum '" + sup.getName() + + "' AND declares its own @values - a shared enum's member set is" + + " owned by the shared declaration; remove the own @values to" + + " inherit it, or extend a non-shared enum instead", + ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT, node.getSource()); + } + // Own @values present and valid — required check not needed. validateEnumFr011Attrs(node); return; diff --git a/server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java b/server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java index b53b7d7af..630233ede 100644 --- a/server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java +++ b/server/java/metadata/src/main/java/com/metaobjects/util/ErrorMessageConstants.java @@ -246,6 +246,15 @@ private ErrorMessageConstants() { */ public static final String ERR_ORIGIN_UNDER_SQL_BODY = "ERR_ORIGIN_UNDER_SQL_BODY"; + /** + * #246: a {@code field.enum} both extends a shared package-level abstract + * enum and declares its own {@code @values}. One shared enum type has one + * member set — the own {@code @values} would be silently dropped in + * codegen. + * Cross-language contract: {@code ERR_ENUM_EXTENDS_VALUES_CONFLICT}. + */ + public static final String ERR_ENUM_EXTENDS_VALUES_CONFLICT = "ERR_ENUM_EXTENDS_VALUES_CONFLICT"; + /** * #208 (design doc §5 R6) warning: an {@code origin.*}-bearing (derived) own * field lives under a host object that declares an {@code @unmanaged} source. diff --git a/server/java/metadata/src/test/java/com/metaobjects/loader/EnumExtendsValuesConflictTest.java b/server/java/metadata/src/test/java/com/metaobjects/loader/EnumExtendsValuesConflictTest.java new file mode 100644 index 000000000..3e01da379 --- /dev/null +++ b/server/java/metadata/src/test/java/com/metaobjects/loader/EnumExtendsValuesConflictTest.java @@ -0,0 +1,151 @@ +/* + * Copyright 2003 Doug Mealing LLC dba Meta Objects + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.metaobjects.loader; + +import com.metaobjects.ErrorCode; +import com.metaobjects.MetaDataException; +import com.metaobjects.registry.SharedRegistryTestBase; +import org.junit.Test; + +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * #246 — a {@code field.enum} that both {@code extends} a shared root-level + * abstract enum AND declares its own {@code @values} must fail to load with + * {@code ERR_ENUM_EXTENDS_VALUES_CONFLICT}: one shared enum type has one + * member set, so the own {@code @values} would be silently dropped by + * codegen's shared-enum collapse. + * + *

Mirrors the TS reference test + * ({@code server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts}) + * and the Java loader also covers Kotlin (Kotlin inherits the JVM loader).

+ * + *

Three cases (the third pins the "root-level" clause of the predicate — + * dropping the {@code sup.getParent() instanceof MetaRoot} check would let a + * non-root abstract super go unrejected):

+ *
    + *
  1. CONFLICT — extends a root-level (metadata.root child) abstract enum, + * and also declares its own {@code @values}.
    2. + *
  2. LEGAL — extends a CONCRETE (non-abstract) enum, and also declares its + * own {@code @values}.
    3. + *
  3. LEGAL — extends an ABSTRACT but NON-ROOT enum (declared as a child of + * an {@code object.entity}, not the shared package level), and also + * declares its own {@code @values}.
    4. + *
+ */ +public class EnumExtendsValuesConflictTest extends SharedRegistryTestBase { + + private MetaDataLoader newTestLoader() { + return createTestLoader("EnumExtendsValuesConflictTest", Collections.emptyList()); + } + + private MetaDataLoader loadThrough(String canonical, String id) { + MetaDataLoader loader = newTestLoader(); + loader.load(List.of(new InMemoryStringSource(canonical, id))); + return loader; + } + + // ------------------------------------------------------------------------ + // 1 — CONFLICT: root-level abstract super + own @values. + // ------------------------------------------------------------------------ + + private static final String CONFLICT_ROOT_LEVEL_ABSTRACT_SUPER = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"field.enum\": { \"name\": \"Status\", \"@isAbstract\": true," + + " \"@values\": [\"A\", \"B\"] } }," + + " { \"object.entity\": { \"name\": \"Order\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.enum\": { \"name\": \"status\", \"extends\": \"acme::Status\"," + + " \"@values\": [\"A\", \"B\", \"C\"] } }," + + " { \"identity.primary\": { \"name\": \"id\", \"@fields\": \"id\" } }" + + " ] } }" + + "] } }"; + + @Test + public void conflictExtendsRootLevelAbstractEnumWithOwnValues() { + try { + loadThrough(CONFLICT_ROOT_LEVEL_ABSTRACT_SUPER, "conflict-root-abstract.json"); + fail("Expected MetaDataException for field.enum extending a shared root-level" + + " abstract enum while also declaring its own @values"); + } catch (MetaDataException e) { + assertTrue("must signal ERR_ENUM_EXTENDS_VALUES_CONFLICT: " + e.getMessage(), + signalsEnumExtendsValuesConflict(e)); + assertTrue("message should name the conflicting field 'status': " + e.getMessage(), + e.getMessage().contains("status")); + } + } + + // ------------------------------------------------------------------------ + // 2 — LEGAL: concrete (non-abstract) super + own @values. + // ------------------------------------------------------------------------ + + private static final String LEGAL_CONCRETE_SUPER = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"field.enum\": { \"name\": \"Status\"," + + " \"@values\": [\"A\", \"B\"] } }," + + " { \"object.entity\": { \"name\": \"Order\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.enum\": { \"name\": \"status\", \"extends\": \"acme::Status\"," + + " \"@values\": [\"A\", \"B\", \"C\"] } }," + + " { \"identity.primary\": { \"name\": \"id\", \"@fields\": \"id\" } }" + + " ] } }" + + "] } }"; + + @Test + public void legalExtendsConcreteEnumWithOwnValues() { + MetaDataLoader loader = loadThrough(LEGAL_CONCRETE_SUPER, "legal-concrete-super.json"); + // Loaded cleanly — no ERR_ENUM_EXTENDS_VALUES_CONFLICT for a concrete super. + assertTrue("expected no warnings", loader.getWarnings().isEmpty()); + } + + // ------------------------------------------------------------------------ + // 3 — LEGAL: abstract but NON-ROOT super (nested inside an object) + own @values. + // ------------------------------------------------------------------------ + + private static final String LEGAL_NON_ROOT_ABSTRACT_SUPER = + "{ \"metadata.root\": { \"package\": \"acme\", \"children\": [" + + " { \"object.entity\": { \"name\": \"Container\", \"@isAbstract\": true, \"children\": [" + + " { \"field.enum\": { \"name\": \"kind\", \"@isAbstract\": true," + + " \"@values\": [\"X\", \"Y\"] } }" + + " ] } }," + + " { \"object.entity\": { \"name\": \"Order\", \"children\": [" + + " { \"field.long\": { \"name\": \"id\" } }," + + " { \"field.enum\": { \"name\": \"status\", \"extends\": \"acme::Container.kind\"," + + " \"@values\": [\"X\", \"Y\", \"Z\"] } }," + + " { \"identity.primary\": { \"name\": \"id\", \"@fields\": \"id\" } }" + + " ] } }" + + "] } }"; + + @Test + public void legalExtendsNonRootAbstractEnumWithOwnValues() { + // Pins the `sup.getParent() instanceof MetaRoot` clause of the predicate — + // dropping it would incorrectly reject this nested-abstract-super case. + MetaDataLoader loader = loadThrough(LEGAL_NON_ROOT_ABSTRACT_SUPER, "legal-non-root-abstract.json"); + assertTrue("expected no warnings", loader.getWarnings().isEmpty()); + } + + /** True if the exception signals ERR_ENUM_EXTENDS_VALUES_CONFLICT via code or message. */ + private static boolean signalsEnumExtendsValuesConflict(MetaDataException e) { + boolean byCode = e.getCode().map(c -> c == ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT).orElse(false); + boolean byMsg = e.getMessage() != null + && e.getMessage().contains("ERR_ENUM_EXTENDS_VALUES_CONFLICT"); + return byCode || byMsg; + } +} diff --git a/server/python/src/metaobjects/errors.py b/server/python/src/metaobjects/errors.py index 470d613c8..535e14360 100644 --- a/server/python/src/metaobjects/errors.py +++ b/server/python/src/metaobjects/errors.py @@ -180,6 +180,11 @@ class ErrorCode(str, Enum): ERR_SQL_BODY_WITH_UNMANAGED = "ERR_SQL_BODY_WITH_UNMANAGED" ERR_SQL_BODY_ON_WRITABLE_KIND = "ERR_SQL_BODY_ON_WRITABLE_KIND" ERR_ORIGIN_UNDER_SQL_BODY = "ERR_ORIGIN_UNDER_SQL_BODY" + # #246 — a field.enum both extends a shared package-level abstract enum and + # declares its own @values. One shared enum type has one member set — the + # own @values would be silently dropped in codegen. Remove the own @values + # to inherit the shared set, or extend a concrete (non-shared) enum instead. + ERR_ENUM_EXTENDS_VALUES_CONFLICT = "ERR_ENUM_EXTENDS_VALUES_CONFLICT" ERR_UNKNOWN = "ERR_UNKNOWN" diff --git a/server/python/src/metaobjects/loader/validation_passes.py b/server/python/src/metaobjects/loader/validation_passes.py index 6ef486f1b..90435dcb6 100644 --- a/server/python/src/metaobjects/loader/validation_passes.py +++ b/server/python/src/metaobjects/loader/validation_passes.py @@ -68,6 +68,7 @@ TYPE_IDENTITY, TYPE_INDEX, TYPE_LAYOUT, + TYPE_METADATA, TYPE_OBJECT, TYPE_ORIGIN, TYPE_RELATIONSHIP, @@ -599,6 +600,33 @@ def _validate_enum_values( ) ) + # #246: own @values AND extends a shared package-level abstract enum — + # one shared enum type has one member set, so the own @values would be + # silently dropped by the shared-enum codegen collapse. "Shared" means + # the resolved super is abstract AND declared at metadata-root (its + # parent is the metadata.root node, not nested inside an object) — a + # concrete super, or a non-root abstract super (e.g. nested inside an + # object), is legal and not flagged. Mirrors the TS reference + # (attr-schema-validate.ts). + sup = node.super_data + if ( + sup is not None + and sup.is_abstract + and sup.parent is not None + and sup.parent.type == TYPE_METADATA + ): + errors.append( + MetaError( + f"{label} declares its own '@{FIELD_ATTR_VALUES}' but extends " + f"a shared package-level abstract enum — one shared enum type " + f"has one member set. Remove the own '@{FIELD_ATTR_VALUES}' to " + f"inherit the shared set, or extend a concrete (non-shared) " + f"enum instead", + ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT, + envelope=node.source, + ) + ) + def _effective_enum_values(node: MetaData) -> list[str]: """The effective ``@values`` members of an enum node (own or inherited via diff --git a/server/python/tests/loader/__init__.py b/server/python/tests/loader/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/server/python/tests/loader/test_enum_extends_values_conflict.py b/server/python/tests/loader/test_enum_extends_values_conflict.py new file mode 100644 index 000000000..2b30aa90e --- /dev/null +++ b/server/python/tests/loader/test_enum_extends_values_conflict.py @@ -0,0 +1,167 @@ +"""#246 — field.enum extends-a-shared-enum-AND-declares-own-@values conflict. + +Mirrors the TS reference test +(server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts): +a field.enum that both `extends` a shared PACKAGE-LEVEL abstract enum AND +declares its own `@values` must fail to load with +ERR_ENUM_EXTENDS_VALUES_CONFLICT — one shared enum type has one member set, so +the own `@values` would be silently dropped by the shared-enum codegen +collapse. + +Three cases (the third pins the "root-level" clause of the predicate — Task 3's +review required it: dropping `sup.parent.type == TYPE_METADATA` would let a +non-root abstract super go unrejected): + + 1. CONFLICT — extends a root-level (metadata.root child) abstract enum, and + also declares its own @values. + 2. LEGAL — extends a CONCRETE (non-abstract) enum, and also declares its own + @values. + 3. LEGAL — extends an ABSTRACT but NON-ROOT enum (declared as a child of an + object.entity, not the shared package level), and also declares its own + @values. +""" +from __future__ import annotations + +import json +import os +import tempfile +from pathlib import Path + +from metaobjects import MetaDataLoader +from metaobjects.core_types import core_provider +from metaobjects.errors import ErrorCode + + +def _load_codes(doc: dict) -> list[ErrorCode]: + """Load a doc dict via a temp file; return the loader's error codes.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, "meta.test.json") + Path(path).write_text(json.dumps(doc)) + result = MetaDataLoader.from_directory(tmpdir, providers=[core_provider]) + return [e.code for e in result.errors] + + +def test_conflict_extends_root_level_abstract_enum_with_own_values() -> None: + """Extending a root-level (shared, package-level) abstract enum while also + declaring own @values must fail with ERR_ENUM_EXTENDS_VALUES_CONFLICT.""" + doc = { + "metadata.root": { + "package": "acme", + "children": [ + { + "field.enum": { + "name": "Status", + "abstract": True, + "@values": ["A", "B"], + } + }, + { + "object.entity": { + "name": "Order", + "children": [ + {"field.long": {"name": "id"}}, + { + "field.enum": { + "name": "status", + "extends": "acme::Status", + "@values": ["A", "B", "C"], + } + }, + {"identity.primary": {"@fields": "id"}}, + ], + } + }, + ], + } + } + codes = _load_codes(doc) + assert codes.count(ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT) == 1, ( + f"Expected exactly one ERR_ENUM_EXTENDS_VALUES_CONFLICT; got {codes}" + ) + + +def test_legal_extends_concrete_enum_with_own_values() -> None: + """Extending a CONCRETE (non-abstract) enum while also declaring own + @values is legal — no ERR_ENUM_EXTENDS_VALUES_CONFLICT.""" + doc = { + "metadata.root": { + "package": "acme", + "children": [ + { + "field.enum": { + "name": "Status", + "@values": ["A", "B"], + } + }, + { + "object.entity": { + "name": "Order", + "children": [ + {"field.long": {"name": "id"}}, + { + "field.enum": { + "name": "status", + "extends": "acme::Status", + "@values": ["A", "B", "C"], + } + }, + {"identity.primary": {"@fields": "id"}}, + ], + } + }, + ], + } + } + codes = _load_codes(doc) + assert ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT not in codes, ( + f"Did not expect ERR_ENUM_EXTENDS_VALUES_CONFLICT; got {codes}" + ) + + +def test_legal_extends_non_root_abstract_enum_with_own_values() -> None: + """Extending an ABSTRACT but NON-ROOT enum (nested inside an object, not + declared at the shared package level) while also declaring own @values is + legal — no ERR_ENUM_EXTENDS_VALUES_CONFLICT. Pins the `sup.parent.type == + TYPE_METADATA` clause of the predicate.""" + doc = { + "metadata.root": { + "package": "acme", + "children": [ + { + "object.entity": { + "name": "Container", + "abstract": True, + "children": [ + { + "field.enum": { + "name": "kind", + "abstract": True, + "@values": ["X", "Y"], + } + } + ], + } + }, + { + "object.entity": { + "name": "Order", + "children": [ + {"field.long": {"name": "id"}}, + { + "field.enum": { + "name": "status", + "extends": "acme::Container.kind", + "@values": ["X", "Y", "Z"], + } + }, + {"identity.primary": {"@fields": "id"}}, + ], + } + }, + ], + } + } + codes = _load_codes(doc) + assert ErrorCode.ERR_ENUM_EXTENDS_VALUES_CONFLICT not in codes, ( + f"Did not expect ERR_ENUM_EXTENDS_VALUES_CONFLICT; got {codes}" + ) diff --git a/server/typescript/packages/metadata/src/attr-schema-validate.ts b/server/typescript/packages/metadata/src/attr-schema-validate.ts index 96e93d626..479b5e8df 100644 --- a/server/typescript/packages/metadata/src/attr-schema-validate.ts +++ b/server/typescript/packages/metadata/src/attr-schema-validate.ts @@ -45,7 +45,7 @@ import type { MetaData } from "./shared/meta-data.js"; import { ParseError } from "./errors.js"; import type { AttrSchema, TypeRegistry } from "./registry.js"; import { childRuleMatches } from "./registry.js"; -import { TYPE_FIELD } from "./shared/base-types.js"; +import { TYPE_FIELD, TYPE_METADATA } from "./shared/base-types.js"; import { ATTR_SUBTYPE_PROPERTIES } from "./core/attr/attr-constants.js"; import { FIELD_SUBTYPE_ENUM, @@ -346,6 +346,25 @@ function validateNode( } } } + + // #246: a field.enum extending a shared package-level abstract enum + // (a root-level abstract field — one whose parent is the metadata root, + // not an object) that ALSO declares its own @values is a conflict: one + // shared enum type has one member set, so codegen's shared-enum collapse + // would silently drop this field's own @values in favor of the shared + // type's. Own-attrs-only (matches the rest of Check 4): only fires when + // THIS node declares @values itself, not when it merely inherits. + const sup = node.superData; + if (sup !== undefined && sup.isAbstract && sup.parent?.type === TYPE_METADATA) { + errors.push( + new ParseError( + `${nodeLabel(node)} extends shared abstract enum '${nodeLabel(sup)}' AND declares its own ` + + `'@${FIELD_ATTR_VALUES}' — a shared enum's member set is owned by the shared declaration; ` + + `remove the own '@${FIELD_ATTR_VALUES}' to inherit it, or extend a non-shared enum instead.`, + { code: "ERR_ENUM_EXTENDS_VALUES_CONFLICT", source: node.source }, + ), + ); + } } // --- Check 5 (FR-011): enum fallback attrs must be a member of @values --- diff --git a/server/typescript/packages/metadata/src/errors.ts b/server/typescript/packages/metadata/src/errors.ts index e937b07ce..ea8c52982 100644 --- a/server/typescript/packages/metadata/src/errors.ts +++ b/server/typescript/packages/metadata/src/errors.ts @@ -195,6 +195,11 @@ export const ERROR_CODES = [ // two sources of truth for the same body (the synthesized derivation/WHERE // vs. the author's verbatim SQL). Fail-closed. "ERR_ORIGIN_UNDER_SQL_BODY", + // #246 — a field.enum both extends a shared package-level abstract enum and + // declares its own @values. One shared enum type has one member set — the + // own @values would be silently dropped in codegen. Remove the own @values + // to inherit the shared set, or extend a concrete (non-shared) enum instead. + "ERR_ENUM_EXTENDS_VALUES_CONFLICT", "ERR_UNKNOWN", ] as const; diff --git a/server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts b/server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts new file mode 100644 index 000000000..40d6623c3 --- /dev/null +++ b/server/typescript/packages/metadata/test/enum-extends-values-conflict.test.ts @@ -0,0 +1,137 @@ +// #246 — a field.enum that both extends a shared package-level abstract enum +// AND declares its own @values must fail to load with +// ERR_ENUM_EXTENDS_VALUES_CONFLICT: one shared enum type has one member set, +// so own @values would be silently dropped by the shared-enum codegen collapse. + +import { describe, it, expect } from "bun:test"; +import { MetaDataLoader } from "../src/loader/meta-data-loader.js"; +import { InMemoryStringSource } from "../src/loader/meta-data-source.js"; + +async function load(doc: unknown) { + const loader = new MetaDataLoader(); + return loader.load([new InMemoryStringSource(JSON.stringify(doc))]); +} + +describe("field.enum — extends a shared abstract enum AND declares own @values", () => { + it("emits ERR_ENUM_EXTENDS_VALUES_CONFLICT on the 'status' field when it extends a root-level abstract enum and also declares own @values", async () => { + const { errors } = await load({ + "metadata.root": { + package: "acme", + children: [ + { + "field.enum": { + name: "Status", + abstract: true, + "@values": ["A", "B"], + }, + }, + { + "object.entity": { + name: "Order", + children: [ + { "field.long": { name: "id" } }, + { + "field.enum": { + name: "status", + extends: "acme::Status", + "@values": ["A", "B", "C"], + }, + }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + ], + }, + }, + ], + }, + }); + + const conflictErrors = errors.filter( + (e) => (e as { code?: string }).code === "ERR_ENUM_EXTENDS_VALUES_CONFLICT", + ); + expect(conflictErrors).toHaveLength(1); + expect(conflictErrors[0]!.message).toContain("status"); + }); + + it("does NOT emit ERR_ENUM_EXTENDS_VALUES_CONFLICT when the field extends a CONCRETE (non-abstract) enum and also declares own @values", async () => { + const { errors } = await load({ + "metadata.root": { + package: "acme", + children: [ + { + "field.enum": { + name: "Status", + "@values": ["A", "B"], + }, + }, + { + "object.entity": { + name: "Order", + children: [ + { "field.long": { name: "id" } }, + { + "field.enum": { + name: "status", + extends: "acme::Status", + "@values": ["A", "B", "C"], + }, + }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + ], + }, + }, + ], + }, + }); + + const conflictErrors = errors.filter( + (e) => (e as { code?: string }).code === "ERR_ENUM_EXTENDS_VALUES_CONFLICT", + ); + expect(conflictErrors).toHaveLength(0); + }); + + it("does NOT emit ERR_ENUM_EXTENDS_VALUES_CONFLICT when the field extends an ABSTRACT but NON-ROOT enum (nested inside an object, not the shared package level) and also declares own @values", async () => { + const { errors } = await load({ + "metadata.root": { + package: "acme", + children: [ + { + "object.entity": { + name: "Container", + abstract: true, + children: [ + { + "field.enum": { + name: "kind", + abstract: true, + "@values": ["X", "Y"], + }, + }, + ], + }, + }, + { + "object.entity": { + name: "Order", + children: [ + { "field.long": { name: "id" } }, + { + "field.enum": { + name: "status", + extends: "acme::Container.kind", + "@values": ["X", "Y", "Z"], + }, + }, + { "identity.primary": { "name": "id", "@fields": "id" } }, + ], + }, + }, + ], + }, + }); + + const conflictErrors = errors.filter( + (e) => (e as { code?: string }).code === "ERR_ENUM_EXTENDS_VALUES_CONFLICT", + ); + expect(conflictErrors).toHaveLength(0); + }); +}); diff --git a/spec/README.md b/spec/README.md index 56099e53d..94cd35ac3 100644 --- a/spec/README.md +++ b/spec/README.md @@ -23,7 +23,7 @@ JSON. Metadata files live in `metaobjects/` at project root, organized by domain ## How parity is enforced -Each implementation passes the same conformance fixtures under [`fixtures/conformance/`](../fixtures/conformance/) -- 249 shared test cases, plus a `CAPABILITIES.json` manifest (which optional behaviours a port declares) and an `ERROR-CODES.json` ledger (the shared error-code vocabulary). It is one of 19 shared corpora; the full corpus x port matrix is in [`docs/CONFORMANCE.md`](../docs/CONFORMANCE.md). Each fixture is a directory with `input/` metadata files and either an `expected.json` (happy-path canonical output) or `expected-errors.json` (error case). The Loader is expected to either produce byte-identical canonical output, or emit the exact set of expected error messages. Without this shared test surface, each language's Loader behavior would drift independently -- overlay merging working one way in TypeScript and a subtly different way in Java, and nobody noticing until a real consumer hit the mismatch. Conformance fixtures make parity testable, not aspirational. Format and contract in [`conformance-tests.md`](conformance-tests.md). +Each implementation passes the same conformance fixtures under [`fixtures/conformance/`](../fixtures/conformance/) -- 258 shared test cases, plus a `CAPABILITIES.json` manifest (which optional behaviours a port declares) and an `ERROR-CODES.json` ledger (the shared error-code vocabulary). It is one of 19 shared corpora; the full corpus x port matrix is in [`docs/CONFORMANCE.md`](../docs/CONFORMANCE.md). Each fixture is a directory with `input/` metadata files and either an `expected.json` (happy-path canonical output) or `expected-errors.json` (error case). The Loader is expected to either produce byte-identical canonical output, or emit the exact set of expected error messages. Without this shared test surface, each language's Loader behavior would drift independently -- overlay merging working one way in TypeScript and a subtly different way in Java, and nobody noticing until a real consumer hit the mismatch. Conformance fixtures make parity testable, not aspirational. Format and contract in [`conformance-tests.md`](conformance-tests.md). ## Documents in this directory diff --git a/spec/roadmap.md b/spec/roadmap.md index 42ae3ddfc..f7ddb6121 100644 --- a/spec/roadmap.md +++ b/spec/roadmap.md @@ -75,7 +75,7 @@ _(FR-032 was developed under the working number "FR-026" — see commit history; ### Cross-port conformance corpora (every port runs the shared corpus) -- **Metamodel conformance** — `fixtures/conformance/` (249 fixtures + CAPABILITIES + ERROR-CODES manifests). TS / C# / Java / Python all green; Kotlin inherits via `metadata-ktx`. Per-corpus counts: `docs/CONFORMANCE.md`. +- **Metamodel conformance** — `fixtures/conformance/` (258 fixtures + CAPABILITIES + ERROR-CODES manifests). TS / C# / Java / Python all green; Kotlin inherits via `metadata-ktx`. Per-corpus counts: `docs/CONFORMANCE.md`. - **Render conformance** — `fixtures/render-conformance/`. TS / C# / Java / Kotlin / Python byte-identical. - **Persistence conformance** — `fixtures/persistence-conformance/` (24 query + 6 migration scenarios). Every port runs the **query** scenarios against Testcontainers Postgres, each provisioning its test DB from the committed, TS-produced `canonical/schema.postgres.sql` (Derby was dropped for the cross-port query corpus, ADR-0015). The **migration** scenarios are TS-only, since TS owns schema. - **API-contract conformance** — `fixtures/api-contract-conformance/` (26 core + tph/m2m/jsonb/write-through sub-corpora). All five ports run it in BOTH lanes — a hand-rolled reference server and the port's own GENERATED API artifact booted over HTTP.