Assert and document the request binder's list-binding contracts - #177
Merged
Conversation
Issue #155 (request binder review, finding 19/19) flagged that whether a present-but-empty required list is valid was neither documented nor asserted. Record the contract: AsRequired constrains a list's presence (non-null), not its element count, so a present-but-empty list binds to an empty list while only an absent (null) list records REQUEST_ARGUMENT_REQUIRED. Document it in the API XML docs of the three list converters (simple, value-type, complex) and in the binder reference docs (EN + FR), and draft ADR-0014 (Proposed) recording the decision for the maintainer to accept. Refs: #155 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HzQtszd156kmTp86rfsQR
Assert the contract documented alongside: a required list that is present but empty binds as success (an empty list) and records nothing, across the simple, value-type, and complex list converters. Coverage alone did not assert this boundary (issue #155, finding 19/19), so the behaviour could have changed silently. Refs: #155 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HzQtszd156kmTp86rfsQR
The property-test suite exercised no binder invariant (issue #155, finding 19/19). Add FirstClassErrors.RequestBinder.PropertyTests — each library keeps its own test projects — with FsCheck invariants for the binder's defining guarantees: collect-all records exactly one error per failing element, and the indexed error paths are positional, so reordering the elements reorders their paths accordingly. Refs: #155 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HzQtszd156kmTp86rfsQR
The maintainer accepts the decision that a required list binding constrains presence, not cardinality — a present-but-empty list is valid. Flip ADR-0014 from Proposed to Accepted (EN + FR) and update the index. Refs: #155 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015HzQtszd156kmTp86rfsQR
18 tasks
Reefact
pushed a commit
that referenced
this pull request
Jul 18, 2026
Renumber ADR-0014 -> ADR-0015 (main took 0014 for the required-list presence contract, #177) and resolve the ADR index.
18 tasks
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
Issue #155 (request binder review, finding 19/19) found two binder behaviours that ran under 100% line/branch coverage but were never asserted. This PR documents and pins the empty-required-list contract, and adds property-based invariants for the binder's collect-all and path-stability guarantees.
Type of change
Changes
AsRequiredconstrains a list's presence (non-null), not its element count — a present-but-empty list binds to an empty list, and only an absent (null) list recordsREQUEST_ARGUMENT_REQUIRED. Updated the XML docs of the three list converters (simple, value-type, complex) and the binder reference docs (RequestBinder.en.md+RequestBinder.fr.md).FirstClassErrors.RequestBinder.PropertyTests(a new test project referencing the binder) with FsCheck invariants: collect-all records exactly one error per failing element, and the indexed error paths are positional — stable under element reordering.Testing
dotnet build FirstClassErrors.slndotnet test FirstClassErrors.slnFirstClassErrors.Analyzers.UnitTests)Full suite green: 975 tests, 0 failures, 0 warnings (includes the 3 new unit tests and the 3 new property tests). Re-verified after rebasing onto the latest
main.Documentation
doc/updated (RequestBinder.en.md, ADR-0014)RequestBinder.fr.mdand the French ADR; no user-facing behaviour change — the contract documents existing behaviour)Architecture decisions
Related issues
Closes #155
Generated by Claude Code