Add the request binder for the primary-adapter boundary#141
Merged
Conversation
Declare the binder scope — FirstClassErrors.RequestBinder, the request binder for the primary-adapter boundary — in the commit linter, the CONTRIBUTING scope table and the CLAUDE.md scope list, ahead of the commits that introduce the component. Refs: #126
Introduce FirstClassErrors.RequestBinder, the framework-agnostic request binder designed in #126: Bind.PropertiesOf(request).FailWith(envelope) opens a binder whose SimpleProperty / ComplexProperty / ListOfSimpleProperties / ListOfComplexProperties stages convert a request DTO into value objects through Outcome-returning converters, collecting every failure — coded, path-carrying, grouped under a PrimaryPortError envelope — instead of stopping at the first. Build assembles the command only when nothing failed, so property handles are safe by construction, and the invalid-input path throws no exception: the binder catches nothing, leaving genuine bugs to the host boundary. The binder ships its own documented errors (REQUEST_ARGUMENT_REQUIRED, REQUEST_ARGUMENT_INVALID, with the full argument path in context), a pluggable per-binder argument-name provider, and an end-to-end unit-test suite covering every converter shape, nesting, list indexing, collect-all and the no-throw contract. Refs: #126
The internal HasErrors property had no caller: recording and surfacing failures both go through Record and Build. Coverage flagged it as dead code; remove it rather than test it. Refs: #126
Line and branch coverage measurement showed the first suite left real gaps beyond guard clauses: the PrimaryPortError converter-failure family, the foreign-family contract violation (which must throw), bare nested failures that bypass the envelope (which must be wrapped so the path survives), the missing required complex list, the failing optional complex list, the Convert-node unwrapping of boxing selectors, and the handles inspection seam. Cover each, plus every null guard and the documentation methods of the binder-owned errors, bringing the assembly to 100% line and branch coverage — measured, with the dead spots inspected rather than assumed. Refs: #126
Reading a bound value out of a property handle relied on `handle.Value`, a pure property that returned the stored value. Read inside `Build` it was safe by construction, but nothing stopped a consumer from reading it *outside* `Build`, on a failed binding: a required reference yielded a silent `null` (NRE far from the binding site), and an optional value-type present-but-invalid was indistinguishable from absent — a silent, diagnosable-nowhere failure the library exists to forbid. The recorded failure was `internal`, so a consumer had no way to even test for it. Replace the value-bearing handles with opaque field tokens read through a `BindingScope`, so an invalid or premature read cannot be written: - `AsRequired`/`AsOptional*` now return `RequiredField<T>` / `OptionalReferenceField<T>` / `OptionalValueField<T>` — tokens with no public value member. - `Build` takes a `BindingAssembler<TCommand>` whose single argument is a `BindingScope`; the value of a token is reachable only through `scope.Get(token)`. - `BindingScope` is a `readonly ref struct` (so it cannot escape the assembler) and `Build` constructs one only on its zero-error success branch. Reading a value before `Build`, or outside the assembler, therefore does not compile; a token from a different binder is rejected with `InvalidOperationException` (the binder's programming-error channel). The happy path stays `new Command(read.Get(email), read.Get(stay), …)`, with a named token at each argument. Collect-all, indexed list paths and nested envelopes are unchanged (failures already lived in the binder's ledger, not in the handles). netstandard2.0 stays green: the compiler synthesises the ref-like attribute. Binder tests keep 100% line and branch coverage (47 tests). BREAKING CHANGE: `handle.Value` is removed; read bound values through the `BindingScope` passed to `Build`. `Build` now takes `BindingAssembler<TCommand>` instead of `Func<TCommand>`. The handle types `RequiredProperty`, `OptionalReferenceProperty` and `OptionalValueProperty` are renamed `RequiredField`, `OptionalReferenceField` and `OptionalValueField`. Refs: #126
Shorten the assembler lambda's parameter from `read` to `s` (for "scope") at every call site — the two doc examples (Bind.cs, README.nuget.md) and the unit tests — so the happy path reads `s.Get(x)`. The BindingAssembler delegate's formal parameter is renamed `scope`, which reads better in the signature than a single letter. No behavior change: build clean, 47 binder tests green. Refs: #126
A complex property (and each element of a complex list) grouped a nested failure by TYPE: `error as PrimaryPortError ?? ArgumentInvalid(path, error)`. The type was a proxy for "is this the nested Build's self-describing envelope?" — but the proxy leaked. A converter is allowed to fail with a bare `PrimaryPortError` leaf; by type it looked like an envelope, so it was recorded as-is and its argument path (`Stay`) or index (`Guests[1]`) was silently dropped. The same failure raised as a `DomainError` was correctly wrapped with the path, and a simple property always wraps — a triple inconsistency, and the one case no test exercised (100% coverage saw the branch run for the legitimate envelope, never asserted the leaf case). Discriminate by INSTANCE instead: `Build` now remembers the envelope it produced (`BuiltEnvelope`), and a nested failure is recorded as-is only when it is that exact instance by `ReferenceEquals`; anything else — including a leaf that merely happens to be a `PrimaryPortError` — is wrapped so the path survives. The shared decision is factored into `NestedFailure.Group`, used by both the complex and the list converters (previously duplicated). The three added tests fail against the old by-type logic (verified) and pass against the fix; binder coverage stays 100% line and branch (50 tests). Refs: #126
The suite had 100% line and branch coverage yet a real bug (#3) had slipped through — because coverage proves a line RUNS, not that a behaviour is ASSERTED. A mutation hunt across every binder source file, each candidate verified empirically in an isolated worktree (apply the mutation, build, run the suite: did it still pass?), surfaced ten behaviours that were executed but never asserted. Close every one: - optional reference / optional value, present-but-invalid: pin the code (REQUEST_ARGUMENT_INVALID, not just "a failure") and the wrapped cause; - a null element in the middle of a complex list must not short-circuit collect-all — the failing elements after it are still collected; - the cross-binder ownership guard is now asserted on ALL three Get overloads (only RequiredField was covered; OptionalReference/OptionalValue were not); - the selector resolver unwraps EVERY stacked Convert node, not just the outer; - the binder-owned errors' public/detailed/diagnostic messages are pinned (short summary, and the argument path named in the detailed and diagnostic text). Verified: re-applying all ten mutations makes at least one test fail (10/10 killed); binder coverage stays 100% line and branch (54 tests). Refs: #126
A second mutation audit against the now-hardened suite surfaced six more behaviours that ran but were never asserted. Close each: - an invalid optional fallback names the misconfigured argument in its exception message (not just the exception type); - a complex-list element binder inherits the parent's options — a custom argument-name provider renames the ELEMENT's inner paths, not only the list prefix (the one genuinely behavioural gap: options inheritance was verified for a single ComplexProperty but never for list elements); - a cross-binder field read is rejected with a message that names the cause; - REQUEST_ARGUMENT_INVALID's diagnostic text still states it is "invalid"; - the REQUEST_ARGUMENT_REQUIRED documentation lists both diagnoses with the right origins (client omission = external, name mismatch = internal). Verified end to end: re-applying all sixteen mutants from both rounds makes at least one test fail (16/16 killed); binder coverage stays 100% line and branch (57 tests). Refs: #126
A third mutation audit against the twice-hardened suite found four remaining survivors — and, tellingly, ZERO behavioural ones: all four are diagnostic message or documentation CONTENT. The behavioural surface has gone dry (two of five source slices returned no survivor at all). Close the four: - the invalid-fallback exception carries the converter's DIAGNOSTIC detail (the raw offending value), not the sanitized public summary; - the foreign-error-family bug exception names the argument path and the offending family type; - the not-a-direct-property selector exception echoes the offending selector; - the REQUEST_ARGUMENT_INVALID documentation's two diagnoses carry the right origins (client-malformed value = external, over-strict parser = internal). Verified: every mutant found across all three rounds now makes at least one test fail; binder coverage stays 100% line and branch (58 tests). Refs: #126
Presence was detected by one test — `value is null` on the reflected property value. That works for a reference or a nullable value type (a missing string or int? is null), but a NON-nullable value type can never be null: a missing field deserializes to default(T) — 0, false — so `value is null` is always false. A missing `int MaxNights` was therefore bound as a legitimately-sent 0, "not sent" became indistinguishable from "sent 0" (silent data corruption at the boundary, the most banal .NET deserialization trap), and `AsRequired()`'s missing branch was dead code for such a property. The information does not exist at runtime, so the only correct answer is to make the DTO declare the property nullable. Guard it at bind time (the binder's programming-error channel, like an invalid selector): a selector pointing at a non-nullable value-type property throws ArgumentException naming the property and telling the developer to declare it as `T?`. A nullable value type, a reference, and a collection are unaffected. (A compile-time analyzer is the natural follow-up; it cannot be a generic constraint because `int` and `int?` both infer TArgument = int.) Verified: the guard's condition is mutation-checked (flipping the nullable test fails the new test); binder coverage stays 100% line and branch (59 tests). Refs: #126
The build terminal accepted only an infallible assembler, so a command produced by a validating factory returning Outcome<TCommand> could not compose (it double-wrapped into Outcome<Outcome<T>>), and a cross-field rule such as CheckOut > CheckIn had nowhere to run. Replace the single Build with two distinctly-named terminals: - New(BindingAssembler<TCommand>): a total constructor, wrapped in Success. - Create(ValidatingAssembler<TCommand>): a validating factory returning Outcome<TCommand>, flattened. The factory runs only once every field is bound, and its failure is returned as-is; only field-binding failures group under the FailWith envelope. Distinct names, not overloads, dissolve the CS0121 ambiguity a lambda returning Outcome<TCommand> would otherwise trigger, and mirror C#'s constructor-vs-factory convention so the terminal name recopies the shape of the assembler. ADR-0007 (Proposed) records the decision and its alternatives, coherent with ADR-0005's plain-factory-name rule. Cover Create with flatten-success, failure-pass-through, factory-untouched- on-binding-failure and null-guard tests (mutation-verified). Refs: #126
main replaced the hardcoded per-script train partition with a data-driven tools/trains.sh, so the binder's release wiring is re-expressed there rather than in each script. The request binder is version-coupled to FirstClassErrors (a ProjectReference), so it ships in lockstep on the lib train instead of a train of its own: a separate train would stamp its FirstClassErrors dependency at a version never co-published, making the package unresolvable (NU1102) on an immutable artifact. - trains.sh: add the binder scope to the lib train and name it in the label. - pack.sh: pack FirstClassErrors.RequestBinder on the lib train, and assert every lib-train package pins its FirstClassErrors dependency to the co-published version. The changelog and release-notes scripts source trains.sh, so binder-scoped commits land in the lib changelog with no further edit. Verified: a binder pack pins its FirstClassErrors dependency to the pack version, and the lockstep guard passes on a match and rejects a mismatch. Refs: #126
The request binder shipped without user-facing documentation: no repo guide, a dead "full guide" link in its NuGet README, and no French coverage at all -- at odds with the library's documented-errors thesis and its EN/FR sync rule. - Add doc/RequestBinder.en.md and doc/RequestBinder.fr.md: an exhaustive, example-driven guide (fluent API, collect-all, the New/Create terminals, the REQUEST_ARGUMENT_* codes, nested and list binding, options and the no-throw invariant, testing). - Index the guide in the README Documentation section and add an install line, in English (README.md) and French (doc/README.fr.md), in lockstep. - Repoint the NuGet README's "full guide" link to the new guide. Refs: #126
This was referenced Jul 16, 2026
Binder: WithOptions is accepted after binding starts, allowing mixed naming within one envelope
#145
Open
Open
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
2 participants
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.
Summary
FirstClassErrors.RequestBinderis a new framework-agnostic package that binds an incoming request DTO into a typed command or query of value objects at the primary-adapter boundary, collecting every invalid field into one documentedPrimaryPortErrorinstead of stopping at the first, and never throwing on the invalid-input path. It implements the design spec in #126, with the terminal API evolved toNew/Create(recorded in ADR-0007).Type of change
Changes
FirstClassErrors.RequestBinderpackage (netstandard2.0):Bind.PropertiesOf(request).FailWith(envelope)fluent API, withSimpleProperty/ComplexProperty/ListOfSimpleProperties/ListOfComplexPropertiesselectors andAsRequired/AsOptional/AsOptionalReference/AsOptionalValueconverters.New(a total constructor) andCreate(a validating factory returningOutcome<TCommand>, flattened) — reading bound values through aBindingScope, areadonly ref structcreated only on the success branch, so a bound value is readable only where every binding is known to have succeeded (enforced at compile time). ADR-0007 records the naming.PrimaryPortErrorenvelope; indexed paths for lists (Guests[1].Email); nested per-object and per-element envelopes; no throw on the invalid-input path — exceptions are reserved for programming errors (a throwing converter, an invalid selector, a non-nullable value-type property, a cross-binder token read).REQUEST_ARGUMENT_REQUIREDandREQUEST_ARGUMENT_INVALID, documented and carrying the full argument path in their context.FirstClassErrors(tools/trains.sh+tools/packaging/pack.sh), with a guard asserting every lib-train package pins itsFirstClassErrorsdependency to the co-published version.doc/RequestBinder.en.mdanddoc/RequestBinder.fr.md, indexed from the README (EN + FR) with an install line; the NuGet README links to the guide.Testing
dotnet build FirstClassErrors.slndotnet test FirstClassErrors.slnFirstClassErrors.Analyzers.UnitTests)Documentation
doc/updateddoc/README.fr.md) updated if user-facing behavior changedArchitecture decisions
Proposed: ADR-0007 (name the binder terminalsNewandCreate)Related issues
New/Create; see ADR-0007).🤖 Generated with Claude Code
Generated by Claude Code