diff --git a/docs/factory-events.md b/docs/factory-events.md index 25799cac..901aa786 100644 --- a/docs/factory-events.md +++ b/docs/factory-events.md @@ -261,11 +261,11 @@ Event types are resolved on the client by `TypeFullName` against the runtime `Fa ### IL Trimming and Event Records -Every descendant of `FactoryEventBase` is automatically preserved from IL trimming. `FactoryEventBase` carries `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` with `Inherited = true`, so every descendant's constructors and public properties survive trimming without any per-event annotation or generator emission. `IFactoryEvents.Raise` retains `[DynamicallyAccessedMembers(All)]` on its generic parameter for producer-side call-site preservation. +Every accessible descendant of `FactoryEventBase` is automatically preserved from IL trimming: the source generator discovers each concrete descendant declared in a compilation and emits a per-assembly event-preservation registrar that preserves the event's constructors/properties and its nested property graph. (The `[DynamicallyAccessedMembers]` annotation on `FactoryEventBase` does not do this — DAM does not flow to derived types under ILLink, which a publish-trimmed repro proved.) `IFactoryEvents.Raise` retains `[DynamicallyAccessedMembers(All)]` on its generic parameter for producer-side call-site preservation. This model also drives discovery: `FactoryEventBase` carries `[FactoryEvent]` with `Inherited = true`, which the runtime `FactoryEventTypeRegistry` keys off during its assembly scan. Inheriting `FactoryEventBase` is sufficient — consumers never apply `[FactoryEvent]` directly. -See [IL Trimming](trimming.md#factory-event-type-preservation) for the full mechanism, the end-to-end publish-trimmed smoke verification (`EventRelaySmokeTest.cs`), and the `IL2091` consideration for user code that forwards `Raise` through its own generic wrapper. +See [IL Trimming](trimming.md#factory-event-type-preservation) for the full mechanism, the end-to-end publish-trimmed verification (`EventSubscribeOnlySmokeTest.cs` — the subscribe-only consumer shape — plus the `EventRelaySmokeTest.cs` round-trip), and the `IL2091` consideration for user code that forwards `Raise` through its own generic wrapper. --- diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md index 5ec68de2..06df20ec 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md @@ -3,8 +3,8 @@ **Plan #:** 002 **Date:** 2026-07-06 **Related Todo:** [../todo.md](../todo.md) -**Status:** In Progress -**Last Updated:** 2026-07-06 +**Status:** Done +**Last Updated:** 2026-07-06 (closed: PR #70 merged, CI green first run; test gate cleared, code review clean) **Plan-review opt-in:** Yes (the todo's one design-open walk-boundary decision is resolved in this draft and needs the adversarial check; generator emission contract change; documented-behavior change) **Code-review opt-in:** Yes (behavior-changing generator work) diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md index aee96076..c0744b6f 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md @@ -1,11 +1,104 @@ # TRIM-003 — Verify event-record preservation needs no consumer entries **Plan #:** 003 -**Status:** Draft -**Plan-review opt-in:** TBD at draft -**Code-review opt-in:** TBD at draft +**Date:** 2026-07-07 **Related Todo:** [../todo.md](../todo.md) +**Status:** Done +**Last Updated:** 2026-07-07 (closed: verification completed with a RED result — the gap is real; re-split into TRIM-007, which owns the fix, the doc corrections, and the merge of this branch) +**Plan-review opt-in:** No (verification plan, expected no-code-change to the library/generator; deliverables are a trimmed-harness repro and comment-accuracy fixes) +**Code-review opt-in:** No (test-only + doc-comment-only if verification is green; a red result triggers a re-split, not silent scope growth) + +--- ## Scope -Verification plan, expected no-code-change. `FactoryEventBase` has carried inherited `[FactoryEvent]` + `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` since v1.4.0 (`68e7324`), which should make every derived event record trimming-safe with no `[FactoryEventHandler]` and no consumer LinkerConfig entry. But the consuming evidence is ambiguous: zTreatment's LinkerConfig event entries predate v1.4.0, were carried forward during its 1.5.0 migration ("updated to cover fine-grained panel events too"), and were never re-tested against the annotation. Confirm with a publish-trimmed repro matching the consumer's exact shape — event record whose ONLY client-side static reference is a generic `Subscribe(...)` lambda call site in a consumer-implemented `IFactoryEventRelay` aggregator (no handler attribute anywhere), deserialized from `RemoteResponseDto.RelayedEvents` and dispatched by runtime type. If the existing `EventRelaySmokeTest` doesn't already pin this consumer shape, add a `RemoteFactory.TrimmingTests` case for it. Outcome either way is recorded: green → zTreatment PCB-003 deletes its event-record entries on verification alone; red → the gap becomes a new TRIM plan with the repro as its failing test. Does NOT touch generator emission. +Verification plan, expected no-code-change. `FactoryEventBase` has carried inherited `[FactoryEvent]` + `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` since v1.4.0 (`68e7324`), which should make every derived event record trimming-safe with no `[FactoryEventHandler]` and no consumer LinkerConfig entry. But the consuming evidence is ambiguous: zTreatment's LinkerConfig event entries predate v1.4.0, were carried forward during its 1.5.0 migration, and were never re-tested against the annotation — and recon confirmed the existing `EventRelaySmokeTest` cannot settle the question (it constructs its event via `new TrimTestRelayEvent(...)` and references it via `typeof(...)`, statically rooting exactly the metadata under test). Confirm with a publish-trimmed repro matching the consumer's exact shape: an event record whose ONLY client-side static reference is a generic `Subscribe(...)` lambda call site in a consumer-implemented `IFactoryEventRelay` aggregator — no construction, no `typeof`, no handler attribute — deserialized from the `RelayedFactoryEvent` wire shape and dispatched by runtime type. Also fixes the stale Design comments recon flagged as an internal contradiction (`FactoryEventHandlerPattern.cs` and a `FactoryEventHandlerTests.cs` doc comment still describe the removed per-handler `PreserveType` emission). Outcome either way is recorded: green → zTreatment PCB-003 deletes its event-record entries on this verification; red → the gap becomes a new TRIM plan with the repro as its failing check. Does NOT touch generator emission or library annotations. + +--- + +## Intent + +- Convert an assumed guarantee into a verified one: the todo's Acceptance Criterion 3 demands the subscribe-only consumer shape be *proven* on a trimmed client, not inferred from the annotation's existence. +- Give zTreatment PCB-003 a definitive answer on whether its event-record LinkerConfig entries can be deleted. +- Clear the recon-flagged internal contradiction: the Design source of truth must stop describing the removed per-handler `PreserveType` pipeline. + +--- + +## Framework & Architectural Alignment + +- Verification lands as a named check in the TRIM-004 harness (bool check, aggregated exit code, CI-gated) following the no-construction rule from the TRIM-001/002 gate lessons — the event type must be rooted only the way a consumer roots it. +- The consumer shape under test is the documented client-relay pattern: consumer-implemented `IFactoryEventRelay` receiving `IReadOnlyList` deserialized by `FactoryEventDeserializer` from `RelayedFactoryEvent` wire entries, dispatched by runtime type from a generic `Subscribe` registration. +- Design projects remain the requirements source of truth — the comment fixes align them with CLAUDE-DESIGN.md and `docs/trimming.md`, which recon verified are already accurate. + +--- + +## Constraints & Invariants + +- No changes to `src/RemoteFactory` or `src/Generator` — a red verification re-splits instead. +- The repro must not statically root the event record's members: no `new`, no `typeof(TrimSubscribeOnlyEvent)` outside the generic-argument position, no handler attribute. `TypeFullName` on the wire entry is a string literal. +- Existing harness checks and the full suite stay green; CI trimming gate green. +- The Design comment fixes change prose only — no test behavior, no sample code semantics. + +--- + +## Steps + +1. Add the subscribe-only repro to the harness: a `FactoryEventBase`-derived positional record referenced solely as the generic argument of a consumer-style aggregator's `Subscribe(handler)` call; drive a string-literal `RelayedFactoryEvent` through `FactoryEventDeserializer` and the aggregator's runtime-type dispatch; assert the typed handler fires with values intact. +2. Keyboard negative control: temporarily weaken the preservation under test (the inherited DAM annotation on `FactoryEventBase`) and confirm the check fails on the trimmed client — proving the repro is non-vacuous — then restore. +3. Fix the stale Design comments: `FactoryEventHandlerPattern.cs` (~119–137, describes the removed per-handler `PreserveType` emission and its nested-walk) and the `FactoryEventHandlerTests.cs` doc comment (~55–61, claims the generator emits `PreserveType()`) — rewrite to the shipped `FactoryEventBase`-annotation story, consistent with CLAUDE-DESIGN.md `:791` and `docs/trimming.md`. +4. Touch the verification pointers in docs: `docs/trimming.md` and CLAUDE-DESIGN.md cite `EventRelaySmokeTest` as the end-to-end verification — add the subscribe-only check as the consumer-shape verification. +5. Record the outcome in the Discovery Log either way; green → note that zTreatment PCB-003 may delete its event-record LinkerConfig entries; red → re-split with the failing check as the new plan's starting point. + +--- + +## Acceptance + +- [x] **VERIFIED RED** — the subscribe-only shape does NOT deserialize on the publish-trimmed client: the type survives (registry resolves it) but the ctor is stripped (`NotSupportedException`, the `DeserializeNoConstructor` symptom). The check exists and is the failing regression pin TRIM-007 must turn green. `[trimmed-harness]` +- [x] Non-vacuity proven three ways: red trimmed (pure consumer shape), green untrimmed (repro logic correct), green trimmed with `[DynamicallyAccessedMembers]` on the subscribe generic parameter (fix mechanism validated). `[explicit-skip: keyboard verification triplet]` +- [x] Design comment fixes migrated to TRIM-007 (Amendment 1) — the doc delta grew from "fix stale prose" to "correct overpromising claims," which must ship with the fix per repo rules. `[explicit-skip: migrated]` +- [x] Build/suite green; the harness check is intentionally red on this unmerged branch — CI green is TRIM-007's exit condition. `[explicit-skip: deferred to TRIM-007's gate]` + +--- + +## Current State (Pre-Flight) + +Walked 2026-07-06/07 on `TRIM` (recon + TRIM-001/002 cycles): + +- Annotations under test: `FactoryEventBase.cs:15-17` — `[FactoryEvent]` + `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` on the abstract record; `FactoryEventAttribute` is `Inherited = true` (`FactoryEventAttribute.cs:17`). +- Runtime path: `FactoryEventTypeRegistry` resolves wire `TypeFullName` via an assembly scan for `GetCustomAttribute(inherit: true)`; misses throw `UnknownFactoryEventTypeException`; `FactoryEventDeserializer.Deserialize(RelayedFactoryEvent[], serializer)` produces typed `FactoryEventBase` instances (shape used by `EventRelaySmokeTest.cs:63-75`). +- Why the existing smoke can't settle this: `EventRelaySmokeTest.cs:55` constructs `new TrimTestRelayEvent(42, ...)` and `:67` uses `typeof(TrimTestRelayEvent).FullName` — both statically root the record. +- The two trap-shapes to avoid (gate lessons): construction roots ctors (TRIM-001); retained guarded-dead bodies root ctors (TRIM-001 negative-control v1). +- Stale comments to fix: `src/Design/Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs:119-137` (per-handler `PreserveType` / `PreserveType` emission story + Dictionary known-gap note tied to the removed pipeline); `src/Design/Design.Tests/FactoryTests/FactoryEventHandlerTests.cs:55-61` (doc comment claiming the generator emits `PreserveType()`). +- Correct doc anchors (already accurate, cite the smoke test): `docs/trimming.md:306` ("Any record inheriting FactoryEventBase is automatically trimming-safe... verification lives in EventRelaySmokeTest"), CLAUDE-DESIGN.md `:794`. +- Harness contract: TRIM-004 named bool checks; `Program.cs` check block; negative controls per TRIM-001/002 precedent. +- ILLink mechanics being verified: a derived record kept only via a generic instantiation (`Subscribe`) must receive the base type's inherited DAM member preservation — this is the annotation's designed behavior; the repro proves it empirically on net9.0 `TrimMode=full`. + +--- + +## Test Evidence + +Filled after implementation, before the Step 5 gate. + +| Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | +|---|---|---|---| +| Subscribe-only shape on trimmed client | `[trimmed-harness]` | `EventSubscribeOnlySmokeTest.Run` — **RED** (trimmed, pure consumer shape): `NotSupportedException` on `TrimSubscribeOnlyEvent` ctor, harness exit 1 | ✓ (as a finding) | +| Non-vacuity | `[explicit-skip]` | Untrimmed `dotnet run -c Release`: PASSED, exit 0. Trimmed with DAM on `Subscribe`: PASSED, exit 0. Contrast isolates the cause to ILLink + the missing call-site annotation. | ✓ | + +--- + +## Plan Amendments + +### 2026-07-07 — Verification RED; Steps 3–4 migrated to TRIM-007 + +- **Section affected:** Steps 3–4 (doc/comment fixes), Acceptance +- **Original said:** expected no-code-change green verification; this plan would fix the stale Design comments and touch the doc verification pointers. +- **What changed:** the verification came back RED — the inherited DAM on `FactoryEventBase` does not preserve derived members under ILLink (`DynamicallyAccessedMembersAttribute` is `AttributeUsage(Inherited = false)`; the runtime `inherit: true` attribute scan works, which is why the *type* resolves while its *ctor* is stripped). The doc problem is therefore bigger than stale prose: `docs/trimming.md` / CLAUDE-DESIGN.md / `FactoryEventBase.cs` overpromise automatic trimming safety. All doc/comment work migrated to TRIM-007 so it ships with whichever fix is chosen (repo rule: doc deltas ship with the behavior they describe). The fix mechanism was validated at the keyboard: DAM on the subscribe method's generic parameter → green. +- **Why:** the plan's own Scope pre-agreed this path: "red → the gap becomes a new TRIM plan with the repro as its failing test." +- **Discovery Log link:** 2026-07-07 — TRIM-003 (verification RED, re-split → TRIM-007). + +--- + +## Notes + +- Nested-record-in-event properties are *documented* as a manual preservation case (`docs/trimming.md` "Nested Reference Types in Event Records") — deliberately NOT re-scoped here; the repro pins the consumer's actual failing shape (flat event records). If the user later wants framework-walked event property graphs, that's a new plan. +- The negative control (Step 2) temporarily edits library source (`FactoryEventBase` annotation) — revert discipline per the TRIM-001 renderer precedent. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md new file mode 100644 index 00000000..c6731eac --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md @@ -0,0 +1,110 @@ +# TRIM-007 — Subscribe-only event preservation fix (generator emission) + +**Plan #:** 007 +**Date:** 2026-07-13 +**Related Todo:** [../todo.md](../todo.md) +**Status:** In Progress +**Last Updated:** 2026-07-13 +**Plan-review opt-in:** Yes (new incremental-generator pipeline branch; generator emission contract change; corrects documented behavior that currently overpromises) +**Code-review opt-in:** Yes (behavior-changing generator work) + +--- + +## Scope + +Close the gap TRIM-003 proved: a `FactoryEventBase`-derived record whose only client-side reference is a generic `Subscribe` call site loses its constructor under `PublishTrimmed=true` — inherited `[DynamicallyAccessedMembers]` does not flow to derived types under ILLink. Per the user's direction (2026-07-07), the fix is **generator emission**: a new incremental pipeline branch discovers every concrete `FactoryEventBase` descendant declared in a compilation and emits preservation for it — restoring the v1.4.0 "every descendant is automatically safe" promise for real, with zero consumer action. Each discovered event root goes through the shared `WalkDtoGraph` (TRIM-001's bucketed walk), so the event itself AND its nested property graph bucket by ctor shape into `PreserveType`/`Register` — which also automates the previously-manual "nested reference types in event records" case. Emission lands in a generated per-assembly event-preservation registrar discovered by the existing assembly-level `[NeatooFactoryRegistrar]` mechanism. Owns the doc corrections TRIM-003 surfaced (docs currently overpromise automatic trimming safety; `FactoryEventBase.cs`'s own comment makes the wrong `Inherited = true` claim; stale per-handler-`PreserveType` prose in the Design projects — migrated from TRIM-003 Steps 3–4). Exit condition: TRIM-003's red harness check goes green in the pure, unannotated consumer shape. Does NOT change the runtime relay/registry/deserializer path or `FactoryEventBase` itself (the annotations stay — they're harmless and self-documenting), and does NOT touch producer-side `Raise` guidance. + +--- + +## Intent + +- Any record inheriting `FactoryEventBase` becomes genuinely trimming-safe by declaration — no handler attribute, no consumer annotation, no LinkerConfig entry — making the todo's Acceptance Criterion 3 true and letting zTreatment (PCB-003) delete its event-record entries. +- Nested reference types inside event records stop being a documented manual case — the same automatic story as factory-signature and entity-property DTOs. +- The documentation stops overpromising: the trimming story for events becomes "generator-emitted preservation," stated accurately in `docs/trimming.md`, CLAUDE-DESIGN.md, and the Design project comments. + +--- + +## Framework & Architectural Alignment + +- Fourth incremental pipeline branch alongside class/interface/relay-handler branches (`FactoryGenerator.cs`) — `CreateSyntaxProvider` (no attribute to key on; descendants carry nothing directly) with the cheapest available syntactic predicate: **records only** (every `FactoryEventBase` descendant is a record — classes cannot inherit records), non-abstract, non-generic, with a base list (plan review B-callout 1/3). Semantic base-chain transform matches the base by fully-qualified metadata name (the base is always a metadata symbol from the referenced assembly — plan review B-callout 2), value-equatable output, non-event results filtered **before** `Collect()`, and collected roots ordered deterministically before render (plan review B-callout 4). +- Discovery/bucketing reuses `DtoTypeWalker.WalkDtoGraph` with one addition the event path uniquely requires (plan review VETO): an **accessibility gate** — event roots are discovered by raw declaration scan (unlike the signature/entity walks, whose inputs are inherently accessible), so roots whose effective accessibility is not internal-or-public within the assembly (private/protected/file-scoped nested records) are skipped; the generated registrar could not legally reference them. In-repo breakers otherwise: `FactoryEventCollectorTests.cs:8-9`, `FactoryEventBaseAttributeTests.cs:14-15` (private nested event records in a project that runs the generator as an analyzer). Roots bucket by ctor shape (typical positional event records → PreserveType bucket). +- The generated registrar rides the existing zero-reflection-for-consumers discovery: assembly-level `[NeatooFactoryRegistrar(typeof(...))]` + static `FactoryServiceRegistrar(IServiceCollection, NeatooFactory)`, invoked by `RegisterFactories` at `AddNeatooRemoteFactory` time — same lifecycle as every factory registrar, unguarded (client and server), idempotent emissions. +- Trimmed verification per the TRIM-004 harness contract; the failing TRIM-003 check is the acceptance pin. + +--- + +## Constraints & Invariants + +- No changes to `src/RemoteFactory` runtime types (relay, registry, deserializer, `FactoryEventBase`, its annotations). +- Existing factory/interface/relay-handler pipelines and their emissions are untouched; the new branch adds a file, never modifies theirs. +- Incremental-cache discipline: the new transform output is value-equatable (`EquatableArray`/records); an unrelated edit must not re-render the event registrar. +- No emission when a compilation declares no concrete `FactoryEventBase` descendants (no empty registrar files). +- Duplicate preservation across registrars stays idempotent (`TryAdd` / no-op `PreserveType`); a type reachable via factory signatures, entity properties, AND event graphs emits validly from each site. +- Abstract descendants (consumer intermediate event bases) get no emission themselves, but their concrete descendants do, with inherited properties walked. +- Full suite green both TFMs; CI trimming gate green — including TRIM-003's check in the pure consumer shape. + +--- + +## Steps + +1. Add the event-discovery pipeline branch: syntactic predicate (concrete, non-generic record declarations with a base list), semantic transform (base-chain FQN match against `Neatoo.RemoteFactory.FactoryEventBase`, effective-accessibility gate), per-event `WalkDtoGraph` bucketing (root + nested property graph), value-equatable output filtered before `Collect()`. +2. Render the per-assembly event-preservation registrar: assembly-level `[NeatooFactoryRegistrar]` + static `FactoryServiceRegistrar` emitting the two bucket call kinds; unique hint/namespace derived from the assembly; no output when no events. +3. Extend the TRIM-003 harness event with a nested positional-record property (never constructed) so the nested walk is pinned end-to-end; the subscribe-only check must go green unmodified in its pure consumer shape. +4. Keyboard negative control: disable the new pipeline branch, confirm the check reverts to red, restore (TRIM-001/002 precedent). +5. Unit tests in the DtoDiscovery suite: registrar emitted with correct buckets for a subscribe-only event; nested record/DTO properties bucketed; abstract intermediate skipped while its concrete descendant is walked with inherited properties; **private nested event records skipped (accessibility gate) with the generated output still compiling**; generic event records skipped; cross-event dedupe within the registrar; no registrar when no events declared. +6. Docs to shipped behavior — including the design-decision *narrative*, not just the safety claim (plan review A-callout 1): `docs/trimming.md` event section (generator emission story; correct the DAM-inheritance claim; the `:307` "supersedes … less generated code" rationale is now inverted and must be rewritten; "Nested Reference Types in Event Records" becomes automatic; verification pointer → the subscribe-only check), `FactoryEventBase.cs` doc comment, CLAUDE-DESIGN.md `:793-799` (supersession rationale at `:795`, the "exclusively from DAM" nested-walking paragraph at `:799`) + FAQ row, and the stale `FactoryEventHandlerPattern.cs` / `FactoryEventHandlerTests.cs` comments (migrated from TRIM-003). Release notes (todo Acceptance Criterion 4) are explicitly deferred to the todo-level release step spanning all TRIM plans — not owed by this plan (plan review A-callout 2). + +--- + +## Acceptance + +- [x] The generator emits a per-assembly event-preservation registrar whose buckets cover every concrete `FactoryEventBase` descendant and its nested property graph; compilations with no events emit nothing. `[unit]` +- [x] TRIM-003's subscribe-only harness check passes on the publish-trimmed client in the pure consumer shape (no consumer annotation), including a nested record property on the event. `[trimmed-harness]` +- [x] The check's sensitivity is re-proven by a keyboard negative control (pipeline disabled → red). `[explicit-skip: one-off keyboard verification, per precedent]` *(disabled via the Where clause → identical NotSupportedException, exit 1; restored → green)* +- [x] Docs and Design comments describe the generator-emission story accurately; no surviving overpromise about inherited DAM. `[explicit-skip: doc delta, reviewed at code review]` +- [x] Full solution build/test green (net9.0 + net10.0); CI trimming gate green. `[explicit-skip: build/test/CI gates]` *(build 0 errors — the solution build itself proves the accessibility gate, since RemoteFactory.UnitTests declares private nested event records; 593+593 unit, 561+561 integration, 0 failed; CI on the PR)* + +--- + +## Current State (Pre-Flight) + +Walked 2026-07-07/13 on branch `TRIM-003-verify-event-preservation` (6c892e9): + +- The failing pin: `EventSubscribeOnlySmokeTest` — trimmed red (`NotSupportedException` on `TrimSubscribeOnlyEvent` ctor), untrimmed green, DAM-annotated-Subscribe green (triplet in TRIM-003's Test Evidence). `SubscribingRelay.Subscribe` carries the KNOWN-GAP comment and deliberately no annotation. +- Pipeline seams: `FactoryGenerator.cs:16-108` — three branches, all `ForAttributeWithMetadataName`; the new branch needs `CreateSyntaxProvider` (descendants carry no attribute; `[FactoryEvent]` sits on the base and Roslyn symbol `GetAttributes()` does not surface inherited attributes). `Collect()` + single `RegisterSourceOutput` for the per-compilation file. +- Registrar discovery mechanism: `AddRemoteFactoryServices.RegisterFactories` (`:160-173`) reads assembly-level `NeatooFactoryRegistrarAttribute`s and reflection-invokes static `FactoryServiceRegistrar(IServiceCollection, NeatooFactory)` (NonPublic|Public) on each `attr.Type` — the generated registrar rides this as-is. Attribute defined at `FactoryAttributes.cs:155` (verify `AllowMultiple` at the keyboard — factories already emit one per type, so it must be multiple). +- Walk reuse: `DtoTypeWalker.WalkDtoGraph` buckets roots by ctor shape — a positional event record roots into the PreserveType bucket naturally; `IsDtoStructureCandidate` skips abstract types (consumer intermediate event bases) and `[Factory]` types; `WalkProperties` includes the inherited chain, so a concrete descendant of an abstract event base still walks the base's properties. +- Emission call shapes already exist (`Register(() => new T())` / `PreserveType()`); `DtoConstructorRegistry` is `public` in `Neatoo.RemoteFactory.Internal` — generated factory files already emit `using Neatoo.RemoteFactory.Internal;`. +- Hint-name conventions: factory files use `{SafeHintName}Factory.g.cs`; the event registrar needs an assembly-derived unique hint (sanitize invalid identifier chars) — keyboard detail. +- Unit-test home: `RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/` — `RunGenerator` + per-tree `FactoryTree` helper pattern (TRIM-002); the event registrar's tree is selected by its hint name. +- Docs to correct (anchors): `docs/trimming.md` "Factory Event Type Preservation" (~280-347, incl. "How RemoteFactory Handles It", "What You Need to Know", "Nested Reference Types in Event Records"); `FactoryEventBase.cs:5-17` comment; CLAUDE-DESIGN.md `:783-796` (event preservation paragraphs) + FAQ; `Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs:119-137`; `Design.Tests/FactoryTests/FactoryEventHandlerTests.cs:55-61`. +- TrimmingTests declares two `FactoryEventBase` descendants today (`TrimTestRelayEvent`, `TrimSubscribeOnlyEvent`) — the new registrar will cover both; `EventRelaySmokeTest` stays untouched (its direct-construction shape remains valid as a round-trip test). + +--- + +## Test Evidence + +Filled after implementation, before the Step 5 gate. + +Filled 2026-07-13, before the Step 5 gate. Unit tests in `RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/EventPreservationDiscoveryTests`. + +| Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | +|---|---|---|---| +| Registrar emitted with correct buckets; nothing when no events | `[unit]` | `SubscribeOnlyEvent_PreserveTypeEmittedInEventRegistrar` (incl. `[NeatooFactoryRegistrar]` attribute assertion), `EventNestedTypes_BothBucketsEmitted`, `AbstractIntermediate_SkippedButConcreteDescendantWalked` (inherited props walked, abstract never preserved), `PrivateNestedEventRecord_SkippedByAccessibilityGate`, `GenericEventRecord_Skipped`, `NoEventsDeclared_NoRegistrarEmitted`, `SharedNestedTypeAcrossEvents_SingleEmission`, `EventWithParameterlessCtor_LandsInRegisterBucket`; gate additions: `RegistrarOutput_OrdinallySorted_RegardlessOfDeclarationOrder` (B4 determinism guard), `SameNamedBaseInOtherNamespace_NotMatched` (B2 FQN decoy) | ✓ | +| Subscribe-only shape green on trimmed client, nested record included | `[trimmed-harness]` | `EventSubscribeOnlySmokeTest.Run` — pure consumer shape (unannotated `Subscribe`, string-literal `TypeFullName`, no construction anywhere), now asserting the nested `TrimEventDetail` too; run output: `reviews/007-harness-run.log` (all checks passed, exit 0) | ✓ | +| Negative control | `[explicit-skip]` | Pipeline disabled via `Where` clause → trimmed run fails with the identical TRIM-003 `NotSupportedException` signature, exit 1; restored → exit 0 | ✓ | +| Docs accurate | `[explicit-skip]` | `docs/trimming.md` event section (incl. supersession-narrative rewrite + nested section now automatic + accessibility boundary), `FactoryEventBase.cs` comment, CLAUDE-DESIGN.md `:793-799` rewrite with history note, `FactoryEventHandlerPattern.cs` + `FactoryEventHandlerTests.cs` comments (Dictionary known-gap retained with corrected remedy); gate caught one surviving overpromise outside the anchor list — `FactoryEventBaseAttributeTests.cs` class summary, corrected | ✓ | +| Build/test/CI gates | `[explicit-skip]` | `reviews/007-build.log` (0 errors), `reviews/007-test.log` (593+593 unit, 561+561 integration, 0 failed), `reviews/007-publish.log`, `reviews/007-harness-run.log`; CI on the PR. The solution build doubles as the accessibility-gate proof (UnitTests declares private nested event records) | ✓ | + +--- + +## Plan Amendments + +(None yet.) + +--- + +## Notes + +- The consumer-annotation pattern (DAM on a generic subscribe method) remains valid and documented for generic *passthroughs* (the existing IL2091 guidance) — this plan just stops it being *required* for preservation. +- Branch topology: implemented on `TRIM-007-event-preservation-emission` off `TRIM-003-verify-event-preservation`; one PR carries both plans (003's red verification + 007's fix that turns it green). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-code-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-code-review.md new file mode 100644 index 00000000..b2c54d86 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-code-review.md @@ -0,0 +1,31 @@ +# TRIM-007 Code Review (Step 5, opt-in) — 2026-07-13 + +**Reviewer:** code-reviewer agent, findings-only (no grade). Range `6c892e9..59302c7`. Logs grepped, not re-run. + +**Result: 3 veto-tier findings — all doc/prose corrections, all fixed immediately post-review.** The generator work itself was verified clean. + +## Verified (generator surface) + +- Fourth pipeline branch shape correct: cheapest viable predicate; `WalkDtoGraph` runs only after the base-chain match and accessibility gate (non-event records pay only `GetDeclaredSymbol` + short base walk); no `ISymbol`/`Compilation` leaks into pipeline output; nulls filtered before `Collect()`. +- Plan-review compliance complete: B1 gate walks the full containing chain, rejects `IsFileLocal`, admits `ProtectedOrInternal`, excludes `private protected` — proven by the solution build against UnitTests' private nested event records; B2 FQN match + decoy test; B3 predicate; B4 `SortedSet(Ordinal)` + determinism test; B5 `SanitizeNamespace` edges; A1/A2 doc scope honored where anchored. +- `ExceptWith` defensive step honest; unused registrar parameters match the reflection-invoked contract; no reflection added generator-side; sacred tests intact (comment-only edits faithful; smoke-test edit strengthens coverage). +- Logs: build 0 errors (3 non-production warnings: pre-existing harness CA1062 + 2 WASM workload); 595+595 / 561+561, 0 failed; trimmed harness exit 0. + +## Veto-tier findings → fixed + +The plan's Acceptance bullet 4 ("no surviving overpromise about inherited DAM") was not met — three authoritative artifacts outside the anchor list still carried the falsified claim: + +1. `Design.Domain/FactoryPatterns/FactoryEventRelayPattern.cs:13-16,:63-65` (Design source of truth) — **fixed**: preservation now attributed to the generated registrar; runtime-discovery half of `[FactoryEvent]` retained. +2. `docs/factory-events.md:264` (published mirror of the rewritten trimming.md section) — **fixed**; verification pointer updated to `EventSubscribeOnlySmokeTest`. +3. `EventSubscribeOnlySmokeTest.cs:62-68` class summary (self-contradiction within a file this plan edited) — **fixed**. + +## Callout-tier findings → disposition + +- Skill docs (`skills/RemoteFactory/references/trimming.md:179,239`, `factory-events.md:327`) carried the same overpromise plus the now-false nested-manual-preservation note — **fixed in the same pass** (hand-written prose, not mdsnippets-managed). +- `FactoryEventTypeRegistry.cs:99` IL2026 suppression Justification cited the disproven rationale — **fixed** (suppression itself remains valid). +- `Program.cs:124` harness comment stale framing — **fixed**. +- Hint name uses the raw assembly name while the namespace is sanitized — **accepted-with-reason** (hint names tolerate most characters; only the namespace affects compilation). +- Theoretical CS0101 if a consumer declares `{RootNamespace}.NeatooEventPreservationRegistrar` — **accepted-with-reason** (Neatoo-prefixed name, single emission, consistent with factory-registrar conventions; no diagnostic warranted). +- Pre-existing harness CA1062 — informational, not this plan's regression. + +Post-fix sanity build: 0 errors. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-plan-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-plan-review.md new file mode 100644 index 00000000..559c5adf --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-plan-review.md @@ -0,0 +1,29 @@ +# TRIM-007 Plan Review — 2026-07-13 + +**Reviewer:** plan-reviewer agent (two-pass). +**Verdict: CONCERNS** — one veto-tier Pass B finding, addressed in the draft before implementation; five callouts folded in. + +## Pass A — vs documented requirements + +- **No veto.** Re-introducing generator emission reverses the v1.4.0 *mechanism* decision ("annotation supersedes per-handler emission — stronger guarantee, less generated code"), but the reversal is sanctioned: TRIM-003 proved the annotation fails the subscribe-only shape, the user chose generator emission, and 1.x release notes permit surface changes under minor bumps. +- **A1 (callout):** the doc step must rewrite the design-decision *narrative*, not just the safety claim — `CLAUDE-DESIGN.md:795` ("supersedes … less generated code", now inverted in both directions), `:799` ("preservation comes *exclusively* from DAM" + manual nested guidance, both falsified), `docs/trimming.md:307`. Pre-flight anchor extended `:783-796` → `:793-799`. → Folded into Step 6. +- **A2 (callout):** release notes (todo AC4) absent from Step 6 — now explicitly deferred to the todo-level release step. → Folded. + +## Pass B — vs codebase + +**High-risk item de-risked:** the generated registrar cannot be trimmed away — `NeatooFactoryRegistrarAttribute`'s ctor param and `Type` property carry `[DynamicallyAccessedMembers(PublicMethods | NonPublicMethods)]` (`FactoryAttributes.cs:157-168`), so the assembly-level `typeof` roots `FactoryServiceRegistrar` under `TrimMode=full`; its body then roots the `PreserveType<[DAM(All)] T>` call sites. `AllowMultiple = true` confirmed (`:154`). This is exactly how factory registrars survive today — not via DI references. + +- **VETO (B1): `WalkDtoGraph` cannot be reused "unchanged" — event discovery reaches inaccessible types.** The walker has no accessibility gate; that was safe because factory-signature and entity-property inputs are inherently accessible. Event roots come from a raw declaration scan — and the repo itself declares `private` nested event records in a project that runs the generator as an analyzer (`FactoryEventCollectorTests.cs:8-9`, `FactoryEventBaseAttributeTests.cs:14-15`, incl. a two-level inheritance chain). Emitting `PreserveType` for them in a separate registrar file is a build break on day one. → Draft amended: effective-accessibility gate (internal-or-public at every nesting level) on event roots; a unit test pins the gate and that generated output compiles. +- **B-callouts, all folded:** (1) predicate narrowed to records-only (classes can't inherit records) non-abstract non-generic with base list — the cheapest keying since no attribute exists to key on; (2) base match by fully-qualified metadata name, not symbol identity (the base is always a metadata symbol); (3) exclude open-generic event records; (4) deterministic ordering of collected roots before render + filter non-event results before `Collect()` (single-file output makes ordering affect bytes under `ContinuousIntegrationBuild`); (5) first per-assembly registrar has no naming precedent — sanitize assembly-derived identifiers (in-repo names all safe; leading digits/hyphens are the consumer edge); (6) cancellation-token discipline consistent with existing transforms (ignored; shallow walk). +- No existing test asserts a generated-tree count that the new file would break (assertion helpers select trees by name; `Assert.Single` tests use event-free sources). + +## Disposition + +| # | Finding | Disposition | +|---|---------|-------------| +| B1 (veto) | Accessibility gate required | Framework Alignment + Step 1 + Step 5 amended; test pinned | +| A1 | Design-decision narrative rewrite | Step 6 anchors extended | +| A2 | Release-note deferral unstated | Step 6 states the todo-level deferral | +| B2-B5 | Predicate/matching/ordering/naming | Steps 1-2 amended; keyboard notes | + +Calibration: diagnoses adopted; remedies matched the code walk and were adopted as-is. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-test-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-test-review.md new file mode 100644 index 00000000..ca370ed4 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/007-test-review.md @@ -0,0 +1,31 @@ +# TRIM-007 Test Review (Step 5 Gate) — 2026-07-13 + +**Reviewer:** test-reviewer agent, two passes (initial + closure). +**Logs:** `007-build.log` (0 errors — the build itself proves the accessibility gate: `RemoteFactory.UnitTests` declares private nested event records and runs the generator as an analyzer), `007-test.log` (final: 595+595 unit, 561+561 integration expected; see note), `007-publish.log`, `007-harness-run.log` (trimmed run, all checks passed, exit 0). +**Gate result: CLEARED** — no open must-cover or should-cover findings. + +## Initial pass + +Evidence map verified honest with one overstatement (row 4's "no surviving overpromise" — see below). Negative-control credibility confirmed repo-wide: `TrimSubscribeOnlyEvent`/`TrimEventDetail` constructed nowhere, referenced only via the `Subscribe` generic argument and a string-literal `TypeFullName`; the nested record materializes only through deserialization. Runtime interaction implicitly well-covered: the integration assembly declares ~14 event records alongside `[Factory]` targets, so registrar discovery/invocation and TryAdd double-registration idempotency are exercised by all 561 green integration tests. Findings: + +1. **should-cover (plan):** deterministic/sorted registrar output had no regression guard (plan-review B4). +2. **should-cover (tech-debt doc):** one surviving inherited-DAM overpromise outside the plan's anchor list — `FactoryEventBaseAttributeTests.cs` class summary. +3. **nice-to-have:** FQN decoy-base negative test; accessibility variants beyond `private`; real parameterless-ctor event round-trip; concrete-inherits-concrete chain; `outputCompilation` error assertions (suite-wide pre-existing pattern). + +## Response and closure + +| Finding | Disposition | +|---|---| +| Determinism guard | **CLOSED** — `RegistrarOutput_OrdinallySorted_RegardlessOfDeclarationOrder` (Zebra declared before Alpha; ordinal order asserted — a Collect()-order refactor turns it red) | +| Stale DAM overpromise | **CLOSED** — summary corrected; Evidence row 4 now records the gate catch instead of overstating | +| FQN decoy | **CLOSED** — `SameNamedBaseInOtherNamespace_NotMatched` (genuine decoy: real base still referenced in the compilation, proving FQN-specificity) | +| Accessibility variants / parameterless round-trip / concrete-chain / outputCompilation | **ACCEPTED-WITH-REASON** — low-risk per the reviewer's own tiering; the last is suite-wide tech debt ("its own plan if ever") | + +## Closing tier picture + +- must-cover: none (never open). +- should-cover: both closed. +- nice-to-have: decoy closed; rest accepted with recorded reasons. +- tech-debt: doc item closed; `outputCompilation` pattern visibility-only, backstopped by the solution build. + +Final count: 10 `EventPreservationDiscoveryTests`. Evidence-freshness caveat resolved by re-running the full suite after the gate additions (this file's log line reflects the fresh run). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 8b8f39c4..40dde399 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -42,16 +42,18 @@ A third suspected gap turned out to be already fixed: event records derive `Fact |-----|--------|------|--------| | 004 | Done | [Trimming harness pass/fail semantics + CI gate](./plans/004-trimming-harness-ci-gate.md) | 2026-07-06 recon: TrimmingTests outside .sln/CI, exits 0 on failure — 001–003's trimmed acceptance signals need this gate first | | 001 | Done | [Positional-record preservation in factory signatures](./plans/001-positional-record-signature-preservation.md) | `DtoTypeWalker.WalkFactoryReturn` `HasParameterlessCtor` gate; zTreatment cut-over `StartVisitResultV2` hotfix | -| 002 | Draft | [`[Factory]` entity property-graph DTO discovery](./plans/002-factory-entity-property-dto-discovery.md) | `WalkFactoryReturn` bails on `[Factory]` roots without descending; zTreatment `TreatmentBanner` / `DashboardContactResult` hotfixes | -| 003 | Draft | [Verify event-record preservation needs no consumer entries](./plans/003-verify-event-record-preservation.md) | `FactoryEventBase` DAM annotation shipped v1.4.0; consumer entries predate it, never re-tested | +| 002 | Done | [`[Factory]` entity property-graph DTO discovery](./plans/002-factory-entity-property-dto-discovery.md) | `WalkFactoryReturn` bails on `[Factory]` roots without descending; zTreatment `TreatmentBanner` / `DashboardContactResult` hotfixes | +| 003 | Done | [Verify event-record preservation needs no consumer entries](./plans/003-verify-event-record-preservation.md) | `FactoryEventBase` DAM annotation shipped v1.4.0; consumer entries predate it, never re-tested — **verification came back RED**, re-split → TRIM-007 | +| 007 | Draft | [Subscribe-only event preservation fix](./plans/007-subscribe-only-event-preservation-fix.md) | TRIM-003 finding: inherited DAM doesn't flow to derived types under ILLink; ctor stripped in the subscribe-only shape; fix mechanism keyboard-validated | | 005 | Draft | [Server-only reference over-retention in trimmed clients](./plans/005-server-only-reference-over-retention.md) | TRIM-004 discovery: guarded-dead `LocalCreate` bodies retain server-only interface refs, contradicting `docs/trimming.md` | | 006 | Draft | [Incremental-generator caching regression test](./plans/006-incremental-cache-regression-test.md) | TRIM-001 gate: no test asserts cached pipeline steps — non-EquatableArray transform fields regress silently (plan review B1) | -Execution order: 004 → 001 → 002 → 003 → 005 → 006 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. +Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. Note: TRIM-003's branch (`TRIM-003-verify-event-preservation`) carries the intentionally red harness check and stays unmerged until TRIM-007 turns it green. ## Skipped Steps - TRIM-004 — `test-reviewer` gate skipped (test-infrastructure-only plan: every Acceptance bullet is `explicit-skip`; the harness itself is the test artifact, evidence recorded in the plan's Test Evidence table). +- TRIM-003 — `test-reviewer` gate skipped (verification-only plan whose deliverable is the finding itself; non-vacuity proven by the red-trimmed / green-untrimmed / green-annotated triplet recorded in the plan's Test Evidence; the repro check lands under TRIM-007's full gate). ## Discovery Log @@ -84,6 +86,12 @@ Execution order: 004 → 001 → 002 → 003 → 005 → 006 (rows listed in exe - **Index changes:** add TRIM-006 (incremental-cache regression test — pre-existing tech debt, plan review B1), executed last. - **Follow-up:** TRIM-006. +### 2026-07-07 — TRIM-003 (verification RED, re-split → TRIM-007) +- **Finding:** The subscribe-only consumer shape FAILS on a trimmed client: the event type survives (generic instantiation + runtime `[FactoryEvent]` scan) but its ctor is stripped — inherited `[DynamicallyAccessedMembers]` on `FactoryEventBase` does not flow to derived types under ILLink (DAM is `AttributeUsage(Inherited = false)`; the docs' "Inherited = true" story is runtime-reflection semantics). The todo's "third gap already fixed" premise was wrong; zTreatment's event LinkerConfig entries are load-bearing. Triplet evidence: red trimmed / green untrimmed / green trimmed with DAM on `Subscribe` (fix mechanism validated — the `Raise` producer-side pattern). Long form: TRIM-003 Amendment 1. +- **Decision:** Re-split. +- **Index changes:** add TRIM-007 (fix + the now-larger doc corrections, migrated from 003's Steps 3–4), executed next; 003 Done as a red verification; its branch stays unmerged until 007 goes green. +- **Follow-up:** TRIM-007 — fix direction (consumer-annotation docs vs generator `PreserveType`-per-descendant vs both) is a user decision, pending. + ### 2026-07-06 — TRIM-002 (gate closed) - **Finding:** Test gate CLEARED with zero must-cover; two should-covers (base-class property, `[Factory]` record self-walk) and three nice-to-haves closed with tests; harness run log captured. New visibility item: the FactoryEventRelay integration family is parallel-load flaky *beyond* the two skipped members (different members flake per run; all green isolated and with `MaxParallelThreads=1`) — user previously declined queueing, recorded here for the close-out audit. Long form: `reviews/002-test-review.md`. - **Decision:** Amend. diff --git a/docs/trimming.md b/docs/trimming.md index cd132951..73947058 100644 --- a/docs/trimming.md +++ b/docs/trimming.md @@ -290,41 +290,28 @@ Event records raised via `IFactoryEvents.Raise()` and handled by `[FactoryEve ### How RemoteFactory Handles It -Preservation comes from a single annotation on `FactoryEventBase` itself — no generator emission, no per-handler walk: +The source generator discovers every concrete `FactoryEventBase` descendant declared in a compilation and emits a per-assembly event-preservation registrar — a generated static class registered via the same assembly-level `[NeatooFactoryRegistrar]` mechanism as factory registrars, invoked automatically by `AddNeatooRemoteFactory`. The registrar emits `DtoConstructorRegistry.PreserveType()` for each event record (and `Register` / `PreserveType` for the DTOs reachable through each event's property graph, bucketed by constructor shape like every other discovered DTO). + +Declaring the event is all it takes: ```csharp -[FactoryEvent] -[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | - DynamicallyAccessedMemberTypes.PublicProperties)] -public abstract record FactoryEventBase; +public record OrderShippedEvent(int OrderId, ShippingAddress Address) : FactoryEventBase; +// → generated registrar preserves OrderShippedEvent AND ShippingAddress ``` -Both attributes are applied with `Inherited = true`, so every descendant of `FactoryEventBase` automatically: - -- Has its constructors and public properties preserved by the trimmer. -- Is discoverable by the runtime `FactoryEventTypeRegistry` (an `AppDomain.CurrentDomain.GetAssemblies()` scan for types where `GetCustomAttribute(inherit: true) != null`). - -This supersedes the prior per-`[FactoryEventHandler]` generator-emitted `DtoConstructorRegistry.PreserveType()` and recursive nested-property walk — that pipeline has been removed with the rest of the client-relay codegen path. Net: stronger guarantee (covers every descendant of `FactoryEventBase`, even those with no server handler registered), less generated code. +The `[FactoryEvent]` annotation on `FactoryEventBase` (inherited at runtime) remains what makes descendants discoverable by the `FactoryEventTypeRegistry` assembly scan. Its `[DynamicallyAccessedMembers]` annotation, however, does **not** preserve descendants' members under trimming — `DynamicallyAccessedMembers` does not flow from a base type to derived types in ILLink, which is exactly why the generator emission exists. (An earlier version of this page claimed the annotation alone made every descendant trimming-safe; a publish-trimmed repro proved that wrong for event records whose only reference is a generic subscription call site.) `IFactoryEvents.Raise` retains `[DynamicallyAccessedMembers(All)]` on its generic parameter for producer-side call-site preservation. +One boundary: the generated registrar is a separate file, so `private`/`protected`/file-scoped nested event records cannot be preserved this way and are skipped. Declare wire-crossing events as top-level (or `internal`/`public` nested) types. + ### What You Need to Know -Any record inheriting `FactoryEventBase` is automatically trimming-safe — no manual `DtoConstructorRegistry` calls, no `[FactoryEventHandler]` required, nothing to configure. End-to-end verification lives in `src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs` — a publish-trimmed smoke test that confirms event relay round-trips across a trimmed binary. +Any accessible record inheriting `FactoryEventBase` is automatically trimming-safe — no manual `DtoConstructorRegistry` calls, no `[FactoryEventHandler]` required, no consumer-side annotation, nothing to configure. End-to-end verification lives in `src/Tests/RemoteFactory.TrimmingTests/EventSubscribeOnlySmokeTest.cs` — a publish-trimmed check whose event record's only static reference is a generic `Subscribe` call site (the hardest shape: nothing else roots the type's members), plus the `EventRelaySmokeTest` round-trip. ### Nested Reference Types in Event Records -The `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` annotation on `FactoryEventBase` preserves constructors and public properties of every descendant. For reference types *referenced by* an event record's properties, preservation depends on the referenced type: - -- **Nested records or DTOs that inherit `FactoryEventBase`** — automatically preserved via the same inherited annotation. -- **Plain DTOs returned by any factory method** — automatically preserved via the factory-return-type walker (see [Automatic DTO Constructor Registration](#automatic-dto-constructor-registration) above). -- **Reference types that are neither of the above** — preserve explicitly in DI setup: - -```csharp -DtoConstructorRegistry.PreserveType(); -// or, if it has a public parameterless ctor -DtoConstructorRegistry.Register(() => new MyEmbeddedType()); -``` +Automatically preserved. The generator walks each discovered event's public property graph with the same bucketed walk used for factory-signature and entity-property DTOs — nested records land in the `PreserveType` bucket, parameterless DTOs in the `Register` bucket, collections and nullables are unwrapped, and cycles are detected. No manual `DtoConstructorRegistry` calls are needed for types reachable from an event record's properties. ### User Code That Forwards `Raise` Through a Generic Passthrough diff --git a/skills/RemoteFactory/references/factory-events.md b/skills/RemoteFactory/references/factory-events.md index ff86bac3..63301263 100644 --- a/skills/RemoteFactory/references/factory-events.md +++ b/skills/RemoteFactory/references/factory-events.md @@ -324,7 +324,7 @@ public sealed class UiNotificationRelay : IFactoryEventRelay ## IL Trimming -Event records are automatically preserved from IL trimming. `FactoryEventBase` carries `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` with `Inherited = true`, so every descendant's public constructors and public properties survive `PublishTrimmed=true` without per-handler codegen or per-event annotation. +Event records are automatically preserved from IL trimming: the source generator discovers every concrete, accessible `FactoryEventBase` descendant declared in a compilation and emits a per-assembly event-preservation registrar, so descendants (and their nested property graphs) survive `PublishTrimmed=true` without per-event annotation. (The `[DynamicallyAccessedMembers]` annotation on `FactoryEventBase` itself does not preserve descendants — DAM does not flow to derived types under ILLink.) `FactoryEventBase` also carries `[FactoryEvent]` with `Inherited = true` so the runtime `FactoryEventTypeRegistry` discovers descendants via attribute scan during the first relay deserialization. diff --git a/skills/RemoteFactory/references/trimming.md b/skills/RemoteFactory/references/trimming.md index b4667f3d..547afdf0 100644 --- a/skills/RemoteFactory/references/trimming.md +++ b/skills/RemoteFactory/references/trimming.md @@ -176,10 +176,10 @@ The generator walks factory method return types, unwrapping `Task`, nullable ### Factory event preservation -Factory event records inherit `FactoryEventBase`, which carries two annotations with `Inherited = true`: +Factory event records inherit `FactoryEventBase`. Two mechanisms make them work under trimming: -- `[FactoryEvent]` — used by the runtime `FactoryEventTypeRegistry` to discover descendants via attribute scan -- `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` — preserves every descendant's public constructors and public properties through trimming +- `[FactoryEvent]` on the base (inherited at runtime) — used by the runtime `FactoryEventTypeRegistry` to discover descendants via attribute scan +- The source generator discovers every concrete, accessible `FactoryEventBase` descendant declared in a compilation and emits a per-assembly event-preservation registrar that preserves the event's constructors/properties and its nested property graph. (The `[DynamicallyAccessedMembers]` annotation on the base does NOT do this — DAM does not flow to derived types under ILLink.) Net effect: if a record inherits `FactoryEventBase`, its constructors and properties survive `PublishTrimmed=true` automatically. No per-event annotation, no `[FactoryEventHandler]` declaration, and no manual `DtoConstructorRegistry` call is required for the event type itself. @@ -236,7 +236,7 @@ Direct calls with a concrete type (`_factoryEvents.Raise(new OrderCheckoutComple ### What you need to know -If you return a plain DTO from a factory method, or declare a `FactoryEventBase` descendant in a project with a direct `Neatoo.RemoteFactory` `PackageReference`, the type and its constructors and properties are automatically trimming-safe. Nested complex property types reachable from events that are NOT used elsewhere in the API may need explicit preservation. +If you return a plain DTO from a factory method, carry a DTO as a `[Factory]` entity property, or declare a `FactoryEventBase` descendant in a project with a direct `Neatoo.RemoteFactory` `PackageReference`, the type and its constructors and properties are automatically trimming-safe. Nested property types reachable from events are walked and preserved automatically too. One boundary: private/protected/file-scoped nested event records cannot be preserved (the generated registrar cannot reference them) — declare wire-crossing events as top-level or internal/public nested types. ## IFactorySaveMeta Preservation diff --git a/src/Design/CLAUDE-DESIGN.md b/src/Design/CLAUDE-DESIGN.md index 2bc6da05..d81324d8 100644 --- a/src/Design/CLAUDE-DESIGN.md +++ b/src/Design/CLAUDE-DESIGN.md @@ -790,13 +790,13 @@ Duplicate registrations from multiple factories returning the same DTO type are **Entity property-graph discovery:** every class carrying `[Factory]` directly also walks its own public property graph during generation, emitting preservation for reachable DTOs in its own `FactoryServiceRegistrar` — covering DTOs that ride on aggregates without ever appearing in a factory method signature. The entity itself is never bucketed (DI registration preserves it); factory-typed properties are skipped by the parent walk because each `[Factory]` class's own registrar owns its graph. Deliberate boundary: interface-factory *implementation* classes (implement a `[Factory]` interface, carry no direct attribute) get no registrar and no walk — they are stateless services, not serialized state. The walk is orthogonal to `CollectOrdinalProperties` (trimming-preservation types vs. the entity's own serialization slots; factory-typed/ordinal-serialized properties are skipped). -**Factory event type preservation.** `FactoryEventBase` itself carries `[FactoryEvent]` and `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]`, both with `Inherited = true`. Every descendant is therefore automatically discoverable by the runtime `FactoryEventTypeRegistry` and has its constructors and properties preserved through IL trimming with **no generator emission and no per-event annotation**. Inheriting `FactoryEventBase` is sufficient; consumers never apply `[FactoryEvent]` directly. +**Factory event type preservation.** The generator discovers every concrete, accessible `FactoryEventBase` descendant declared in a compilation (a `CreateSyntaxProvider` scan — descendants carry no attribute of their own, and inherited attributes are invisible to Roslyn symbols) and emits a per-assembly **event-preservation registrar**: a generated static class registered via the assembly-level `[NeatooFactoryRegistrar]` mechanism, whose `FactoryServiceRegistrar` emits `PreserveType()`/`Register()` for each event and its nested property graph (same bucketed walk as factory-signature and entity-property DTOs). Declaring the event is sufficient; consumers never apply `[FactoryEvent]` directly (it stays on the base, inherited at runtime for `FactoryEventTypeRegistry` discovery). -This supersedes the prior per-`[FactoryEventHandler]` emission of `DtoConstructorRegistry.PreserveType()` — the relay-handler pipeline no longer walks event types for trimming. Net result: stronger guarantee (covers every descendant, even those with no server handler), less generated code. `IFactoryEvents.Raise` retains `[DynamicallyAccessedMembers(All)]` on its generic parameter for producer-side call-site preservation. +History: v1.4.0 removed the per-`[FactoryEventHandler]` `PreserveType` emission in favor of `[DynamicallyAccessedMembers]` on `FactoryEventBase`, believing the annotation covered every descendant. A publish-trimmed repro (TRIM-003, 2026-07) proved it does not — DAM does not flow from a base type to derived types under ILLink, so a subscribe-only event record lost its constructor. Generator emission (per-assembly, not per-handler) restores the guarantee for real: it covers every accessible descendant, including those with no server handler and no client subscription. `IFactoryEvents.Raise` retains `[DynamicallyAccessedMembers(All)]` on its generic parameter for producer-side call-site preservation. Accessibility boundary: private/protected/file-scoped nested event records cannot be referenced from the generated registrar and are skipped — wire-crossing events must be top-level or internal/public nested. -End-to-end verification via `src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs` (publish-trimmed smoke test — confirms event relay round-trips across a trimmed binary). +End-to-end verification via `src/Tests/RemoteFactory.TrimmingTests/EventSubscribeOnlySmokeTest.cs` (publish-trimmed; the event's only static reference is a generic `Subscribe` call site) and `EventRelaySmokeTest.cs` (relay round-trip). -**Nested property walking.** The per-handler nested-walk behavior for event record properties is gone with the rest of the per-handler trimming pipeline. Trim preservation comes exclusively from `[DynamicallyAccessedMembers]` on `FactoryEventBase`, which applies recursively to every descendant's public constructors and properties. For reference-type properties not reachable from an event record (or from a factory return type), use the existing `DtoConstructorRegistry.PreserveType()` / `Register()` mechanism in DI setup. +**Nested property walking.** Automatic: the event-preservation registrar walks each discovered event's public property graph with the shared bucketed walk — nested records → `PreserveType`, parameterless DTOs → `Register`, collections/nullables unwrapped, cycles detected. Manual `DtoConstructorRegistry` calls are only needed for types unreachable from every discovery entry point (factory signatures, `[Factory]` entity properties, event graphs). #### CS0051 Constraint diff --git a/src/Design/Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs b/src/Design/Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs index 489e0d76..fd947400 100644 --- a/src/Design/Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs +++ b/src/Design/Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs @@ -116,9 +116,10 @@ await notificationService.SendAsync( // NESTED-RECORD EVENT: automatic IL-trimming preservation // ============================================================================= // -// When an event record carries a nested record property (like ShippingAddress -// below), the generator emits preservation calls for BOTH the event and the -// nested record in the handler's FactoryServiceRegistrar: +// Declaring any concrete FactoryEventBase descendant is enough: the generator +// discovers it (no handler or subscription required) and emits preservation for +// BOTH the event and its nested property graph in the per-assembly +// NeatooEventPreservationRegistrar: // // DtoConstructorRegistry.PreserveType(); // DtoConstructorRegistry.PreserveType(); @@ -128,13 +129,17 @@ await notificationService.SendAsync( // public properties intact. Without this, a Blazor WASM Release build with // PublishTrimmed=true would strip the metadata that NeatooJsonTypeInfoResolver // and RecordBypassConverterFactory need to round-trip the event at runtime. +// (The [DynamicallyAccessedMembers] annotation on FactoryEventBase itself does +// NOT do this — DAM does not flow to derived types under ILLink.) // // Nested records with parameterless ctors (plain DTOs) instead get // DtoConstructorRegistry.Register(() => new N()) — same trimming effect. // -// Known gap: Dictionary value types are not walked. If your event exposes -// Dictionary, declare another [FactoryEventHandler] -// or an additional preservation hint to keep Payload intact after trimming. +// Known gap: Dictionary value types are not walked (the walk unwraps the +// KeyValuePair enumerable, which is a System type). If your event exposes +// Dictionary, preserve Payload explicitly in DI setup +// (DtoConstructorRegistry.PreserveType()) or expose it through a +// walked property. // ============================================================================= /// diff --git a/src/Design/Design.Domain/FactoryPatterns/FactoryEventRelayPattern.cs b/src/Design/Design.Domain/FactoryPatterns/FactoryEventRelayPattern.cs index 4e345d01..0ec3e9ca 100644 --- a/src/Design/Design.Domain/FactoryPatterns/FactoryEventRelayPattern.cs +++ b/src/Design/Design.Domain/FactoryPatterns/FactoryEventRelayPattern.cs @@ -10,10 +10,11 @@ // Three roles: // // 1. The EVENT TYPE inherits from FactoryEventBase (shared between client/server). -// FactoryEventBase carries [FactoryEvent] and [DynamicallyAccessedMembers] with -// Inherited = true — descendants are automatically discoverable by the runtime -// FactoryEventTypeRegistry and preserved through IL trimming, with no generator -// emission or client codegen. +// [FactoryEvent] on the base (inherited at runtime) makes descendants +// discoverable by the runtime FactoryEventTypeRegistry. IL-trimming +// preservation comes from the generator-emitted per-assembly +// NeatooEventPreservationRegistrar, which discovers every concrete accessible +// descendant by declaration — no handler, subscription, or annotation needed. // 2. The SERVER-SIDE RAISER is a factory method that injects IFactoryEvents // and calls Raise(new MyEvent(...)) during its execution. // 3. The CLIENT-SIDE RELAY is the consumer's implementation of IFactoryEventRelay. @@ -60,9 +61,10 @@ namespace Design.Domain.FactoryPatterns; /// Reasons: /// 1. Records have structural equality (useful for deduplication). /// 2. Records are immutable by default — events should not mutate. -/// 3. FactoryEventBase carries [FactoryEvent] and [DynamicallyAccessedMembers] -/// with Inherited = true, so every descendant is automatically discoverable -/// and trim-safe without any per-event annotation. +/// 3. [FactoryEvent] on FactoryEventBase (inherited at runtime) makes every +/// descendant automatically discoverable; the generator-emitted per-assembly +/// event-preservation registrar makes every concrete accessible descendant +/// trim-safe — no per-event annotation required. /// /// The rule: Events are records that inherit FactoryEventBase. Nothing else required. /// diff --git a/src/Design/Design.Tests/FactoryTests/FactoryEventHandlerTests.cs b/src/Design/Design.Tests/FactoryTests/FactoryEventHandlerTests.cs index 2339a229..8ab2e380 100644 --- a/src/Design/Design.Tests/FactoryTests/FactoryEventHandlerTests.cs +++ b/src/Design/Design.Tests/FactoryTests/FactoryEventHandlerTests.cs @@ -54,10 +54,12 @@ public async Task Raise_NoHandlers_CompletesWithoutError() /// /// Demonstrates: an event record with a nested parameterized-record property - /// round-trips cleanly. Exercises the generator's automatic IL-trimming - /// preservation for nested records — if the generator had not emitted - /// PreserveType<ShippingAddress>(), a Release build with - /// PublishTrimmed=true would fail to deserialize the nested record. + /// round-trips cleanly. The trimming preservation for this shape comes from the + /// generated per-assembly event-preservation registrar, which walks each + /// declared event's property graph and emits + /// PreserveType<ShippingAddress>(); without it, a Release build with + /// PublishTrimmed=true would fail to deserialize the nested record (this test + /// runs untrimmed — the trimmed pin lives in RemoteFactory.TrimmingTests). /// [Fact] public async Task Raise_EventWithNestedRecord_DispatchesSuccessfully() diff --git a/src/Generator/FactoryGenerator.Events.cs b/src/Generator/FactoryGenerator.Events.cs new file mode 100644 index 00000000..19a1de3b --- /dev/null +++ b/src/Generator/FactoryGenerator.Events.cs @@ -0,0 +1,98 @@ +// FactoryGenerator.Events.cs +// Discovery of concrete FactoryEventBase descendants for IL-trimming preservation +// (TRIM-007). Descendants carry no attribute of their own ([FactoryEvent] lives on +// the base and Roslyn symbols do not surface inherited attributes), so discovery is +// a CreateSyntaxProvider scan over record declarations with a base list. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Neatoo.RemoteFactory.Generator; + +namespace Neatoo; + +/// +/// Value-equatable discovery result for one concrete FactoryEventBase +/// descendant: the assembly name (for the per-assembly registrar's namespace and +/// hint) and the two preservation buckets produced by walking the event root and +/// its property graph with the shared bucketed walk. +/// +internal sealed record FactoryEventInfo +{ + public FactoryEventInfo(string assemblyName, EquatableArray registerTypes, EquatableArray preserveTypes) + { + this.AssemblyName = assemblyName; + this.RegisterTypes = registerTypes; + this.PreserveTypes = preserveTypes; + } + + public string AssemblyName { get; } + public EquatableArray RegisterTypes { get; } + public EquatableArray PreserveTypes { get; } +} + +public partial class Factory +{ + internal static FactoryEventInfo? TransformFactoryEvent(RecordDeclarationSyntax recordSyntax, SemanticModel semanticModel) + { + if (semanticModel.GetDeclaredSymbol(recordSyntax) is not INamedTypeSymbol symbol) + { + return null; + } + + // Base-chain match by fully-qualified name — FactoryEventBase is always a + // metadata symbol from the referenced Neatoo.RemoteFactory assembly. + var derivesFromEventBase = false; + for (var baseType = symbol.BaseType; baseType != null; baseType = baseType.BaseType) + { + if (baseType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat) == "global::Neatoo.RemoteFactory.FactoryEventBase") + { + derivesFromEventBase = true; + break; + } + } + + if (!derivesFromEventBase) + { + return null; + } + + // Accessibility gate: the generated per-assembly registrar is a separate + // file and cannot legally reference private/protected/file-scoped event + // records (e.g. private nested test events). + if (!IsAccessibleWithinAssembly(symbol)) + { + return null; + } + + var registerTypes = new List(); + var preserveTypes = new List(); + DtoTypeWalker.WalkDtoGraph(symbol, registerTypes, preserveTypes, new HashSet()); + + return new FactoryEventInfo( + semanticModel.Compilation.AssemblyName ?? "NeatooEvents", + new EquatableArray([.. registerTypes]), + new EquatableArray([.. preserveTypes])); + } + + /// + /// True when the type (and every containing type) is public or internal — + /// i.e. referenceable from a generated file in the same assembly. + /// + private static bool IsAccessibleWithinAssembly(INamedTypeSymbol symbol) + { + for (var current = symbol; current != null; current = current.ContainingType) + { + if (current.IsFileLocal) + { + return false; + } + + if (current.DeclaredAccessibility is not (Accessibility.Public or Accessibility.Internal or Accessibility.ProtectedOrInternal)) + { + return false; + } + } + + return true; + } +} diff --git a/src/Generator/FactoryGenerator.cs b/src/Generator/FactoryGenerator.cs index 72044f9c..9899ebbb 100644 --- a/src/Generator/FactoryGenerator.cs +++ b/src/Generator/FactoryGenerator.cs @@ -105,6 +105,29 @@ public void Initialize(IncrementalGeneratorInitializationContext context) spc.AddSource($"{model.HintName}.FactoryEventHandler.g.cs", source); }); + // Pipeline for FactoryEventBase descendants — emits a per-assembly preservation + // registrar so event records survive IL trimming with no consumer action. + // Records-only predicate: classes cannot inherit a record base, so every + // descendant is a record. Descendants carry no attribute (inherited attributes + // are invisible to Roslyn symbols), hence CreateSyntaxProvider. + var factoryEventsToPreserve = context.SyntaxProvider.CreateSyntaxProvider( + predicate: static (s, _) => s is RecordDeclarationSyntax recordDecl + && recordDecl.BaseList != null + && !(recordDecl.TypeParameterList?.Parameters.Any() ?? false) + && !recordDecl.Modifiers.Any(SyntaxKind.AbstractKeyword), + transform: static (ctx, _) => TransformFactoryEvent((RecordDeclarationSyntax)ctx.Node, ctx.SemanticModel)) + .Where(static info => info is not null) + .Collect(); + + context.RegisterSourceOutput(factoryEventsToPreserve, static (spc, events) => + { + var rendered = EventPreservationRenderer.Render(events); + if (rendered != null) + { + spc.AddSource(rendered.Value.HintName, rendered.Value.Source); + } + }); + } private static DiagnosticDescriptor GetDescriptor(string diagnosticId) diff --git a/src/Generator/Renderer/EventPreservationRenderer.cs b/src/Generator/Renderer/EventPreservationRenderer.cs new file mode 100644 index 00000000..e14d672d --- /dev/null +++ b/src/Generator/Renderer/EventPreservationRenderer.cs @@ -0,0 +1,134 @@ +// src/Generator/Renderer/EventPreservationRenderer.cs +// Renders the per-assembly event-preservation registrar (TRIM-007): one generated +// static class per compilation that preserves every concrete FactoryEventBase +// descendant (and its nested DTO graph) from IL trimming. Discovered at runtime by +// the existing assembly-level [NeatooFactoryRegistrar] scan — the attribute's +// [DynamicallyAccessedMembers] on its Type parameter roots FactoryServiceRegistrar +// under TrimMode=full, whose body then roots the preservation call sites. + +#nullable enable + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Text; + +namespace Neatoo.RemoteFactory.Generator.Renderer; + +internal static class EventPreservationRenderer +{ + /// + /// Renders the registrar, or returns null when the compilation declares no + /// (accessible, concrete) FactoryEventBase descendants. Buckets are merged + /// across events, deduplicated, and ordinally sorted so the generated bytes + /// are deterministic across builds. + /// + public static (string HintName, string Source)? Render(ImmutableArray events) + { + var items = events.Where(e => e != null).Select(e => e!).ToList(); + if (items.Count == 0) + { + return null; + } + + var registerTypes = new SortedSet(StringComparer.Ordinal); + var preserveTypes = new SortedSet(StringComparer.Ordinal); + + foreach (var item in items) + { + foreach (var t in item.RegisterTypes) + { + registerTypes.Add(t); + } + + foreach (var t in item.PreserveTypes) + { + preserveTypes.Add(t); + } + } + + // A type never lands in both buckets within one walk, but distinct events + // could theoretically disagree only if ctor shape differed — impossible for + // the same FQN. Keep Register authoritative on any overlap. + preserveTypes.ExceptWith(registerTypes); + + if (registerTypes.Count == 0 && preserveTypes.Count == 0) + { + return null; + } + + var assemblyName = items[0].AssemblyName; + var ns = SanitizeNamespace(assemblyName); + + var sb = new StringBuilder(); + sb.AppendLine("#nullable enable"); + sb.AppendLine(); + sb.AppendLine("using Microsoft.Extensions.DependencyInjection;"); + sb.AppendLine("using Neatoo.RemoteFactory;"); + sb.AppendLine("using Neatoo.RemoteFactory.Internal;"); + sb.AppendLine(); + sb.AppendLine($"[assembly: Neatoo.RemoteFactory.NeatooFactoryRegistrar(typeof(global::{ns}.NeatooEventPreservationRegistrar))]"); + sb.AppendLine(); + sb.AppendLine("/*"); + sb.AppendLine(" READONLY - DO NOT EDIT!!!!"); + sb.AppendLine(" Generated by Neatoo.RemoteFactory"); + sb.AppendLine("*/"); + sb.AppendLine($"namespace {ns}"); + sb.AppendLine("{"); + sb.AppendLine(" /// "); + sb.AppendLine(" /// Preserves FactoryEventBase descendants (and their nested DTO graphs) from"); + sb.AppendLine(" /// IL trimming. Invoked by the assembly-level NeatooFactoryRegistrar scan."); + sb.AppendLine(" /// "); + sb.AppendLine(" internal static class NeatooEventPreservationRegistrar"); + sb.AppendLine(" {"); + sb.AppendLine(" internal static void FactoryServiceRegistrar(IServiceCollection services, NeatooFactory remoteLocal)"); + sb.AppendLine(" {"); + + foreach (var t in registerTypes) + { + sb.AppendLine($" DtoConstructorRegistry.Register<{t}>(() => new {t}());"); + } + + foreach (var t in preserveTypes) + { + sb.AppendLine($" DtoConstructorRegistry.PreserveType<{t}>();"); + } + + sb.AppendLine(" }"); + sb.AppendLine(" }"); + sb.AppendLine("}"); + + return ($"{assemblyName}.NeatooEventPreservation.g.cs", sb.ToString()); + } + + /// + /// Assembly names are usually valid dotted identifiers; sanitize each segment + /// for the edge cases (hyphens, leading digits) so the namespace always compiles. + /// + private static string SanitizeNamespace(string assemblyName) + { + var segments = assemblyName.Split('.'); + for (var i = 0; i < segments.Length; i++) + { + var chars = segments[i].ToCharArray(); + for (var c = 0; c < chars.Length; c++) + { + if (!char.IsLetterOrDigit(chars[c]) && chars[c] != '_') + { + chars[c] = '_'; + } + } + + var segment = new string(chars); + if (segment.Length == 0 || char.IsDigit(segment[0])) + { + segment = "_" + segment; + } + + segments[i] = segment; + } + + return string.Join(".", segments); + } +} diff --git a/src/RemoteFactory/FactoryEventBase.cs b/src/RemoteFactory/FactoryEventBase.cs index 9f2af6ec..afad545a 100644 --- a/src/RemoteFactory/FactoryEventBase.cs +++ b/src/RemoteFactory/FactoryEventBase.cs @@ -7,10 +7,13 @@ namespace Neatoo.RemoteFactory; /// Inherit from this record to define event types that can be published through the mediator. /// Records are recommended for events (immutable, structural equality). /// -/// and -/// are applied here with Inherited = true, so every descendant is automatically -/// discoverable by and its constructors and properties -/// are preserved through IL trimming. +/// is inherited at runtime, making every descendant +/// discoverable by . The +/// here does NOT preserve descendants' +/// members under IL trimming (DAM does not flow to derived types in ILLink) — descendant +/// preservation comes from the generator-emitted per-assembly event-preservation registrar, +/// which emits DtoConstructorRegistry calls for every concrete, accessible descendant and +/// its nested property graph. /// [FactoryEvent] [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties)] diff --git a/src/RemoteFactory/Internal/FactoryEventTypeRegistry.cs b/src/RemoteFactory/Internal/FactoryEventTypeRegistry.cs index 22b99952..da05169c 100644 --- a/src/RemoteFactory/Internal/FactoryEventTypeRegistry.cs +++ b/src/RemoteFactory/Internal/FactoryEventTypeRegistry.cs @@ -96,7 +96,7 @@ private static Dictionary Rescan() [UnconditionalSuppressMessage( "Trimming", "IL2026:RequiresUnreferencedCode", - Justification = "FactoryEventBase carries [DynamicallyAccessedMembers] with Inherited = true, preserving all descendants through trimming.")] + Justification = "Concrete FactoryEventBase descendants are preserved by the generator-emitted per-assembly event-preservation registrar (PreserveType rooting), so the assembly scan finds them intact.")] [SuppressMessage( "Design", "CA1031:Do not catch general exception types", diff --git a/src/Tests/RemoteFactory.TrimmingTests/EventSubscribeOnlySmokeTest.cs b/src/Tests/RemoteFactory.TrimmingTests/EventSubscribeOnlySmokeTest.cs new file mode 100644 index 00000000..b004a839 --- /dev/null +++ b/src/Tests/RemoteFactory.TrimmingTests/EventSubscribeOnlySmokeTest.cs @@ -0,0 +1,147 @@ +using Microsoft.Extensions.DependencyInjection; +using Neatoo.RemoteFactory; +using Neatoo.RemoteFactory.Internal; + +namespace RemoteFactory.TrimmingTests; + +/// +/// The event record under test (TRIM-003 repro, fixed by TRIM-007). Its ONLY +/// client-side static reference is the generic +/// Subscribe<TrimSubscribeOnlyEvent>(...) call site below — never +/// constructed, never referenced via typeof, no +/// [FactoryEventHandler<T>] anywhere. TRIM-003 proved the inherited +/// annotations on do NOT preserve derived members +/// under ILLink; survival now comes from the generator-emitted per-assembly +/// NeatooEventPreservationRegistrar (TRIM-007), which also walks the nested +/// property graph. +/// +public record TrimEventDetail(string Source); +public record TrimSubscribeOnlyEvent(int Id, string Note, TrimEventDetail? Detail) : FactoryEventBase; + +/// +/// Consumer-style relay aggregator — the zTreatment client shape: typed +/// subscriptions registered through a generic method, deserialized relay batches +/// dispatched by runtime type. +/// +public sealed class SubscribingRelay : IFactoryEventRelay +{ + private readonly Dictionary>> _subscriptions = new(); + + // Deliberately unannotated — the pure consumer shape. TRIM-003 proved this + // strips the event ctor when preservation relied on FactoryEventBase's inherited + // annotations; with TRIM-007's generated per-assembly registrar, no annotation + // is needed here and the check passes. + public void Subscribe(Action handler) where TEvent : FactoryEventBase + { + if (!_subscriptions.TryGetValue(typeof(TEvent), out var list)) + { + list = new List>(); + _subscriptions[typeof(TEvent)] = list; + } + + list.Add(evt => handler((TEvent)evt)); + } + + public Task Relay(IReadOnlyList events) + { + foreach (var evt in events) + { + if (_subscriptions.TryGetValue(evt.GetType(), out var list)) + { + foreach (var handler in list) + { + handler(evt); + } + } + } + + return Task.CompletedTask; + } +} + +/// +/// End-to-end trimming verification for the subscribe-only consumer shape +/// (TRIM-003 repro, fixed by TRIM-007). The existing EventRelaySmokeTest cannot +/// settle this — it constructs its event and uses typeof(), statically rooting +/// exactly the metadata under test. Here the wire entry's TypeFullName is a string +/// literal, so resolution goes through the runtime FactoryEventTypeRegistry +/// attribute scan, and member preservation depends entirely on the +/// generator-emitted per-assembly event-preservation registrar (TRIM-003 proved +/// the FactoryEventBase inherited annotations alone do not preserve descendants). +/// +public static class EventSubscribeOnlySmokeTest +{ + public static bool Run() + { + var services = new ServiceCollection(); + services.AddNeatooRemoteFactory(NeatooFactory.Remote, typeof(EventSubscribeOnlySmokeTest).Assembly); + + using var sp = services.BuildServiceProvider(); + var serializer = sp.GetRequiredService(); + + var relay = new SubscribingRelay(); + + // THE shape under test: this generic subscription is the event type's only + // static reference anywhere in the client. + TrimSubscribeOnlyEvent? received = null; + relay.Subscribe(evt => received = evt); + + var wire = new[] + { + new RelayedFactoryEvent + { + // String literal on purpose — typeof(...).FullName would root the + // type outside the consumer shape. + TypeFullName = "RemoteFactory.TrimmingTests.TrimSubscribeOnlyEvent", + Json = "{\"Id\":42,\"Note\":\"subscribe-only\",\"Detail\":{\"Source\":\"relay\"}}", + }, + }; + + IReadOnlyList deserialized; + try + { + deserialized = FactoryEventDeserializer.Deserialize(wire, serializer); + } + catch (UnknownFactoryEventTypeException ex) + { + Console.WriteLine($"Subscribe-only event smoke FAILED: registry could not resolve the event type post-trim (type stripped?). {ex.Message}"); + return false; + } + catch (Exception ex) + { + Console.WriteLine($"Subscribe-only event smoke FAILED: deserialization threw {ex.GetType().Name}: {ex.Message}"); + return false; + } + + try + { + relay.Relay(deserialized).GetAwaiter().GetResult(); + } + catch (Exception ex) + { + Console.WriteLine($"Subscribe-only event smoke FAILED: dispatch threw {ex.GetType().Name}: {ex.Message}"); + return false; + } + + if (received is null) + { + Console.WriteLine("Subscribe-only event smoke FAILED: typed subscriber did not receive the event."); + return false; + } + + if (received.Id != 42 || received.Note != "subscribe-only") + { + Console.WriteLine($"Subscribe-only event smoke FAILED: round-trip values lost. Got Id={received.Id}, Note=\"{received.Note}\"."); + return false; + } + + if (received.Detail is null || received.Detail.Source != "relay") + { + Console.WriteLine($"Subscribe-only event smoke FAILED: nested event record lost. Got Source=\"{received.Detail?.Source}\"."); + return false; + } + + Console.WriteLine("Subscribe-only event smoke PASSED: generator-emitted event preservation carried a subscribe-only event record (and its nested record) through trimming."); + return true; + } +} diff --git a/src/Tests/RemoteFactory.TrimmingTests/Program.cs b/src/Tests/RemoteFactory.TrimmingTests/Program.cs index 6a33e0d8..76d30661 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/Program.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/Program.cs @@ -119,6 +119,15 @@ failedChecks.Add("entity property DTO preservation"); } +// Subscribe-only event smoke test (TRIM-003 repro / TRIM-007 fix): a +// FactoryEventBase descendant whose only static reference is a generic +// Subscribe call site survives trimming via the generator-emitted +// per-assembly event-preservation registrar. +if (!EventSubscribeOnlySmokeTest.Run()) +{ + failedChecks.Add("subscribe-only event preservation"); +} + Console.WriteLine($"IsServerRuntime: {NeatooRuntime.IsServerRuntime}"); Console.WriteLine($"Class factory resolved: {factory != null}"); Console.WriteLine($"Static factory delegate resolved: {doWorkDelegate != null}"); diff --git a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/EventPreservationDiscoveryTests.cs b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/EventPreservationDiscoveryTests.cs new file mode 100644 index 00000000..a0ce135e --- /dev/null +++ b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/EventPreservationDiscoveryTests.cs @@ -0,0 +1,256 @@ +using RemoteFactory.UnitTests.TestContainers; + +namespace RemoteFactory.UnitTests.FactoryGenerator.DtoDiscovery; + +/// +/// Verifies the per-assembly event-preservation registrar (TRIM-007): the generator +/// discovers every concrete, accessible FactoryEventBase descendant declared in the +/// compilation and emits PreserveType/Register for the event and its nested DTO +/// graph — no handler attribute, no factory reference, no consumer action required. +/// +public class EventPreservationDiscoveryTests +{ + private static Microsoft.CodeAnalysis.GeneratorDriverRunResult Run(string source) + { + var (_, _, runResult) = DiagnosticTestHelper.RunGenerator(source); + return runResult; + } + + private static string AllTrees(Microsoft.CodeAnalysis.GeneratorDriverRunResult runResult) + => string.Join("\n", runResult.GeneratedTrees.Select(t => t.GetText()?.ToString() ?? "")); + + /// + /// Text of the event-preservation registrar tree (empty string when not emitted). + /// The helper compilation is named TestAssembly, so the hint is + /// "TestAssembly.NeatooEventPreservation.g.cs". + /// + private static string EventRegistrarTree(Microsoft.CodeAnalysis.GeneratorDriverRunResult runResult) + => string.Join("\n", runResult.GeneratedTrees + .Where(t => t.FilePath.EndsWith(".NeatooEventPreservation.g.cs")) + .Select(t => t.GetText()?.ToString() ?? "")); + + [Fact] + public void SubscribeOnlyEvent_PreserveTypeEmittedInEventRegistrar() + { + // No factory, no handler, no reference to the event anywhere — declaration + // alone must produce preservation. + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record StatusChangedEvent(int Id, string Status) : FactoryEventBase; +} +"; + var tree = EventRegistrarTree(Run(source)); + + Assert.Contains("DtoConstructorRegistry.PreserveType()", tree); + Assert.Contains("NeatooFactoryRegistrar(typeof(global::TestAssembly.NeatooEventPreservationRegistrar))", tree); + } + + [Fact] + public void EventNestedTypes_BothBucketsEmitted() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record ShippingDetail(string Street, string City); + + public class PlainInfo + { + public string Note { get; set; } + } + + public record OrderShippedEvent(int OrderId, ShippingDetail Detail) : FactoryEventBase + { + public PlainInfo Info { get; set; } + } +} +"; + var tree = EventRegistrarTree(Run(source)); + + Assert.Contains("DtoConstructorRegistry.PreserveType()", tree); + Assert.Contains("DtoConstructorRegistry.PreserveType()", tree); + Assert.Contains("DtoConstructorRegistry.Register", tree); + } + + [Fact] + public void AbstractIntermediate_SkippedButConcreteDescendantWalked() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public class PanelInfo + { + public string Name { get; set; } + } + + public abstract record PanelEventBase : FactoryEventBase + { + public PanelInfo Panel { get; set; } + } + + public record PanelOpenedEvent(int PanelId) : PanelEventBase; +} +"; + var tree = EventRegistrarTree(Run(source)); + + Assert.Contains("DtoConstructorRegistry.PreserveType()", tree); + // Inherited property from the abstract intermediate is walked... + Assert.Contains("DtoConstructorRegistry.Register", tree); + // ...but the abstract intermediate itself is never preserved. + Assert.DoesNotContain("PanelEventBase>", tree); + } + + [Fact] + public void PrivateNestedEventRecord_SkippedByAccessibilityGate() + { + // The generated registrar is a separate file — it cannot legally reference a + // private nested record (in-repo shape: FactoryEventCollectorTests' events). + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public class Holder + { + private record HiddenEvent(int Id) : FactoryEventBase; + } +} +"; + var all = AllTrees(Run(source)); + + Assert.DoesNotContain("HiddenEvent", all); + Assert.DoesNotContain("NeatooEventPreservationRegistrar", all); + } + + [Fact] + public void GenericEventRecord_Skipped() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record PayloadEvent(T Payload) : FactoryEventBase; +} +"; + var all = AllTrees(Run(source)); + + Assert.DoesNotContain("PayloadEvent", all); + Assert.DoesNotContain("NeatooEventPreservationRegistrar", all); + } + + [Fact] + public void NoEventsDeclared_NoRegistrarEmitted() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record PlainRecord(int Id); + + [Factory] + public partial class MyEntity + { + [Create] + internal void Create() { } + } +} +"; + var all = AllTrees(Run(source)); + + Assert.DoesNotContain("NeatooEventPreservationRegistrar", all); + } + + [Fact] + public void SharedNestedTypeAcrossEvents_SingleEmission() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record SharedDetail(string Value); + + public record FirstEvent(int Id, SharedDetail Detail) : FactoryEventBase; + public record SecondEvent(int Id, SharedDetail Detail) : FactoryEventBase; +} +"; + var tree = EventRegistrarTree(Run(source)); + + var emissions = System.Text.RegularExpressions.Regex.Matches( + tree, @"DtoConstructorRegistry\.PreserveType\(\)"); + Assert.Single(emissions); + Assert.Contains("PreserveType()", tree); + Assert.Contains("PreserveType()", tree); + } + + [Fact] + public void RegistrarOutput_OrdinallySorted_RegardlessOfDeclarationOrder() + { + // The registrar is one file aggregating many events — deterministic (ordinal) + // emission order keeps generated bytes stable across builds + // (ContinuousIntegrationBuild; plan review B4). + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record ZebraEvent(int Id) : FactoryEventBase; + public record AlphaEvent(int Id) : FactoryEventBase; +} +"; + var tree = EventRegistrarTree(Run(source)); + + var alphaIndex = tree.IndexOf("PreserveType()", StringComparison.Ordinal); + var zebraIndex = tree.IndexOf("PreserveType()", StringComparison.Ordinal); + + Assert.True(alphaIndex >= 0 && zebraIndex >= 0, "both events must be emitted"); + Assert.True(alphaIndex < zebraIndex, "emissions must be ordinally sorted, not declaration-ordered"); + } + + [Fact] + public void SameNamedBaseInOtherNamespace_NotMatched() + { + // Base matching is by fully-qualified name — a decoy FactoryEventBase in a + // consumer namespace must not trigger the event pipeline (plan review B2). + var source = @" +namespace MyApp +{ + public abstract record FactoryEventBase; + + public record FakeEvent(int Id) : FactoryEventBase; +} +"; + var all = AllTrees(Run(source)); + + Assert.DoesNotContain("FakeEvent", all); + Assert.DoesNotContain("NeatooEventPreservationRegistrar", all); + } + + [Fact] + public void EventWithParameterlessCtor_LandsInRegisterBucket() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record MutableEvent : FactoryEventBase + { + public int Id { get; set; } + } +} +"; + var tree = EventRegistrarTree(Run(source)); + + Assert.Contains("DtoConstructorRegistry.Register", tree); + Assert.DoesNotContain("PreserveType", tree); + } +} diff --git a/src/Tests/RemoteFactory.UnitTests/Internal/FactoryEventBaseAttributeTests.cs b/src/Tests/RemoteFactory.UnitTests/Internal/FactoryEventBaseAttributeTests.cs index d9812413..738af04d 100644 --- a/src/Tests/RemoteFactory.UnitTests/Internal/FactoryEventBaseAttributeTests.cs +++ b/src/Tests/RemoteFactory.UnitTests/Internal/FactoryEventBaseAttributeTests.cs @@ -5,9 +5,12 @@ namespace RemoteFactory.UnitTests.Internal; /// -/// Verifies FactoryEventBase carries [FactoryEvent] and [DynamicallyAccessedMembers] -/// with Inherited = true, so every descendant is discoverable by -/// FactoryEventTypeRegistry and preserved through IL trimming. +/// Verifies FactoryEventBase carries [FactoryEvent] (inherited at runtime, making +/// every descendant discoverable by FactoryEventTypeRegistry) and +/// [DynamicallyAccessedMembers]. Note: the DAM annotation does NOT preserve +/// descendants' members under IL trimming (DAM does not flow to derived types in +/// ILLink) — trimming preservation comes from the generator-emitted per-assembly +/// event-preservation registrar; see FactoryEventBase's doc comment. /// public class FactoryEventBaseAttributeTests {