diff --git a/CHANGELOG.md b/CHANGELOG.md index b977a8710..f9acf8b29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,62 @@ 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] + +**Coordinated PATCH (all 5 ports)** — will release across npm / PyPI / Maven Central / NuGet +together when cut; #228 is a cross-port fix. Existing `meta gen` output is byte-identical for +every model without a cross-package short-name collision. + +### Fixed — extract/output-parser tier and build-time `@payloadRef`/`@responseRef` resolution under a cross-package payload collision (#228) + +ADR-0044 (#219/#220) gave every port's *payload-record* emitter collision-scoped naming: two +cross-package `object.value`s sharing a bare short name (`acme::alpha::Note` / +`acme::beta::Note`) now each emit a distinct, package-qualified type instead of silently +colliding. The **extract tier** — the `template.output`/`template.toolcall` parser generator +that reads a rendered payload back off an LLM response — was a sibling generator ADR-0044 +flagged as a recurrence risk but did not fix: it named and imported nested value-object +classes by bare short name, so under a real collision it referenced a class the payload +generator no longer emits. This half is **latent** — the only shipped collision fixture +(`fixtures/template-output-render-conformance/xpkg-collision/`) used `@format: html`, which the +extract tier never runs against (it only engages for `@format: json | xml`) — closed by a new +`xpkg-collision-json` fixture plus a hardcoded per-port collision test, all five ports. + +A second, **reachable** bug surfaced auditing the fix: several build-time +`@payloadRef`/`@responseRef` resolvers — feeding the extract tier, the render-helper / +output-prompt generators, and (JVM ports) the `meta:verify` template-drift check — resolved a +bare ref package-blind (first-match by load order, or a bare-tail fallback), while the loader +validates the same ref package-local per ADR-0042. Under a genuine cross-package bare +collision this meant the loader accepted object A while codegen silently emitted against +object B. Fixed by routing every one of these resolvers through each port's canonical +package-local resolver, threading the referring template's package: Python +(`resolve_payload_vo` + `render_helper_generator` + `@responseRef`), C# (`VerifyCommand` / +`BuildPayloadFieldTree`), Java (five call sites consolidated into a new `SpringNaming` helper, +plus a `LlmTraceHelperGenerator` bare-tail fix), Kotlin (`KotlinGenUtil` — reusing the loader's +own `SymbolTable` — plus the render-helper, output-prompt, and api-docs generators, and the +shared `codegen-base/TemplateVerify.java` used by both JVM ports). + +A third fix closes a **generated-runtime** analog: TS, Python, and C#'s generated +output-parser code resolved its *own* `@payloadRef` payload by a bare runtime lookup — wrong +under a cross-package bare collision between two `template.output`s (or a payload bare-name +colliding with another root object). All three now bake the fully-qualified name and resolve +package-locally only when the bare name is genuinely ambiguous; byte-identical when it isn't. +Java and Kotlin already baked the FQN in generated code and needed no change here. + +TS additionally extended its entity-tier collision-scoped naming (previously payload-record +only) to cover every value-object *reference* site reachable from an entity module — +write-through read-views, projection declarations, and view declarations — plus a `runner.ts` +load-order package-binding misbind found in the same pass (the same class of bug #244 fixed +elsewhere), and fixed a reachable **runtime** wrong-data bug in `runtime-ts`'s +`extract-object.ts` (a bare-tail fallback resolver that extracted a nested colliding +value-object using the wrong package's shape). + +Byte-identical for every non-colliding model, all ports. Gated by the new +`xpkg-collision-json` fixture plus a per-port collision test (compile-and-run proof where the +port's toolchain supports it). Reuses `ERR_PAYLOAD_NAME_COLLISION` — no new error code, no new +metamodel vocabulary (ADR-0023 unaffected). See +[ADR-0044](spec/decisions/ADR-0044-payload-record-naming-cross-package-collision.md), whose +Consequences section now marks this recurrence closed. + ## [0.20.9] — 2026-07-28 **npm-only** — `migrate-ts` + `codegen-ts` (schema migrations and projection-view codegen diff --git a/docs/CONFORMANCE.md b/docs/CONFORMANCE.md index 8f438ced2..5f076a32f 100644 --- a/docs/CONFORMANCE.md +++ b/docs/CONFORMANCE.md @@ -39,7 +39,7 @@ regenerate with `ls -d fixtures//*/ | wc -l`. | [`fixtures/object-model-conformance/`](../fixtures/object-model-conformance/) | 1 shared metadata fixture (per-port scenarios) | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/codegen-conformance/`](../fixtures/codegen-conformance/) | 4 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/template-codegen-conformance/`](../fixtures/template-codegen-conformance/) | 3 | ✓ | ✓ | ✓ | ✓ | ✓ | -| [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 4 | ✓ | ✓ | ✓ | ✓ | ✓ | +| [`fixtures/template-output-render-conformance/`](../fixtures/template-output-render-conformance/) | 5 | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/generator-registry-conformance/`](../fixtures/generator-registry-conformance/) | 1 canonical manifest | ✓ | ✓ | ✓ | ✓ | ✓ | | [`fixtures/provider-composition-conformance/`](../fixtures/provider-composition-conformance/) | 5 cases | ✓ | ✓ | — (JVM registry via Java) | ✓ | ✓ | | [`fixtures/agent-context-conformance/`](../fixtures/agent-context-conformance/) | 4 | ✓ (the emitter is TS-owned) | — | — | — | — | diff --git a/docs/superpowers/plans/2026-07-29-issue-228-extract-tier-collision-naming.md b/docs/superpowers/plans/2026-07-29-issue-228-extract-tier-collision-naming.md new file mode 100644 index 000000000..c851271d9 --- /dev/null +++ b/docs/superpowers/plans/2026-07-29-issue-228-extract-tier-collision-naming.md @@ -0,0 +1,165 @@ +# #228 — Collision-scoped payload naming in the extract/output-parser tier (all 5 ports) Implementation Plan + +> **For agentic workers:** Execute with superpowers:subagent-driven-development, one fresh implementer per task + per-task review. Steps use checkbox syntax. + +**Goal:** ADR-0044 made payload-record naming collision-scoped in every port's PAYLOAD generator (two cross-package same-short-name `object.value`s → package-qualified names like `AcmeAlphaNotePayload`). #228: the **extract / output-parser tier** — which imports those nested classes — still names/imports them by BARE short name, so under a collision it references a class the payload generator no longer emits. Extend the collision-scoped naming to that tier in all 5 ports, gated by a new json/xml collision fixture. **Latent today** (the only collision fixture is `@format: html`; the extract tier gates on `@format ∈ {json,xml}`), so nothing shipped is wrong — but it's the last known instance of the bare-name bug class (sibling of #219/#220/#244). + +**Design settled (fable ruling, 2026-07-29):** TS uses **Option A** — the extractor's strict type is the per-VO ENTITY module (not the payload interface, which deliberately differs: `?:T|null` vs `?:T`, decimal `number` vs `string`, uuid/uri/inet/map → `unknown`, and lives in the optional relocatable `promptRender()` output). Cross-port invariant: the extractor's strict type = **each port's canonical strict artifact** — payload record (Py/C#/Kotlin), flavored class (Java), per-VO entity module (TS). So the 4 non-TS ports reuse their OWN payload name-map (mechanical); TS additionally brings its entity tier into ADR-0044 scope (the entity-file clobber is the load-bearing half in TS — NOT deferrable). + +**Reference:** ADR-0044 (`spec/decisions/ADR-0044-payload-record-naming-cross-package-collision.md`). Backstop `ERR_PAYLOAD_NAME_COLLISION` already in the shared ledger since 0.19.3. + +## Global Constraints + +- **Byte-identical, all ports, when non-colliding.** Qualification activates ONLY when two closure/domain members share a bare short name. A collision-free model emits today's exact names/paths. Pin with no-churn tests. **TS: the golden byte-gate lives OUTSIDE the per-package suite — run `cd server/typescript/packages/codegen-ts && bun test test/golden/` and regen only with proof.** +- **The collision domain differs by tier (TS-specific, load-bearing):** the PAYLOAD/prompts artifact keeps its per-payload-closure domain; the ENTITY tier's domain is the run/target's emitted-object SET (filenames + `runner.ts` `packageOf` are per-outDir global). These two artifacts may assign different names to the same VO — each internally consistent. The extract tier imports from entity modules, so it MUST use the entity-domain name map, never payload-codegen's closure map. +- **Resolution stays ADR-0041/0042 FQN-exact/package-local.** Where a port's extract tier resolves a `@objectRef` by bare name (Python `ref_vo`/`_find_object`, C# `RefVo`/`FindObject`), route it through the port's canonical resolver (`resolve_object_ref` / `NamingRefs.ResolveObjectRef`) — this fixes a wrong-node resolution bug (the #219 disease), not just naming. +- Reuse `ERR_PAYLOAD_NAME_COLLISION` as the backstop (already central, all ports). No new vocabulary; no metamodel change (ADR-0023 unaffected). +- Each port compiles + tests locally before its commit. Scope tests to the port (`scripts/ci-local.sh --only ` or the port's native runner). Never bare repo-root `bun test`. +- Stage explicit paths; never `git add -A` (untracked `.serena/`). Commit to this branch. +- Detailed per-file:line scope lives in each task below (captured from the scoping pass). + +--- + +### Task 1: Shared json/xml collision fixture + +**Files:** Create `fixtures/template-output-render-conformance/xpkg-collision-json/{meta.alpha.json,meta.beta.json,meta.app.json}`; modify that corpus's `README.md`; modify `docs/CONFORMANCE.md`. + +The extract/parser tier gates on `@format ∈ {json,xml}`; the existing `xpkg-collision/` fixture is `@format: html`, so it never exercises the tier. This fixture is a near-copy that DOES. + +- [ ] **Step 1:** Copy the three metadata files from `fixtures/template-output-render-conformance/xpkg-collision/` verbatim (`meta.alpha.json` = pkg `acme::alpha`, `object.value Note{alphaText @required}`; `meta.beta.json` = pkg `acme::beta`, `object.value Note{betaText @required}`; `meta.app.json` = pkg `acme::app`, `object.value Digest` with `field.object fromAlpha @objectRef=acme::alpha::Note` + `fromBeta @objectRef=acme::beta::Note`, and a `template.output DigestDoc @payloadRef=Digest @textRef=xpkg/digest`). In the copy, change the `template.output`'s `"@format": "html"` → `"@format": "json"`. Everything else identical. No `expected.json` (this corpus pins expectations in prose + inline per-port test assertions). +- [ ] **Step 2:** Add a section to `fixtures/template-output-render-conformance/README.md` mirroring the existing "Cross-package short-name collision" section, describing the json variant and that it exercises the extract/output-parser tier; state the expected emitted nested names (`AcmeAlphaNotePayload`/`AcmeBetaNotePayload` for Py/Java/Kotlin; `AcmeAlphaNote`/`AcmeBetaNote` for TS/C#). +- [ ] **Step 3:** Bump the fixture count in `docs/CONFORMANCE.md` (the template-output-render-conformance row). +- [ ] **Step 4:** Validate the three JSON files parse (`node -e "JSON.parse(require('fs').readFileSync(''))"` each). Commit `test(#228): add json xpkg-collision fixture for the extract/output-parser tier`. + +Note: `template-output-render-conformance` has NO auto-discovery — each port's test hardcodes the dir + filenames. So this fixture does nothing until a port adds a test method referencing it (done per-port in Tasks 4-8). + +--- + +### Task 2: TS — shared collision-naming module (pure refactor, no behavior change) + +**Files:** Create `server/typescript/packages/codegen-ts/src/naming/collision-names.ts`; modify `src/payload-codegen.ts`; Test `test/naming/collision-names.test.ts`. + +`assignEmittedNames` (payload-codegen.ts:139) and `packageQualifiedName` (within payload-codegen.ts:111-176) are pure functions of `(fqn, bareName, package)` triples. Extract them to a shared module so the entity + extract tiers can reuse the identical algorithm. + +- [ ] **Step 1 (test):** Write `collision-names.test.ts`: assert `assignEmittedNames` over a closure with a unique bare name → bare emitted name; over a closure with two same-bare-name FQNs → both package-qualified (`AcmeAlphaNote`/`AcmeBetaNote`); a still-colliding derived name → throws `ERR_PAYLOAD_NAME_COLLISION`. Mirror the existing payload-codegen collision tests. +- [ ] **Step 2:** Move `assignEmittedNames` + `packageQualifiedName` (+ the `ERR_PAYLOAD_NAME_COLLISION` throw) verbatim into `collision-names.ts`, exported. Keep signatures identical. +- [ ] **Step 3:** `payload-codegen.ts` re-imports them (delete the local copies). Run `bun test test/payload-codegen.test.ts` — must stay green (byte-identical payload output). Run the golden gate `bun test test/golden/`. +- [ ] **Step 4:** Typecheck (`cd server/typescript && bun run --filter '@metaobjectsdev/codegen-ts' typecheck`). Commit `refactor(#228): extract collision-name assignment into a shared module`. + +--- + +### Task 3: TS — entity tier into ADR-0044 scope (Option A) + +**Files:** modify `src/templates/entity-file.ts` (+ `src/generators/entity-file.ts` if the filename is decided there), `src/templates/inferred-types.ts`, `src/templates/zod-validators.ts`, `src/templates/drizzle-schema.ts`, `src/import-path.ts`, `src/runner.ts`; Tests: extend the relevant per-template tests + a new collision test. + +Make the per-VO entity module (interface name + output filename) collision-aware, keyed by the **run/target emitted-object set** (NOT the payload closure). This is the load-bearing half of TS #228 (the extract tier imports FROM here). + +- [ ] **Step 1 (test):** Add a collision test: two same-short-name `object.value`s across packages, run entity-file generation, assert both emit distinct interfaces (`AcmeAlphaNote`/`AcmeBetaNote`) to distinct module paths, and every `valueObjectModuleSpecifier`-routed reference (Zod `InsertSchema`, Drizzle `.$type<>()`, inferred-types field.object/map) uses the qualified name. Assert non-colliding case byte-identical (no-churn). +- [ ] **Step 2 (impl):** + - Build the entity-domain name map once (over the run's emitted `object.value` set) using `collision-names.ts` `assignEmittedNames`. + - `runner.ts` (~146-148): `packageOf` currently `Map` keyed by bare `o.name` — the second same-named object overwrites the first (the #244 disease, load-order-dependent misbinding in package layout). Key it by the emitted name (unique by construction via the backstop), so `valueObjectModuleSpecifier` resolves the right module. + - `inferred-types.ts` (`valueObjectFieldType` field.object/field.map ref branches, ~260-264/274-278): `stripPackage(ref)` → name-map lookup by `resolutionKey()`. `renderValueObjectInterface` (~314) declaration name → emitted name. + - `entity-file.ts` output filename → emitted name; `import-path.ts` `valueObjectModuleSpecifier` (~69-78) → emitted name. + - `zod-validators.ts` `InsertSchema` imports; `drizzle-schema.ts` `.$type<>()` imports → emitted names. + - Enum alias names follow the emitted owner name (`enumUnionAliasName(ownerName, …)`), matching what payload-codegen.ts:265 already does. +- [ ] **Step 3:** Run the codegen-ts suite + the golden gate. Non-colliding output MUST be byte-identical (regen golden only with proof the diff is collision-only). Typecheck. +- [ ] **Step 4:** Commit `fix(#228): TS entity tier emits collision-scoped value-object names (Option A)`. + +--- + +### Task 4: TS — extract/output-parser tier + TS collision test + +**Files:** modify `src/templates/extractor.ts`, `src/templates/extract-delegate-emitter.ts`, `src/templates/output-parser.ts`; Test: a TS test method vs the Task-1 fixture + inline collision tests. + +Thread the entity-domain name map through the extract tier; replace every bare `vo.name` name/dedupe key with `resolutionKey()`-keyed lookups. + +- [ ] **Step 1 (test):** TS test loading `fixtures/template-output-render-conformance/xpkg-collision-json/` (hardcoded path, like the existing render-helper-conformance xpkg test at `test/render-helper-conformance.test.ts`), running `extractor()` + `outputParser()` + `entityFile()`, asserting the emitted extractor/parser source imports/references `AcmeAlphaNote`/`AcmeBetaNote` (matching Task 3's entity module) and NOT bare `Note`; and that both mirror types/mappers are emitted (not dropped). Compile the generated output if the test harness supports it. +- [ ] **Step 2 (impl):** + - `extract-delegate-emitter.ts`: `mirrorName`(54)/`mapperName`(59) → name-map; the four `seen`-by-`vo.name`/`cur.name` sets (109,171,245,288) → `resolutionKey()`. Signature changes ripple to exported `nestedMirrorInterfaces`/`nestedMappers`/`mirrorName`/`usedHelpers`/`hasNested` (thread the map). + - `extractor.ts`: `mapperName`(91-93), `emitMapper` dedupe/mir (181,186), `reachablePayloadGroups` dedupe+naming+module-target (229-230,235,243 — module target = the entity-domain emitted name from Task 3), `reachableMirrorTypes` (259,264-266), `strictType = vo.name`(308) → entity-domain name map. + - `output-parser.ts`: the extract-delegate calls (180,205,207) thread the map. (Its inline Zod schema has no type names — no change. The `root.findObject(vo.name)` runtime lookup at 196/224 is a separate FQN-runtime hazard — note it, but keep in scope only if a fixture exercises it; otherwise leave a `// #228: bare findObject...` comment and a follow-up note in the report.) + - `refVo()` in both files already resolves FQN-exact (no change needed). +- [ ] **Step 3:** codegen-ts suite + golden gate + typecheck. Non-colliding byte-identical. +- [ ] **Step 4:** Commit `fix(#228): TS extract/output-parser tier uses collision-scoped names`. + +--- + +### Task 5: Python — extract tier collision naming + wrong-node fix + +**Files:** modify `server/python/src/metaobjects/codegen/extract_delegate_emitter.py`, `server/python/src/metaobjects/codegen/generators/extractor_generator.py`; promote 2 funcs from `codegen/generators/payload_vo_generator.py` (or a small shared module); Test: `server/python/tests/codegen/` new collision test. + +Python has TWO bug classes here (worse than naming): `reachable_vos` drops the 2nd colliding VO; `ref_vo` mis-resolves. + +- [ ] **Step 1 (test):** pytest loading `xpkg-collision-json/` (hardcoded path like `test_render_helper_conformance.py`), asserting the extractor + output-parser emit BOTH `AcmeAlphaNotePayload`/`AcmeBetaNotePayload` mirror/mapper/imports (not a dropped 2nd VO, not bare `NotePayload`). +- [ ] **Step 2 (impl):** + - Promote `_assign_nested_names` + `_package_qualified_name` (payload_vo_generator.py:471-479,547-582) to shared/exported; reuse `ERR_PAYLOAD_NAME_COLLISION` (errors.py:138). + - `extract_delegate_emitter.py`: `ref_vo`/`_find_object` (38-63) → `resolve_object_ref(root, ref, referrer_pkg)` (naming_refs.py:190), drop the bare-tail fallback; `reachable_vos` (131-148) dedupe key `cur.name` → `cur.resolution_key()`; `mirror_name`/`_mapper_name` (72-79) → name-map; thread the map through `_nested_mirror_type`(109), `nested_mirror_dataclasses`/`_one_mirror`(177,190), `nested_mappers`/`_one_mapper`/`_mapper_arg`(219-270). + - `extractor_generator.py`: `_strict_class`(62-68), `_mapper_name`(71-73), strict_imports loop (181-185) → name-map. +- [ ] **Step 3:** `cd server/python && uv run --extra integration pytest tests/codegen/` (scope to codegen). Commit `fix(#228): Python extract tier collision-scoped naming + FQN resolution`. + +--- + +### Task 6: C# — extract tier collision naming (shared ExtractDelegateEmitter) + +**Files:** modify `server/csharp/MetaObjects.Codegen/Generators/ExtractDelegateEmitter.cs`, `Generators/ExtractorGenerator.cs`, `Generators/OutputParserGenerator.cs`, `MetaObjects.Codegen/PayloadCodegen.cs` (visibility); Test: `MetaObjects.Codegen.Tests/` new collision test. + +Both generators funnel through `ExtractDelegateEmitter` — highest leverage. + +- [ ] **Step 1 (test):** test loading `xpkg-collision-json/`, asserting extractor + output-parser emit `AcmeAlphaNote`/`AcmeBetaNote` mirror/mapper/refs, not bare/dropped. Mirror `PayloadGeneratorTests.cs:121`. +- [ ] **Step 2 (impl):** + - `PayloadCodegen.cs`: promote `CollectClosure`(123)+`AssignEmittedNames`(171) private→internal (or add one internal wrapper returning `(order, byFqn, nameMap)`). `ResolveEmittedName`(218) already internal. + - `ExtractDelegateEmitter.cs`: `FindObject`(40-42)/`RefVo`(49-57) → `NamingRefs.ResolveObjectRef`/`EffectivePackage` (NamingRefs.cs:49,70, public); `MirrorName`(68)/`MapperName`(71) → name-map; thread through all consumers. + - `ExtractorGenerator.cs`: root strict/mirror/class (87-89), `EmitMapper`(157-158), `StrictArg`(187-190), `EnumTypeRef`(238 — pass the EMITTED owner name to `PayloadCodegen.EnumTypeName`). + - `OutputParserGenerator.cs`: payload-root resolution (95/107 `StripPkg`+root-scan) → FQN-aware. +- [ ] **Step 3:** `cd server/csharp && dotnet test` (or `scripts/ci-local.sh --only csharp`). Commit `fix(#228): C# extract/output-parser tier collision-scoped naming`. + +--- + +### Task 7: Java — SpringOutputParserGenerator collision naming + +**Files:** modify `server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java`, `SpringPayloadGenerator.java` (visibility); Test: `SpringOutputParserGeneratorTest.java` (or a new test). + +Smallest port — single-file fix + reuse the payload name-map. + +- [ ] **Step 1 (test):** test loading `xpkg-collision-json/` (hardcoded, like `SpringPayloadGeneratorTest.java:812`), asserting the output-parser's `from` mappers reference `AcmeAlphaNotePayload`/`AcmeBetaNotePayload`. +- [ ] **Step 2 (impl):** + - `SpringPayloadGenerator.java`: `computePayloadNameMap`(159-207)+`collectNestedClosure`+`nestedTargetOf`+`packageQualifiedName` protected/instance → `public static`. + - `SpringOutputParserGenerator.java`: `execute()`(113-131) gather ALL `MetaTemplate` (not just `SUBTYPE_OUTPUT`) so the nameMap domain matches the payload generator's; thread a `Map nameMap` through `emit → emitMapperMethods → emitMapper → mapperArgForField`; fix `nestedPayloadClass`(369-371) to consult it. +- [ ] **Step 3:** `cd server/java && mvn -pl codegen-spring test` (or `scripts/ci-local.sh --only java`; NO `-T`). Commit `fix(#228): Java output-parser tier collision-scoped payload naming`. + +--- + +### Task 8: Kotlin — extract tier collision naming (three-tier) + +**Files:** modify `server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt` (or lift to `KotlinGenUtil.kt`), `KotlinOutputParserGenerator.kt`, `KotlinExtractSchemaEmitter.kt`, `KotlinExtractMapperEmitter.kt`, `KotlinExtractorGenerator.kt`; Test: `KotlinOutputParserGeneratorTest.kt`/new. + +Heaviest — three tiers (Payload → Extracted mirror → strict Payload); Kotlin `protected` ≠ same-package. + +- [ ] **Step 1 (test):** test loading `xpkg-collision-json/`, asserting the extractor/parser reference `AcmeAlphaNotePayload`/`AcmeBetaNotePayload` (strict) AND collision-scoped `...Extracted` mirror names. +- [ ] **Step 2 (impl):** + - LIFT `computePayloadNameMap`+`collectNestedClosure`+`nestedTargetOf`+`packageQualifiedName` from `KotlinPayloadGenerator.kt`(107-221) into `KotlinGenUtil` (public object). + - `KotlinOutputParserGenerator.kt`: `execute()` gather ALL MetaTemplate; thread nameMap. + - `KotlinExtractSchemaEmitter.kt`: `nestedExtractedClass`(66-67) — collision-scope the "Extracted" mirror family (its own second naming scheme); thread nameMap through `extractedClassDeclsNested→emitMirror→nestedNullableTypeName`. + - `KotlinExtractMapperEmitter.kt`: 74,91 — thread nameMap. + - `KotlinExtractorGenerator.kt`: 239-242,271 — use the strict Payload nameMap (for `toStrict`) AND the mirror nameMap (for `Extracted`). +- [ ] **Step 3:** `cd server/java && mvn -pl codegen-kotlin test` (or `scripts/ci-local.sh --only kotlin`). Commit `fix(#228): Kotlin extract/output-parser tier collision-scoped naming`. + +--- + +### Task 9: Docs + CHANGELOG + +**Files:** modify `CHANGELOG.md`; touch ADR-0044 Consequences (mark the extract-tier follow-up shipped) if apt. + +- [ ] **Step 1:** `CHANGELOG.md` `## [Unreleased]` — coordinated cross-port bug fix (all 5 ports): the extract/output-parser tier now uses ADR-0044 collision-scoped payload names (was bare — under a cross-package short-name collision it referenced a class the payload generator no longer emits; Python/C# additionally mis-resolved the wrong node, TS additionally clobbered the entity module). Note it's LATENT (no shipped-wrong output; the collision fixture was html-only), byte-identical for non-colliding models, gated by the new json fixture. TS = Option A (entity tier). `ERR_PAYLOAD_NAME_COLLISION` reused. +- [ ] **Step 2:** In ADR-0044 Consequences, note the extract/output-parser sibling-generator recurrence (line ~51) is now addressed by #228. +- [ ] **Step 3:** Commit `docs(#228): CHANGELOG + ADR-0044 note for the extract-tier collision-naming fix`. + +--- + +## Self-Review + +**Spec coverage:** fixture → Task 1; TS Option A (naming module → entity tier → extract tier) → Tasks 2-4; Python → 5; C# → 6; Java → 7; Kotlin → 8; docs → 9. Each port task carries its own collision test vs the shared fixture (per-port hardcoded path — no auto-discovery). Byte-identical-when-non-colliding pinned per port. Wrong-node resolution (Python/C#) fixed via each port's canonical resolver. + +**Domain correctness:** the TS entity tier uses the run/target emitted-object domain (Task 3), the extract tier consumes THAT map (Task 4) — not payload-codegen's per-payload-closure domain. The 4 non-TS ports reuse their own payload name-map (their strict artifact IS the payload record/flavored class). + +**Ordering:** Task 1 (fixture) first so every port test can reference it. Task 2 (shared module) before 3-4. Ports 5-8 independent. Task 9 last. diff --git a/fixtures/template-output-render-conformance/README.md b/fixtures/template-output-render-conformance/README.md index cbcaa3ae7..9a959026e 100644 --- a/fixtures/template-output-render-conformance/README.md +++ b/fixtures/template-output-render-conformance/README.md @@ -166,6 +166,19 @@ The render-output pins above (`"Alpha=AA Beta=BB"`) are UNCHANGED by this contra idiomatic (Tier-1 codegen); this sub-corpus gates it by compile + construct + render + name assertions, strengthening the gate rather than weakening it. +## Cross-package short-name collision with extract/output-parser tier — `xpkg-collision-json/` + +Identical to `xpkg-collision/` above (same three metadata files, same `Digest` +payload with colliding `Note` VOs from `acme::alpha` and `acme::beta`), but the +`DigestDoc` `template.output` has `@format="json"` instead of `@format="html"`. +This variant exercises the extract/output-parser tier (which gates on +`@format ∈ {json,xml}`); the html variant does not. The generated render helper, +collision-aware payload naming, and render output remain identical — see +[Cross-package short-name collision](#cross-package-short-name-collision--xpkg-collision-digestdoc) +above for the full contract and expected payload names +(`AcmeAlphaNotePayload`/`AcmeBetaNotePayload` for Java/Kotlin/Python; +`AcmeAlphaNote`/`AcmeBetaNote` for TS/C#). + ## Expected build-time drift FAILURE — `drift/` `drift/meta.json` declares the same `Welcome` VO and a `document` diff --git a/fixtures/template-output-render-conformance/xpkg-collision-json/meta.alpha.json b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.alpha.json new file mode 100644 index 000000000..dd60ca834 --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.alpha.json @@ -0,0 +1,15 @@ +{ + "metadata.root": { + "package": "acme::alpha", + "children": [ + { + "object.value": { + "name": "Note", + "children": [ + { "field.string": { "name": "alphaText", "@required": true } } + ] + } + } + ] + } +} diff --git a/fixtures/template-output-render-conformance/xpkg-collision-json/meta.app.json b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.app.json new file mode 100644 index 000000000..f7b1c3cf7 --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.app.json @@ -0,0 +1,25 @@ +{ + "metadata.root": { + "package": "acme::app", + "children": [ + { + "object.value": { + "name": "Digest", + "children": [ + { "field.object": { "name": "fromAlpha", "@objectRef": "acme::alpha::Note" } }, + { "field.object": { "name": "fromBeta", "@objectRef": "acme::beta::Note" } } + ] + } + }, + { + "template.output": { + "name": "DigestDoc", + "@kind": "document", + "@payloadRef": "Digest", + "@textRef": "xpkg/digest", + "@format": "json" + } + } + ] + } +} diff --git a/fixtures/template-output-render-conformance/xpkg-collision-json/meta.beta.json b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.beta.json new file mode 100644 index 000000000..74ddf76fa --- /dev/null +++ b/fixtures/template-output-render-conformance/xpkg-collision-json/meta.beta.json @@ -0,0 +1,15 @@ +{ + "metadata.root": { + "package": "acme::beta", + "children": [ + { + "object.value": { + "name": "Note", + "children": [ + { "field.string": { "name": "betaText", "@required": true } } + ] + } + } + ] + } +} diff --git a/server/csharp/MetaObjects.Cli.Tests/VerifyCommandTests.cs b/server/csharp/MetaObjects.Cli.Tests/VerifyCommandTests.cs index c5e585121..602ed3402 100644 --- a/server/csharp/MetaObjects.Cli.Tests/VerifyCommandTests.cs +++ b/server/csharp/MetaObjects.Cli.Tests/VerifyCommandTests.cs @@ -201,4 +201,41 @@ public void Output_with_unresolved_payload_ref_is_flagged_as_output_drift() d.Code == VerifyCommand.ERR_PAYLOAD_REF_UNRESOLVED && d.Path == "NoSuchPayload"); } + + // #228 — the build-time @payloadRef resolver bug class (cross-port checkpoint for this + // task). Two packages each declare their OWN bare-colliding "Report" and a template.output + // with a BARE (same-package) @payloadRef "Report" — the realistic common case (a template + // referencing its own package's local value-object). Before this fix, + // PayloadCodegen.BuildPayloadFieldTree had no referrer-package parameter and resolved via a + // GLOBAL bare-name-first-match scan, so ONE of these two templates' drift check would + // silently bind to the OTHER package's "Report" (load-order dependent) and spuriously fail + // with ERR_VAR_NOT_ON_PAYLOAD for its own field. Passing each template's own effective + // package now binds each to ITS OWN "Report". + [Fact] + public void Bare_payloadRef_collision_across_packages_binds_own_package_report() + { + const string alphaModel = """ + { "metadata.root": { "package": "acme::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ { "field.string": { "name": "alphaVal" } } ] } }, + { "template.output": { "name": "ReportDocAlpha", "@payloadRef": "Report", + "@textRef": "t/alpha", "@format": "json" } } + ]}} + """; + const string betaModel = """ + { "metadata.root": { "package": "acme::beta", "children": [ + { "object.value": { "name": "Report", "children": [ { "field.string": { "name": "betaVal" } } ] } }, + { "template.output": { "name": "ReportDocBeta", "@payloadRef": "Report", + "@textRef": "t/beta", "@format": "json" } } + ]}} + """; + File.WriteAllText(Path.Combine(MetaDir, "meta.ai.json"), alphaModel); + File.WriteAllText(Path.Combine(MetaDir, "meta.beta.json"), betaModel); + WriteAt("t/alpha", "{{alphaVal}}"); + WriteAt("t/beta", "{{betaVal}}"); + + var o = VerifyCommand.Run(MetaDir, TplDir); + Assert.True(o.Ok, string.Join("; ", + o.LoadErrors.Concat(o.UnresolvedText).Concat(o.Errors.Select(e => $"{e.Code}({e.Path})")))); + Assert.Empty(o.Errors); + } } diff --git a/server/csharp/MetaObjects.Cli/VerifyCommand.cs b/server/csharp/MetaObjects.Cli/VerifyCommand.cs index 5ff5c6713..bcde9c9e2 100644 --- a/server/csharp/MetaObjects.Cli/VerifyCommand.cs +++ b/server/csharp/MetaObjects.Cli/VerifyCommand.cs @@ -273,7 +273,11 @@ public static Outcome Run(string metadataDir, string templatesRoot, bool strict // Both subtypes: @payloadRef must resolve to a loaded object.value (=> // non-empty derived field tree). Catches a renamed VO before codegen. - var fields = PayloadCodegen.BuildPayloadFieldTree(load.Root, payloadRef); + // ADR-0042/#228: a bare @payloadRef resolves in the template's OWN package FIRST — + // never whichever same-short-named object.value another package happens to declare + // (the loader validates @payloadRef package-locally too; verify must agree). + var referrerPkg = global::MetaObjects.NamingRefs.EffectivePackage(tmpl); + var fields = PayloadCodegen.BuildPayloadFieldTree(load.Root, payloadRef, referrerPkg); if (fields.Count == 0) { errors.Add(new Drift(tmpl.Name, kind, ERR_PAYLOAD_REF_UNRESOLVED, payloadRef)); diff --git a/server/csharp/MetaObjects.Codegen.Tests/ExtractTierCollisionTests.cs b/server/csharp/MetaObjects.Codegen.Tests/ExtractTierCollisionTests.cs new file mode 100644 index 000000000..b7be715e7 --- /dev/null +++ b/server/csharp/MetaObjects.Codegen.Tests/ExtractTierCollisionTests.cs @@ -0,0 +1,299 @@ +// #228 — extract / output-parser tier collision-scoped naming (C# port). +// +// Loads the SHARED cross-port corpus at +// fixtures/template-output-render-conformance/xpkg-collision-json/ — a Digest payload with +// two field.object children, each FQN-@objectRef-ing a DIFFERENT package's same-bare-named +// Note (acme::alpha::Note / acme::beta::Note). The SAME corpus the TS +// (extract-tier-collision.test.ts) and Python (test_extract_tier_collision.py) #228 tasks load. +// +// Design invariant (cross-port ruling): each port's extractor STRICT type = that port's +// CANONICAL strict artifact = the payload record. For C# that is the PayloadCodegen record +// (AcmeAlphaNote / AcmeBetaNote under ADR-0044 collision-scoped naming) — reused (never +// re-derived) by ExtractDelegateEmitter/ExtractorGenerator/OutputParserGenerator via the +// shared PayloadCodegen closure name-map (PayloadCodegen.ComputeClosureAndNames / +// PayloadCodegen.EmittedNameOf). +// +// Before the fix: ExtractDelegateEmitter.FindObject/RefVo matched by bare Name only (falling +// back to a bare-tail short-name match for an FQN ref — the #219/#244 "wrong node" pattern), +// MirrorName/MapperName were bare-Name-keyed (so a cross-package short-name collision either +// bound the WRONG package's Note or silently DROPPED the second one via bare-Name-keyed `seen` +// sets), and OutputParserGenerator/ExtractorGenerator resolved their OWN top-level @payloadRef +// via a bare/global-scan (no referrer-package awareness) rather than PayloadCodegen's own +// collision-aware emitted name. +using System.Reflection; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using MetaObjects.Codegen; +using MetaObjects.Codegen.Generators; +using MetaObjects.Loader; +using MetaObjects.Meta; +using Xunit; + +namespace MetaObjects.Codegen.Tests; + +public sealed class ExtractTierCollisionTests +{ + private static string Corpus() + { + var dir = AppContext.BaseDirectory; + while (dir is not null && + !Directory.Exists(Path.Combine(dir, "fixtures", "template-output-render-conformance", "xpkg-collision-json"))) + dir = Directory.GetParent(dir)?.FullName; + if (dir is null) + throw new InvalidOperationException("fixtures/template-output-render-conformance/xpkg-collision-json not found"); + return Path.Combine(dir, "fixtures", "template-output-render-conformance", "xpkg-collision-json"); + } + + private static MetaRoot LoadCorpus() + { + var dir = Corpus(); + var sources = new[] { "meta.alpha.json", "meta.beta.json", "meta.app.json" } + .Select((f, i) => (IMetaDataSource)new InMemoryStringSource(File.ReadAllText(Path.Combine(dir, f)), id: $"corpus{i}.json")) + .ToArray(); + var r = new MetaDataLoader().Load(sources); + Assert.Empty(r.Errors); + return r.Root; + } + + private static GenContext Ctx(MetaRoot root) => new() + { + Entities = root.Objects(), + Root = root, + Config = new GenConfig { OutDir = "/tmp", Namespace = "Acme.Generated" }, + }; + + [Fact] + public void Output_parser_and_extractor_emit_both_qualified_collision_members_never_bare_note() + { + var root = LoadCorpus(); + var ctx = Ctx(root); + + var parserSrc = Assert.Single(new OutputParserGenerator().Generate(ctx), f => f.Path == "DigestDoc.output.cs").Content; + var extractorFile = Assert.Single(new ExtractorGenerator().Generate(ctx)); + var extractorSrc = extractorFile.Content; + + // The extractor class + file are named off the ROOT payload's emitted name (Digest + // itself doesn't collide, so it stays bare) — never off the colliding NESTED Note. + Assert.Equal("DigestExtractor.cs", extractorFile.Path); + Assert.Contains("public static class DigestExtractor", extractorSrc); + + // ---- output-parser: BOTH mirror records + mappers present, collision-scoped ---- + // (never bare/dropped — the #219-class dedupe-by-name defect this fix closes). + Assert.Contains("public sealed record AcmeAlphaNoteExtracted", parserSrc); + Assert.Contains("public sealed record AcmeBetaNoteExtracted", parserSrc); + Assert.DoesNotContain("public sealed record NoteExtracted", parserSrc); + Assert.Contains("FromAcmeAlphaNoteExtracted(", parserSrc); + Assert.Contains("FromAcmeBetaNoteExtracted(", parserSrc); + Assert.DoesNotContain("FromNoteExtracted(", parserSrc); + + // The root Digest mirror's fields reference the qualified nested mirror types. + Assert.Contains("AcmeAlphaNoteExtracted? fromAlpha { get; init; }", parserSrc); + Assert.Contains("AcmeBetaNoteExtracted? fromBeta { get; init; }", parserSrc); + + // ---- extractor: mappers for BOTH qualified STRICT payload types (PayloadCodegen's + // own record names — the reused, never-re-derived, canonical strict artifact) ---- + Assert.Contains("ToStrict_AcmeAlphaNote(", extractorSrc); + Assert.Contains("ToStrict_AcmeBetaNote(", extractorSrc); + Assert.DoesNotContain("ToStrict_Note(", extractorSrc); + + // No bare "Note" identifier/type token anywhere in either generated file (word-boundary + // — "AcmeAlphaNote"/"AcmeBetaNote" do NOT match \bNote\b since "a"/"N" share no boundary). + Assert.DoesNotMatch(@"\bNote\b", parserSrc); + Assert.DoesNotMatch(@"\bNote\b", extractorSrc); + } + + // Compile + RUN — the strongest proof: each nested VO extracts into ITS OWN shape, never the + // other's (the #219/#244 "wrong node" bug), and neither is dropped. + [Fact] + public void Generated_extractor_compiles_and_extracts_each_nested_vo_into_its_own_shape() + { + var root = LoadCorpus(); + var ctx = Ctx(root); + + var parserSrc = Assert.Single(new OutputParserGenerator().Generate(ctx), f => f.Path == "DigestDoc.output.cs").Content; + var extractorSrc = Assert.Single(new ExtractorGenerator().Generate(ctx)).Content; + // GENERATOR-emitted payload records (PayloadCodegen — never hand-authored), resolved via + // the FQN root VO (mirrors RenderHelperConformanceTests' xpkg-collision precedent). + var payloadSrc = "using System.Collections.Generic;\nnamespace Acme.Generated;\n" + + PayloadCodegen.GeneratePayloadRecords(root, "acme::app::Digest"); + + var asm = Compile(parserSrc, extractorSrc, payloadSrc); + + var extractorType = asm.GetType("Acme.Generated.DigestExtractor")!; + var extract = extractorType.GetMethod("Extract", new[] { typeof(MetaObject), typeof(string) })!; + + MetaObject digestMo = root.FindObject("Digest")!; + + const string text = "{ \"fromAlpha\": { \"alphaText\": \"AA\" }, \"fromBeta\": { \"betaText\": \"BB\" } }"; + + var digest = extract.Invoke(null, new object?[] { digestMo, text })!; + + var fromAlpha = digest.GetType().GetProperty("fromAlpha")!.GetValue(digest)!; + Assert.Equal("AcmeAlphaNote", fromAlpha.GetType().Name); + Assert.Equal("AA", fromAlpha.GetType().GetProperty("alphaText")!.GetValue(fromAlpha)); + // Proves no wrong-node cross-wire: alpha's record has NO betaText property at all. + Assert.Null(fromAlpha.GetType().GetProperty("betaText")); + + var fromBeta = digest.GetType().GetProperty("fromBeta")!.GetValue(digest)!; + Assert.Equal("AcmeBetaNote", fromBeta.GetType().Name); + Assert.Equal("BB", fromBeta.GetType().GetProperty("betaText")!.GetValue(fromBeta)); + Assert.Null(fromBeta.GetType().GetProperty("alphaText")); + } + + // --------------------------------------------------------------------- + // no-churn — a non-colliding template.output payload keeps bare names (qualification + // never fires). Global constraint: byte-identical when there is no collision. + // --------------------------------------------------------------------- + + [Fact] + public void No_churn_non_colliding_payload_keeps_bare_names() + { + const string m = """ + { "metadata.root": { "package": "acme::demo", "children": [ + { "object.value": { "name": "Detail", "children": [ + { "field.string": { "name": "note", "@required": true } } + ]}}, + { "object.value": { "name": "Widget", "children": [ + { "field.object": { "name": "detail", "@objectRef": "Detail", "@required": true } } + ]}}, + { "template.output": { "name": "WidgetOut", "@payloadRef": "Widget", + "@textRef": "x/y", "@format": "json" } } + ]}} + """; + var r = new MetaDataLoader().Load([new InMemoryStringSource(m, id: "widget.json")]); + Assert.Empty(r.Errors); + var ctx = Ctx(r.Root); + + var parserSrc = Assert.Single(new OutputParserGenerator().Generate(ctx)).Content; + var extractorFile = Assert.Single(new ExtractorGenerator().Generate(ctx)); + + Assert.Equal("WidgetExtractor.cs", extractorFile.Path); + Assert.Contains("public static Widget Parse(string text)", parserSrc); + Assert.Contains("public static class WidgetExtractor", extractorFile.Content); + Assert.DoesNotContain("AcmeDemo", parserSrc); + Assert.DoesNotContain("AcmeDemo", extractorFile.Content); + } + + // --------------------------------------------------------------------- + // #228 fix round 1 — the RUNTIME PAYLOAD_FQN lookup wrong-node bug. Payload + // records/extractors/output-parsers ALWAYS emit into ONE FLAT namespace + // (ctx.Config.Namespace), while entities (and owned value-objects referenced by an + // entity) can emit into PER-PACKAGE namespaces via PackageBindingResolver (FR-019 — + // the recommended setup for multi-package projects). So an object.value "Report" + // (this @payloadRef, in acme::beta) and an UNRELATED object.entity "Report" (in + // acme::alpha) can BOTH load (ADR-0042 makes cross-package bare short names legal) + // AND compile cleanly (different namespaces — no duplicate-type error) while + // sharing the exact same bare short name. Before this fix, the generated + // ExtractLenient(MetaRoot, string) overload resolved its payload via a bare + // root.FindObject("Report") (MetaRoot's public runtime API — package-blind, + // first-match) — reachable, COMPILING, and silently wrong-node at runtime (the + // build-time resolver fix earlier in this file does NOT cover this: it only + // decides which node the GENERATOR walks, not what the GENERATED CODE looks up + // at runtime). + // --------------------------------------------------------------------- + + [Fact] + public void ExtractLenient_MetaRoot_overload_binds_the_payload_not_a_same_named_entity_in_another_package() + { + const string alphaEntity = """ + { "metadata.root": { "package": "acme::alpha", "children": [ + { "object.entity": { "name": "Report", "children": [ + { "source.rdb": { "@table": "reports" } }, + { "field.long": { "name": "id" } }, + { "identity.primary": { "@fields": "id" } } + ]}} + ]}} + """; + const string betaPayload = """ + { "metadata.root": { "package": "acme::beta", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "betaVal", "@required": true } } + ]}}, + { "template.output": { "name": "ReportDoc", "@payloadRef": "Report", + "@textRef": "x/y", "@format": "json" } } + ]}} + """; + // Adversarial load order: alpha's ENTITY "Report" loads FIRST, so a package-blind + // bare-name-first-match runtime lookup would bind IT, never beta's payload value. + var r = new MetaDataLoader().Load([ + new InMemoryStringSource(alphaEntity, id: "alpha.json"), + new InMemoryStringSource(betaPayload, id: "beta.json"), + ]); + Assert.Empty(r.Errors); + var root = r.Root; + + var config = new GenConfig + { + OutDir = "/tmp", + Namespace = "Acme.Generated", + // FR-019 per-package namespace binding — acme::alpha's entities land in a + // DIFFERENT namespace than the flat payload/output-parser namespace, so the + // two same-bare-named "Report"s do NOT collide at compile time (proving the + // scenario is reachable in a real, recommended multi-package setup). + PackageNamespaces = new Dictionary { ["acme::alpha"] = "Acme.Alpha" }, + }; + var ctx = new GenContext { Entities = root.Objects(), Root = root, Config = config }; + + var entitySrc = Assert.Single(new EntityGenerator().Generate(ctx)).Content; + var parserSrc = Assert.Single(new OutputParserGenerator().Generate(ctx), f => f.Path == "ReportDoc.output.cs").Content; + var payloadSrc = "using System.Collections.Generic;\nnamespace Acme.Generated;\n" + + PayloadCodegen.GeneratePayloadRecords(root, "acme::beta::Report"); + + // The entity lands in its OWN per-package namespace, distinct from the flat + // payload namespace — proves the "no compile collision" premise this bug relies on. + Assert.Contains("namespace Acme.Alpha;", entitySrc); + Assert.Contains("public class Report", entitySrc); + Assert.Contains("namespace Acme.Generated;", parserSrc); + + // The runtime PAYLOAD_FQN lookup bakes the FULL FQN (never the bare, ambiguous + // "Report") and resolves via the canonical FQN-exact matcher — never MetaRoot's + // bare-name-first-match FindObject. + Assert.Contains("public const string PAYLOAD_FQN = \"acme::beta::Report\";", parserSrc); + Assert.Contains("global::MetaObjects.NamingRefs.ResolveObjectRef(root, PAYLOAD_FQN, \"\")", parserSrc); + Assert.DoesNotContain("root.FindObject(PAYLOAD_FQN)", parserSrc); + + var asm = Compile(entitySrc, parserSrc, payloadSrc); + + var parserType = asm.GetType("Acme.Generated.ReportDocParser")!; + var extractLenientWithLoader = parserType.GetMethods() + .Single(m => m.Name == "ExtractLenient" && m.GetParameters()[0].ParameterType == typeof(MetaRoot)); + + const string text = "{ \"betaVal\": \"BV\" }"; + var result = extractLenientWithLoader.Invoke(null, new object?[] { root, text, null })!; + var data = result.GetType().GetProperty("Data")!.GetValue(result)!; + + // Binds the PAYLOAD value's shape (betaVal), NOT the entity's (id) — the wrong-node + // bug would have assembled the graph against alpha's entity schema instead, losing + // betaVal entirely (no such property on the entity's field set). + Assert.Equal("BV", data.GetType().GetProperty("betaVal")!.GetValue(data)); + } + + private static Assembly Compile(params string[] sources) + { + var trees = sources.Select(s => + CSharpSyntaxTree.ParseText(s, new CSharpParseOptions(LanguageVersion.CSharp12))).ToArray(); + + var refs = ((string)AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES")!) + .Split(Path.PathSeparator).Where(p => p.Length > 0) + .Select(p => (MetadataReference)MetadataReference.CreateFromFile(p)).ToList(); + refs.Add(MetadataReference.CreateFromFile(typeof(MetaObjects.Render.Extract.ExtractSchema).Assembly.Location)); + refs.Add(MetadataReference.CreateFromFile(typeof(MetaObject).Assembly.Location)); + refs.Add(MetadataReference.CreateFromFile(typeof(MetaObjects.Codegen.Runtime.ExtractObject).Assembly.Location)); + + var options = new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary) + .WithSpecificDiagnosticOptions(new Dictionary + { + ["CS8619"] = ReportDiagnostic.Error, // nullable-covariance mismatch must fail the proof + }); + var comp = CSharpCompilation.Create("extract_collision_" + Guid.NewGuid().ToString("N"), trees, refs, options); + + using var ms = new MemoryStream(); + var emit = comp.Emit(ms); + var errors = emit.Diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error) + .Select(d => $"{d.Id}: {d.GetMessage()}").ToList(); + Assert.True(errors.Count == 0, "generated code should compile, got: " + string.Join("; ", errors)); + + ms.Seek(0, SeekOrigin.Begin); + return Assembly.Load(ms.ToArray()); + } +} diff --git a/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs b/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs index c35265445..5d132bc0b 100644 --- a/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs +++ b/server/csharp/MetaObjects.Codegen.Tests/PayloadCodegenTests.cs @@ -126,6 +126,47 @@ public void BuildPayloadFieldTree_resolves_fqn_nested_objectRef_across_package_c Assert.Equal("betaText", Assert.Single(fromBeta.Fields!).Name); } + // #228 — the build-time @payloadRef resolver bug class (Python's round-2 fix; the CROSS-PORT + // checkpoint for this task). Two packages each declare their OWN bare-colliding "Report" and a + // template.output with a BARE (same-package) @payloadRef "Report" — the realistic common case + // (a template referencing its own package's local value-object). Before this fix, + // BuildPayloadFieldTree(root, "Report") had no referrerPkg parameter at all and resolved via a + // GLOBAL bare-name-first-match scan — VerifyCommand's drift check for EITHER template's + // "Report" would silently bind to WHICHEVER "Report" happened to load first, regardless of + // which package the declaring template belonged to. Passing each template's OWN effective + // package now binds each to ITS OWN "Report" — never the other's, never load-order-dependent. + [Fact] + public void BuildPayloadFieldTree_bare_payloadRef_binds_own_package_not_first_loaded() + { + const string alpha = """ + { "metadata.root": { "package": "acme::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ { "field.string": { "name": "alphaVal" } } ] } } ] } } + """; + const string beta = """ + { "metadata.root": { "package": "acme::beta", "children": [ + { "object.value": { "name": "Report", "children": [ { "field.string": { "name": "betaVal" } } ] } } ] } } + """; + var root = new MetaDataLoader().Load([ + new InMemoryStringSource(alpha, id: "a.json"), + new InMemoryStringSource(beta, id: "b.json"), + ]).Root; + + // A BARE "Report" ref resolved with alpha's own package binds alpha's Report... + var alphaTree = PayloadCodegen.BuildPayloadFieldTree(root, "Report", "acme::alpha"); + Assert.Equal("alphaVal", Assert.Single(alphaTree).Name); + + // ...and the SAME bare ref resolved with beta's own package binds beta's Report — never + // whichever "Report" happened to load first (both alpha-first and beta-first orderings + // give the SAME per-referrer result, proving this is referrer-scoped, not load-order). + var betaTree = PayloadCodegen.BuildPayloadFieldTree(root, "Report", "acme::beta"); + Assert.Equal("betaVal", Assert.Single(betaTree).Name); + + // A bare ref with NO referrer package (today's pre-#228 call convention) keeps the + // permissive global-scan fallback — unaffected callers see unchanged behavior. + var noReferrerTree = PayloadCodegen.BuildPayloadFieldTree(root, "Report"); + Assert.Single(noReferrerTree); + } + [Fact] public void Emits_render_handle_binding_textRef_and_format() { diff --git a/server/csharp/MetaObjects.Codegen/Generators/ExtractDelegateEmitter.cs b/server/csharp/MetaObjects.Codegen/Generators/ExtractDelegateEmitter.cs index 8f7d01f7d..b9fbe547a 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/ExtractDelegateEmitter.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/ExtractDelegateEmitter.cs @@ -37,23 +37,31 @@ internal static class ExtractDelegateEmitter // VO / field discovery (object-before-isArray order — the cross-port fix) // ========================================================================= - internal static MetaData? FindObject(MetaData root, string name) => - // ADR-0039: Children() — resolving root scan (behavior-identical; root has no super). - root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && c.Name == name); + /// + /// Resolve an OBJECT reference under the ADR-0042 package-local contract (FQN-exact when + /// qualified; else the referrer's own package, else root-level) — never a bare-tail + /// fallback (the #219/#228 "wrong node" class: under a cross-package short-name collision, + /// a bare-tail match binds WHICHEVER same-named object happens to load first, regardless of + /// which package actually points at). + /// + internal static MetaData? FindObject(MetaData root, string name, string referrerPkg) => + global::MetaObjects.NamingRefs.ResolveObjectRef(root, name, referrerPkg); /// /// The @objectRef target VO for a nested-object field, or null when unresolvable. - /// Matches by the bare ref first, then by the package-stripped short name. Shared with + /// The referrer package is the FIELD's own declaring package (ADR-0042 — the field's parent + /// object, not necessarily the walk's root VO, since a field may be inherited via extends + /// from an abstract VO declared in a different package). Shared with /// (the extract tier walks the same VO graph). /// internal static MetaData? RefVo(MetaData field, MetaData root) { // ADR-0039: resolving — @objectRef may be inherited via extends (TS reads f.attr). if (field.Attr(FIELD_ATTR_OBJECT_REF) is not string objectRef) return null; - var direct = FindObject(root, objectRef); - if (direct is not null) return direct; - int sep = objectRef.LastIndexOf(PACKAGE_SEPARATOR, System.StringComparison.Ordinal); - return sep >= 0 ? FindObject(root, objectRef[(sep + PACKAGE_SEPARATOR.Length)..]) : null; + var referrerPkg = field.Parent is not null + ? global::MetaObjects.NamingRefs.EffectivePackage(field.Parent) + : ""; + return FindObject(root, objectRef, referrerPkg); } /// @@ -64,11 +72,30 @@ internal static class ExtractDelegateEmitter /// internal static bool IsObjectField(MetaData field) => field.SubType == FIELD_SUBTYPE_OBJECT; - /// The extracted-mirror record name for a value-object (<Name>Extracted). - public static string MirrorName(MetaData vo) => $"{vo.Name}Extracted"; + /// ADR-0044 (#228) — the ADR-0044 emitted name for from the + /// shared closure name-map: bare when unique in the closure, + /// package-qualified on a cross-package short-name collision. Reused (never re-derived) so + /// the mirror/mapper types the extract tier emits always agree with PayloadCodegen's own + /// record names. + private static string EmittedName(MetaData vo, IReadOnlyDictionary nameMap) => + PayloadCodegen.EmittedNameOf(vo, nameMap); + + /// The extracted-mirror record name for a value-object (<EmittedName>Extracted). + public static string MirrorName(MetaData vo, IReadOnlyDictionary nameMap) => + $"{EmittedName(vo, nameMap)}Extracted"; - /// The mapper-method name for a value-object (From<Name>Extracted). - private static string MapperName(MetaData vo) => $"From{vo.Name}Extracted"; + /// The mapper-method name for a value-object (From<EmittedName>Extracted). + private static string MapperName(MetaData vo, IReadOnlyDictionary nameMap) => + $"From{EmittedName(vo, nameMap)}Extracted"; + + /// ADR-0044 (#228) — the closure name-map for 's OWN reference + /// closure (the same closure would walk + /// for this VO). is already-resolved, so its ResolutionKey() is + /// used as a self-resolving FQN reference (referrer package is irrelevant for an FQN, and for + /// a root-level VO its own effective package — "" — correctly self-resolves). + private static IReadOnlyDictionary ClosureNameMap(MetaData vo, MetaData root) => + PayloadCodegen.ComputeClosureAndNames( + root, vo.ResolutionKey(), global::MetaObjects.NamingRefs.EffectivePackage(vo)).NameMap; // ========================================================================= // "Has nested" — only emit the delegating overload + mappers when worthwhile @@ -83,7 +110,10 @@ public static bool HasNested(MetaData vo, MetaData root) while (stack.Count > 0) { var cur = stack.Pop(); - if (!seen.Add(cur.Name)) continue; + // Dedupe by ResolutionKey (FQN) — NOT the bare Name — so a cross-package same-short- + // named VO reached from a DIFFERENT branch is still walked (the #219-class defect: + // bare-name dedupe would prune it as "already seen" and could miss its nested fields). + if (!seen.Add(cur.ResolutionKey())) continue; foreach (var f in Fr010FieldMapping.Fields(cur)) { if (!IsObjectField(f)) continue; @@ -100,14 +130,14 @@ public static bool HasNested(MetaData vo, MetaData root) // ========================================================================= /// The nullable mirror C# type for one field — nested-aware (recurses into nested mirror names). - private static string NestedMirrorType(MetaData field, MetaData root) + private static string NestedMirrorType(MetaData field, MetaData root, IReadOnlyDictionary nameMap) { // Object BEFORE array (the object-before-isArray fix): an array-of-objects must map to // a list of nested mirrors, NOT a string list. if (IsObjectField(field)) { var target = RefVo(field, root); - string baseName = target is not null ? MirrorName(target) : "object"; + string baseName = target is not null ? MirrorName(target, nameMap) : "object"; return Fr010FieldMapping.IsArray(field) ? $"global::System.Collections.Generic.IReadOnlyList<{baseName}?>?" : $"{baseName}?"; @@ -122,23 +152,25 @@ private static string NestedMirrorType(MetaData field, MetaData root) /// /// Emit the PAYLOAD mirror record (nested-aware, so object fields are typed as nested mirrors, - /// not object?) plus every reachable NESTED mirror record, deduped by simple name - /// (cycle-safe). The PAYLOAD mirror keeps the canonical <Payload>Extracted name - /// () so the delegating overload's one shared mirror type can - /// carry populated nested components. + /// not object?) plus every reachable NESTED mirror record, deduped by ResolutionKey + /// (cycle-safe; ADR-0044 #228 — never the bare metadata name, which would silently collapse two + /// cross-package same-short-named VOs onto one emitted mirror). The PAYLOAD mirror keeps the + /// canonical <Payload>Extracted name () so the + /// delegating overload's one shared mirror type can carry populated nested components. /// public static string NestedMirrorRecords(MetaData vo, MetaData root, string payloadMirror) { + var nameMap = ClosureNameMap(vo, root); var sb = new StringBuilder(); var seen = new HashSet(System.StringComparer.Ordinal); - EmitMirror(vo, root, payloadMirror, seen, sb); + EmitMirror(vo, root, payloadMirror, nameMap, seen, sb); return sb.ToString(); } private static void EmitMirror(MetaData vo, MetaData root, string recordName, - HashSet seen, StringBuilder sb) + IReadOnlyDictionary nameMap, HashSet seen, StringBuilder sb) { - if (!seen.Add(vo.Name)) return; + if (!seen.Add(vo.ResolutionKey())) return; string baseName = recordName.EndsWith("Extracted", System.StringComparison.Ordinal) ? recordName[..^"Extracted".Length] : recordName; sb.AppendLine(); @@ -146,12 +178,12 @@ private static void EmitMirror(MetaData vo, MetaData root, string recordName, sb.AppendLine($"public sealed record {recordName}"); sb.AppendLine("{"); foreach (var f in Fr010FieldMapping.Fields(vo)) - sb.AppendLine($" public {NestedMirrorType(f, root)} {f.Name} {{ get; init; }}"); + sb.AppendLine($" public {NestedMirrorType(f, root, nameMap)} {f.Name} {{ get; init; }}"); sb.AppendLine("}"); foreach (var f in Fr010FieldMapping.Fields(vo)) if (IsObjectField(f) && RefVo(f, root) is { } target) - EmitMirror(target, root, MirrorName(target), seen, sb); + EmitMirror(target, root, MirrorName(target, nameMap), nameMap, seen, sb); } // ========================================================================= @@ -168,6 +200,28 @@ private static void EmitMirror(MetaData vo, MetaData root, string recordName, public static string DelegatingMembers(MetaData vo, MetaData root, string payloadFqn, string rootMirror, string formatEnum) { + var nameMap = ClosureNameMap(vo, root); + + // ADR-0044/#228 fix round 1 — root.FindObject(name) (MetaRoot's public runtime API) is + // a bare-Name-only, first-match lookup with NO package awareness. Payload + // records/extractors/output-parsers emit into ONE FLAT namespace (config.Namespace), + // while entities and owned value-objects can emit into PER-PACKAGE namespaces (FR-019 + // PackageBindingResolver) — so two DIFFERENT-package objects sharing this payload's bare + // short name (e.g. an object.value "Report" used as this @payloadRef in one package, and + // an unrelated object.entity "Report" in another) can BOTH load and compile cleanly (no + // duplicate-type error, since they land in different namespaces), yet + // root.FindObject(bareName) at RUNTIME silently binds whichever one loaded first — + // reachable, compiling, silent wrong-node extraction. Bake the FULL ResolutionKey (FQN) + // and resolve via the canonical NamingRefs.ResolveObjectRef matcher (FQN-exact, + // load-order-independent) ONLY when this payload's bare name is actually AMBIGUOUS at + // the metadata root (more than one root-level object.* shares it — the SAME domain + // MetaRoot.FindObject itself searches). A UNIQUE (the overwhelmingly common) payload + // name keeps TODAY'S exact bare-name + root.FindObject() path, byte-identical to + // pre-fix output — a naive "always bake the FQN" would REGRESS the unique case, since + // MetaRoot.FindObject matches bare child names only and would return null for an FQN. + bool payloadNameAmbiguous = root.Children().Count(c => c.Type == TYPE_OBJECT && c.Name == vo.Name) > 1; + string bakedPayloadFqn = payloadNameAmbiguous ? vo.ResolutionKey() : payloadFqn; + var sb = new StringBuilder(); sb.AppendLine(); sb.AppendLine(" // FR-010 — runtime-delegating extraction (the single metadata-driven extract path)."); @@ -175,8 +229,11 @@ public static string DelegatingMembers(MetaData vo, MetaData root, string payloa sb.AppendLine(" // FULL object graph (nested objects + arrays-of-objects) reflection-free by reading the live"); sb.AppendLine(" // metadata, then maps it into the typed nullable mirror via From*Extracted."); sb.AppendLine(); - sb.AppendLine($" /// The payload's metadata name — resolve it against a loaded MetaRoot to obtain the runtime MetaObject."); - sb.AppendLine($" public const string PAYLOAD_FQN = \"{Fr010FieldMapping.CSharpStringLiteral(payloadFqn)}\";"); + var ambiguousNote = payloadNameAmbiguous + ? " ADR-0042 FQN (this payload's bare name collides with a same-short-name object elsewhere in the run)." + : ""; + sb.AppendLine($" /// The payload's metadata name — resolve it against a loaded MetaRoot to obtain the runtime MetaObject.{ambiguousNote}"); + sb.AppendLine($" public const string PAYLOAD_FQN = \"{Fr010FieldMapping.CSharpStringLiteral(bakedPayloadFqn)}\";"); sb.AppendLine(); sb.AppendLine($" /// Tolerant best-effort extraction delegating to the runtime; fully populates nested-object and"); sb.AppendLine($" /// array-of-object components by reading the live metadata. Never throws."); @@ -194,7 +251,12 @@ public static string DelegatingMembers(MetaData vo, MetaData root, string payloa sb.AppendLine($" public static global::MetaObjects.Render.Extract.ExtractionResult<{rootMirror}> ExtractLenient("); sb.AppendLine($" global::MetaObjects.Meta.MetaRoot root, string text, ExtractOptions? opts = null)"); sb.AppendLine(" {"); - sb.AppendLine(" var mo = root.FindObject(PAYLOAD_FQN)"); + // NamingRefs.ResolveObjectRef returns MetaData?; ExtractLenient(MetaObject, ...) needs a + // MetaObject — the "as" narrows (never a hard cast throw) matching root.FindObject's own + // MetaObject? return type on the unique path. + sb.AppendLine(payloadNameAmbiguous + ? " var mo = global::MetaObjects.NamingRefs.ResolveObjectRef(root, PAYLOAD_FQN, \"\") as global::MetaObjects.Meta.MetaObject" + : " var mo = root.FindObject(PAYLOAD_FQN)"); sb.AppendLine(" ?? throw new global::System.InvalidOperationException("); sb.AppendLine(" $\"payload object \\\"{PAYLOAD_FQN}\\\" not found in the loaded metadata\");"); sb.AppendLine(" return ExtractLenient(mo, text, opts);"); @@ -202,26 +264,28 @@ public static string DelegatingMembers(MetaData vo, MetaData root, string payloa // ---- mappers (root + nested, deduped). Root mapper is named distinctly so it can carry // the canonical payload-mirror return type (the template name may differ from the VO). - EmitMappers(sb, vo, root, rootMirror); + EmitMappers(sb, vo, root, rootMirror, nameMap); // ---- shared helpers AppendHelpers(sb); return sb.ToString(); } - private static void EmitMappers(StringBuilder sb, MetaData rootVo, MetaData root, string rootMirror) + private static void EmitMappers(StringBuilder sb, MetaData rootVo, MetaData root, string rootMirror, + IReadOnlyDictionary nameMap) { var seen = new HashSet(System.StringComparer.Ordinal); // Root mapper: a distinctly-named method returning the canonical payload mirror. - EmitRootMapper(sb, rootVo, root, rootMirror); - seen.Add(rootVo.Name); + EmitRootMapper(sb, rootVo, root, rootMirror, nameMap); + seen.Add(rootVo.ResolutionKey()); // Nested mappers (each named FromExtracted, returning Extracted). foreach (var f in Fr010FieldMapping.Fields(rootVo)) if (IsObjectField(f) && RefVo(f, root) is { } target) - EmitNestedMapper(sb, target, root, seen); + EmitNestedMapper(sb, target, root, seen, nameMap); } - private static void EmitRootMapper(StringBuilder sb, MetaData vo, MetaData root, string mirror) + private static void EmitRootMapper(StringBuilder sb, MetaData vo, MetaData root, string mirror, + IReadOnlyDictionary nameMap) { sb.AppendLine(); sb.AppendLine($" /// Map an assembled ValueObject graph into a typed {mirror}. Generated; null-tolerant."); @@ -231,34 +295,35 @@ private static void EmitRootMapper(StringBuilder sb, MetaData vo, MetaData root, sb.AppendLine($" return new {mirror}"); sb.AppendLine(" {"); foreach (var f in Fr010FieldMapping.Fields(vo)) - sb.AppendLine($" {f.Name} = {MapperArg(f, root)},"); + sb.AppendLine($" {f.Name} = {MapperArg(f, root, nameMap)},"); sb.AppendLine(" };"); sb.AppendLine(" }"); } - private static void EmitNestedMapper(StringBuilder sb, MetaData vo, MetaData root, HashSet seen) + private static void EmitNestedMapper(StringBuilder sb, MetaData vo, MetaData root, HashSet seen, + IReadOnlyDictionary nameMap) { - if (!seen.Add(vo.Name)) return; - string mirror = MirrorName(vo); + if (!seen.Add(vo.ResolutionKey())) return; + string mirror = MirrorName(vo, nameMap); sb.AppendLine(); sb.AppendLine($" /// Map an assembled ValueObject graph into a typed {mirror}. Generated; null-tolerant."); - sb.AppendLine($" private static {mirror}? {MapperName(vo)}(object? o)"); + sb.AppendLine($" private static {mirror}? {MapperName(vo, nameMap)}(object? o)"); sb.AppendLine(" {"); sb.AppendLine(" if (o is null) return null;"); sb.AppendLine($" return new {mirror}"); sb.AppendLine(" {"); foreach (var f in Fr010FieldMapping.Fields(vo)) - sb.AppendLine($" {f.Name} = {MapperArg(f, root)},"); + sb.AppendLine($" {f.Name} = {MapperArg(f, root, nameMap)},"); sb.AppendLine(" };"); sb.AppendLine(" }"); foreach (var f in Fr010FieldMapping.Fields(vo)) if (IsObjectField(f) && RefVo(f, root) is { } target) - EmitNestedMapper(sb, target, root, seen); + EmitNestedMapper(sb, target, root, seen, nameMap); } /// The mirror-field initializer expression that reads from the assembled object. - private static string MapperArg(MetaData field, MetaData root) + private static string MapperArg(MetaData field, MetaData root, IReadOnlyDictionary nameMap) { string key = $"\"{Fr010FieldMapping.CSharpStringLiteral(field.Name)}\""; @@ -267,7 +332,7 @@ private static string MapperArg(MetaData field, MetaData root) { var target = RefVo(field, root); if (target is null) return "null /* unresolved @objectRef */"; - string fn = MapperName(target); + string fn = MapperName(target, nameMap); return Fr010FieldMapping.IsArray(field) ? $"MapObjectList(ReadProp(o, {key}), {fn})" : $"{fn}(ReadProp(o, {key}))"; diff --git a/server/csharp/MetaObjects.Codegen/Generators/ExtractorGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/ExtractorGenerator.cs index 2cc472654..e75a86c70 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/ExtractorGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/ExtractorGenerator.cs @@ -67,7 +67,11 @@ public virtual IEnumerable Generate(GenContext ctx) bool formatSupportsExtract = format.Equals("json", System.StringComparison.OrdinalIgnoreCase) || format.Equals("xml", System.StringComparison.OrdinalIgnoreCase); - var vo = ExtractDelegateEmitter.FindObject(ctx.Root, payloadRef); + // ADR-0042/#228: a bare @payloadRef resolves in the template's OWN package first + // (never a bare-tail/global-scan fallback that could bind the WRONG package's + // same-short-named object under a cross-package collision). + var referrerPkg = global::MetaObjects.NamingRefs.EffectivePackage(tmpl); + var vo = ExtractDelegateEmitter.FindObject(ctx.Root, payloadRef, referrerPkg); // The extract tier sits over the NESTED-CAPABLE delegating extract, which // OutputParserGenerator emits only for json/xml payloads that have a nested object / @@ -84,9 +88,18 @@ protected virtual EmittedFile EmitExtractor(MetaData tmpl, MetaData vo, string p { string templateName = tmpl.Name; string parserClass = CSharpNaming.ParserClassName(templateName); - string strictType = payloadRef; // PayloadCodegen emits the bare VO name. - string rootMirror = $"{payloadRef}Extracted"; - string extractorClass = CSharpNaming.ExtractorClassName(payloadRef); + // ADR-0042/0044 (#228): resolve the SAME closure name-map PayloadCodegen/PayloadGenerator + // computed for this template's OWN @payloadRef, and name the strict/mirror/extractor types + // after the resolved VO's EMITTED (bare-unless-colliding) name — never the raw (possibly + // FQN) payloadRef attribute string, which diverges from the actual PayloadCodegen record + // name under a within-closure short-name collision (e.g. the closure root "Digest" stays + // bare, but a nested collision member resolved as a DIFFERENT template's own payload would + // need qualification). + var referrerPkg = global::MetaObjects.NamingRefs.EffectivePackage(tmpl); + var nameMap = PayloadCodegen.ComputeClosureAndNames(ctx.Root, payloadRef, referrerPkg).NameMap; + string strictType = PayloadCodegen.EmittedNameOf(vo, nameMap); + string rootMirror = $"{strictType}Extracted"; + string extractorClass = CSharpNaming.ExtractorClassName(strictType); var sb = new StringBuilder(); sb.AppendLine("// "); @@ -137,34 +150,38 @@ protected virtual EmittedFile EmitExtractor(MetaData tmpl, MetaData vo, string p sb.AppendLine($" {parserClass}.ExtractLenient(mo, text);"); // ---- recursive mirror->strict mappers (payload + nested, deduped, cycle-safe) ---- - EmitMappers(sb, vo, ctx.Root); + EmitMappers(sb, vo, ctx.Root, nameMap); sb.AppendLine("}"); return new EmittedFile($"{extractorClass}.cs", sb.ToString()); } - private static void EmitMappers(StringBuilder sb, MetaData vo, MetaData root) + private static void EmitMappers(StringBuilder sb, MetaData vo, MetaData root, IReadOnlyDictionary nameMap) { var seen = new HashSet(System.StringComparer.Ordinal); - EmitMapper(sb, vo, root, seen); + EmitMapper(sb, vo, root, seen, nameMap); } - private static void EmitMapper(StringBuilder sb, MetaData vo, MetaData root, HashSet seen) + private static void EmitMapper(StringBuilder sb, MetaData vo, MetaData root, HashSet seen, + IReadOnlyDictionary nameMap) { - if (!seen.Add(vo.Name)) return; + // Dedupe by ResolutionKey (FQN), never the bare metadata name — the #219-class defect: + // a bare-name dedupe would drop the SECOND cross-package same-short-named VO's mapper. + if (!seen.Add(vo.ResolutionKey())) return; + var voName = PayloadCodegen.EmittedNameOf(vo, nameMap); sb.AppendLine(); - sb.AppendLine($" /// Map the all-nullable {vo.Name}Extracted mirror onto the strict {vo.Name} payload. Generated."); - sb.AppendLine($" private static {vo.Name} ToStrict_{vo.Name}({vo.Name}Extracted m) => new {vo.Name}"); + sb.AppendLine($" /// Map the all-nullable {voName}Extracted mirror onto the strict {voName} payload. Generated."); + sb.AppendLine($" private static {voName} ToStrict_{voName}({voName}Extracted m) => new {voName}"); sb.AppendLine(" {"); foreach (var f in Fr010FieldMapping.Fields(vo)) - sb.AppendLine($" {f.Name} = {StrictArg(vo, f, root)},"); + sb.AppendLine($" {f.Name} = {StrictArg(vo, f, root, nameMap)},"); sb.AppendLine(" };"); // Recurse into nested-object targets (single + array) for their mappers. foreach (var f in Fr010FieldMapping.Fields(vo)) if (ExtractDelegateEmitter.IsObjectField(f) && ExtractDelegateEmitter.RefVo(f, root) is { } target) - EmitMapper(sb, target, root, seen); + EmitMapper(sb, target, root, seen, nameMap); } /// @@ -173,7 +190,7 @@ private static void EmitMapper(StringBuilder sb, MetaData vo, MetaData root, Has /// field is mapped as required (no optional-null variant). is the field's /// value-object — needed to compute the nested enum type name for an enum field. /// - private static string StrictArg(MetaData owner, MetaData field, MetaData root) + private static string StrictArg(MetaData owner, MetaData field, MetaData root, IReadOnlyDictionary nameMap) { string name = field.Name; @@ -184,7 +201,7 @@ private static string StrictArg(MetaData owner, MetaData field, MetaData root) if (target is null) // Unresolved @objectRef — PayloadCodegen types this as the bare ref name; pass through. return $"m.{name}!"; - string fn = $"ToStrict_{target.Name}"; + string fn = $"ToStrict_{PayloadCodegen.EmittedNameOf(target, nameMap)}"; return Fr010FieldMapping.IsArray(field) ? $"m.{name}!.Select({fn}).ToList()" : $"{fn}(m.{name}!)"; @@ -196,7 +213,7 @@ private static string StrictArg(MetaData owner, MetaData field, MetaData root) // with no cast). This is the string-LIST -> enum-list bridge. if (field.SubType == FIELD_SUBTYPE_ENUM && Fr010FieldMapping.IsArray(field)) { - string et = EnumTypeRef(owner, field); + string et = EnumTypeRef(owner, field, nameMap); return $"(m.{name} ?? global::System.Linq.Enumerable.Empty()).Where(x => x is not null)" + $".Select(x => System.Enum.Parse<{et}>(x!)).ToList()"; } @@ -217,7 +234,7 @@ private static string StrictArg(MetaData owner, MetaData field, MetaData root) // enum type. Coerce via Enum.Parse<> (which returns , so no cast mismatch). // Engine already validated the member, so Parse is safe. if (field.SubType == FIELD_SUBTYPE_ENUM) - return $"System.Enum.Parse<{EnumTypeRef(owner, field)}>(m.{name}!)"; + return $"System.Enum.Parse<{EnumTypeRef(owner, field, nameMap)}>(m.{name}!)"; // Scalar (single): the strict record is non-null. Value-type scalars (int/long/double/bool) // are typed T? in the mirror, so `m.F!` keeps type T? — unwrap via `.Value`. Reference-type @@ -231,9 +248,14 @@ private static string StrictArg(MetaData owner, MetaData field, MetaData root) /// /// The fully-qualified reference to a payload field's nested enum type. PayloadCodegen emits the - /// enum NESTED inside the owning record <Owner>, so the mapper (a separate top-level - /// class) must qualify it as <Owner>.<EnumType>. + /// enum NESTED inside the owning record <Owner> under its ADR-0044 EMITTED (possibly + /// package-qualified) name — never the bare metadata name, which could collide with a sibling + /// collision member's own same-named enum field — so the mapper (a separate top-level class) + /// must qualify it as <EmittedOwner>.<EnumType>. /// - private static string EnumTypeRef(MetaData owner, MetaData field) => - $"{owner.Name}.{PayloadCodegen.EnumTypeName(owner.Name, field)}"; + private static string EnumTypeRef(MetaData owner, MetaData field, IReadOnlyDictionary nameMap) + { + var ownerName = PayloadCodegen.EmittedNameOf(owner, nameMap); + return $"{ownerName}.{PayloadCodegen.EnumTypeName(ownerName, field)}"; + } } diff --git a/server/csharp/MetaObjects.Codegen/Generators/OutputParserGenerator.cs b/server/csharp/MetaObjects.Codegen/Generators/OutputParserGenerator.cs index 54649e1b1..bdcade4de 100644 --- a/server/csharp/MetaObjects.Codegen/Generators/OutputParserGenerator.cs +++ b/server/csharp/MetaObjects.Codegen/Generators/OutputParserGenerator.cs @@ -89,10 +89,22 @@ protected virtual EmittedFile EmitParser(MetaData tmpl, string payloadRef, GenCo { var templateName = tmpl.Name; var parserClass = CSharpNaming.ParserClassName(templateName); - // FR-032: @payloadRef is an FQN after the desugar/sweep; the generated C# TYPE - // NAME is the resolved value-object's bare name (an FQN like "acme::ai::Payload" - // is not a valid C# identifier). Mirrors RenderHelperGenerator's StripPkg use. - var payloadType = CSharpNaming.StripPkg(payloadRef); + + // ADR-0042/#228: resolve @payloadRef package-aware — never a bare-tail/global-scan + // fallback that could bind the WRONG package's same-short-named object under a + // cross-package collision (the #219/#244 "wrong node" class). The loader validates + // @payloadRef through this SAME canonical resolver; codegen must never silently walk + // a DIFFERENT node than what was validated. + var referrerPkg = global::MetaObjects.NamingRefs.EffectivePackage(tmpl); + var vo = global::MetaObjects.NamingRefs.ResolveObjectRef(ctx.Root, payloadRef, referrerPkg); + // FR-032/ADR-0044: @payloadRef may be an FQN after the desugar/sweep; the generated C# + // TYPE NAME is PayloadCodegen's OWN emitted name for the resolved VO — bare unless its + // within-closure short name collides (never a raw StripPkg of the possibly-FQN attribute + // string, which would diverge from the ACTUAL record PayloadGenerator/PayloadCodegen + // emits under a collision). Falls back to StripPkg only when payloadRef is unresolvable + // (mirrors the pre-#228 permissive behavior for a dangling/malformed @payloadRef). + var payloadType = PayloadCodegen.ResolveEmittedName(ctx.Root, payloadRef, referrerPkg) + ?? CSharpNaming.StripPkg(payloadRef); var extractedType = $"{payloadType}Extracted"; // FR-010: emit the tolerant extract() API alongside strict Parse/TryParse when the @@ -103,8 +115,6 @@ protected virtual EmittedFile EmitParser(MetaData tmpl, string payloadRef, GenCo bool formatSupportsExtract = format.Equals("json", StringComparison.OrdinalIgnoreCase) || format.Equals("xml", StringComparison.OrdinalIgnoreCase); - // ADR-0039: Children() — resolving root scan (behavior-identical; root has no super). - var vo = ctx.Root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && CSharpNaming.StripPkg(c.Name) == payloadType); bool emitExtract = formatSupportsExtract && vo is not null; var sb = new StringBuilder(); diff --git a/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs b/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs index 54124b8a9..4f842c4f6 100644 --- a/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs +++ b/server/csharp/MetaObjects.Codegen/PayloadCodegen.cs @@ -65,21 +65,6 @@ public static class PayloadCodegen [FIELD_SUBTYPE_TIMESTAMP] = "string", }; - // ADR-0041: the verify field-tree resolver — a FULLY-QUALIFIED ref (contains ::) resolves - // EXACTLY on the package-qualified name (ResolutionKey()/Fqn()), never a bare-tail fallback - // that would bind a same-named object.value in the WRONG package on a cross-package - // short-name collision. A bare ref matches by short name (first-wins). Used ONLY by the - // verify field-tree walk (BuildTree) below — record emission resolves through the shared - // NamingRefs.ResolveObjectRef matcher instead (ADR-0044; see CollectClosure). The render - // engine + verify field-tree resolve against METADATA, not record names, and are UNCHANGED - // by the ADR-0044 naming rule. - private static MetaData? ResolveObjectRef(MetaData root, string reference) - { - bool fqn = reference.Contains("::"); - return root.Children().FirstOrDefault(c => c.Type == TYPE_OBJECT && - (fqn ? c.ResolutionKey() == reference || c.Fqn() == reference : c.Name == reference)); - } - // ADR-0039: resolve array-ness through the super chain (isArray is a native // property, not an attr; the former OwnAttr("isArray") clause was dead code). private static bool IsArrayField(MetaData field) => field.ResolvedIsArray(); @@ -211,6 +196,32 @@ private static Dictionary AssignEmittedNames(IReadOnlyDictionary return nameMap; } + /// + /// ADR-0044 — compute the full reference closure + collision-aware emitted-name map for + /// , for reuse by OTHER codegen tiers that must reference the SAME + /// payload record names would emit (the extract / + /// output-parser tier — #228 — reuses this rather than re-deriving its own naming). Returns + /// the traversal order (FQNs), the resolved-node-by-FQN map, and the emitted-name map (FQN + /// -> the bare-or-package-qualified C# identifier). A closure containing an unresolvable + /// still-colliding derived name throws ERR_PAYLOAD_NAME_COLLISION (see ). + /// + internal static (List Order, Dictionary ByFqn, Dictionary NameMap) + ComputeClosureAndNames(MetaData root, string voRef, string referrerPkg) + { + var order = new List(); + var byFqn = new Dictionary(StringComparer.Ordinal); + CollectClosure(root, voRef, referrerPkg, order, byFqn); + var nameMap = AssignEmittedNames(byFqn); + return (order, byFqn, nameMap); + } + + /// The ADR-0044 emitted name for from a closure name-map + /// computed by — falls back to the bare metadata name + /// when isn't a key in the map (defensive; a map from a closure walk + /// that actually reached always contains it). + internal static string EmittedNameOf(MetaData vo, IReadOnlyDictionary nameMap) => + nameMap.GetValueOrDefault(vo.ResolutionKey(), vo.Name); + /// Resolve 's emitted C# record name under the /// ADR-0044 naming rule, scoped to its OWN reference closure (the same closure /// would emit). Returns null when @@ -219,10 +230,8 @@ private static Dictionary AssignEmittedNames(IReadOnlyDictionary { var vo = ResolveForEmission(root, reference, referrerPkg); if (vo is null) return null; - var order = new List(); - var byFqn = new Dictionary(StringComparer.Ordinal); - CollectClosure(root, reference, referrerPkg, order, byFqn); - return AssignEmittedNames(byFqn).GetValueOrDefault(vo.ResolutionKey()); + var (_, _, nameMap) = ComputeClosureAndNames(root, reference, referrerPkg); + return nameMap.GetValueOrDefault(vo.ResolutionKey()); } /// @@ -339,10 +348,7 @@ private static void EmitClosureRecords( /// public static string GeneratePayloadRecords(MetaData root, string voName, string referrerPkg = "") { - var order = new List(); - var byFqn = new Dictionary(StringComparer.Ordinal); - CollectClosure(root, voName, referrerPkg, order, byFqn); - var nameMap = AssignEmittedNames(byFqn); + var (order, byFqn, nameMap) = ComputeClosureAndNames(root, voName, referrerPkg); var output = new List(); EmitClosureRecords(root, order, byFqn, nameMap, output); return string.Join("\n\n", output) + "\n"; @@ -352,29 +358,44 @@ public static string GeneratePayloadRecords(MetaData root, string voName, string /// Derive the verify field tree (the input to Verify.Check) from an /// object.value view-object: scalars become leaves, object-ref fields recurse /// into nested element trees. This is the metadata→verify bridge a `dotnet meta verify` - /// command uses to drift-check a template against its @payloadRef. + /// command uses to drift-check a template against its @payloadRef. + /// (ADR-0042, #228) is the declaring template's effective package — a bare + /// resolves there FIRST (else root-level, else the pre-ADR-0044 global bare-name scan — the + /// SAME fallback record emission uses, see ), so a template + /// whose bare @payloadRef collides with a same-short-named object.value in ANOTHER + /// package binds its OWN package's object — never whichever one loads first. /// - public static IReadOnlyList BuildPayloadFieldTree(MetaData root, string voName) => - BuildTree(root, voName, new HashSet(StringComparer.Ordinal)); + public static IReadOnlyList BuildPayloadFieldTree(MetaData root, string voName, string referrerPkg = "") => + BuildTree(root, voName, referrerPkg, new HashSet(StringComparer.Ordinal)); - private static IReadOnlyList BuildTree(MetaData root, string voName, HashSet visiting) + private static IReadOnlyList BuildTree(MetaData root, string voName, string referrerPkg, HashSet visiting) { - // ADR-0041: FQN-exact resolution for the verify field-tree (a SEPARATE resolver from - // record emission's CollectClosure — see the ResolveObjectRef doc comment above) so a - // fully-qualified nested @objectRef binds its own package. - var vo = ResolveObjectRef(root, voName); - if (vo is null || !visiting.Add(voName)) return []; + // Shares the record-emission resolver (ADR-0042/0044): FQN-exact when qualified, else + // referrer-package-local, else the pre-ADR-0044 global bare-name-scan fallback. + var vo = ResolveForEmission(root, voName, referrerPkg); + // ADR-0039: dedupe/cycle-guard by ResolutionKey (FQN), never the bare ref string — two + // DIFFERENT same-short-named nodes reached via different bare refs must not collapse + // onto "already visiting" (the #219-class dedupe defect). + if (vo is null || !visiting.Add(vo.ResolutionKey())) return []; + var voPkg = global::MetaObjects.NamingRefs.EffectivePackage(vo); var fields = new List(); foreach (var f in vo.Children().Where(c => c.Type == TYPE_FIELD)) { // ADR-0039: resolving — @objectRef may be inherited via extends (TS reads f.attr). - // ADR-0041: pass the FULL (possibly FQN) ref — ResolveObjectRef resolves it exactly. if (f.SubType == FIELD_SUBTYPE_OBJECT && f.Attr(FIELD_ATTR_OBJECT_REF) is string refName) - fields.Add(new PayloadField(f.Name, BuildTree(root, refName, visiting))); + { + // ADR-0042: a nested @objectRef resolves in the FIELD's OWN declaring package, + // which may differ from this VO's when the field is inherited via extends from + // an abstract VO in another package (mirrors CollectClosure's fieldPkg). + var fieldPkg = f.Parent is not null + ? global::MetaObjects.NamingRefs.EffectivePackage(f.Parent) + : voPkg; + fields.Add(new PayloadField(f.Name, BuildTree(root, refName, fieldPkg, visiting))); + } else fields.Add(new PayloadField(f.Name)); } - visiting.Remove(voName); + visiting.Remove(vo.ResolutionKey()); return fields; } diff --git a/server/java/codegen-base/src/main/java/com/metaobjects/generator/verify/TemplateVerify.java b/server/java/codegen-base/src/main/java/com/metaobjects/generator/verify/TemplateVerify.java index 9b230811c..2e4e38543 100644 --- a/server/java/codegen-base/src/main/java/com/metaobjects/generator/verify/TemplateVerify.java +++ b/server/java/codegen-base/src/main/java/com/metaobjects/generator/verify/TemplateVerify.java @@ -12,6 +12,7 @@ import com.metaobjects.template.MetaTemplate; import com.metaobjects.template.PromptTemplate; import com.metaobjects.template.TemplateConstants; +import com.metaobjects.validation.SymbolTable; import java.nio.file.Path; import java.util.ArrayList; @@ -95,6 +96,14 @@ public static Outcome run(MetaDataLoader loader, Path templateRoot) { List warnings = new ArrayList<>(); List unresolved = new ArrayList<>(); + // ADR-0042 — resolve @payloadRef through the loader's OWN package-local symbol table + // (#228), so a bare ref binds the template's own package (else root-level) and an FQN + // binds exactly — the same contract the loader validated the ref under. The prior + // package-blind bare-tail scan bound a same-short-named value-object in the WRONG + // package under a cross-package collision (load-order-dependent), deriving the wrong + // field tree and mis-reporting {{field}} drift. + SymbolTable symbols = SymbolTable.build(loader.getRoot()); + for (com.metaobjects.MetaData child : loader.getRoot().getChildren()) { if (!(child instanceof MetaTemplate tmpl)) continue; @@ -107,7 +116,7 @@ public static Outcome run(MetaDataLoader loader, Path templateRoot) { // Both subtypes: @payloadRef must resolve to a loaded object.value (a // non-empty derived field tree). Catches a renamed VO before codegen. - MetaObject payloadVo = resolveValueObject(loader, payloadRef); + MetaObject payloadVo = resolveValueObject(symbols, payloadRef, tmpl.getPackage()); List fields = payloadVo == null ? List.of() : derivePayloadFieldTree(loader, payloadVo, new LinkedHashSet<>()); @@ -237,14 +246,15 @@ private static void addIfPresent(List refs, String ref) { if (ref != null && !ref.isEmpty()) refs.add(ref); } - /** Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities). */ - private static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - for (MetaObject obj : loader.getMetaObjects()) { - if (!MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) continue; - if (obj.getName().equals(ref)) return obj; - if (shortName(obj.getName()).equals(ref)) return obj; - } - return null; + /** + * Resolve {@code @payloadRef} to its {@code object.value} target under the loader's ADR-0042 + * package-local contract (rejects entities). {@code referrerPkg} is the template's own package + * ({@code ""} for a root-level template): a bare ref binds {@code ::} first, + * else a root-level object; an FQN binds exactly — no cross-package bare-name fallback. + */ + private static MetaObject resolveValueObject(SymbolTable symbols, String ref, String referrerPkg) { + MetaObject obj = symbols.resolveObject(ref, referrerPkg == null ? "" : referrerPkg); + return (obj != null && MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) ? obj : null; } /** Last {@code ::} segment of a (possibly packaged) metadata name. */ diff --git a/server/java/codegen-base/src/test/java/com/metaobjects/generator/verify/TemplateVerifyTest.java b/server/java/codegen-base/src/test/java/com/metaobjects/generator/verify/TemplateVerifyTest.java index 374ce0566..97f69ae59 100644 --- a/server/java/codegen-base/src/test/java/com/metaobjects/generator/verify/TemplateVerifyTest.java +++ b/server/java/codegen-base/src/test/java/com/metaobjects/generator/verify/TemplateVerifyTest.java @@ -279,6 +279,90 @@ public void nestedFqnObjectRefRejectsFieldFromTheCollidingRow() throws Exception assertEquals("somethingElse", out.errors().get(0).path()); } + // === #228 — the @payloadRef resolver is package-local (ADR-0042). Two packages each + // === declare their OWN payload VO `Report` (distinct field) AND a template.prompt with a + // === BARE @payloadRef "Report". The prior package-blind bare-tail scan bound whichever + // === Report loaded first, deriving the WRONG package's field tree and mis-reporting + // === {{field}} drift. The fix binds each template's OWN package's Report — in BOTH orders. + + /** Package pv::alpha — its OWN Report (field alphaField) + a bare-@payloadRef prompt. */ + private static final String PAYLOAD_ALPHA_FIXTURE = """ + { + "metadata.root": { "package": "pv::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "alphaField" } } + ] } }, + { "template.prompt": { + "name": "ReportPrompt", + "@payloadRef": "Report", + "@textRef": "alpha/tmpl" + } } + ] } + } + """; + + /** Package pv::beta — a DIFFERENT Report (same short name, field betaField) + its own prompt. */ + private static final String PAYLOAD_BETA_FIXTURE = """ + { + "metadata.root": { "package": "pv::beta", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "betaField" } } + ] } }, + { "template.prompt": { + "name": "ReportPrompt", + "@payloadRef": "Report", + "@textRef": "beta/tmpl" + } } + ] } + } + """; + + private void assertBarePayloadRefBindsOwnPackage(String baseName, String first, String second) + throws Exception { + Path templateRoot = Files.createTempDirectory("tv-payloadref-" + baseName); + // Each template references ONLY its own package's field: clean iff each @payloadRef + // binds its own package's Report (a wrong-package bind would drift on the other field). + writeTemplate(templateRoot, "alpha/tmpl", "{{alphaField}}"); + writeTemplate(templateRoot, "beta/tmpl", "{{betaField}}"); + + MetaDataLoader loader = loadFixtures(baseName, first, second); + + TemplateVerify.Outcome out = TemplateVerify.run(loader, templateRoot); + + assertTrue("bare @payloadRef must bind each template's own package's Report; got: " + out, + out.ok()); + } + + @Test + public void barePayloadRefBindsOwnPackageAcrossCollision_alphaFirst() throws Exception { + assertBarePayloadRefBindsOwnPackage("alpha-first", PAYLOAD_ALPHA_FIXTURE, PAYLOAD_BETA_FIXTURE); + } + + @Test + public void barePayloadRefBindsOwnPackageAcrossCollision_betaFirst() throws Exception { + assertBarePayloadRefBindsOwnPackage("beta-first", PAYLOAD_BETA_FIXTURE, PAYLOAD_ALPHA_FIXTURE); + } + + @Test + public void barePayloadRefRejectsOtherPackagesField() throws Exception { + Path templateRoot = Files.createTempDirectory("tv-payloadref-reject"); + // pv::alpha's prompt references betaField (only on pv::beta::Report) — must drift, proving + // the alpha prompt bound pv::alpha::Report, not the colliding pv::beta::Report. + writeTemplate(templateRoot, "alpha/tmpl", "{{betaField}}"); + writeTemplate(templateRoot, "beta/tmpl", "{{betaField}}"); + + MetaDataLoader loader = loadFixtures("reject", PAYLOAD_ALPHA_FIXTURE, PAYLOAD_BETA_FIXTURE); + + TemplateVerify.Outcome out = TemplateVerify.run(loader, templateRoot); + + assertFalse("a field from the colliding package's Report must NOT resolve", out.ok()); + assertTrue("expected an ERR_VAR_NOT_ON_PAYLOAD for betaField on the alpha prompt; got: " + out, + out.errors().stream().anyMatch(d -> + "pv::alpha::ReportPrompt".equals(d.template()) + && "ERR_VAR_NOT_ON_PAYLOAD".equals(d.code()) + && "betaField".equals(d.path()))); + } + // === helpers ============================================================ private static void writeTemplate(Path root, String ref, String body) throws IOException { diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractMapperEmitter.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractMapperEmitter.kt index 7e548a969..9504de752 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractMapperEmitter.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractMapperEmitter.kt @@ -38,10 +38,14 @@ internal object KotlinExtractMapperEmitter { * [rootExtractedClass]) plus the shared `asMap` / `mapObjectList` helpers. Returns the * concatenated Kotlin source, each member prefixed with a blank line for readability. */ - fun mapperMethods(rootVo: MetaObject, rootExtractedClass: String): String { + fun mapperMethods( + rootVo: MetaObject, + rootExtractedClass: String, + nameMap: Map, + ): String { val out = StringBuilder() val emitted = LinkedHashSet() - emitMapper(rootVo, rootExtractedClass, out, emitted) + emitMapper(rootVo, rootExtractedClass, out, emitted, nameMap) appendHelpers(out) return out.toString() } @@ -51,12 +55,13 @@ internal object KotlinExtractMapperEmitter { extractedClass: String, out: StringBuilder, emitted: LinkedHashSet, + nameMap: Map, ) { if (!emitted.add(vo.name)) return // dedupe + cycle guard val nested = mutableListOf() val args = vo.metaFields.joinToString(",\n") { field -> - " ${mapperArgForField(field, nested)}" + " ${mapperArgForField(field, nested, nameMap)}" } out.append("\n") @@ -71,7 +76,10 @@ internal object KotlinExtractMapperEmitter { // Recurse into nested mappers (post-order, deduped). for (nestedVo in nested) { - emitMapper(nestedVo, KotlinExtractSchemaEmitter.nestedExtractedClass(nestedVo), out, emitted) + emitMapper( + nestedVo, KotlinExtractSchemaEmitter.nestedExtractedClass(nestedVo, nameMap), + out, emitted, nameMap + ) } } @@ -81,14 +89,18 @@ internal object KotlinExtractMapperEmitter { * object recurses into its generated mapper; an array-of-objects maps each element. * Records the discovered nested VO into [nested] so the caller emits its mapper. */ - private fun mapperArgForField(field: MetaField<*>, nested: MutableList): String { + private fun mapperArgForField( + field: MetaField<*>, + nested: MutableList, + nameMap: Map, + ): String { val name = KotlinExtractSchemaEmitter.kotlinStringLiteral(field.name) // Nested object / array-of-objects (NOT enum — that is a string-backed scalar). val target = KotlinExtractSchemaEmitter.objectRefValueObject(field) if (target != null) { nested.add(target) - val nestedClass = KotlinExtractSchemaEmitter.nestedExtractedClass(target) + val nestedClass = KotlinExtractSchemaEmitter.nestedExtractedClass(target, nameMap) return if (field.isArrayType()) { // List?: map each element Map; the assembled value is a List. // `it` is a non-null Map here, so from never returns null — `!!` keeps diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractSchemaEmitter.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractSchemaEmitter.kt index e78630076..649d9f7dc 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractSchemaEmitter.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractSchemaEmitter.kt @@ -47,41 +47,51 @@ internal object KotlinExtractSchemaEmitter { *

