Type handlers: runtime SqlMapper.AddTypeHandler registrations honored end-to-end

notes/harness-baseline.md

@@ -199,6 +199,22 @@ list expansion, TVPs, custom params), TypeHandlerTests ×16 (type-handler story)
 ×16 (coercions + tokens), Async/Literal (literals), plus the First-pipeline drain pair and
 the small tail. Nothing unexplained.
 
+## Round 13: type handlers (PR #206) - 705/793
+
+Runtime `SqlMapper.AddTypeHandler` registrations honored end-to-end by deferring to
+vanilla's `LookupDbType` at execution time (writes) and a generated-code bridge into the
+AOT readers (reads - the lib cannot reference Dapper without splitting the registry
+against StrongName consumers). The whole runtime-handler family cleared, plus the
+bare-DataTable TVP pair and Xml types for free (vanilla registers those handlers by
+default). Full account: [typehandlers-design.md](typehandlers-design.md), including the
+three probed contracts (DBNull-not-null to handlers; char excluded - StringFixedLength
+pads; typeof needs TypeOfName) and the silent-harness-build-failure lesson.
+
+**705 passed / 88 failed.** Remaining: Misc x11 (privates/fields, inheritance,
+Int16/Int32 + nullable-char coercions, message parity, multi-exec object[]), Literal x5 +
+async x3, deferred-by-decision (AnsiString pair, SetTypeMap pair - see PR #206), BigInt
+coercion, Constructor x2, singles.
+
 ## Round 11: dynamic-record fidelity (PR #200) - 672/793
 
 `DynamicRecord` diverged from vanilla's `DapperRow` three ways, all caught by the suite

notes/parity.md

@@ -90,8 +90,8 @@ Two levers change several complexity scores and are worth naming up front:
 | enum results (string→enum case-insens., widening, `ShortEnum`) | ⚠️❓ | high | low-med | Dapper recently changed precedence (prefer type handlers, #2200) — match the *new* behavior |
 | `MatchNamesWithUnderscores` | ❓ | med-high | low | snake_case databases; needs a compile-time equivalent (global option/attr) |
 | `SetTypeMap` / `CustomPropertyTypeMap` / `ITypeMap` / `TypeMapProvider` | ❌ 🚫? | med | med | runtime config by definition; AOT spelling is `[Column]`+`[UseColumnAttribute]`. Proposal: declare 🚫 for the runtime API, ship attribute equivalents + migration guidance |
-| `AddTypeHandler` / `TypeHandler<T>` / `StringTypeHandler` | ❌→⚠️ | **high** | med-high | AOT has its own `TypeHandler<T>`; needs the unification story (how does a *Dapper* handler registration become an AOT one?) |
-| `AddTypeMap` / `RemoveTypeMap` (scalar DbType map) | ❌ ❓ | low-med | low | e.g. `DateTime`→`DateTime2`; global compile-time option |
+| `AddTypeHandler` / `TypeHandler<T>` / `StringTypeHandler` | ✅ | — | — | runtime registrations honored end-to-end: writes dispatch through vanilla's `LookupDbType` (unrecognized member types), reads through a generated-code bridge into the AOT readers (the lib cannot reference Dapper: StrongName would split the registry). Whole-type handlers override generated row factories, matching vanilla. See [typehandlers-design.md](typehandlers-design.md); the announced-attribute tier (static dispatch, prior art #117/#162) remains as an optimization |
+| `AddTypeMap` / `RemoveTypeMap` (scalar DbType map) | ⚠️ | low-med | low | honored for member types the generator does not recognize (they route through `LookupDbType` at execution); *recognized* scalars (the string→AnsiString tests) keep their baked DbType — honoring those means a per-parameter lookup on the hottest types, a trade to take explicitly |
 | `Settings.ApplyNullValues` | ❓ | low | low | |
 | coercion matrix (`char`, `Nullable<T>`, `Convert.ChangeType` fidelity) | ❓ | high | med | silent-wrongness risk; test-driven, differential against Dapper |
 | column-level error reporting (`ThrowDataException` names column+value) | ❓ | med | low | DX parity worth keeping |

notes/typehandlers-design.md

@@ -0,0 +1,129 @@
+# Type handlers: the unification story
+
+## What the failing tests actually are
+
+"TypeHandlerTests ×16/provider" decomposes into four families; only the first two are
+type-handler work:
+
+1. **Runtime `AddTypeHandler` registrations** (Issue136, Issue1959 ×2, Issue461,
+   Issue253 ×2, SO24740733 ×2, EnumTypeHandler-preference): the suite registers handlers
+   at runtime; write-side members bind raw ("No mapping exists from object type
+   LocalDate…" from the provider) and read-side members never consult the handler.
+   Issue253 is the sharp one: a *handled collection type* — vanilla checks handlers
+   **before** list expansion, and our #197 expansion now wins incorrectly.
+2. **`AddTypeMap`** (AnsiString ×2): runtime remap of a *recognized* scalar's DbType.
+3. **`SetTypeMap`/`CustomPropertyTypeMap`** (TestCustomTypeMap, Test_RemoveTypeMap):
+   runtime *column-name mapping* — genuinely incompatible with compile-time row
+   factories; the parity table's 🚫-proposal stands (decision needed).
+4. **Coercion tail wearing the wrong filename** (TestBigIntForEverythingWorks: enum
+   from float/double column needs the pre-convert vanilla does; Issue149 strictness):
+   not handler work at all.
+
+## Tier 1: runtime dispatch, delegating to vanilla's own decision procedure
+
+`SqlMapper.LookupDbType(Type, name, demand, out ITypeHandler)` is public and
+`[Obsolete(…, false)]` — the same suppressible tier as `PackListParameters`, and it *is*
+the whole vanilla decision: handlers, the `AddTypeMap` remap, LinqBinary,
+`Settings.PreferTypeHandlersForEnums`, and `EnumerableMultiParameter` (i.e. the
+handler-before-expansion ordering), evaluated at execution time. `ITypeHandler` itself
+(`SetValue(IDbDataParameter, object)` / `Parse(Type, object)`) is public and
+non-obsolete. (`TypeHandlerCache<T>` is obsolete-as-**error** — unusable from generated
+C#, which is why vanilla can only call it from IL; no new Dapper API is needed, so no
+DAP052 gate.)
+
+- **Write, unknown member type** (today: raw `p.Value = …`, provider throws): emit
+  `LookupDbType(typeof(X), name, demand: true, out var handler)`; handler present →
+  `handler.SetValue(p, value)` with the **raw** value (null stays null — Issue1959 pins
+  that the handler sees null; vanilla only sanitizes on the non-handler path); else
+  apply the returned DbType if any and bind as today. `demand: true` also restores
+  vanilla's *"The member X of type Y cannot be used as a parameter value"* — which is
+  exactly what MiscTests.TestUnexpectedDataMessage pins, so that clears too.
+- **Write, expandable member**: same lookup *first*; handler present → single handled
+  parameter, else `PackListParameters` (vanilla's ordering; fixes Issue253).
+- **Write, enum member**: branch on `Settings.PreferTypeHandlersForEnums` (static bool,
+  default false — cheap short-circuit) before the baked enum path.
+- **Read, unknown member/result type**: `LookupDbType(typeof(X), "", demand: false,
+  out var handler)`; handler present → `(X)handler.Parse(typeof(X), reader.GetValue(i))`,
+  else the current `As<X>` fallback. Per-row lookup for tier 1 — registrations are
+  mutable (the suite re-registers), so per-shape caching is a later optimization with a
+  staleness story, not a first cut. Covers constructor binding (Issue461) and the
+  single-column scalar form (SO24740733).
+
+Deliberately *not* in tier 1:
+
+- **`AddTypeMap` on recognized scalars** (the AnsiString pair): honoring it means every
+  string member pays a runtime lookup where today the DbType is baked. Possible, small,
+  but a per-parameter cost on the most common parameter type — decision to take
+  explicitly rather than slip in.
+- **`SetTypeMap` family**: propose 🚫 (runtime column-mapping vs compile-time row
+  factories); the attribute equivalents (`[Column]` + `[UseColumnAttribute]`) are the
+  AOT spelling.
+
+## Tier 2: the announced-attribute layer (compile-time)
+
+`[TypeHandler<TValue, THandler>]` and `TypeHandler<T>` already ship in Dapper.AOT — the
+generator just never consults them (dormant API). Wiring them gives static dispatch
+(no lookup, no mutable registry, trim-friendly) and is the AOT-strict spelling to point
+people at. Prior art: external PRs #117 (samcragg — the attribute shape, plus a
+`Read(DbDataReader, int)` addition to `TypeHandler<T>`) and #162 (7amou3 — static
+per-file handler instances instead of per-call `new`). Both are the right *shape*;
+neither implementation can land as-is post-phase-2: #162's `TypeHandlerInstanceRegistry`
+keys a dictionary on `INamedTypeSymbol` inside generator state, which is exactly the
+Roslyn-objects-in-cached-state trap the plain-data model exists to prevent (ModelShapeTests
+enforces it). Tier 2 = their design, re-done as plain-data plans, with credit.
+
+Tier 1 first: it is what the test suite actually measures, needs no consumer changes,
+and works with every shipped Dapper.
+
+## Outcomes (recorded after implementation)
+
+- **695 -> 705/793**: the whole runtime-handler family cleared (Issue136, Issue1959 x4,
+  Issue253 x2, Issue461, SO24740733 x2, Issue149, the enum-preference test), plus the bare
+  `DataTable` TVP pair and the Xml tests - vanilla registers `DataTableHandler` and the XML
+  handlers *by default*, so the dispatch reaches them for free.
+- **`demand: false`, not vanilla's `demand: true`**, deliberately: when nothing matches we
+  keep the previous raw bind, because modern providers natively handle types vanilla's map
+  does not (DateOnly until the Dapper re-enable ships being the live case). Message parity
+  for genuinely-unusable types (TestUnexpectedDataMessage) is deferred to that bump.
+- **A handler receives DBNull, never null** - `SqlMapper.TypeHandler<T>`'s explicit
+  interface impl special-cases DBNull and NREs on a raw null (struct cast); vanilla's IL
+  coalesces first, so we do too.
+- **`char`/`char?` stay excluded from dispatch**: their map entry is StringFixedLength
+  *with* SetType, and applying it pads the round-trip (TestCharInputAndOutput). Vanilla
+  converts char members to length-1 strings on the way out - coercion-tail work, not
+  handler work.
+- **The build-exit lesson, again**: the first measurement showed zero movement because the
+  harness build had silently failed (generated `typeof` on an annotated reference type is
+  CS8639, on `dynamic` CS1962 - hence `ParamMember.TypeOfName`, mirroring `RowMember`'s)
+  and `--no-build` ran stale binaries. Check the exit code, not the presence of output.
+
+## Direction (agreed 2026-08-21): declarative config attributes as the primary spelling
+
+A static `Dapper.SomeConfigCall(...)` becomes `[module: SomeDapperConfig(...)]` - and that
+is *better scoped*, not merely equivalent: per-assembly instead of process-global mutable
+state, deterministic (no startup-ordering races), reviewable in the diff, and
+compile-time-visible so the generator bakes it at zero runtime cost. The protobuf-net
+precedent carries over whole, including the cross-assembly hand-off: the generator gathers
+assembly-level declarations from *references*, so a package can ship handlers for the
+types it owns (the [ProtoSurrogate] pattern, probed and shipped there).
+
+The config-call surface mapped onto attributes:
+
+| runtime call | declarative spelling |
+| --- | --- |
+| `SqlMapper.AddTypeHandler(typeof(T), h)` | `[module: TypeHandler<T, THandler>]` - already ships (dormant); generator wires it, announced handlers *elide* the runtime dispatch for that type |
+| `SqlMapper.AddTypeMap(type, dbType)` | `[module: TypeMap(typeof(string), DbType.AnsiString)]` (new) - also answers the deferred AnsiString pair with zero per-parameter cost |
+| `Settings.*` globals (CommandTimeout, list-expansion knobs...) | `[module: DapperSettings(...)]`-style (new); several parity rows already wanted "a compile-time global" |
+| `DefaultTypeMap.MatchNamesWithUnderscores` | same treatment (parity row already asks for it) |
+| `SetTypeMap` / `CustomPropertyTypeMap` | stays 🚫; `[Column]` + `[UseColumnAttribute]` is the spelling |
+
+Layering (unchanged from the PR, sharpened by the discussion):
+
+1. tier 1 (PR #206) stays: runtime registrations keep working, and after the enum gate the
+   cost is confined to the people using the feature;
+2. tier 2 attributes become the *recommended* spelling, statically dispatched; the
+   migration story is tooling, not docs alone - an analyzer that spots
+   `SqlMapper.AddTypeHandler(...)` in a [DapperAot] compilation and offers the attribute
+   as a code fix (the AotMigrationAnalyzer pattern from protobuf-net);
+3. a strict switch turns the runtime bridge off entirely (closed world, trimmable) for
+   consumers who want the full protobuf-net posture.