Make the OneOf family uniform: both shapes everywhere, and on AnyPattern - #193
Make the OneOf family uniform: both shapes everywhere, and on AnyPattern#193Reefact wants to merge 5 commits into
Conversation
AnyString was the only generator of twenty-three carrying an IEnumerable<T> overload beside its params one. The other twenty-two now carry it too, with the same contract, validation and conflicts, so a set already held as a list or a LINQ result reaches a typed builder directly. Any.ElementOf is not that route: it returns a pool generator carrying only the exclusion pair, so it drops the type's own constraints, and Any.String().OneOf(list).WithLength(3) would have no spelling left. No existing call changes meaning — T[] is a better conversion target than IEnumerable<T>, so arrays, params lists and collection expressions all still bind to the array form. Held by a reflection guard rather than by convention: the algebra table compares method names and was structurally blind to an overload set, which is how the gap survived to be found by an audit. Sonar's S3220 is the one collateral effect, and it is narrower than it looks: ordinary calls stay silent, while a collection expression, a bare null and a single value of an unresolved type parameter now trip it. Two call sites met it and name the intended overload. Refs: #185
AnyPattern carried Except and DifferentFrom and no OneOf — the one partial trio on the surface, and a gap rather than a refusal: no record stated it, and the type described itself as carrying "the exclusion pair", which reads as a complete answer. ADR-0033 already settles it. A caller-supplied value set is a domain, not a layout: once the values are supplied there is nothing to build, and every other constraint becomes a test each value passes or fails. A pattern is exactly that test, and the verifier that performs it has been there since ADR-0027 — so this needs no new generation machinery. With the domain supplied it is also enumerable, which upgrades the diagnostics: a set the pattern admits nothing of, and an exclusion that empties what it left, are refused at declaration naming both sides, where the same exclusion on an unpooled pattern can only spend its redraw budget at Generate. A pooled pattern reports its survivors and rejections and answers a distinct collection with its surviving size. Declaring a set forces the verifier to compile at declaration, which a draw would have deferred. That is the trade: judging the caller's values is what makes the conflict eager. Refs: #185
|
chore (blocking): PR description contains unnatural English [P2] Several sentences read like literal or overly compressed translations rather than natural English, which makes the rationale harder to follow. Please rewrite at least these passages:
There are other similarly compressed constructions in the same description; please give the full text a natural-English pass rather than only applying these four replacements. |
|
Resolved in the description — rewritten in full rather than patched in four places, as asked. Your four passages are fixed as suggested. Beyond them, the pass broke up the compressed constructions throughout: the two Two factual updates while I was in there, both because the old text had gone stale rather than because it read badly:
No code changed, so the head is still Generated by Claude Code |
Reefact
left a comment
Reefact
left a comment
There was a problem hiding this comment.
note: Inline review finding attached
Reefact
left a comment
Reefact
left a comment
There was a problem hiding this comment.
note: Additional documentation findings attached
Reefact
left a comment
Reefact
left a comment
There was a problem hiding this comment.
note: One additional French wording finding attached
OneOf documents duplicates as ignored and promises nothing about order,
so OneOf("a", "b"), OneOf("b", "a") and OneOf("a", "b", "b") all declare
one domain. The identity check compared the rendered call, so two of the
three were refused as a second, conflicting set — a conflict naming a
constraint the caller has nothing to loosen in.
The comparison is now the deduplicated ordinal set. The rendered call is
still kept for the messages, because a conflict has to quote the caller
their own words, and a genuinely different set still conflicts.
Refs: #185
The changelog entry, one ADR-0099 sentence and the new French prose read as literal or over-compressed translations. Reviewer-named cases are fixed and the same pass runs over the rest of what this branch added. English: the changelog's "the only generator of twenty-three carrying" becomes "the only one of the twenty-three generators with", and the clause chains around it become sentences; ADR-0099's "an impossible arrange" becomes "an impossible test arrangement", naming the test phase it meant. French: "le test que chacune passe ou échoue" becomes "le motif sert alors à vérifier chaque valeur"; "un helper partagé possède le format" becomes "définit le format"; "un site d'appel le resserre sur les références qu'une fixture détient" becomes "le site d'appel limite le générateur aux références réellement disponibles dans la fixture"; "un chemin qui a vouché pour le motif" becomes "un chemin qui a déjà validé que le motif est générable"; "cela achète un diagnostic" becomes "le gain n'est pas seulement une symétrie : c'est un meilleur diagnostic". Both ADRs and both user pages get the same treatment. Refs: #185
Reefact
left a comment
Reefact
left a comment
There was a problem hiding this comment.
chore (blocking): Le français de l'ADR reste partiellement calqué
[P2] La passe a corrigé plusieurs formulations, mais deux calques restent dans la version actuelle de l'ADR-0099 : Un *arrange* impossible... n'est pas du français naturel, et La distinction entre refus immédiat et refus différé est une couture reste une traduction littérale de is a seam.
Pour le premier, utiliser par exemple Une configuration de test impossible... ou expliciter la phase Arrange. Pour le second, exprimer directement l'idée, par exemple Cette différence de comportement constitue un point de friction ou une subtilité de l'API.
The earlier prose pass fixed the English "an impossible arrange" and missed its French twin, which is exactly the drift the twin-page rule exists to prevent. It now reads "Une configuration de test impossible, signalée au moment de la génération". "est une couture" was a literal rendering of "is a seam" and becomes "constitue un point de friction". A sweep over the rest of the French this branch added finds no other calque of the same kind. Refs: #185
|
Both fixed in
The first one is the more embarrassing of the two, and worth naming: I fixed I then swept the rest of the French this branch adds — both Documentation suite green (11/11, parity and samples included). No code changed. Generated by Claude Code |
2 participants
Summary
OneOfwas inconsistent in two opposite ways.AnyStringwas the only one of the twenty-three generators with anIEnumerable<T>overload, andAnyPatternwas the only one with an incomplete trio: it hadExceptandDifferentFrom, but noOneOf. This PR closes both gaps and records both decisions.Type of change
Changes
OneOf(IEnumerable<T>)added to the twenty-two typed builders that lacked it. Each one delegates to theparamsform, so the contract, the validation and the conflicts are identical.Any.ElementOfis not a substitute here. It returns a pool generator that carries only the exclusion pair, so it drops the constraints belonging to the type:Any.String().OneOf(list).WithLength(3)would have no equivalent.No existing call site changes meaning.
T[]is a better conversion target thanIEnumerable<T>, so arrays,paramslists and collection expressions all continue to bind to the array form. The only calls the new overload affects are those that previously did not compile.AnyPattern.OneOfadded, in both shapes. Once you supply the values, the pattern stops generating them and becomes the test that each supplied value must pass. This is exactly how ADR-0033 describes a caller-supplied value set, and the verifier that performs the test has existed since ADR-0027, so nothing new was needed in the generation engine.Once the domain is supplied, it can be enumerated, and that makes two failures decidable at declaration time. A value set that the pattern rejects entirely, and an exclusion that empties whatever the pattern left, are both refused where they are declared, with a message naming both sides. On a pattern with no value set, the same exclusion can only exhaust its redraw budget at
Generate().AnyPatternnow implementsICardinalityHint<string>andIPoolInspection<string>, as ADR-0068 requires of any generator that accepts a caller-supplied pool.A redeclared value set is judged by its values, not by the call as written (review finding,
3636c38).OneOfdocuments duplicates as ignored and promises nothing about order, soOneOf("a", "b"),OneOf("b", "a")andOneOf("a", "b", "b")all declare one domain, and the second declaration is the harmless no-op the surface promises. The identity check compared the rendered call, so two of the three were refused as a conflicting second set. It now compares the deduplicated ordinal set, while the rendered call is still kept for the messages.A reflection guard on the shape of
OneOf, inSurfaceParityTests. The existing algebra table compares method names, so it could not see that one generator had an overload the others lacked. That is why an audit found this gap rather than the test suite. The new guard also covers generators that have not been written yet.Public API baseline updated: 24 entries on
net8.0, 19 onnetstandard2.0.User documentation updated, in English and French. The value-set section of
strings, and the pool rule inenums-and-choices, where the type-inference trap inAny.OneOf<T>is now clearly separated from the typed builders, which have noTto infer and therefore no trap.Changelog entry covering both halves.
One side effect worth reviewing
Sonar's
S3220now fires on three call shapes where it did not before: a collection expression such asOneOf([1, 2]), a barenull, and a single value whose type is an unresolved type parameter.Ordinary calls are unaffected. I measured this rather than inferring it:
OneOf(1),OneOf(Suit.Hearts),OneOf(1, 2),OneOf(array)andOneOf(list)all stay silent. The repository hits two of the three shapes, in one test each.On a collection expression the two rules contradict each other, because
S3878asks for the array to be removed again. That site therefore takes a justified suppression, following the precedent already recorded inSuppressionJustification.S3220.One thing left for you to decide
The redeclaration fix above is applied to
AnyPatternonly, because that is the line the review flagged. The same comparison exists at nine sites, eight of them predating this PR, soAny.String().OneOf("a", "b").OneOf("b", "a")still throws onmaintoday. Extending the fix would change declaration-time behaviour on twenty-three generators this PR does not otherwise touch, so I left that call to you rather than widening the PR. The open thread onAnyPattern.cscarries the measurements.Testing
dotnet build JustDummies.sln— 0 warnings, 0 errorsdotnet test JustDummies.sln— 2366 passed, 0 failed, 1 skippedJustDummies.Analyzers.UnitTests) — 430 passedI ran both suites on the
net10.0target only, because the container I worked in has no .NET Framework runtime. CI covers the rest — Library on the .NET Framework 4.7.2 floor, Build & test (windows-latest), Build & test (ubuntu-latest), SonarQube Cloud analysis and CodeQL — and has run green on every head this branch has had, including the current one. The checks list is the live answer; this paragraph deliberately names no commit, so it cannot go stale behind a push.I did not run mutation testing. The
JustDummies mutation gatecheck is green, but per ADR-0025 it only reports and enforces no threshold, so that is not a claim about a score.Documentation
doc/updatedArchitecture decisions
Proposed: ADR-0098Proposed: ADR-0099I wrote two records rather than one, because either can be adopted without the other. Both are in this branch, under
doc/handwritten/for-maintainers/adr/:0098-offer-every-oneof-in-both-shapes.md— offer everyOneOfin both shapes.0099-offer-oneof-wherever-the-generator-would-otherwise-build.md— offerOneOfwherever the generator would otherwise build the value.The second decision deserves more explanation. Refusing
AnyPattern.OneOfwould have been the cheaper answer, and I considered it seriously. The problem is that every argument for refusing it — the pattern stops generating, it only validates what it is given,Any.OneOf(...)already draws from a pool — applies just as well toAny.String().OneOf(...)used beside a shape constraint. ADR-0033 weighed exactly those arguments and admitted that case. Refusing it here would have contradicted an accepted record, which seemed worse than leaving the gap open.ADR-0099 also states one trade-off explicitly rather than hiding it. Declaring a value set forces
AnyPatternto compile its verifier at declaration time, whereas the code deliberately deferred that, so that a pattern the generation ceiling rejects never compiles aRegexat all. Validating the caller's values at declaration is precisely what makes the conflict eager, so the two cannot both be had. I measured the case the precaution was written for,^(?:a{2147483647,})$: it compiles in 7 ms and 30 MB. That was on the modern engine only. The comment in the code names a different implementation, and I could not measure the 4.7.2 floor locally, but CI runs that leg green.Related issues
Closes #185
Contributor agreement