Cycle/depth bounding is handled upstream by `MetaObjectExtractor`; the per-FQN * dedupe set here also stops the emitter from recursing forever on a cyclic graph.

* + * @param nameMap ADR-0044 collision-scoped nested-mirror name map (VO FQN -> + * `Extracted`, or `AcmeAlphaNoteExtracted` on a cross-package short-name + * collision) from [KotlinGenUtil.computeExtractedNameMap] (#228). * @return Kotlin source: the root mirror declaration followed by the nested ones, * separated by blank lines. Returns the same shape as [extractedClassDecl] * when [rootVo] has no nested object fields. */ - fun extractedClassDeclsNested(rootVo: MetaObject, rootClassName: String): String { + fun extractedClassDeclsNested( + rootVo: MetaObject, + rootClassName: String, + nameMap: Map, + ): String { val out = StringBuilder() val emitted = LinkedHashSet() - emitMirror(rootVo, rootClassName, out, emitted) + emitMirror(rootVo, rootClassName, out, emitted, nameMap) return out.toString().trimEnd() } /** - * The nested mirror class name for a value-object: `Extracted`. Mirrors - * [KotlinPayloadGenerator]'s nested payload naming (`Payload`) but for the - * extracted (all-nullable) mirror. Public so the parser generator names mappers consistently. + * The nested mirror class name for a value-object: the ADR-0044 collision-scoped name from + * [nameMap] (`AcmeAlphaNoteExtracted` on a cross-package short-name collision), else the bare + * `Extracted`. Mirrors [KotlinPayloadGenerator]'s nested payload naming + * (`Payload`) but for the extracted (all-nullable) mirror. Public so the parser + * generator names mappers consistently. */ - fun nestedExtractedClass(vo: MetaObject): String = - PackageMapping.splitFqn(vo.name).second + "Extracted" + fun nestedExtractedClass(vo: MetaObject, nameMap: Map): String = + nameMap[vo.name] ?: KotlinNaming.extractedName(PackageMapping.splitFqn(vo.name).second) private fun emitMirror( vo: MetaObject, className: String, out: StringBuilder, emitted: LinkedHashSet, + nameMap: Map, ) { if (!emitted.add(vo.name)) return // dedupe + cycle guard val nested = mutableListOf() val props = vo.metaFields.joinToString(",\n") { field -> - " val ${field.name}: ${nestedNullableTypeName(field, nested)} = null" + " val ${field.name}: ${nestedNullableTypeName(field, nested, nameMap)} = null" } out.append("data class $className(\n$props,\n)\n\n") for (nestedVo in nested) { - emitMirror(nestedVo, nestedExtractedClass(nestedVo), out, emitted) + emitMirror(nestedVo, nestedExtractedClass(nestedVo, nameMap), out, emitted, nameMap) } } @@ -90,12 +100,17 @@ internal object KotlinExtractSchemaEmitter { * `@objectRef` resolves to a value-object become the nested mirror type (single) or * `List<Extracted>?` (array-of-objects); the discovered nested VO is * recorded into [nested] so the caller emits its mirror. All other fields fall back - * to the scalar mapping in [nullableTypeName]. + * to the scalar mapping in [nullableTypeName]. The nested mirror name is resolved through + * the collision-scoped [nameMap] (#228). */ - private fun nestedNullableTypeName(field: MetaField<*>, nested: MutableList): String { + private fun nestedNullableTypeName( + field: MetaField<*>, + nested: MutableList, + nameMap: Map, + ): String { val target = objectRefValueObject(field) if (target != null) { - val nestedClass = nestedExtractedClass(target) + val nestedClass = nestedExtractedClass(target, nameMap) nested.add(target) return if (field.isArrayType()) "List<$nestedClass>?" else "$nestedClass?" } diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractorGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractorGenerator.kt index 61a8817a9..f25cbc60a 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractorGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinExtractorGenerator.kt @@ -70,23 +70,40 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() parseArgs() val outRoot = Paths.get(outDir.absolutePath) - // Stable name order — matches the sibling generators' deterministic emission. + // ADR-0044 (#228) — the extractor references BOTH the strict payload records (`toStrict`) + // and the `...Extracted` mirrors, so it consults BOTH collision-scoped name maps. Both are + // computed over ALL templates (not just outputs) so their domain / package assignment match + // the payload + parser generators, keeping all three tiers' nested names in lockstep. // ADR-0039: root-scan discipline — resolving children accessor. + val allTemplates = loader.root.getChildren(MetaTemplate::class.java, true) + .sortedBy { it.name } + val payloadNameMap = KotlinGenUtil.computePayloadNameMap(allTemplates, loader) + val extractedNameMap = KotlinGenUtil.computeExtractedNameMap(allTemplates, loader) + + // Only template.output gets an extractor file. Stable name order — matches the sibling + // generators' deterministic emission. val outputs = loader.root.getChildren(OutputTemplate::class.java, true) .sortedBy { it.name } for (tmpl in outputs) { - emit(tmpl, loader, outRoot) + emit(tmpl, loader, outRoot, payloadNameMap, extractedNameMap) } } - protected open fun emit(template: MetaTemplate, loader: MetaDataLoader, outRoot: Path) { + protected open fun emit( + template: MetaTemplate, + loader: MetaDataLoader, + outRoot: Path, + payloadNameMap: Map, + extractedNameMap: Map, + ) { val payloadRef = template.payloadRef if (payloadRef.isNullOrEmpty()) { LOG.warn("skipping extractor for {} — missing @payloadRef", template.name) return } - val payloadVo = resolveViewObject(loader, payloadRef) + // ADR-0042 — resolve @payloadRef under the loader's package-local contract (#228). + val payloadVo = KotlinGenUtil.resolveValueObjectRef(loader, payloadRef, template.getPackage()) if (payloadVo == null) { LOG.warn( "skipping extractor for {} — @payloadRef '{}' does not resolve to an object.value", @@ -112,7 +129,9 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() // nested mirrors/payloads are keyed on the value-object short name. val extractorClass = KotlinNaming.extractorName(templateShort) val parserClass = KotlinNaming.parserName(templateShort) - val rootMirror = templateShort + "Extracted" + // Root mirror + strict payload are template-named (unique — never collision-scoped); + // nested targets consult the collision-scoped name maps (#228). + val rootMirror = KotlinNaming.extractedName(templateShort) val rootStrict = KotlinNaming.payloadName(templateShort) val src = buildString { @@ -178,7 +197,7 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() append(parserClass) append(".extractLenient(loader, text, opts)\n") // Recursive mirror->strict mappers (root + nested, deduped, cycle-safe). - appendMappers(payloadVo, rootMirror, rootStrict) + appendMappers(payloadVo, rootMirror, rootStrict, payloadNameMap, extractedNameMap) append("}\n") } @@ -196,9 +215,15 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() * mapper is named on its value-object short name (`Extracted`/`Payload`), * matching the nested classes those generators emit.

*/ - private fun StringBuilder.appendMappers(rootVo: MetaObject, rootMirror: String, rootStrict: String) { + private fun StringBuilder.appendMappers( + rootVo: MetaObject, + rootMirror: String, + rootStrict: String, + payloadNameMap: Map, + extractedNameMap: Map, + ) { val emitted = LinkedHashSet() - appendMapper(rootVo, rootMirror, rootStrict, emitted) + appendMapper(rootVo, rootMirror, rootStrict, emitted, payloadNameMap, extractedNameMap) } private fun StringBuilder.appendMapper( @@ -206,13 +231,15 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() mirror: String, strict: String, emitted: LinkedHashSet, + payloadNameMap: Map, + extractedNameMap: Map, ) { if (!emitted.add(vo.name)) return // dedupe + cycle guard val nested = mutableListOf() val args = vo.metaFields.joinToString(",\n") { field -> - " ${field.name} = ${strictArg(field, vo, nested)}" + " ${field.name} = ${strictArg(field, vo, nested, payloadNameMap)}" } append("\n") @@ -235,10 +262,15 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() append(" )\n") // Recurse into nested-object targets (single + array) for their mappers (post-order). - // Nested mappers are keyed on the value-object short name. + // Nested mapper names are collision-scoped: the `...Extracted` mirror from + // [extractedNameMap] and the strict `...Payload` from [payloadNameMap] (#228), falling + // back to the bare `Extracted`/`Payload` when a target is not in the map + // (non-colliding — byte-identical to pre-#228 output). for (nestedVo in nested) { val nestedShort = PackageMapping.splitFqn(nestedVo.name).second - appendMapper(nestedVo, nestedShort + "Extracted", nestedShort + "Payload", emitted) + val nestedMirror = extractedNameMap[nestedVo.name] ?: KotlinNaming.extractedName(nestedShort) + val nestedStrict = payloadNameMap[nestedVo.name] ?: KotlinNaming.payloadName(nestedShort) + appendMapper(nestedVo, nestedMirror, nestedStrict, emitted, payloadNameMap, extractedNameMap) } } @@ -261,14 +293,23 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() *
  • scalar (single) → `m.f!!`.
    • * */ - private fun strictArg(field: MetaField<*>, owner: MetaObject, nested: MutableList): String { + private fun strictArg( + field: MetaField<*>, + owner: MetaObject, + nested: MutableList, + payloadNameMap: Map, + ): String { val name = field.name // Object BEFORE array: array-of-objects maps element-wise (checked before isArray). val target = KotlinExtractSchemaEmitter.objectRefValueObject(field) if (target != null) { nested.add(target) - val nestedStrict = PackageMapping.splitFqn(target.name).second + "Payload" + // ADR-0044 (#228) — the strict `toStrict` target is collision-scoped via + // [payloadNameMap] (the SAME map the payload generator + the nested recursion use), + // so the call and the emitted `toStrict` definition agree under a collision. + val nestedStrict = payloadNameMap[target.name] + ?: KotlinNaming.payloadName(PackageMapping.splitFqn(target.name).second) return if (field.isArrayType()) { // The mirror element type for an array-of-objects is the NON-NULL nested mirror // (KotlinExtractSchemaEmitter.nestedNullableTypeName emits `List<Extracted>?`), @@ -373,11 +414,6 @@ open class KotlinExtractorGenerator : MultiFileDirectGeneratorBase() } } - /** Resolve a `@payloadRef` to its `object.value` (rejects entities — payloads must be VOs). */ - private fun resolveViewObject(loader: MetaDataLoader, ref: String): MetaObject? = - KotlinGenUtil.resolveObjectByShortOrFqn(loader, ref) - ?.takeIf { it.subType == MetaObject.SUBTYPE_VALUE } - // === MultiFileDirectGeneratorBase abstract-method stubs ==================== override fun writeSingleFile(md: MetaObject, writer: GeneratorIOWriter<*>?) { /* unused */ } override fun ?> getSingleWriter( diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinGenUtil.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinGenUtil.kt index 1e2d023de..03795a4d0 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinGenUtil.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinGenUtil.kt @@ -3,13 +3,18 @@ package com.metaobjects.generator.kotlin import com.metaobjects.MetaData import com.metaobjects.field.DateField import com.metaobjects.field.MetaField +import com.metaobjects.field.ObjectField import com.metaobjects.field.TimeField import com.metaobjects.field.TimestampField +import com.metaobjects.generator.GeneratorException import com.metaobjects.loader.MetaDataLoader import com.metaobjects.`object`.MetaObject import com.metaobjects.origin.AggregateOrigin +import com.metaobjects.origin.CollectionOrigin import com.metaobjects.origin.MetaOrigin import com.metaobjects.source.RdbSource +import com.metaobjects.template.MetaTemplate +import com.metaobjects.validation.SymbolTable /** * Helpers shared by the codegen-kotlin generators. Extracted to keep @@ -44,6 +49,38 @@ public object KotlinGenUtil { return null } + // ========================================================================= + // ADR-0042 — canonical package-local object-ref resolution (@payloadRef). + // + // The loader validates a template's @payloadRef via the SAME package-local + // contract (ValidationPhase.resolveRootObject, backed by SymbolTable): an FQN ref + // binds EXACTLY; a bare ref binds the referrer's own package, else a root-level + // object; NO cross-package bare-name / bare-tail fallback. Reusing the loader's own + // public [SymbolTable] (rather than a divergent codegen copy) keeps codegen's + // @payloadRef resolution identical to the loader's, so under a cross-package + // short-name collision codegen binds the SAME value-object the loader validated — + // never a load-order-dependent decoy (the #244 class). The bare-tail/first-match + // [resolveObjectByShortOrFqn] above is deliberately left as-is: it backs the + // @from/@of/@via dotted-ref navigation (a different ref kind, #244's own domain). + // ========================================================================= + + /** + * Resolve a metadata OBJECT reference (bare or FQN) under the loader's ADR-0042 + * package-local contract, or null. [referrerPkg] is the effective package of the node + * carrying the ref (a template's own `getPackage()` for a @payloadRef); "" for root-level. + */ + fun resolveObjectRef(loader: MetaDataLoader, ref: String?, referrerPkg: String?): MetaObject? { + if (ref == null) return null + return SymbolTable.build(loader.root).resolveObject(ref, referrerPkg ?: "") + } + + /** + * Resolve [ref] to its `object.value` target under the same ADR-0042 package-local + * contract as [resolveObjectRef] (rejects entities — a @payloadRef must be a VO). + */ + fun resolveValueObjectRef(loader: MetaDataLoader, ref: String?, referrerPkg: String?): MetaObject? = + resolveObjectRef(loader, ref, referrerPkg)?.takeIf { it.subType == MetaObject.SUBTYPE_VALUE } + /** * The first `source.rdb` child of [obj], RESOLVED through the `extends` super chain * (ADR-0039). `MetaObject.getSources(true)` walks the inheritance chain, so an entity @@ -230,4 +267,162 @@ public object KotlinGenUtil { } return sb.toString() } + + // ========================================================================= + // ADR-0044 — collision-scoped payload / extracted-mirror naming. + // + // Lifted here (was private on [KotlinPayloadGenerator]) so the strict payload + // record, the `...Extracted` mirror family, and the extractor all share ONE + // name-map algorithm. Kotlin `protected` is NOT same-package-visible, so the + // extract-tier emitters ([KotlinExtractSchemaEmitter] / [KotlinExtractMapperEmitter] / + // [KotlinExtractorGenerator], all in this package) reach these public helpers here. + // ========================================================================= + + /** + * ADR-0044 — the run's nested-PAYLOAD name map (VO FQN -> `Payload`, or the + * package-qualified `AcmeAlphaNotePayload` on a same-output-package short-name collision). + * See [computeNameMap]. Consumed by [KotlinPayloadGenerator] (the strict record files) and + * the extractor's `toStrict` / mapper-return references. + */ + fun computePayloadNameMap(templates: List, loader: MetaDataLoader): Map = + computeNameMap(templates, loader) { KotlinNaming.payloadName(it) } + + /** + * ADR-0044 — the run's nested-EXTRACTED-mirror name map (VO FQN -> `Extracted`, or the + * package-qualified `AcmeAlphaNoteExtracted` on a collision). Uses the SAME [computeNameMap] + * closure + collision grouping as [computePayloadNameMap] (differing only in the leaf suffix), + * so the `...Extracted` mirror and the `...Payload` strict record qualify in lockstep. + */ + fun computeExtractedNameMap(templates: List, loader: MetaDataLoader): Map = + computeNameMap(templates, loader) { KotlinNaming.extractedName(it) } + + /** + * ADR-0044 pass 1/2 — the run's nested-class name map, keyed by value-object FQN + * (`MetaObject.name`), scoped per OUTPUT PACKAGE. Kotlin is a one-class-per-file emitter, + * so its collision domain is the output prompts package: two value-objects sharing a bare + * short name written into the same package would clobber one `NotePayload.kt` / + * `NoteExtracted` declaration. A nested VO whose bare short name is UNIQUE in its output + * package is named `nameOf()` (byte-identical to pre-ADR-0044 output); a COLLISION + * names every member `nameOf()` (`acme::alpha::Note` -> `AcmeAlphaNote...`). + * A still-colliding derived name fails loud with [KotlinPayloadGenerator.ERR_PAYLOAD_NAME_COLLISION]. + * Pure function of the templates — never of emission order. + */ + private fun computeNameMap( + templates: List, + loader: MetaDataLoader, + nameOf: (String) -> String, + ): Map { + // FQN -> output package (first reaching template in caller-sorted order wins, matching + // the run-wide dedupe). The primary VO is template-named, so excluded. + val voOutPkg = LinkedHashMap() + val orderedFqns = ArrayList() + for (tmpl in templates) { + val payloadRef = tmpl.payloadRef ?: continue + // ADR-0042 — resolve @payloadRef under the loader's own package-local contract. + val vo = resolveValueObjectRef(loader, payloadRef, tmpl.getPackage()) ?: continue + val nestedPkg = KotlinNaming.promptsPackage(PackageMapping.splitFqn(tmpl.name).first) + collectNestedClosure(vo, loader, nestedPkg, voOutPkg, orderedFqns, mutableSetOf(vo.name)) + } + // Group by (output package, bare short name). + val byPkgShort = LinkedHashMap>() + for (fqn in orderedFqns) { + val key = voOutPkg[fqn] + " " + PackageMapping.splitFqn(fqn).second + byPkgShort.getOrPut(key) { ArrayList() }.add(fqn) + } + val nameMap = LinkedHashMap() + for (fqns in byPkgShort.values) { + if (fqns.size == 1) { + val fqn = fqns[0] + nameMap[fqn] = nameOf(PackageMapping.splitFqn(fqn).second) + } else { + for (fqn in fqns) { + val (pkg, short) = PackageMapping.splitFqn(fqn) + nameMap[fqn] = nameOf(packageQualifiedName(pkg, short)) + } + } + } + // Backstop — per output package, two DISTINCT FQNs deriving the same class name. + // Sorted so the named pair (and whether any collision fires) is order-independent. + val ownerByPkgName = HashMap() + for (fqn in nameMap.keys.sorted()) { + val pkgName = voOutPkg[fqn] + " " + nameMap[fqn] + val prev = ownerByPkgName.putIfAbsent(pkgName, fqn) + if (prev != null && prev != fqn) { + throw GeneratorException( + "${KotlinPayloadGenerator.ERR_PAYLOAD_NAME_COLLISION}: payload record name collision: \"${nameMap[fqn]}\" " + + "derives from both \"$prev\" and \"$fqn\" — rename one value-object or move " + + "it to a package that derives a distinct name" + ) + } + } + return nameMap + } + + /** + * ADR-0044 pass 1 — walk [vo]'s transitive nested-payload closure (plain + * `field.object @objectRef` + `origin.collection @via` edges), assigning each + * not-yet-seen target VO to [outPkg] (first reaching template wins) and recording it + * in [orderedFqns]. [seen] is seeded with the primary VO's FQN and is the cycle guard. + */ + private fun collectNestedClosure( + vo: MetaObject, + loader: MetaDataLoader, + outPkg: String, + voOutPkg: MutableMap, + orderedFqns: MutableList, + seen: MutableSet, + ) { + for (field in vo.metaFields) { + val target = nestedTargetOf(field, loader) ?: continue + val fqn = target.name + if (!seen.add(fqn)) continue + if (!voOutPkg.containsKey(fqn)) { + voOutPkg[fqn] = outPkg + orderedFqns.add(fqn) + } + collectNestedClosure(target, loader, outPkg, voOutPkg, orderedFqns, seen) + } + } + + /** + * The nested-payload target VO a [field] contributes to the closure, or `null` when it + * contributes no nested class. Passthrough / aggregate / computed / first origins yield + * scalar types (no nested class). NOTE: the `origin.collection @via` and `field.objectRef` + * navigation here uses [resolveObjectByShortOrFqn] / the loader-bound `objectRef` — the + * origin-navigation ref kind (#244's domain), intentionally NOT the ADR-0042 @payloadRef + * resolver (which is only for the template's own @payloadRef). + */ + private fun nestedTargetOf(field: MetaField<*>, loader: MetaDataLoader): MetaObject? { + val origin = field.children.filterIsInstance().firstOrNull() + if (origin is CollectionOrigin) { + val via = origin.via ?: return null + val (parentName, relName) = splitDottedRef(via) ?: return null + val parent = resolveObjectByShortOrFqn(loader, parentName) ?: return null + val rel = parent.relationships + .firstOrNull { it.name == relName || it.name.substringAfterLast("::") == relName } + ?: return null + val targetRef = rel.objectRef ?: return null + return resolveObjectByShortOrFqn(loader, targetRef) + } + if (origin != null) return null // passthrough / aggregate / computed / first -> scalar + if (field is ObjectField) { + val target = try { field.objectRef } catch (e: RuntimeException) { null } ?: return null + if (target.subType != MetaObject.SUBTYPE_VALUE) return null + return target + } + return null + } + + /** + * ADR-0044 — PascalCase each dotted segment of [kotlinPkg] (already `::`->`.` + * converted by [PackageMapping.splitFqn]), concatenate, append the bare [shortName] + * (`"acme.alpha"` + `"Note"` -> `"AcmeAlphaNote"`). A root-level (empty-package) node + * keeps its bare short name. + */ + fun packageQualifiedName(kotlinPkg: String, shortName: String): String { + if (kotlinPkg.isEmpty()) return shortName + return kotlinPkg.split(".") + .filter { it.isNotEmpty() } + .joinToString("") { it.replaceFirstChar { c -> c.uppercaseChar() } } + shortName + } } diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinNaming.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinNaming.kt index ceb081769..870a9fa1c 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinNaming.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinNaming.kt @@ -102,6 +102,14 @@ object KotlinNaming { /** [KotlinPayloadGenerator]: `templateShort + "Payload"`. */ fun payloadName(templateShort: String): String = templateShort + "Payload" + /** + * [KotlinExtractSchemaEmitter] / [KotlinOutputParserGenerator] / [KotlinExtractorGenerator]: + * `templateShort + "Extracted"` — the all-nullable extract mirror class name. The peer of + * [payloadName] for the lenient `...Extracted` mirror family; the SSOT so the root mirror, + * the nested mirrors, and the extractor's mirror references stay in lockstep. + */ + fun extractedName(templateShort: String): String = templateShort + "Extracted" + /** [KotlinRenderHelperGenerator]: `capitalizeFirst(templateShort) + "RenderHelper"`. */ fun renderHelperName(templateShort: String): String = capitalizeFirst(templateShort) + "RenderHelper" diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputParserGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputParserGenerator.kt index 8a2766562..cac67394b 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputParserGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputParserGenerator.kt @@ -80,17 +80,30 @@ open class KotlinOutputParserGenerator : MultiFileDirectGeneratorBase, + ) { val payloadRef = template.payloadRef if (payloadRef.isNullOrEmpty()) { // Loader validation normally catches this first; defensive only. @@ -100,7 +113,8 @@ open class KotlinOutputParserGenerator : MultiFileDirectGeneratorBase? (array-of-objects) so the runtime-delegating // extractLenient(loader, ...) overload can populate the full graph (FR-010 nested gap). - append(KotlinExtractSchemaEmitter.extractedClassDeclsNested(payloadVo, extractedClass)) + append(KotlinExtractSchemaEmitter.extractedClassDeclsNested(payloadVo, extractedClass, extractedNameMap)) append("\n\n") } append("/** Parser for LLM responses matching the `") @@ -219,7 +235,7 @@ open class KotlinOutputParserGenerator : MultiFileDirectGeneratorBase typed Extracted-mirror mappers (root + nested, deduped) ---- - append(KotlinExtractMapperEmitter.mapperMethods(payloadVo, extractedClass)) + append(KotlinExtractMapperEmitter.mapperMethods(payloadVo, extractedClass, extractedNameMap)) } append("}\n") } @@ -229,11 +245,6 @@ open class KotlinOutputParserGenerator : MultiFileDirectGeneratorBase?) { /* unused */ } override fun ?> getSingleWriter( diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputPromptGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputPromptGenerator.kt index 771842064..23568a41d 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputPromptGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinOutputPromptGenerator.kt @@ -101,7 +101,8 @@ open class KotlinOutputPromptGenerator : MultiFileDirectGeneratorBase?) { /* unused */ } diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt index 42e8940f0..8782620c6 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt @@ -3,7 +3,6 @@ package com.metaobjects.generator.kotlin import com.metaobjects.field.EnumField import com.metaobjects.field.MetaField import com.metaobjects.field.ObjectField -import com.metaobjects.generator.GeneratorException import com.metaobjects.generator.GeneratorIOWriter import com.metaobjects.generator.direct.MultiFileDirectGeneratorBase import com.metaobjects.loader.MetaDataLoader @@ -85,141 +84,14 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase() { val templates = loader.root.getChildren(MetaTemplate::class.java, true) .sortedBy { it.name } // ADR-0044 — collision-scoped nested-payload class names, a pure function of the - // loaded templates (order-independent). Keyed by VO FQN. - val nameMap = computePayloadNameMap(templates, loader) + // loaded templates (order-independent). Keyed by VO FQN. Lifted to [KotlinGenUtil] + // so the extract-tier emitters reuse the SAME name-map algorithm (#228). + val nameMap = KotlinGenUtil.computePayloadNameMap(templates, loader) for (md in templates) { emit(md, loader, outRoot, emittedNestedFqns, emittedEnumFqns, nameMap) } } - /** - * ADR-0044 pass 1/2 — the run's nested-payload name map, keyed by value-object FQN - * (`MetaObject.name`), scoped per OUTPUT PACKAGE. Kotlin is a one-class-per-file - * emitter (KotlinPoet `FileSpec(outPkg, className)`), so its collision domain is the - * output prompts package: two value-objects sharing a bare short name written into - * the same package would clobber one `NotePayload.kt`. A nested VO whose bare short - * name is UNIQUE in its output package emits `Payload` (byte-identical to - * pre-ADR-0044 output); a COLLISION emits every member under its package-qualified - * derived name (`acme::alpha::Note` -> `AcmeAlphaNotePayload`). A still-colliding - * derived name fails loud with [ERR_PAYLOAD_NAME_COLLISION]. Pure function of the - * templates — never of emission order. - */ - protected open fun computePayloadNameMap( - templates: List, - loader: MetaDataLoader, - ): Map { - // FQN -> output package (first reaching template in sorted order wins, matching - // the run-wide dedupe). The primary VO is template-named, so excluded. - val voOutPkg = LinkedHashMap() - val orderedFqns = ArrayList() - for (tmpl in templates) { - val payloadRef = tmpl.payloadRef ?: continue - val vo = resolveViewObject(loader, payloadRef) ?: continue - val nestedPkg = KotlinNaming.promptsPackage(PackageMapping.splitFqn(tmpl.name).first) - collectNestedClosure(vo, loader, nestedPkg, voOutPkg, orderedFqns, mutableSetOf(vo.name)) - } - // Group by (output package, bare short name). - val byPkgShort = LinkedHashMap>() - for (fqn in orderedFqns) { - val key = voOutPkg[fqn] + " " + PackageMapping.splitFqn(fqn).second - byPkgShort.getOrPut(key) { ArrayList() }.add(fqn) - } - val nameMap = LinkedHashMap() - for (fqns in byPkgShort.values) { - if (fqns.size == 1) { - val fqn = fqns[0] - nameMap[fqn] = KotlinNaming.payloadName(PackageMapping.splitFqn(fqn).second) - } else { - for (fqn in fqns) { - val (pkg, short) = PackageMapping.splitFqn(fqn) - nameMap[fqn] = KotlinNaming.payloadName(packageQualifiedName(pkg, short)) - } - } - } - // Backstop — per output package, two DISTINCT FQNs deriving the same class name. - // Sorted so the named pair (and whether any collision fires) is order-independent. - val ownerByPkgName = HashMap() - for (fqn in nameMap.keys.sorted()) { - val pkgName = voOutPkg[fqn] + " " + nameMap[fqn] - val prev = ownerByPkgName.putIfAbsent(pkgName, fqn) - if (prev != null && prev != fqn) { - throw GeneratorException( - "$ERR_PAYLOAD_NAME_COLLISION: payload record name collision: \"${nameMap[fqn]}\" " + - "derives from both \"$prev\" and \"$fqn\" — rename one value-object or move " + - "it to a package that derives a distinct name" - ) - } - } - return nameMap - } - - /** - * ADR-0044 pass 1 — walk [vo]'s transitive nested-payload closure (plain - * `field.object @objectRef` + `origin.collection @via` edges), assigning each - * not-yet-seen target VO to [outPkg] (first reaching template wins) and recording it - * in [orderedFqns]. [seen] is seeded with the primary VO's FQN and is the cycle guard. - */ - protected fun collectNestedClosure( - vo: MetaObject, - loader: MetaDataLoader, - outPkg: String, - voOutPkg: MutableMap, - orderedFqns: MutableList, - seen: MutableSet, - ) { - for (field in vo.metaFields) { - val target = nestedTargetOf(field, loader) ?: continue - val fqn = target.name - if (!seen.add(fqn)) continue - if (!voOutPkg.containsKey(fqn)) { - voOutPkg[fqn] = outPkg - orderedFqns.add(fqn) - } - collectNestedClosure(target, loader, outPkg, voOutPkg, orderedFqns, seen) - } - } - - /** - * The nested-payload target VO a [field] contributes to the closure, or `null` when - * it contributes no nested class. Mirrors the resolution in [resolveObjectFieldType] - * (plain `field.object @objectRef`) and [resolveCollectionType] (`origin.collection - * @via`) EXACTLY, so the closure walk and the emission walk agree. Passthrough / - * aggregate / computed / first origins yield scalar types (no nested class). - */ - protected fun nestedTargetOf(field: MetaField<*>, loader: MetaDataLoader): MetaObject? { - val origin = field.children.filterIsInstance().firstOrNull() - if (origin is CollectionOrigin) { - val via = origin.via ?: return null - val (parentName, relName) = KotlinGenUtil.splitDottedRef(via) ?: return null - val parent = KotlinGenUtil.resolveObjectByShortOrFqn(loader, parentName) ?: return null - val rel = parent.relationships - .firstOrNull { it.name == relName || it.name.substringAfterLast("::") == relName } - ?: return null - val targetRef = rel.objectRef ?: return null - return KotlinGenUtil.resolveObjectByShortOrFqn(loader, targetRef) - } - if (origin != null) return null // passthrough / aggregate / computed / first -> scalar - if (field is ObjectField) { - val target = try { field.objectRef } catch (e: RuntimeException) { null } ?: return null - if (target.subType != MetaObject.SUBTYPE_VALUE) return null - return target - } - return null - } - - /** - * ADR-0044 — PascalCase each dotted segment of [kotlinPkg] (already `::`->`.` - * converted by [PackageMapping.splitFqn]), concatenate, append the bare [shortName] - * (`"acme.alpha"` + `"Note"` -> `"AcmeAlphaNote"`). A root-level (empty-package) node - * keeps its bare short name. - */ - protected fun packageQualifiedName(kotlinPkg: String, shortName: String): String { - if (kotlinPkg.isEmpty()) return shortName - return kotlinPkg.split(".") - .filter { it.isNotEmpty() } - .joinToString("") { it.replaceFirstChar { c -> c.uppercaseChar() } } + shortName - } - protected open fun emit( template: MetaTemplate, loader: MetaDataLoader, @@ -229,7 +101,8 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase() { nameMap: Map, ) { val payloadRef = template.payloadRef ?: return - val payloadVo = resolveViewObject(loader, payloadRef) ?: return + // ADR-0042 — resolve @payloadRef under the loader's package-local contract (#228). + val payloadVo = KotlinGenUtil.resolveValueObjectRef(loader, payloadRef, template.getPackage()) ?: return val (templatePkg, templateShort) = PackageMapping.splitFqn(template.name) val outPkg = KotlinNaming.promptsPackage(templatePkg) @@ -544,11 +417,6 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase() { } } - /** Resolve a `@payloadRef` to its `object.value` (rejects entities — payloads must be VOs). */ - private fun resolveViewObject(loader: MetaDataLoader, ref: String): MetaObject? = - KotlinGenUtil.resolveObjectByShortOrFqn(loader, ref) - ?.takeIf { it.subType == MetaObject.SUBTYPE_VALUE } - // === MultiFileDirectGeneratorBase abstract-method stubs ==================== override fun writeSingleFile(md: MetaObject, writer: GeneratorIOWriter<*>?) { /* unused */ } override fun ?> getSingleWriter( diff --git a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperGenerator.kt b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperGenerator.kt index 3f3bda92a..75dabd9a5 100644 --- a/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperGenerator.kt +++ b/server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperGenerator.kt @@ -94,7 +94,9 @@ open class KotlinRenderHelperGenerator : MultiFileDirectGeneratorBasePayload` record ([KotlinPayloadGenerator]), + * 2. the `Extracted` all-nullable mirror family ([KotlinExtractSchemaEmitter] — its OWN + * second naming scheme, emitted into the parser file), + * 3. the extractor's strict-payload + mirror references ([KotlinExtractorGenerator]: + * `toStrict` over `Extracted`). + * + * ADR-0044 already scoped tier 1 (shipped 0.19.3). This test proves tiers 2 + 3 are scoped in + * lockstep with tier 1, so the generated parser + extractor compile (no duplicate `NoteExtracted` + * class / `fromNoteExtracted` / `toStrictNotePayload` function) and reference BOTH the + * package-qualified strict records AND the package-qualified mirrors. + * + * Loads the shared `fixtures/template-output-render-conformance/xpkg-collision-json/` corpus + * (`@format: json`, so the extract tier fires) — two `Note` VOs (`acme::alpha` / `acme::beta`) + * reached by FQN `@objectRef` from `acme::app::Digest`, the `DigestDoc` output's `@payloadRef`. + */ +@OptIn(org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi::class) +class KotlinExtractTierCollisionTest { + + private val corpus: Path = run { + var p: Path? = Path.of(System.getProperty("user.dir")).toAbsolutePath() + while (p != null && !Files.exists(p.resolve("fixtures/template-output-render-conformance"))) { + p = p.parent + } + assertTrue(p != null, "could not locate fixtures/template-output-render-conformance from user.dir") + p!!.resolve("fixtures/template-output-render-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() + } + + @Test fun `extract tier collision-scopes payload, mirror and extractor refs across a cross-package Note collision`() { + val outDir = Files.createTempDirectory("kext-xpkg-") + try { + val loader = loadDirectory("kext-xpkg", corpus.resolve("xpkg-collision-json")) + + // All three extract-tier generators run for one payload graph. + for (gen in listOf( + KotlinPayloadGenerator(), + KotlinOutputParserGenerator(), + KotlinExtractorGenerator(), + )) { + gen.setArgs(mapOf("outputDir" to outDir.toString())) + gen.execute(loader) + } + + val produced = Files.walk(outDir).filter { it.isRegularFile() }.toList() + val names = produced.map { it.fileName.toString() }.toSet() + + // ---- Tier 1: strict payload records (ADR-0044, the existing guarantee) ---- + assertTrue("AcmeAlphaNotePayload.kt" in names, "expected AcmeAlphaNotePayload.kt; files=$names") + assertTrue("AcmeBetaNotePayload.kt" in names, "expected AcmeBetaNotePayload.kt; files=$names") + assertTrue("NotePayload.kt" !in names, "must NOT emit a clobbered bare NotePayload.kt; files=$names") + + // ---- Tier 2 + 3: the parser file (mirror family) ---- + val parserSrc = produced.first { it.fileName.toString() == "DigestDocParser.kt" }.readText() + // Collision-scoped nested mirror declarations (their OWN naming scheme). + assertTrue("data class AcmeAlphaNoteExtracted(" in parserSrc, + "parser must declare AcmeAlphaNoteExtracted; saw:\n$parserSrc") + assertTrue("data class AcmeBetaNoteExtracted(" in parserSrc, + "parser must declare AcmeBetaNoteExtracted; saw:\n$parserSrc") + // The bare mirror name must NOT be emitted twice (the pre-fix duplicate-class compile error). + assertTrue("data class NoteExtracted(" !in parserSrc, + "must NOT emit a bare (colliding) NoteExtracted; saw:\n$parserSrc") + // The root mirror types its object fields as the collision-scoped nested mirrors. + assertTrue("AcmeAlphaNoteExtracted?" in parserSrc && "AcmeBetaNoteExtracted?" in parserSrc, + "root DigestDocExtracted must type fromAlpha/fromBeta as the scoped mirrors; saw:\n$parserSrc") + // Collision-scoped mappers — never a bare (duplicated) fromNoteExtracted. + assertTrue("fun fromAcmeAlphaNoteExtracted(" in parserSrc, parserSrc) + assertTrue("fun fromAcmeBetaNoteExtracted(" in parserSrc, parserSrc) + assertTrue("fun fromNoteExtracted(" !in parserSrc, + "must NOT emit a bare (colliding) fromNoteExtracted mapper; saw:\n$parserSrc") + + // ---- Tier 3: the extractor file references BOTH strict records AND mirrors ---- + val extractorSrc = produced.first { it.fileName.toString() == "DigestDocExtractor.kt" }.readText() + // Strict payload references (mapper name + return type). + assertTrue("toStrictAcmeAlphaNotePayload" in extractorSrc, extractorSrc) + assertTrue("toStrictAcmeBetaNotePayload" in extractorSrc, extractorSrc) + assertTrue("toStrictNotePayload(" !in extractorSrc, + "must NOT emit a bare (colliding) toStrictNotePayload; saw:\n$extractorSrc") + // Mirror references (the mapper parameter type). + assertTrue("AcmeAlphaNoteExtracted" in extractorSrc, extractorSrc) + assertTrue("AcmeBetaNoteExtracted" in extractorSrc, extractorSrc) + + // ---- The whole graph COMPILES — proves the scoped classes/functions are real, distinct, + // and the cross-file references (payload <-> parser <-> extractor) resolve. ---- + val result = compile(outDir) + assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode, result.messages) + } finally { + outDir.toFile().deleteRecursively() + } + } + + // === Checkpoint #1 — the build-time @payloadRef resolver is package-local (ADR-0042). === + // Two packages each declare their OWN payload VO `Report` (distinct field) AND a template + // with a BARE @payloadRef "Report". The prior first-match/bare-tail resolver bound whichever + // Report loaded first, so a template could emit the OTHER package's payload shape. The + // canonical resolver binds each template's OWN package's Report — in BOTH load orders. + + private val alphaFixture = """{ + "metadata.root": { "package": "pkg::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "alphaVal" } } + ] } }, + { "template.prompt": { "name": "ReportPrompt", + "@payloadRef": "Report", "@textRef": "alpha/x" } } + ] } + }""".trimIndent() + + private val betaFixture = """{ + "metadata.root": { "package": "pkg::beta", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "betaVal" } } + ] } }, + { "template.prompt": { "name": "ReportPrompt", + "@payloadRef": "Report", "@textRef": "beta/x" } } + ] } + }""".trimIndent() + + private fun assertBarePayloadRefBindsOwnPackage(firstAlpha: Boolean) { + val outDir = Files.createTempDirectory("kpay-bareref-") + try { + val loader = MetaDataLoader.createManual(false, "bareref-${firstAlpha}") + loader.init() + val sources = if (firstAlpha) + listOf(InMemoryStringSource(alphaFixture, "alpha"), InMemoryStringSource(betaFixture, "beta")) + else + listOf(InMemoryStringSource(betaFixture, "beta"), InMemoryStringSource(alphaFixture, "alpha")) + loader.load(sources) + loader.register() + + KotlinPayloadGenerator().apply { setArgs(mapOf("outputDir" to outDir.toString())) }.execute(loader) + + val alphaPayload = outDir.resolve("pkg/alpha/prompts/ReportPromptPayload.kt").readText() + val betaPayload = outDir.resolve("pkg/beta/prompts/ReportPromptPayload.kt").readText() + + // Each template's payload record must carry its OWN package's field — never the other's. + assertTrue("alphaVal" in alphaPayload && "betaVal" !in alphaPayload, + "pkg::alpha ReportPrompt must bind pkg::alpha::Report (alphaVal); saw:\n$alphaPayload") + assertTrue("betaVal" in betaPayload && "alphaVal" !in betaPayload, + "pkg::beta ReportPrompt must bind pkg::beta::Report (betaVal); saw:\n$betaPayload") + } finally { + outDir.toFile().deleteRecursively() + } + } + + @Test fun `bare payloadRef binds own package's payload — alpha loaded first`() = + assertBarePayloadRefBindsOwnPackage(firstAlpha = true) + + @Test fun `bare payloadRef binds own package's payload — beta loaded first`() = + assertBarePayloadRefBindsOwnPackage(firstAlpha = false) + + // === Checkpoint #1 (fix round 1) — the RENDER-HELPER and OUTPUT-PROMPT generators also === + // resolve @payloadRef; they must bind package-locally too. Two packages each declare their + // OWN `Report` output VO (distinct field) + a template.output with a BARE @payloadRef. + // The render-helper's build-time drift gate would THROW (wrong VO field-tree) and the + // output-prompt fragment would list the OTHER package's field, under the prior first-match + // resolver. Both must bind each template's own package's Report — in BOTH load orders. + + private val alphaOutFixture = """{ + "metadata.root": { "package": "po::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "alphaVal" } } + ] } }, + { "template.output": { "name": "ReportOut", + "@payloadRef": "Report", "@textRef": "alpha/t", "@format": "json" } } + ] } + }""".trimIndent() + + private val betaOutFixture = """{ + "metadata.root": { "package": "po::beta", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "betaVal" } } + ] } }, + { "template.output": { "name": "ReportOut", + "@payloadRef": "Report", "@textRef": "beta/t", "@format": "json" } } + ] } + }""".trimIndent() + + private fun writeTemplate(root: Path, ref: String, body: String) { + val file = root.resolve("$ref.mustache") + Files.createDirectories(file.parent) + Files.writeString(file, body) + } + + private fun assertRenderHelperAndOutputPromptBindOwnPackage(firstAlpha: Boolean) { + val outDir = Files.createTempDirectory("krh-op-bareref-") + val templateRoot = Files.createTempDirectory("krh-op-tmpl-") + try { + // Each template's mustache references ONLY its own package's field: a wrong-package + // bind makes the render-helper drift gate THROW (the referenced var is not on the + // mis-bound field-tree). + writeTemplate(templateRoot, "alpha/t", "{{alphaVal}}") + writeTemplate(templateRoot, "beta/t", "{{betaVal}}") + + val loader = MetaDataLoader.createManual(false, "rhop-bareref-$firstAlpha") + loader.init() + val sources = if (firstAlpha) + listOf(InMemoryStringSource(alphaOutFixture, "alpha"), InMemoryStringSource(betaOutFixture, "beta")) + else + listOf(InMemoryStringSource(betaOutFixture, "beta"), InMemoryStringSource(alphaOutFixture, "alpha")) + loader.load(sources) + loader.register() + + // Render-helper: its drift gate would throw under a wrong-package @payloadRef bind. + KotlinRenderHelperGenerator().apply { + setArgs(mapOf("outputDir" to outDir.toString(), "templateRoot" to templateRoot.toString())) + }.execute(loader) + // Output-prompt: emits a field-name fragment derived from the resolved VO. + KotlinOutputPromptGenerator().apply { setArgs(mapOf("outputDir" to outDir.toString())) }.execute(loader) + + val alphaPrompt = outDir.resolve("po/alpha/prompts/ReportOutPrompt.kt").readText() + val betaPrompt = outDir.resolve("po/beta/prompts/ReportOutPrompt.kt").readText() + assertTrue("alphaVal" in alphaPrompt && "betaVal" !in alphaPrompt, + "po::alpha ReportOut output-prompt must list po::alpha::Report (alphaVal); saw:\n$alphaPrompt") + assertTrue("betaVal" in betaPrompt && "alphaVal" !in betaPrompt, + "po::beta ReportOut output-prompt must list po::beta::Report (betaVal); saw:\n$betaPrompt") + + // Render-helper files emitted for both (no drift throw aborted generation). + assertTrue(Files.exists(outDir.resolve("po/alpha/prompts/ReportOutRenderHelper.kt")), + "po::alpha ReportOut render-helper must be emitted (drift gate bound the right VO)") + assertTrue(Files.exists(outDir.resolve("po/beta/prompts/ReportOutRenderHelper.kt")), + "po::beta ReportOut render-helper must be emitted (drift gate bound the right VO)") + } finally { + outDir.toFile().deleteRecursively() + templateRoot.toFile().deleteRecursively() + } + } + + @Test fun `render-helper and output-prompt bind own package's payload — alpha loaded first`() = + assertRenderHelperAndOutputPromptBindOwnPackage(firstAlpha = true) + + @Test fun `render-helper and output-prompt bind own package's payload — beta loaded first`() = + assertRenderHelperAndOutputPromptBindOwnPackage(firstAlpha = false) +} diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/apidocs/JavaFieldShapes.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/apidocs/JavaFieldShapes.java index 403f6c486..58853c336 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/apidocs/JavaFieldShapes.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/apidocs/JavaFieldShapes.java @@ -10,6 +10,7 @@ import com.metaobjects.loader.MetaDataLoader; import com.metaobjects.object.MetaObject; import com.metaobjects.template.MetaTemplate; +import com.metaobjects.util.MetaDataUtil; import java.nio.file.Files; import java.nio.file.Path; @@ -94,7 +95,8 @@ public static List payloadFields(MetaData template, MetaDataLoader l if (!(template instanceof MetaTemplate tmpl)) return List.of(); String payloadRef = tmpl.getPayloadRef(); if (payloadRef == null || payloadRef.isEmpty()) return List.of(); - MetaObject vo = SpringPayloadGenerator.resolveValueObject(loader, payloadRef); + MetaObject vo = SpringPayloadGenerator.resolveValueObject( + loader, payloadRef, MetaDataUtil.findPackageForMetaData(tmpl)); if (vo == null) return List.of(); SpringPayloadGenerator gen = new SpringPayloadGenerator(); diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/LlmTraceHelperGenerator.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/LlmTraceHelperGenerator.java index bd44ebd45..2b7b8cc66 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/LlmTraceHelperGenerator.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/LlmTraceHelperGenerator.java @@ -149,7 +149,11 @@ protected void emit(MetaObject entity, MetaDataLoader loader, Path outRoot) { String responseRef = prompt.getResponseRef(); if (responseRef == null || responseRef.isEmpty()) return; // @responseRef gates the helper - MetaObject responseVo = resolveValueObject(loader, responseRef); + // #228 — referrer is the PROMPT (the @responseRef is authored on it), not the + // entity: findPackageForMetaData walks parents, so a nested prompt still + // resolves the entity's effective package when the prompt itself carries none. + MetaObject responseVo = resolveValueObject(loader, responseRef, + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(prompt)); if (responseVo == null) { throw new GeneratorException( "trace-helper: entity \"" + entity.getName() + "\" prompt @responseRef \"" @@ -276,15 +280,18 @@ protected static PromptTemplate firstPrompt(MetaObject entity) { return null; } - /** Resolve a {@code @responseRef} to its {@code object.value} target (by FQN or short name). */ - protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - String refShort = SpringNaming.splitFqn(ref)[1]; - for (MetaObject obj : loader.getMetaObjects()) { - if (!MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) continue; - if (obj.getName().equals(ref)) return obj; - if (SpringNaming.splitFqn(obj.getName())[1].equals(refShort)) return obj; - } - return null; + /** + * Resolve a {@code @responseRef} to its {@code object.value} target under the + * ADR-0042 package-local contract (#228). Was a package-BLIND bare-name scan that, + * worse, reduced the REF ITSELF to its trailing {@code ::} segment before matching + * — the #219/#244 bare-tail-fallback pattern: an FQN {@code @responseRef} that + * failed an exact match would still bind ANY same-bare-named {@code object.value} + * in a WRONG package (first match, load-order-dependent), not just a genuinely + * bare ref. Now delegates to the shared {@link SpringNaming#resolveValueObjectRef} + * (FQN matches exactly, never a bare-tail fallback). + */ + protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref, String referrerPkg) { + return SpringNaming.resolveValueObjectRef(loader, ref, referrerPkg); } /** Java string-literal quoting with the common escapes. */ diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringNaming.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringNaming.java index 96ad3e319..f0fed366b 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringNaming.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringNaming.java @@ -1,6 +1,7 @@ package com.metaobjects.generator.spring; import com.metaobjects.MetaData; +import com.metaobjects.loader.MetaDataLoader; import com.metaobjects.object.MetaObject; import com.metaobjects.source.RdbSource; @@ -50,6 +51,60 @@ public static String[] splitFqn(String fqn) { }; } + /** + * ADR-0042 — resolve a metadata OBJECT reference (bare or FQN) to a {@link MetaObject} + * under the package-local contract, or {@code null} when nothing matches: + *
      + *
    • FQN {@code ref} (contains {@code "::"}) → EXACT match on + * {@link MetaData#getName()}. No bare-tail fallback, so an FQN pointing at one + * package never binds a same-named object in another.
      • + *
    • bare {@code ref} (no {@code "::"}) → the referrer's OWN package + * ({@code ::}) first, else a root-level (unpackaged) object + * whose name IS {@code ref}. Package-local BEFORE root-level; no cross-package + * short-name scan, no bare-tail fallback.
      • + *
    + * Mirrors the loader's own {@code ValidationPhase#resolveRootObject} (the same contract + * the TS/Python/C# ports' canonical resolvers implement), so a codegen-time + * {@code @payloadRef}/{@code @responseRef} resolution agrees with the loader's own + * validation of the same ref under a cross-package short-name collision. + * + * @param referrerPkg the effective package of the node carrying the ref ("" for root-level) + */ + public static MetaObject resolveObjectRef(MetaDataLoader loader, String ref, String referrerPkg) { + if (ref == null) return null; + String pkg = referrerPkg == null ? "" : referrerPkg; + if (ref.contains("::")) { + for (MetaObject obj : loader.getMetaObjects()) { + if (ref.equals(obj.getName())) return obj; + } + return null; + } + String localKey = pkg.isEmpty() ? ref : pkg + "::" + ref; + MetaObject own = null; + MetaObject rootLevel = null; + for (MetaObject obj : loader.getMetaObjects()) { + String key = obj.getName(); + if (key == null) continue; + if (key.equals(localKey)) own = obj; + if (key.equals(ref)) rootLevel = obj; + } + if (own != null) return own; + return localKey.equals(ref) ? null : rootLevel; + } + + /** + * Resolve {@code ref} to its {@code object.value} target under the same ADR-0042 + * package-local contract as {@link #resolveObjectRef} (rejects entities / other + * subtypes). The shared home for every {@code @payloadRef}/{@code @responseRef} + * resolution in this package — callers derive {@code referrerPkg} via + * {@code MetaDataUtil.findPackageForMetaData(referrerNode)} (walks parents, so it + * works for both a root-level template and a nested {@code template.prompt}). + */ + public static MetaObject resolveValueObjectRef(MetaDataLoader loader, String ref, String referrerPkg) { + MetaObject obj = resolveObjectRef(loader, ref, referrerPkg); + return (obj != null && MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) ? obj : null; + } + /** * Naive pluralisation: lowercase + "s". Matches the cross-port reference * (TS / C# / Kotlin all use the same trivial rule for the default route diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java index cb33b023e..29015ea44 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputParserGenerator.java @@ -21,8 +21,10 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collection; +import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -115,18 +117,28 @@ public void execute(MetaDataLoader loader) { parseArgs(); Path outRoot = Paths.get(outDir.getAbsolutePath()); - // Stable name order — matches the other ports' deterministic emission. + // ADR-0044 (#228) — gather ALL MetaTemplate (prompt/output/toolcall), matching + // SpringPayloadGenerator.execute()'s domain EXACTLY, so the run-wide nested-payload + // name map (below) agrees with the payload tier's — even though only + // template.output gets a parser FILE emitted here (the SUBTYPE_OUTPUT filter moves + // to the `outputs` sublist, after the nameMap is computed over every template). // ADR-0039: root-scan discipline — resolving children accessor. + List allTemplates = + new ArrayList<>(loader.getRoot().getChildren(MetaTemplate.class, true)); + allTemplates.sort(Comparator.comparing(MetaTemplate::getName)); + + Map nameMap = SpringPayloadGenerator.computePayloadNameMap(allTemplates, loader); + + // Stable name order — matches the other ports' deterministic emission. List outputs = new ArrayList<>(); - for (MetaTemplate t : loader.getRoot().getChildren(MetaTemplate.class, true)) { + for (MetaTemplate t : allTemplates) { if (TemplateConstants.SUBTYPE_OUTPUT.equals(t.getSubType())) { outputs.add(t); } } - outputs.sort((a, b) -> a.getName().compareTo(b.getName())); for (MetaTemplate tmpl : outputs) { - emit(tmpl, loader, outRoot); + emit(tmpl, loader, outRoot, nameMap); } } @@ -146,14 +158,15 @@ public static boolean appliesTo(MetaData node, MetaDataLoader loader) { if (!TemplateConstants.SUBTYPE_OUTPUT.equals(template.getSubType())) return false; String payloadRef = template.getPayloadRef(); if (payloadRef == null || payloadRef.isEmpty()) return false; - return resolveValueObject(loader, payloadRef) != null; + return resolveValueObject(loader, payloadRef, MetaDataUtil.findPackageForMetaData(template)) != null; } - protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot) { + protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot, Map nameMap) { if (!appliesTo(template, loader)) { return; // missing @payloadRef, or not a VO — same contract as SpringPayloadGenerator } - MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef()); + MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef(), + MetaDataUtil.findPackageForMetaData(template)); String[] split = SpringNaming.splitFqn(template.getName()); String templatePkg = split[0]; @@ -228,7 +241,7 @@ protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot) src.append(" }\n"); // ---- Generated ValueObject(Map) -> typed-record mappers (payload + nested, deduped) ---- - emitMapperMethods(src, payloadVo, loader, payloadClass); + emitMapperMethods(src, payloadVo, loader, payloadClass, nameMap); appendMapperHelpers(src); } src.append("}\n"); @@ -258,15 +271,20 @@ protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot) * here also stops the emitter from recursing forever on a cyclic value-object graph.

    */ protected void emitMapperMethods(StringBuilder src, MetaObject rootVo, - MetaDataLoader loader, String rootPayloadClass) { + MetaDataLoader loader, String rootPayloadClass, + Map nameMap) { Set emitted = new LinkedHashSet<>(); - emitMapper(src, rootVo, loader, rootPayloadClass, emitted); + emitMapper(src, rootVo, loader, rootPayloadClass, emitted, nameMap); } protected void emitMapper(StringBuilder src, MetaObject vo, MetaDataLoader loader, - String payloadClass, Set emitted) { + String payloadClass, Set emitted, Map nameMap) { if (!emitted.add(vo.getName())) { - return; // already emitted (dedupe + cycle guard) + return; // already emitted (dedupe + cycle guard) — vo.getName() is already the + // FQN (Java's MetaObject.getName() is package-qualified), so this key + // is never bare — a cross-package same-short-name collision does NOT + // silently drop the second VO's mapper (unlike the #219/#244 bare-key + // dedupe bug other ports hit here). } // Discover nested mappers to emit AFTER this one (declaration order is irrelevant @@ -285,7 +303,7 @@ protected void emitMapper(StringBuilder src, MetaObject vo, MetaDataLoader loade List fields = new ArrayList<>(vo.getMetaFields()); for (int i = 0; i < fields.size(); i++) { MetaField field = fields.get(i); - String arg = mapperArgForField(field, vo, payloadClass, loader, nestedVos); + String arg = mapperArgForField(field, vo, payloadClass, loader, nestedVos, nameMap); body.append(" ").append(arg); if (i < fields.size() - 1) body.append(','); body.append('\n'); @@ -296,8 +314,8 @@ protected void emitMapper(StringBuilder src, MetaObject vo, MetaDataLoader loade // Recurse into nested payloads (post-order, deduped). for (MetaObject nested : nestedVos) { - String nestedClass = nestedPayloadClass(nested); - emitMapper(src, nested, loader, nestedClass, emitted); + String nestedClass = nestedPayloadClass(nested, nameMap); + emitMapper(src, nested, loader, nestedClass, emitted, nameMap); } } @@ -309,7 +327,8 @@ protected void emitMapper(StringBuilder src, MetaObject vo, MetaDataLoader loade */ @SuppressWarnings("rawtypes") protected String mapperArgForField(MetaField field, MetaObject owner, String payloadClass, - MetaDataLoader loader, List nestedVos) { + MetaDataLoader loader, List nestedVos, + Map nameMap) { String name = field.getName(); // Nested object / array-of-objects (but NOT enum, which is a string-backed scalar). @@ -318,7 +337,7 @@ protected String mapperArgForField(MetaField field, MetaObject owner, String MetaObject target = MetaDataUtil.getObjectRef(field); if (target != null && MetaObject.SUBTYPE_VALUE.equals(target.getSubType())) { nestedVos.add(target); - String nestedClass = nestedPayloadClass(target); + String nestedClass = nestedPayloadClass(target, nameMap); if (field.isArrayType()) { // List: map each element Map; the assembled value is a List. // from is a static method in scope within this generated parser class. @@ -365,8 +384,22 @@ protected String mapperArgForField(MetaField field, MetaObject owner, String return "ExtractMap.asString(d, \"" + name + "\")"; } - /** {@code Payload} — mirrors {@link SpringPayloadGenerator}'s nested naming. */ - protected static String nestedPayloadClass(MetaObject vo) { + /** + * {@code Payload} — consults the ADR-0044 collision-scoped + * {@code nameMap} ({@link SpringPayloadGenerator#computePayloadNameMap}) FIRST (#228): + * a nested VO whose bare short name is unique in the run's payload domain keeps its + * bare {@code Payload} derivation (byte-identical to pre-#228 output); a + * cross-package short-name collision resolves to the SAME package-qualified name + * {@link SpringPayloadGenerator} actually emitted (e.g. {@code AcmeAlphaNotePayload}) — + * otherwise this generator would reference a bare {@code NotePayload} class the payload + * generator never emits under collision (a compile error: two same-named + * {@code fromPayload} mapper methods would ALSO collide). Falls back to the bare + * derivation when {@code vo} isn't in the map (the primary VO — template-named, outside + * the map's domain — or a caller without a precomputed map). + */ + protected static String nestedPayloadClass(MetaObject vo, Map nameMap) { + String mapped = nameMap.get(vo.getName()); + if (mapped != null) return mapped; return SpringNaming.payloadName(SpringNaming.splitFqn(vo.getName())[1]); } @@ -417,15 +450,16 @@ private static String escapeJava(String value) { return sb.toString(); } - /** Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities). */ - protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - for (MetaObject obj : loader.getMetaObjects()) { - if (!MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) continue; - if (obj.getName().equals(ref)) return obj; - String[] split = SpringNaming.splitFqn(obj.getName()); - if (split[1].equals(ref)) return obj; - } - return null; + /** + * Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities) + * under the ADR-0042 package-local contract (#228) — was a package-BLIND bare-name + * scan over every loaded {@code object.value} (first match wins, load-order-dependent); + * now delegates to the shared {@link SpringNaming#resolveValueObjectRef} so a bare + * {@code @payloadRef} binds the referrer's OWN package first, agreeing with the + * loader's own {@code ValidationPhase} validation of the same ref. + */ + protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref, String referrerPkg) { + return SpringNaming.resolveValueObjectRef(loader, ref, referrerPkg); } // === MultiFileDirectGeneratorBase abstract-method stubs ==================== diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputPromptGenerator.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputPromptGenerator.java index e6ab7286d..b510f6ea3 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputPromptGenerator.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringOutputPromptGenerator.java @@ -113,14 +113,16 @@ public static boolean appliesTo(MetaData node, MetaDataLoader loader) { if (!supported) return false; String payloadRef = template.getPayloadRef(); if (payloadRef == null || payloadRef.isEmpty()) return false; - return resolveValueObject(loader, payloadRef) != null; + return resolveValueObject(loader, payloadRef, + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)) != null; } protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot) { if (!appliesTo(template, loader)) { return; // unsupported @format, missing @payloadRef, or not a VO } - MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef()); + MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef(), + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)); String[] split = SpringNaming.splitFqn(template.getName()); String templatePkg = split[0]; @@ -170,15 +172,14 @@ protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot) } } - /** Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities). */ - protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - for (MetaObject obj : loader.getMetaObjects()) { - if (!MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) continue; - if (obj.getName().equals(ref)) return obj; - String[] split = SpringNaming.splitFqn(obj.getName()); - if (split[1].equals(ref)) return obj; - } - return null; + /** + * Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities) + * under the ADR-0042 package-local contract (#228) — was a package-BLIND bare-name + * scan (first match wins, load-order-dependent); now delegates to the shared + * {@link SpringNaming#resolveValueObjectRef}. + */ + protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref, String referrerPkg) { + return SpringNaming.resolveValueObjectRef(loader, ref, referrerPkg); } // === MultiFileDirectGeneratorBase abstract-method stubs ==================== diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringPayloadGenerator.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringPayloadGenerator.java index 80a4e2d28..f63659001 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringPayloadGenerator.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringPayloadGenerator.java @@ -155,14 +155,20 @@ public void execute(MetaDataLoader loader) { * {@code AcmeAlphaNotePayload}). A still-colliding derived name fails loud with * {@link #ERR_PAYLOAD_NAME_COLLISION}. Pure function of the templates — never of * emission order. + * + *

    public static (promoted from {@code protected} instance) so + * {@link SpringOutputParserGenerator} — a sibling generator consuming the SAME + * {@code @payloadRef} closure — reuses this ONE name map rather than re-deriving + * naming (#228: extract/output-parser tier collision-scoped naming). */ - protected Map computePayloadNameMap(List templates, MetaDataLoader loader) { + public static Map computePayloadNameMap(List templates, MetaDataLoader loader) { // FQN -> output package (first reaching template in sorted order wins, matching // the run-wide dedupe below). The primary VO is template-named, so excluded. Map voOutPkg = new LinkedHashMap<>(); List orderedFqns = new ArrayList<>(); for (MetaTemplate tmpl : templates) { - MetaObject vo = resolveValueObject(loader, tmpl.getPayloadRef()); + MetaObject vo = resolveValueObject(loader, tmpl.getPayloadRef(), + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(tmpl)); if (vo == null) continue; String nestedPkg = SpringNaming.promptsPackage(SpringNaming.splitFqn(tmpl.getName())[0]); Set seen = new HashSet<>(); @@ -212,8 +218,11 @@ protected Map computePayloadNameMap(List templates * assigning each not-yet-seen target VO to {@code outPkg} (first reaching * template wins) and recording it in {@code orderedFqns}. {@code seen} is seeded * with the primary VO's FQN and doubles as the cycle guard. + * + *

    public static (promoted from {@code protected} instance, #228) — see + * {@link #computePayloadNameMap}. */ - protected void collectNestedClosure(MetaObject vo, + public static void collectNestedClosure(MetaObject vo, MetaDataLoader loader, String outPkg, Map voOutPkg, @@ -239,8 +248,11 @@ protected void collectNestedClosure(MetaObject vo, * {@link #resolveCollectionType} ({@code origin.collection @via}) EXACTLY, so the * closure walk and the emission walk agree on the target set. Passthrough / * aggregate origins yield scalar types (no nested record). + * + *

    public static (promoted from {@code protected} instance, #228) — see + * {@link #computePayloadNameMap}. */ - protected MetaObject nestedTargetOf(MetaField field, MetaDataLoader loader) { + public static MetaObject nestedTargetOf(MetaField field, MetaDataLoader loader) { MetaOrigin origin = firstOriginChild(field); if (origin instanceof CollectionOrigin co) { String via = co.getVia(); @@ -273,8 +285,11 @@ protected MetaObject nestedTargetOf(MetaField field, MetaDataLoader loader) { * {@code ::}->{@code .} converted by {@link SpringNaming#splitFqn}), concatenate, * append the bare {@code shortName} ({@code "acme.alpha"} + {@code "Note"} -> * {@code "AcmeAlphaNote"}). A root-level (empty-package) node keeps its bare name. + * + *

    public static (widened from package-private-visible {@code protected + * static}, #228) — see {@link #computePayloadNameMap}. */ - protected static String packageQualifiedName(String javaPkg, String shortName) { + public static String packageQualifiedName(String javaPkg, String shortName) { if (javaPkg == null || javaPkg.isEmpty()) return shortName; StringBuilder sb = new StringBuilder(); for (String seg : javaPkg.split("\\.")) { @@ -295,7 +310,8 @@ public static boolean appliesTo(MetaData node, MetaDataLoader loader) { if (!(node instanceof MetaTemplate template)) return false; String payloadRef = template.getPayloadRef(); if (payloadRef == null || payloadRef.isEmpty()) return false; - return resolveValueObject(loader, payloadRef) != null; + return resolveValueObject(loader, payloadRef, + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)) != null; } protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot, @@ -303,7 +319,8 @@ protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot, if (!appliesTo(template, loader)) { return; // missing @payloadRef, or not a VO — same contract as Kotlin / C# / Python } - MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef()); + MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef(), + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)); String[] split = SpringNaming.splitFqn(template.getName()); String templatePkg = split[0]; @@ -673,11 +690,17 @@ protected static MetaObject resolveObjectByShortOrFqn(MetaDataLoader loader, Str return null; } - /** Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities). */ - public static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - MetaObject obj = resolveObjectByShortOrFqn(loader, ref); - if (obj == null) return null; - return MetaObject.SUBTYPE_VALUE.equals(obj.getSubType()) ? obj : null; + /** + * Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities) + * under the ADR-0042 package-local contract (#228): a bare ref resolves in + * {@code referrerPkg} first, else root-level; an FQN ref matches exactly. Distinct + * from {@link #resolveObjectByShortOrFqn} (used only by the {@code origin.@from}/ + * {@code @of}/{@code @via} dotted-ref walk above, a different ref kind out of this + * fix's scope) — {@code @payloadRef} is the one every port's canonical resolver + * gates, matching the loader's own {@code ValidationPhase} validation of the same ref. + */ + public static MetaObject resolveValueObject(MetaDataLoader loader, String ref, String referrerPkg) { + return SpringNaming.resolveValueObjectRef(loader, ref, referrerPkg); } /** diff --git a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringRenderHelperGenerator.java b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringRenderHelperGenerator.java index 48aaade2a..e3ead4ee3 100644 --- a/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringRenderHelperGenerator.java +++ b/server/java/codegen-spring/src/main/java/com/metaobjects/generator/spring/SpringRenderHelperGenerator.java @@ -134,7 +134,8 @@ public static boolean appliesTo(MetaData node, MetaDataLoader loader) { if (!TemplateConstants.SUBTYPE_OUTPUT.equals(template.getSubType())) return false; String payloadRef = template.getPayloadRef(); if (payloadRef == null || payloadRef.isEmpty()) return false; - return resolveValueObject(loader, payloadRef) != null; + return resolveValueObject(loader, payloadRef, + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)) != null; } protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot, @@ -142,7 +143,8 @@ protected void emit(MetaTemplate template, MetaDataLoader loader, Path outRoot, if (!appliesTo(template, loader)) { return; // missing @payloadRef, or not a VO — same contract as SpringPayloadGenerator } - MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef()); + MetaObject payloadVo = resolveValueObject(loader, template.getPayloadRef(), + com.metaobjects.util.MetaDataUtil.findPackageForMetaData(template)); String[] split = SpringNaming.splitFqn(template.getName()); String templatePkg = split[0]; @@ -408,15 +410,16 @@ private static String attr(MetaTemplate template, String attr) { return template.getMetaAttr(attr).getValueAsString(); } - /** Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities). */ - protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref) { - for (MetaObject obj : loader.getMetaObjects()) { - if (!MetaObject.SUBTYPE_VALUE.equals(obj.getSubType())) continue; - if (obj.getName().equals(ref)) return obj; - String[] split = SpringNaming.splitFqn(obj.getName()); - if (split[1].equals(ref)) return obj; - } - return null; + /** + * Resolve {@code @payloadRef} to its {@code object.value} target (rejects entities) + * under the ADR-0042 package-local contract (#228) — was a package-BLIND bare-name + * scan (first match wins, load-order-dependent); now delegates to the shared + * {@link SpringNaming#resolveValueObjectRef}. Distinct from this file's OWN + * {@link #resolveNestedObjectRef} (the {@code @objectRef} field-tree walk), which was + * ALREADY package-local-correct. + */ + protected static MetaObject resolveValueObject(MetaDataLoader loader, String ref, String referrerPkg) { + return SpringNaming.resolveValueObjectRef(loader, ref, referrerPkg); } /** Java string-literal quoting with the common escapes. */ diff --git a/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/GeneratedTraceHelperCompileRunTest.java b/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/GeneratedTraceHelperCompileRunTest.java index d28b1a7a2..af00479e3 100644 --- a/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/GeneratedTraceHelperCompileRunTest.java +++ b/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/GeneratedTraceHelperCompileRunTest.java @@ -209,6 +209,64 @@ public void skipsEntityNotDerivedFromLlmCallBase() throws Exception { Files.exists(gen.resolve("acme/ai/PlainEntityTraceHelper.java"))); } + /** + * #228 checkpoint 4 — {@code resolveValueObject}'s pre-fix bare-tail-fallback bug + * (the #219/#244 "wrong node despite a VALID FQN target" pattern): the old + * implementation checked, PER CANDIDATE in loader iteration order, "does this + * object's bare short name equal the ref's bare tail?" — so a same-bare-named + * DECOY {@code object.value} visited BEFORE the true FQN target would win + * immediately, even though the correctly-FQN-qualified target also exists and + * loads later. Package {@code acme::other} declares a decoy {@code GreetResponse} + * (loaded FIRST); {@code acme::ai} declares its OWN {@code GreetResponse} and an + * FQN {@code @responseRef: "acme::ai::GreetResponse"} that unambiguously names it. + * Asserts the generated helper derives its typed result record from {@code acme::ai}'s + * shape ({@code greeting}/{@code score}) — never the decoy's ({@code otherField}). + */ + @Test + public void responseRefFqnBindsOwnPackageNotABareTailDecoyLoadedFirst() throws Exception { + String decoyMeta = "{ \"metadata.root\": {" + + " \"package\": \"acme::other\"," + + " \"children\": [" + + " { \"object.value\": { \"name\": \"GreetResponse\", \"children\": [" + + " { \"field.string\": { \"name\": \"otherField\", \"@required\": true } }" + + " ]}}" + + " ]" + + "}}"; + + MetaDataLoader loader = new MetaDataLoader( + LoaderOptions.create(false, false, true), + MetaDataLoader.SUBTYPE_MANUAL, "trace-responseref-fqn"); + loader.init(); + // Decoy loads FIRST — under the pre-fix bare-tail-fallback bug this would win. + loader.load(List.of( + new InMemoryStringSource(decoyMeta, "trace-responseref-fqn/meta.other.json"), + new InMemoryStringSource(META, "trace-responseref-fqn/meta.ai.json"))); + + Path gen = tmp.newFolder("gen-responseref-fqn").toPath(); + LlmTraceHelperGenerator generator = new LlmTraceHelperGenerator(); + Map args = new HashMap<>(); + args.put("outputDir", gen.toString()); + generator.setArgs(args); + generator.execute(loader); + + Path helper = gen.resolve("acme/ai/GreetingCallTraceHelper.java"); + assertTrue("GreetingCallTraceHelper.java must be emitted at " + helper, Files.exists(helper)); + String src = Files.readString(helper); + + // The baked FQN string is the load-bearing proof: LlmTraceHelperGenerator bakes + // the RESOLVED responseVo's OWN name (not the raw @responseRef attr verbatim), so + // a pre-fix bare-tail-fallback mis-resolution to the decoy would have baked + // "acme::other::GreetResponse" here instead. + assertTrue("must resolve + bake acme::ai's OWN GreetResponse FQN; saw:\n" + src, + src.contains("getMetaObjectByName(\"acme::ai::GreetResponse\")")); + assertFalse("must NEVER bind/bake the decoy acme::other::GreetResponse; saw:\n" + src, + src.contains("acme::other") || src.contains("otherField")); + + // Compile it too — proves the resolved MetaObject is a real, loadable node + // (not just a text match), same rigor as the other tests in this file. + compileGenerated(gen); + } + // ----------------------------------------------------------------------------------------- // helpers // ----------------------------------------------------------------------------------------- diff --git a/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/OutputParserExtractTierCollisionTest.java b/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/OutputParserExtractTierCollisionTest.java new file mode 100644 index 000000000..bb47ad0fc --- /dev/null +++ b/server/java/codegen-spring/src/test/java/com/metaobjects/generator/spring/OutputParserExtractTierCollisionTest.java @@ -0,0 +1,266 @@ +package com.metaobjects.generator.spring; + +import com.metaobjects.loader.LoaderOptions; +import com.metaobjects.loader.MetaDataLoader; +import com.metaobjects.loader.uri.URIHelper; +import com.metaobjects.registry.SharedRegistryTestBase; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * #228 — Java port of the extract/output-parser tier collision-scoped naming fix. + * {@link SpringOutputParserGenerator} now consumes {@link SpringPayloadGenerator}'s + * OWN ADR-0044 name map (never re-derives naming) so a cross-package short-name + * collision on a NESTED {@code field.object} target VO gets the SAME + * package-qualified record name the payload tier emits — a bare {@code NotePayload} + * reference would either be a dangling class reference or (worse) a duplicate-method + * compile error when two colliding VOs both derive {@code fromNotePayload(...)}. + * + *

    Also covers the ADR-0042 build-time {@code @payloadRef} resolver fix + * (checkpoint 3): {@code resolveValueObject} was previously a package-BLIND + * bare-name-anywhere scan (first match in load order wins); it now resolves in the + * referring template's OWN package first, matching the loader's own + * {@code ValidationPhase} validation of the same ref. + */ +public class OutputParserExtractTierCollisionTest extends SharedRegistryTestBase { + + @Rule + public TemporaryFolder tempFolder = new TemporaryFolder(); + + // ------------------------------------------------------------------------- + // Step 1 (brief) — shared xpkg-collision-json corpus: nested field.object + // collision (acme::alpha::Note / acme::beta::Note), both reachable from one + // payload (Digest) via FQN @objectRef. + // ------------------------------------------------------------------------- + + @Test + public void xpkgCollisionJsonEmitsDistinctMappersForBothCollidingNestedVos() throws Exception { + Path corpus = findCorpus(); + assertTrue("shared corpus fixtures/template-output-render-conformance must be reachable", + corpus != null && Files.exists(corpus.resolve("xpkg-collision-json/meta.app.json"))); + Path xpkg = corpus.resolve("xpkg-collision-json"); + + Path outDir = tempFolder.newFolder("outputparser-xpkg").toPath(); + MetaDataLoader loader = loadMultiFile("xpkg-op", + xpkg.resolve("meta.alpha.json"), + xpkg.resolve("meta.beta.json"), + xpkg.resolve("meta.app.json")); + + SpringOutputParserGenerator gen = new SpringOutputParserGenerator(); + Map args = new HashMap<>(); + args.put("outputDir", outDir.toString()); + gen.setArgs(args); + gen.execute(loader); + + Path parser = outDir.resolve("acme/app/prompts/DigestDocParser.java"); + assertTrue("expected DigestDocParser.java at " + parser, Files.exists(parser)); + String src = Files.readString(parser); + + // Both colliding nested VOs get their OWN distinct, collision-scoped mapper — + // never the bare `NotePayload` the payload generator no longer emits under + // collision, and never a dropped/clobbered second mapper. + assertTrue("expected a fromAcmeAlphaNotePayload mapper; saw:\n" + src, + src.contains("private static AcmeAlphaNotePayload fromAcmeAlphaNotePayload(java.util.Map d)")); + assertTrue("expected a fromAcmeBetaNotePayload mapper; saw:\n" + src, + src.contains("private static AcmeBetaNotePayload fromAcmeBetaNotePayload(java.util.Map d)")); + assertFalse("must NEVER reference/emit the shadowed bare fromNotePayload mapper; saw:\n" + src, + src.contains("fromNotePayload(")); + assertFalse("must NEVER reference the shadowed bare NotePayload type; saw:\n" + src, + src.contains("NotePayload fromNotePayload") || src.contains(" NotePayload)")); + + // The root mapper's fromAlpha/fromBeta fields route to their OWN qualified mapper. + assertTrue("fromAlpha field must recurse into fromAcmeAlphaNotePayload; saw:\n" + src, + src.contains("fromAcmeAlphaNotePayload(asMap(d.get(\"fromAlpha\")))")); + assertTrue("fromBeta field must recurse into fromAcmeBetaNotePayload; saw:\n" + src, + src.contains("fromAcmeBetaNotePayload(asMap(d.get(\"fromBeta\")))")); + } + + // ------------------------------------------------------------------------- + // No-churn: a non-colliding nested VO keeps its bare mapper name/type — proves + // the nameMap consultation is a no-op absent a collision (byte-identical to + // pre-#228 output). + // ------------------------------------------------------------------------- + + private static final String NO_CHURN_FIXTURE = """ + { + "metadata.root": { "package": "acme::ai", "children": [ + { "object.value": { "name": "Detail", "children": [ + { "field.string": { "name": "note", "@required": true } } + ] } }, + { "object.value": { "name": "WidgetOut", "children": [ + { "field.string": { "name": "title", "@required": true } }, + { "field.object": { "name": "detail", "@objectRef": "Detail" } } + ] } }, + { "template.output": { + "name": "WidgetDoc", + "@payloadRef": "WidgetOut", + "@textRef": "widget/doc", + "@format": "json" + } } + ] } + } + """; + + @Test + public void noChurnNonCollidingNestedVoKeepsBareMapperName() throws Exception { + Path outDir = tempFolder.newFolder("outputparser-nochurn").toPath(); + Path workspace = tempFolder.newFolder("outputparser-nochurn-fx").toPath(); + MetaDataLoader loader = SpringTestFixtures.loadFixture(workspace, "nochurn", NO_CHURN_FIXTURE); + + SpringOutputParserGenerator gen = new SpringOutputParserGenerator(); + Map args = new HashMap<>(); + args.put("outputDir", outDir.toString()); + gen.setArgs(args); + gen.execute(loader); + + Path parser = outDir.resolve("acme/ai/prompts/WidgetDocParser.java"); + assertTrue("expected WidgetDocParser.java at " + parser, Files.exists(parser)); + String src = Files.readString(parser); + + assertTrue("non-colliding nested VO must keep its BARE mapper; saw:\n" + src, + src.contains("private static DetailPayload fromDetailPayload(java.util.Map d)")); + assertTrue("detail field must recurse into the bare fromDetailPayload; saw:\n" + src, + src.contains("fromDetailPayload(asMap(d.get(\"detail\")))")); + assertFalse("must NOT package-qualify a non-colliding VO", src.contains("AcmeAiDetailPayload")); + } + + // ------------------------------------------------------------------------- + // Checkpoint 3 — build-time @payloadRef resolver: a BARE @payloadRef that + // cross-package-collides on its OWN name must bind the referring template's + // OWN package, regardless of load order (was package-blind, first-match-wins). + // ------------------------------------------------------------------------- + + private static String alphaReportJson() { + return """ + { "metadata.root": { "package": "acme::alpha", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "alphaVal", "@required": true } } + ] } }, + { "template.output": { + "name": "ReportDocAlpha", + "@payloadRef": "Report", + "@textRef": "report/alpha", + "@format": "json" + } } + ] } } + """; + } + + private static String betaReportJson() { + return """ + { "metadata.root": { "package": "acme::beta", "children": [ + { "object.value": { "name": "Report", "children": [ + { "field.string": { "name": "betaVal", "@required": true } } + ] } }, + { "template.output": { + "name": "ReportDocBeta", + "@payloadRef": "Report", + "@textRef": "report/beta", + "@format": "json" + } } + ] } } + """; + } + + @Test + public void barePayloadRefCollisionBindsOwnPackage_alphaLoadedFirst() throws Exception { + assertBarePayloadRefBindsOwnPackage(true); + } + + @Test + public void barePayloadRefCollisionBindsOwnPackage_betaLoadedFirst() throws Exception { + assertBarePayloadRefBindsOwnPackage(false); + } + + private void assertBarePayloadRefBindsOwnPackage(boolean alphaFirst) throws Exception { + Path workspace = tempFolder.newFolder("bare-payloadref-" + alphaFirst).toPath(); + Path alphaFile = workspace.resolve("meta.alpha.json"); + Path betaFile = workspace.resolve("meta.beta.json"); + Files.writeString(alphaFile, alphaReportJson()); + Files.writeString(betaFile, betaReportJson()); + + MetaDataLoader loader = alphaFirst + ? loadMultiFile("bare-" + alphaFirst, alphaFile, betaFile) + : loadMultiFile("bare-" + alphaFirst, betaFile, alphaFile); + + Path outDir = tempFolder.newFolder("bare-payloadref-out-" + alphaFirst).toPath(); + + // SpringPayloadGenerator: each template's record must carry its OWN + // package's field, never the other's, regardless of load order. + SpringPayloadGenerator payloadGen = new SpringPayloadGenerator(); + Map args = new HashMap<>(); + args.put("outputDir", outDir.toString()); + payloadGen.setArgs(args); + payloadGen.execute(loader); + + String alphaPayloadSrc = Files.readString(outDir.resolve("acme/alpha/prompts/ReportDocAlphaPayload.java")); + String betaPayloadSrc = Files.readString(outDir.resolve("acme/beta/prompts/ReportDocBetaPayload.java")); + assertTrue("ReportDocAlphaPayload must carry alphaVal (own package); saw:\n" + alphaPayloadSrc, + alphaPayloadSrc.contains("String alphaVal")); + assertFalse("ReportDocAlphaPayload must NOT carry betaVal (wrong package); saw:\n" + alphaPayloadSrc, + alphaPayloadSrc.contains("betaVal")); + assertTrue("ReportDocBetaPayload must carry betaVal (own package); saw:\n" + betaPayloadSrc, + betaPayloadSrc.contains("String betaVal")); + assertFalse("ReportDocBetaPayload must NOT carry alphaVal (wrong package); saw:\n" + betaPayloadSrc, + betaPayloadSrc.contains("alphaVal")); + + // SpringOutputParserGenerator: same resolver, same guarantee — the generated + // mapper for each template's OWN root payload must read its OWN field name. + SpringOutputParserGenerator parserGen = new SpringOutputParserGenerator(); + parserGen.setArgs(args); + parserGen.execute(loader); + + String alphaParserSrc = Files.readString(outDir.resolve("acme/alpha/prompts/ReportDocAlphaParser.java")); + String betaParserSrc = Files.readString(outDir.resolve("acme/beta/prompts/ReportDocBetaParser.java")); + assertTrue("ReportDocAlphaParser's mapper must read alphaVal; saw:\n" + alphaParserSrc, + alphaParserSrc.contains("ExtractMap.asString(d, \"alphaVal\")")); + assertFalse("ReportDocAlphaParser's mapper must NOT read betaVal; saw:\n" + alphaParserSrc, + alphaParserSrc.contains("betaVal")); + assertTrue("ReportDocBetaParser's mapper must read betaVal; saw:\n" + betaParserSrc, + betaParserSrc.contains("ExtractMap.asString(d, \"betaVal\")")); + assertFalse("ReportDocBetaParser's mapper must NOT read alphaVal; saw:\n" + betaParserSrc, + betaParserSrc.contains("alphaVal")); + } + + // ------------------------------------------------------------------------- + // Helpers (mirrors SpringPayloadGeneratorTest's private helpers of the same name). + // ------------------------------------------------------------------------- + + /** Walk up from {@code user.dir} to the repo-root shared corpus, or {@code null}. */ + private static Path findCorpus() { + Path p = Paths.get(System.getProperty("user.dir")).toAbsolutePath(); + while (p != null && !Files.exists(p.resolve("fixtures/template-output-render-conformance"))) { + p = p.getParent(); + } + return p != null ? p.resolve("fixtures/template-output-render-conformance") : null; + } + + /** Load several metadata files into one merged loader (multi-package fixtures), in the + * EXACT order given (MetaDataLoader does not re-sort an explicit URI list). */ + private MetaDataLoader loadMultiFile(String baseName, Path... files) throws Exception { + List uris = new ArrayList<>(); + for (Path f : files) { + uris.add(URIHelper.toURI("model:file:" + f.toAbsolutePath().toString().replace('\\', '/'))); + } + MetaDataLoader loader = new MetaDataLoader( + LoaderOptions.create(false, false, true), + MetaDataLoader.SUBTYPE_MANUAL, + "spring-test-" + baseName); + loader.setSourceURIs(uris); + loader.init(); + return loader; + } +} diff --git a/server/python/src/metaobjects/apidocs/builder.py b/server/python/src/metaobjects/apidocs/builder.py index fe7357620..de70bd2d1 100644 --- a/server/python/src/metaobjects/apidocs/builder.py +++ b/server/python/src/metaobjects/apidocs/builder.py @@ -55,11 +55,24 @@ from metaobjects.meta.persistence.source.source_constants import SOURCE_KIND_TABLE from metaobjects.meta.template import template_constants as tc from metaobjects.shared.base_types import TYPE_OBJECT, TYPE_TEMPLATE +from metaobjects.shared.separators import PACKAGE_SEP # Structured formats that get an output-format prompt + a tolerant extractor. _STRUCTURED_FORMATS = frozenset({tc.TEMPLATE_FORMAT_JSON, tc.TEMPLATE_FORMAT_XML}) +def _pkg_of(node: MetaData) -> str: + """The effective package of a node — its ``resolution_key()`` minus the + trailing ``::`` ("" for a root-level node). Duplicated (not imported) to + match the existing per-generator convention. Used to derive a template's + referrer package for ``resolve_payload_vo`` (#228) — see that function's + docstring for why this ancestor-walk-aware form is used instead of the + loader's bare ``tpl.package or tpl.file_default_package or ""``.""" + key = node.resolution_key() + i = key.rfind(PACKAGE_SEP) + return "" if i == -1 else key[:i] + + # --------------------------------------------------------------------------- # Applies-predicates — each REUSES the matching generator's own gate helpers, so # inclusion can never drift from emission. (The Python generators gate inline; @@ -95,7 +108,9 @@ def _payload_resolves(tmpl: MetaData, root: MetaData) -> MetaObject | None: payload_ref = tmpl.get_meta_attr(tc.TEMPLATE_ATTR_PAYLOAD_REF) # ADR-0039: template attr resolves via extends (not origin; templates CAN extend) if not isinstance(payload_ref, str) or not payload_ref: return None - return resolve_payload_vo(root, payload_ref) + # ADR-0042 (#228): the referrer is THIS template — a bare @payloadRef resolves + # in ITS OWN package first. + return resolve_payload_vo(root, payload_ref, _pkg_of(tmpl)) def _is_email_kind(tmpl: MetaData) -> bool: diff --git a/server/python/src/metaobjects/cli.py b/server/python/src/metaobjects/cli.py index 11522d4e9..b4b45e3a0 100644 --- a/server/python/src/metaobjects/cli.py +++ b/server/python/src/metaobjects/cli.py @@ -84,6 +84,17 @@ verify as render_verify, ) from metaobjects.shared.base_types import TYPE_TEMPLATE +from metaobjects.shared.separators import PACKAGE_SEP + + +def _pkg_of(node: MetaData) -> str: + """The effective package of a node — its ``resolution_key()`` minus the + trailing ``::`` ("" for a root-level node). Duplicated (not imported) to + match the existing per-generator convention. Used to derive a template's + referrer package for ``_resolve_payload_vo`` (#228).""" + key = node.resolution_key() + i = key.rfind(PACKAGE_SEP) + return "" if i == -1 else key[:i] def _default_generators() -> list[Generator]: @@ -642,7 +653,9 @@ def _verify_templates(args: argparse.Namespace) -> int: print(f"error: [{tmpl.name}] missing @payloadRef.", file=sys.stderr) error_count += 1 continue - vo = _resolve_payload_vo(root, payload_ref) + # ADR-0042 (#228): the referrer is THIS template — a bare @payloadRef + # resolves in ITS OWN package first. + vo = _resolve_payload_vo(root, payload_ref, _pkg_of(tmpl)) if vo is None: print( f"error: [{tmpl.name}] @payloadRef '{payload_ref}' did not " diff --git a/server/python/src/metaobjects/codegen/collision_names.py b/server/python/src/metaobjects/codegen/collision_names.py new file mode 100644 index 000000000..e03b0bfb4 --- /dev/null +++ b/server/python/src/metaobjects/codegen/collision_names.py @@ -0,0 +1,110 @@ +"""ADR-0044 — collision-scoped nested-VO name assignment (shared). + +Promoted out of ``payload_vo_generator.py`` (formerly the module-private +``_package_qualified_name`` / ``_assign_nested_names``) so every Python generator that +walks a payload's nested-value-object closure derives IDENTICAL emitted names for an +IDENTICAL closure. Today that's the payload-record tier (``payload_vo_generator.py``) +AND the extract/output-parser tier (``extract_delegate_emitter.py`` / +``extractor_generator.py`` / ``output_parser_generator.py``) — a nested class an +extractor module IMPORTS from the sibling payload module must be spelled exactly the +way the payload module itself emitted it, so both tiers MUST share one naming +function rather than re-derive a second (and possibly-diverging) copy (issue #228). + +:func:`assign_nested_names` returns BASE names only — a bare short name when it is +unique across the closure, else its package-qualified derived form +(``acme::alpha`` + ``Note`` → ``AcmeAlphaNote``). It never bakes in a suffix; each +caller applies its OWN transform on top of the shared base (the payload tier: +``payload_class_name(base)`` → ``...Payload``; the extract tier: ``f"{base}Extracted"`` +for the mirror dataclass, ``f"_to_strict_{snake(base)}"`` / ``f"_from_{snake(base)} +_extracted"`` for the mapper function names) — one pure function of the closure feeds +every naming scheme that must agree on the same base. +""" +from __future__ import annotations + +from collections.abc import Callable, Mapping + +from metaobjects.errors import ErrorCode +from metaobjects.meta.meta_data import MetaData +from metaobjects.shared.separators import PACKAGE_SEP + +#: ADR-0044 backstop error code — REUSED (never redefined) from the shared cross-port +#: error-code ledger (``metaobjects.errors.ErrorCode``), which already carries it. +ERR_PAYLOAD_NAME_COLLISION = ErrorCode.ERR_PAYLOAD_NAME_COLLISION.value + + +def pascal_segment(name: str) -> str: + """``priority`` → ``Priority`` (leading char upper-cased only; no snake-splitting) + — matches the cross-port rule for PascalCasing a bare field/package segment.""" + return name[:1].upper() + name[1:] if name else name + + +def package_qualified_name(pkg: str, short_name: str) -> str: + """PascalCase each ``::``-segment of *pkg*, concatenate, append the bare + *short_name* (``acme::alpha`` + ``Note`` → ``AcmeAlphaNote``). A root-level + (empty-package) node keeps its bare short name — the loader's own-package + uniqueness already precludes two root-level nodes sharing a name, so this can't + silently under-qualify.""" + if pkg == "": + return short_name + return "".join(pascal_segment(seg) for seg in pkg.split(PACKAGE_SEP)) + short_name + + +def _pkg_of(node: MetaData) -> str: + """The effective package of an object — its ``resolution_key()`` minus the + trailing ``::`` ("" for a root-level object). Derived from the resolution + key so it is correct for BOTH loaded trees (file_default_package) and + programmatically-built trees (package only on the root).""" + key = node.resolution_key() + i = key.rfind(PACKAGE_SEP) + return "" if i == -1 else key[:i] + + +def assign_nested_names( + closure: Mapping[str, MetaData], + class_name_fn: Callable[[str], str] | None = None, +) -> dict[str, str]: + """ADR-0044 pass 2 — ``resolution_key()`` → emitted name. A PURE function of the + closure's ``(key, short-name, package)`` triples, never of traversal order: a bare + short name unique in the closure emits its bare form (byte-identical to + pre-ADR-0044 output); a short-name collision emits EVERY member under its + package-qualified derived form. If two distinct keys still derive the same name, + fails loud with ``ERR_PAYLOAD_NAME_COLLISION`` — never silently collides a second + time. + + *class_name_fn*, when supplied, transforms each derived BASE name (bare or + package-qualified) into the caller's final emitted name — e.g. + ``payload_vo_generator`` passes ``payload_class_name`` (bare ``"Note"`` → + ``"NotePayload"``) so its own collision backstop message names the actual emitted + class. Omitted (``None``, the default) → identity, returning bare BASE names — the + extract tier's callers apply their OWN suffix/transform on top (see module + docstring) so every naming scheme derives from ONE shared base-assignment pass. + """ + name_fn: Callable[[str], str] = class_name_fn if class_name_fn is not None else (lambda base: base) + + by_short: dict[str, list[str]] = {} + for key, node in closure.items(): + by_short.setdefault(node.name, []).append(key) + + name_map: dict[str, str] = {} + for short, keys in by_short.items(): + if len(keys) == 1: + name_map[keys[0]] = name_fn(short) + continue + for key in keys: + node = closure[key] + name_map[key] = name_fn(package_qualified_name(_pkg_of(node), short)) + + # Backstop — sorted by key so both the emptiness of the colliding set and the + # pair named in the message are a pure function of the closure, not dict order. + owner: dict[str, str] = {} + for key in sorted(name_map): + emitted = name_map[key] + existing = owner.get(emitted) + if existing is not None and existing != key: + raise ValueError( + f"{ERR_PAYLOAD_NAME_COLLISION}: payload record name collision: " + f'"{emitted}" derives from both "{existing}" and "{key}" — rename one ' + "value-object or move it to a package that derives a distinct name" + ) + owner[emitted] = key + return name_map diff --git a/server/python/src/metaobjects/codegen/extract_delegate_emitter.py b/server/python/src/metaobjects/codegen/extract_delegate_emitter.py index ae282563a..5c2e7ba8f 100644 --- a/server/python/src/metaobjects/codegen/extract_delegate_emitter.py +++ b/server/python/src/metaobjects/codegen/extract_delegate_emitter.py @@ -24,43 +24,55 @@ Bounded by the cross-port ``MAX_NEST_DEPTH`` via the runtime — codegen here only mirrors the runtime's resolved object graph, so depth/cycle guarding lives in ``object_extract``. -The emitter dedupes mirrors/mappers by VO simple name (cycle-safe). +The emitter dedupes mirrors/mappers by ``resolution_key()`` (the package-qualified FQN, +cycle-safe) — NOT the bare VO ``name``, which would silently collapse two same-short-name +value-objects from different packages into one (ADR-0044, #228). A bare-name collision +resolves to a package-qualified emitted name via the shared +:func:`~metaobjects.codegen.collision_names.assign_nested_names` pass (see +:func:`build_name_map`) — the SAME naming pass the payload-record tier +(``payload_vo_generator``) runs, so this tier's names never diverge from the payload +module's own. """ from __future__ import annotations from metaobjects.codegen import fr010_field_mapping as fm +from metaobjects.codegen.collision_names import assign_nested_names from metaobjects.meta.core.field import field_constants as fc from metaobjects.meta.meta_data import MetaData -from metaobjects.shared.base_types import TYPE_OBJECT +from metaobjects.naming_refs import resolve_object_ref from metaobjects.shared.separators import PACKAGE_SEP -def _find_object(root: MetaData, name: str) -> MetaData | None: - """The top-level ``object.*`` node named *name*, or ``None``. - - ADR-0039 sanctioned own: top-level object lookup on the loader ROOT - (metadata.root is never extended, so own == effective) — mirrors the TS - reference (``root.ownChildren()``). - """ - for c in root.own_children(): - if c.type == TYPE_OBJECT and c.name == name: - return c - return None +def _pkg_of(node: MetaData) -> str: + """The effective package of an object — its ``resolution_key()`` minus the + trailing ``::`` ("" for a root-level object). Duplicated (not imported) to + match the existing per-generator convention — ``payload_vo_generator.py`` and + ``render_helper_generator.py`` each carry their own identical copy.""" + key = node.resolution_key() + i = key.rfind(PACKAGE_SEP) + return "" if i == -1 else key[:i] def ref_vo(field: MetaData, root: MetaData) -> MetaData | None: """The ``@objectRef`` target VO for a nested-object field, or ``None`` when - unresolvable. Matches first on the full ref, then the trailing simple-name - segment (mirrors the runtime ``_resolve_object_ref`` short-name fallback).""" + unresolvable. + + ADR-0042 (#228) — resolves via the canonical `resolve_object_ref` package-local + contract: an FQN ref resolves EXACTLY; a bare ref resolves in the DECLARING + field's own package first, else a root-level object. NO bare-tail short-name + fallback — that pattern (matching an FQN ref by its trailing simple-name segment + against ANY same-named object root-wide) is the #219/ADR-0042-banned "wrong + node" bug: under a cross-package short-name collision it silently binds + whichever same-named object happens to load first, regardless of which package + the ref actually pointed at. *referrer_pkg* is the field's OWN declaring + package (which differs from the VO's when the field is inherited via `extends` + from an abstract VO in another package) — mirrors payload_vo_generator's + `_resolve_object_field_type`.""" ref = field.attrs().get(fc.FIELD_ATTR_OBJECT_REF) if not isinstance(ref, str) or not ref: return None - direct = _find_object(root, ref) - if direct is not None: - return direct - if PACKAGE_SEP in ref: - return _find_object(root, ref.rsplit(PACKAGE_SEP, 1)[-1]) - return None + referrer_pkg = _pkg_of(field.parent) if field.parent is not None else "" + return resolve_object_ref(root, ref, referrer_pkg) def _is_object_field(field: MetaData) -> bool: @@ -69,14 +81,20 @@ def _is_object_field(field: MetaData) -> bool: return field.sub_type == fc.FIELD_SUBTYPE_OBJECT -def mirror_name(vo: MetaData) -> str: - """The extracted-mirror dataclass name for a value-object (``Extracted``).""" - return f"{vo.name}Extracted" +def mirror_name(vo: MetaData, name_map: dict[str, str]) -> str: + """The extracted-mirror dataclass name for a value-object + (``Extracted``) — ADR-0044 (#228) collision-scoped: *base* is the bare + ``vo.name`` unless a cross-package bare-name collision requires the + package-qualified derived form (see :func:`build_name_map`).""" + base = name_map.get(vo.resolution_key(), vo.name) + return f"{base}Extracted" -def _mapper_name(vo: MetaData) -> str: - """The mapper function name for a value-object (``_from__extracted``).""" - return f"_from_{_snake(vo.name)}_extracted" +def _mapper_name(vo: MetaData, name_map: dict[str, str]) -> str: + """The mapper function name for a value-object (``_from__extracted``) + — *base* per :func:`mirror_name`.""" + base = name_map.get(vo.resolution_key(), vo.name) + return f"_from_{_snake(base)}_extracted" def root_mapper_name(template_name: str) -> str: @@ -101,12 +119,12 @@ def _snake(name: str) -> str: # ============================================================================= -def _nested_mirror_type(field: MetaData, root: MetaData) -> str: +def _nested_mirror_type(field: MetaData, root: MetaData, name_map: dict[str, str]) -> str: """The nullable mirror annotation for one field — nested-aware (nested objects become ``Extracted``; array-of-objects become ``list[...]``).""" if _is_object_field(field): target = ref_vo(field, root) - base = f'"{mirror_name(target)}"' if target is not None else "object" + base = f'"{mirror_name(target, name_map)}"' if target is not None else "object" elem = f"{base} | None" return f"list[{elem}] | None" if fm.is_array(field) else elem if fm.is_array(field): @@ -130,20 +148,29 @@ def _nested_mirror_type(field: MetaData, root: MetaData) -> str: def reachable_vos(vo: MetaData, root: MetaData) -> list[MetaData]: """``vo`` + every value-object reachable through nested ``@objectRef`` fields, in - stable BFS order, deduped by simple name (cycle-safe).""" + stable BFS order, deduped by ``resolution_key()`` (cycle-safe). + + ADR-0044 (#228) — deduping by the bare ``name`` (pre-fix) silently DROPPED a + second cross-package value-object sharing the first one's bare short name (e.g. + ``acme::alpha::Note`` + ``acme::beta::Note``): once the first ``Note`` was seen, + the second's bare name matched ``seen`` and it was never queued/emitted — a + silent shape loss, not merely a naming cosmetic. ``resolution_key()`` is the + package-qualified FQN, so two same-short-name VOs from different packages are + two distinct keys and both survive the walk.""" out: list[MetaData] = [] seen: set[str] = set() queue: list[MetaData] = [vo] while queue: cur = queue.pop(0) - if cur.name in seen: + key = cur.resolution_key() + if key in seen: continue - seen.add(cur.name) + seen.add(key) out.append(cur) for f in fm.fields(cur): if _is_object_field(f): target = ref_vo(f, root) - if target is not None and target.name not in seen: + if target is not None and target.resolution_key() not in seen: queue.append(target) return out @@ -156,30 +183,55 @@ def has_nested(vo: MetaData, root: MetaData) -> bool: return False +def build_name_map(vo: MetaData, root: MetaData) -> dict[str, str]: + """ADR-0044 (#228) — the collision-scoped BASE name map for ``vo``'s reachable + nested-VO closure, keyed by ``resolution_key()``. ``vo`` itself (the PRIMARY — + named after the enclosing template/payload, never its own bare name) is + excluded from the collision domain, mirroring payload_vo_generator's + `_collect_nested_closure` (which seeds ``seen`` with the primary's own key for + the identical reason). + + Reuses the SAME shared :func:`~metaobjects.codegen.collision_names.assign_nested_names` + pass the payload-record tier runs, so a nested VO's derived BASE here agrees + exactly with the payload module's own emitted class name (modulo the + ``Payload``/``Extracted`` suffix each tier applies on top) — the extractor's + imports and the payload module's declarations can never diverge.""" + primary_key = vo.resolution_key() + closure: dict[str, MetaData] = { + cur.resolution_key(): cur + for cur in reachable_vos(vo, root) + if cur.resolution_key() != primary_key + } + return assign_nested_names(closure) + + # ============================================================================= # Nested-aware mirror dataclasses # ============================================================================= def nested_mirror_dataclasses( - vo: MetaData, root: MetaData, payload_mirror: str + vo: MetaData, root: MetaData, payload_mirror: str, name_map: dict[str, str] ) -> list[str]: """Emit the nested-aware mirror dataclass for ``vo`` and every reachable nested VO (deduped). The payload mirror keeps the canonical ``