Type handlers via attribute registration, with a shim for vanilla handlers - #208
Merged
Merged
Conversation
…for vanilla handlers
The shipped [TypeHandler<TValue, THandler>] was never read by anything, so
there is no behavior to preserve - only metadata. Both it and its TypeHandler<T>
constraint are now [Obsolete(error)], naming the replacement; the types stay, so
binary compatibility holds.
The replacement is non-generic on purpose. .NET Framework's GetCustomAttributes
throws NotSupportedException for the whole call when an assembly or type carries
a generic attribute, poisoning unrelated reflection - the same wall protobuf-net
hit, with the same typeof-based resolution:
[module: TypeHandler(typeof(LocalDate), typeof(LocalDateHandler))]
and deliberately not [Conditional("DEBUG")], unlike most of our attributes, so a
package can declare handlers for the types it owns.
The runtime contract is IDbValueHandler<T> (with a DbValueHandler<T> base for
convenience): SetValue / SetNullValue - so a handler over a struct is never
handed a null it cannot express - Parse for output parameters, and a
Tokenize/Parse(reader, ordinal, token) pair mirroring how generated row
factories already work.
It is an interface rather than a base class because that is what makes existing
Dapper handlers portable across the snk boundary. The runtime library cannot
reference Dapper (a consumer may use Dapper *or* Dapper.StrongName, and
referencing either loads both and splits the registry) - but the generator can
see the consumer's symbols, and generated code compiles against whichever Dapper
they actually have. So a registration naming a SqlMapper.ITypeHandler emits
new VanillaTypeHandler<Money>(new MoneyHandler())
where VanillaTypeHandler<T> : IDbValueHandler<T> is generated into the same
file: our interface, their implementation, a shim in between.
Generated code dispatches on all four paths - AddParameters, the in-place
UpdateParameters used by command caching, PostProcess for output parameters, and
the row-factory read - with one static per registration, emitted only for
registrations the code actually reached. A handler owns the parameter's type and
size, so CanPrepare is cleared for those sites.
DAP053 reports SqlMapper.AddTypeHandler in a Dapper.AOT compilation with no
attribute registration for that type, naming the attribute to paste. It only
sees calls in the compilation being built, so it narrows the silent-wrongness
surface rather than closing it.
Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
This was referenced Aug 23, 2026
… to DAP054 Three things that stood between the declarative registration and being usable. **Tokenize is wired.** A handler's per-column decision was being thrown away - generated code always passed token 0 - so anything the protocol exists for (which shape did the provider actually give us?) had to be re-decided per row. The handler's token now travels in the row-factory's existing `state` channel: Tokenize allocates one int array per query, a second pass fills it by walking the token span (so it works the same whichever shape the mapping loop took, and does not care whether that loop advanced columnOffset), and Read indexes it positionally. Members sharing a handler share one case-arm. Costs one array per query and nothing per row. TypeHandlerProtocolTests pins that contract from the outside: a hand-written factory in the shape the generator emits, asserting Tokenize runs once per column no matter how many rows are read, that its token reaches every Parse, and that a factory reading a slice asks about its own column rather than column 0. Written against the public API, so it fails if either side drifts. **DAP055** reports a registration naming something generated code cannot use - implements neither contract, handles the wrong value type, no public parameterless constructor, abstract, static, inaccessible. These were skipped silently, which is exactly the failure mode declarative registration exists to remove; the message names the reason and says the registration is ignored. It is a warning, not an error: the code still compiles and runs, it just binds without the handler. **DAP053 becomes DAP054** here, because #206 takes DAP053 for the [UseRuntimeTypeHandlers]/PublishAot warning; ids are allocated so the two PRs can land in either order. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
The attribute accepted a one-argument, member-scoped form that nothing consults - which is precisely the silent no-op the shipped generic attribute was obsoleted for. Targets are now assembly/module only, matching what the generator actually reads. Both halves of this are non-breaking to add back: widening AttributeUsage and adding a constructor. So the narrower scope stays a plausible future addition, just not a shipped promise. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
marked this pull request as ready for review
August 25, 2026 10:50
mgravell
added a commit
that referenced
this pull request
Aug 25, 2026
#206's rule doc prescribes the attribute that only exists in #208, so merging #206 first would ship a diagnostic whose documented fix does not compile. #208 goes first, or #206's doc gets softened. Also records the id allocation (DAP053/#206, DAP054+DAP055/#208) so the two can land in either order, and that closing #206 entirely remains a live option. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
mgravell
added a commit
that referenced
this pull request
Aug 25, 2026
* Notes: type-handler registration - the feature never landed, and the route out The attribute and base class ship, but nothing in the analyzer or generator reads them, so every [module: TypeHandler<,>] written to date is a silent no-op. Records that, the three unmerged attempts and what each is worth, the richer protocol sitting on the incomplete type-handler branch, the constraints on a replacement (binary compat; no generic attributes, per the netfx GetCustomAttributes throw; not Conditional if cross-assembly), and the agreed route: obsolete the old pair, new typeof-based registration plus a new runtime API, and a diagnostic when a runtime registration has no attribute counterpart. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Notes: a state-of-play page, and the harness recipe that actually works Losing a session cost most of a morning re-deriving which branch held what, so: state-of-play.md records where each piece of work is, the agreed landing order and why, the diagnostic-id allocation, and the adjacent things that are easy to forget - the incomplete in-repo type-handler branch, the two external PRs awaiting a decision, and the unreleased Dapper APIs we are pinned behind. The harness section gets the three ways a repack silently measures stale code (pack does not build; pack skips when the nupkg exists; the package cache is redirected on this machine), plus the golden-regeneration workaround for deterministic source paths - all of which bit today. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY * Notes: #208 is complete, and the landing order was wrong #206's rule doc prescribes the attribute that only exists in #208, so merging #206 first would ship a diagnostic whose documented fix does not compile. #208 goes first, or #206's doc gets softened. Also records the id allocation (DAP053/#206, DAP054+DAP055/#208) so the two can land in either order, and that closing #206 entirely remains a live option. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
… is free The migration diagnostic becomes DAP053 and the unusable-handler refusal DAP054, keeping the library block contiguous now that nothing is going to take DAP053 for [UseRuntimeTypeHandlers]. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
…shapes Both external PRs propose the same fixture - module-scoped registration of a handler for a custom type, then three call sites exercising it - so it is a usable statement of intent. Translated verbatim (same call sites, type names and member names) into the registration and handler contract this PR ships, so the two can be compared line for line. All three generate through the handler: the read of a handled member on a mapped row type, the write of a handled member on an *anonymous* parameter type (shape witness intact - a case the existing fixture did not cover), and the output parameter read back via Parse. The handler is emitted once as a static readonly field shared by every factory in the file. That was #162's improvement over #117, which allocated at each parameter and each column read; static readonly also avoids the `??=` its review flagged as non-atomic. Two spellings differ, both deliberate: the registration is typeof-based rather than generic, because .NET Framework's GetCustomAttributes throws for the whole call when an assembly carries a generic attribute; and SetValueCore/Parse are abstract rather than defaulted, so a handler that handles nothing is a compiler error instead of a silent pass-through. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
…ping one Reading #162 for the prior-art fixture surfaced a gap it had already solved and this PR had not: two handlers registered for one type, with the second silently ignored. That is the same silent-wrongness the declarative form exists to remove, so it gets a diagnostic - worded like DAP021, which is the identical situation for parameters, naming both handlers and which one loses. First registration wins, deterministically: module scope before assembly scope, source order within each, and the generator no longer carries the losers. An exact repeat stays quiet, since the outcome is identical either way; a dropped duplicate is not also graded for usability, so a registration that is both duplicated and unusable produces one message rather than two. Also rewords DAP053 and DAP054 as single sentences, per the auto-review: the analyzer-design rule wants no trailing period on a single sentence and one on a multi-sentence message, and single-sentence is what DAP050-052 already do. Claude-Session: https://claude.ai/code/session_01GMLcMi7PXmALVsydfmkcmY
This was referenced Aug 25, 2026
Closed
This was referenced Aug 25, 2026
mgravell
added a commit
that referenced
this pull request
Aug 26, 2026
…ront (#210) parity.md still described type handlers as an open unification question, and still claimed DAP050 was the next free id. Both moved: #208 shipped declarative registration, and DAP050-055 are now taken. The type-handler row splits in two, because the answer was different for each half: declaring a handler is supported (and a vanilla SqlMapper.ITypeHandler can be named as-is, adapted by a generated shim), while registering one at runtime is now an explicit non-goal with the reasoning recorded. XML types, UDTs and bare DataTable members move from "not supported" to "expressible, no built-in declaration shipped" - which is a different question and a much smaller one. Adds a delta summary at the top so the file answers "what is actually left" without reading five tables: the corpus number, the interception ratio, and the remaining work ordered by weight, including which items are corpus adoption rather than product work, and the two type-handler ceilings that will not convert at all.
mgravell
added a commit
that referenced
this pull request
Sep 11, 2026
`state-of-play.md` is the "read first after a break" page and was the most wrong thing in the repo: it still listed #206/#207/#208 as in flight with a landing order to follow, and knew nothing of #209-#216. It now says what is true - nothing of ours in flight, the four open PRs are all external and all awaiting a decision. It also absorbs the account of **why #206 was closed**, which was sitting unmerged on the `typehandler-registration-note` branch and existed nowhere in main: gating stopped the bridge emitting for consumers who did not opt in, but the read-side check still sat in `RowFactory.GetValue<T>` - the type-flexible arm of every mapped member - so everyone paid for a feature almost nobody would enable. The emission-time fix that was considered and rejected is recorded with it, since that is the part most likely to be re-proposed. Two things promoted, because they gate real work and were buried: - the behavioural harness is local-only and does **not** exist on this machine (no `aot-harness` branch, no SQL Server, Windows-shaped repack recipe), so no phase-3 round can be closed and 677/793 cannot be re-measured until it is rebuilt; - net48 is unverified since #216 and #214, both of which changed interceptor goldens. Corrections: #117 and #162 are closed, not awaiting a decision; the `type-handler` branch harvest was filed as "before #208 settles", and #208 has settled; diagnostic ids as actually shipped (DAP053-056, next free DAP057). `parity.md`: the `CommandDefinition` row still said "27 overloads, every one skipped silently" - #214 moved the `Type`+`CommandDefinition` combinations out, so it is 21 of the 22 in that bucket. Cites the report rather than restating a number, which is what #213 was for. Also: `typehandlers-design.md` never existed in main - it was a file on the closed #206 branch - leaving dangling links in three notes. Pointed at the note that did land, `typehandler-registration.md`.
mgravell
added a commit
that referenced
this pull request
Sep 11, 2026
Pre-release verification, and the rig had to be rebuilt first: the old one was local-only on the Windows box and did not survive the move. The step-by-step for standing it up from nothing is now at the top of harness-baseline.md, Linux-shaped - databases from the Dapper suite's own docker compose, the SqlServerConnectionString env var, the local feed, and the .globalconfig severity downgrades without which DAP036/DAP037 stop the build outright. Round 15, net10.0: **729 passed / 800**, 41 failed, against a vanilla control of 770/800 on the same box. Scorecard: handled 432 of 736 enabled call-sites. All 41 divergences are known gaps at x2 providers - TypeHandler, Literal, Misc, Parameter, Async, plus scattered singles - so no new failure class, and the pass count is up on round 12's 677. The question this was run to answer: **the round-12 generator (b411eb4), packed and run against this same rig, also reads 432 of 736.** So #208-#217 changed interception not at all, and moved behaviour only upward. Recorded honestly rather than smoothed over: the 533/725 in round 12 is *not* reproducible here, and since the round-12 generator does not reproduce it either, the difference is rig configuration that no longer exists to inspect. DAP051 firing 244 times is the likely candidate but is a hypothesis. The durable lesson is in the note: absolute call-site counts are rig-specific, so compare within a rig, never across. Also corrected: the preamble still described round 14's [module: UseRuntimeTypeHandlers] as part of the setup, and that attribute does not exist - #206 was closed.
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
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.
Replaces the shipped-but-never-implemented
[TypeHandler<TValue, THandler>]with a registration the generator actually reads.TypeHandlerAttribute<,>andTypeHandler<T>ship in the package, but nothing in the analyzer or generator has ever consulted them — so every[module: TypeHandler<,>]written to date is a silent no-op (issues #159, #165, #173). There is no behavior to preserve, only metadata: both are now[Obsolete(..., error: true)]naming the replacement, and stay put, so binary compatibility holds.Registration
Non-generic on purpose: on .NET Framework,
GetCustomAttributes()over an assembly or type carrying a generic attribute throwsNotSupportedExceptionfor the whole call, which poisons unrelated third-party reflection, not just ours. Same wall we hit in protobuf-net, sametypeofresolution.Assembly/module scope only. A narrower scope (a handler for one member or parameter) is a plausible future addition — widening
AttributeUsageand adding a constructor are both non-breaking — but shipping a form nothing reads is the problem this PR exists to remove. Deliberately not[Conditional("DEBUG")], unlike our enablement attributes: a package should be able to declare handlers for the types it owns, which needs the metadata to survive the build.Runtime API
IDbValueHandler<T>, with aDbValueHandler<T>base for convenience:SetValue/SetNullValue(so a handler over a struct is never handed a null it cannot express),Parsefor output parameters, andTokenize/Parse(reader, ordinal, token)mirroring how generated row factories already work.Existing Dapper handlers stay usable, across the snk boundary
An interface rather than a base class is what makes that possible. The library cannot reference Dapper — a consumer may be on Dapper or Dapper.StrongName, and referencing either loads both and splits the registry — but the generator sees the consumer's symbols, and generated code compiles against whichever Dapper they actually have. So a registration naming a
SqlMapper.ITypeHandleremitswith
file sealed class VanillaTypeHandler<T> : IDbValueHandler<T>generated into the same file. Our interface, their implementation, a generated shim between. Migrating an existing handler is a matter of moving the registration out of a startup method, not rewriting the handler.What generated code does
All four paths dispatch:
AddParameters, the in-placeUpdateParametersused by command caching,PostProcessfor output parameters, and the row-factory read. One static per registration, emitted only for registrations the code actually reached;CanPrepareis cleared at handler sites, since the handler owns the parameter's type and size.Tokenizeis wired, which is what makes the read path worth having: a handler's per-column decision travels in the row factory's existingstatechannel — one int array per query, filled by a second pass over the token span (so it works the same whichever shape the mapping loop took), indexed positionally inRead. Members sharing a handler share a case-arm. One array per query, nothing per row.TypeHandlerProtocolTestspins that contract from outside the generator: a hand-written factory in the shape the generator emits, assertingTokenizeruns once per column however many rows are read, that its token reaches everyParse, and that a factory reading a slice asks about its own column rather than column 0.Diagnostics
SqlMapper.AddTypeHandlerin a Dapper.AOT compilation with no declarative registration for that type, naming the attribute to paste. Quiet when declared, quiet outside Dapper.AOT. It only sees calls in the compilation being built, so it narrows the silent-wrongness surface rather than closing it.All three have rule docs.
Does this satisfy #117 and #162?
Both external PRs ship the same fixture — module-scoped registration of a handler for a custom type, then three call sites — so it works as a statement of intent.
TypeHandlerPriorArt.input.csis that fixture translated verbatim (same call sites, type names, member names) into this PR's shapes, so the two can be read side by side. All three scenarios generate through the handler:Query<MyType>whereMyType.Cis the handled typeresult.C = ... TypeHandler0.Parse(reader, columnOffset, handlerTokens[i])Query<int>("def", new { Param = new CustomClass() })— anonymous parameterif (typed.Param is null) TypeHandler0.SetNullValue(p); else TypeHandler0.SetValue(p, typed.Param);, shape witness intact[DbValue(Direction = Output)] CustomClass OutputValueargs.OutputValue = TypeHandler0.Parse(ps[0]);The handler is emitted once, as a
static readonlyfield shared by every factory in the file — #162's improvement over #117 (which allocated at every parameter and every column read), without the??=its review flagged as non-atomic. #117's proposed read contract,Read(DbDataReader, int columnOffset), is satisfied as a superset:Parse(DbDataReader, int ordinal, int token)adds the per-column token.Two spellings differ from theirs, both deliberate:
typeof-based, not generic — a generic attribute cannot be read by .NET Framework'sGetCustomAttributes, which throws for the whole call and poisons unrelated reflection over the assembly;SetValueCoreandParseare abstract, where their base class defaulted them and their fixture's handler was empty. A handler that handles nothing should be a compiler error, not a silent pass-through.Reading #162 for this also surfaced a gap it had solved and this PR had not: duplicate registrations were silently dropping the loser. That is now DAP055 — the credit is theirs.
One thing from #162 is deliberately not carried over: its
TypeHandlerInstanceRegistrykeys generator state onINamedTypeSymbol, which the plain-data model introduced in #187/#188 exists to prevent (andModelShapeTestsnow enforces). The registry idea survives asTypeHandlerRegistration, projected to strings at parse time.Verification
Full suite green on net8.0 (366) and net48 (359); solution builds clean. Golden fixtures cover a native handler, a vanilla handler through the shim, a nullable member, an output parameter, a command-cache update, a handler with a real
Tokenize, and the prior-art scenarios above.Relationship to #206
#206 (honor runtime registrations, behind an opt-in) is now closed: even gated, its read-side check landed in
RowFactory.GetValue<T>— the type-flexible arm of every mapped member — so consumers who never enabled it still paid for it. This PR is the answer on its own, and existing Dapper handlers migrate by being named in the attribute rather than rewritten.