Discover EventProjection published types semantically, not syntactically - #611
Merged
Merged
Conversation
Closes the trap that cost compliance wave 2 (#608) a red test and is far worse for users than for suite authors. AggregateAnalyzer.DiscoverDocumentTypesFromMethodBodies matched only GenericNameSyntax, so an explicit ApplyAsync override that wrote operations.Store<AuditRecord>(record); registered AuditRecord as a published type, while the equally valid operations.Store(record); compiled and registered nothing at all. Nothing at the call site says so, and nothing fails at runtime either: the store provisions that document's storage on demand, so only the ahead-of-time surfaces come up short -- schema creation, known document types, rebuild teardown. See marten#4166 for the original. Binding the invocation gives the same answer for both spellings, because the type argument is on the method symbol whether or not it was written down. The syntactic path stays as a fallback for trees that do not bind (mid-edit, broken code), so nothing regresses when the semantic model has nothing to say. Also fixes a hole the semantic path made visible: IsFrameworkType cannot decide whether a type is registrable, because object and string render through ToDisplayString() as their C# keywords rather than System.Object / System.String, so a name-prefix test let them through. Store<object>(...) was registering `object` as a published document type. Registrability is now a SpecialType / TypeKind question, and IsFrameworkType is only the last check. New JFXEVT005 (Info) covers what is left: a call that binds to the projection's own session but whose document type cannot be named -- object, dynamic, an open type parameter. Info rather than Warning on purpose. Registration is not always required (the type may be registered through store options, and storage is provisioned on demand regardless), so this must not break a TreatWarningsAsErrors build over a legitimate call. Receiver identification is deliberate rather than name-based: the call must bind to the projection's own TOperations (directly, through an interface, or as an extension method's first parameter), so an unrelated Store method on some other object produces neither a registration nor a diagnostic. JasperFx.Events.SourceGenerator.Tests 30/30 with four new cases covering both spellings, the unregistrable case, and the unrelated-receiver case. EventTests 657/0/0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde
The published-type discovery change ships in JasperFx.Events.SourceGenerator, so it needs its own version. The publish workflow pushes with --skip-duplicate: run at an already-published version it skips every package and still reports success, so a shippable change merged without a bump goes out as nothing at all.
This was referenced Aug 3, 2026
jeremydmiller
added a commit
that referenced
this pull request
Aug 6, 2026
…ide (marten#5192) (#637) * fix(generator): register published types via a PublishedTypes() override (marten#5192) An EventProjection's discovered published document types were registered by emitting `public MyProjection() { RegisterPublishedType(...); }` into the user's partial class. A constructor is the wrong extension point, and it failed two ways. It is illegal on a type that declares a primary constructor -- `partial class MyProjection(ILogger logger) : EventProjection` -- because C# requires every other constructor to chain through the primary one, so the generated file broke the build outright with CS8862. That is what marten#5192 reported. Worse, and silent: a projection that takes dependencies has to be registered through Marten's AddProjectionWithServices, so the container calls the dependency-taking constructor and the generated parameterless one never ran. Published types went unregistered, which also left #626's teardown registration -- it reads PublishedTypes() -- with nothing to register. Emit an override of the virtual ProjectionBase.PublishedTypes() instead. It does not care how the instance was constructed, and chaining through base.PublishedTypes() keeps hand-written RegisterPublishedType calls and Options.StorageTypes flowing. The generator yields when the author already wrote their own override. Both emission sites are covered: EmitEventProjectionTypeRegistrationPartial (an ApplyAsync override) and the conventional-method path. Neither defect was reachable before 2.38.0: discovery was syntactic, so only an explicit `Store<Doc>(x)` produced a registration and the far more common `Store(doc)` produced none. #611 made discovery semantic and both spellings started emitting the constructor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde * 2.42.1 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the trap that cost compliance wave 2 (#608) a red test. It is far worse for users than it was for suite authors.
The trap
AggregateAnalyzer.DiscoverDocumentTypesFromMethodBodiesmatched onlyGenericNameSyntax. So an explicitApplyAsyncoverride that wrotebehaved completely differently for two spellings of the same call — and nothing tells you. It compiles, and it does not fail at runtime either: the store provisions that document's storage on demand, so only the ahead-of-time surfaces come up short (schema creation, known document types, rebuild teardown). Original context in marten#4166.
The fix
Bind the invocation. The type argument is on the method symbol whether or not it was written down, so both spellings give the same answer. The syntactic path stays as a fallback for trees that do not bind (mid-edit, broken code), so nothing regresses when the semantic model has nothing to say.
Receiver identification is deliberate rather than name-based: the call must bind to the projection's own
TOperations— directly, through an interface, or as an extension method's first parameter — so an unrelatedStoremethod on some other object produces neither a registration nor a diagnostic. There is a test for exactly that.A hole the semantic path made visible
IsFrameworkTypecannot decide whether a type is registrable.objectandstringrender throughToDisplayString()as their C# keywords, not asSystem.Object/System.String, so a name-prefix test lets them straight through —Store<object>(...)was registeringobjectas a published document type. Registrability is now aSpecialType/TypeKindquestion, withIsFrameworkTypeonly as the last check.New diagnostic: JFXEVT005 (Info)
Covers what is left after the fix — a call that binds to the projection's own session but whose document type cannot be named:
object,dynamic, an open type parameter.Info rather than Warning, on purpose. Registration is not always required — the type may be registered through store options, and storage is provisioned on demand regardless — so this must not break a
TreatWarningsAsErrorsbuild over a perfectly legitimate call.Verification
JasperFx.Events.SourceGenerator.Tests30/30, with four new cases: explicit spelling, inferred spelling, the unregistrable case, and the unrelated-receiver case.EventTests657 / 0 / 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde