Repository navigation
Wrapper set arities don't check that an element's text form round-trips #53
Description
The problem
A wrapper set (StringSet<T>, GuidSet<T>, the integer arities, DecimalSet<T>, the four temporal arities) writes an element as element.ToString(format, invariant) and reads it as TElement.Parse(text). Nothing checks that the two are inverses. When they aren't, the set writes text that can't be read back, or that reads back as a different element with no error.
Measured on main (c4f0272) through the array literal:
| Element | Write | Read back |
|---|---|---|
StringSet<Decorated>: ToString → CUST-123, Parse accepts digits only |
{CUST-123} |
FormatException from the element. The data can't be read back |
StringSet<Lenient>: ToString → CUST-123, Parse keeps any text |
{CUST-123} |
{CUST-CUST-123}, Equals false. Silent, and the prefix grows on every round trip |
Int32Set<OffByOne>: ToString → V + 1, Parse → V |
{124} |
element 124 instead of 123, Equals false. Silent |
The same text reaches JSON, the SQL literal and the bound parameter, so the same thing happens at every persistence boundary. The probing round (#52) first found it as claim S07 for StringSet<T>: JSON gives ["CUST-123"], the SQL literal ARRAY['CUST-123']::text[], and the bound parameter ["CUST-123"].
The integer, decimal and temporal arities look as if they refuse, but only because their primitive grammar rejects text that isn't a number or ISO. A decoration that still parses goes straight through. The five NodaTime arities already check the round trip: the element's ISO text must read back as the same element, in From, when reading, and as a probe. The core arities don't.
The fix (10.0.0)
Check the round trip for every core wrapper arity, as the NodaTime arities do:
- write:
Parse(Format(x))must equalx, or throw an error naming the text-form contract - read:
Format(Parse(text))must equaltext, which catches theLenientcase in data written before the fix
Decide where the write check lives. At construction (From, collection expressions, Add, probes) matches the NodaTime arities and fails earliest. At the boundaries only, it's cheaper. In either case, keep the EF bridge (requireRoundTrip) and the JSON element converter in step.
It's in 10.0.0 because it tightens behaviour. A consumer who only writes, such as an API that intentionally sends "CUST-123", gets an exception where it got output before. docs/migration.md needs a v9.x entry saying so.
Tests
DecorativeElementContractTestscurrently registers the threeStringSet<DecoratedText>rows intest/oracles/candidates.jsonagainst this issue, and usesInt32Set<T>as the reference row that refuses. Add an off-by-one integer row and a lenient string row. Both are candidates against this issue until the fix lands.- Seed the fix by removing the check for one arity at a time. The oracle must catch each one.
- The fix's PR removes the candidate rows, and the commit carries
Found-By: OT.
The 9.1.0 doc correction that states the contract is #59.
Activity
CaffeinatedCoder commentedon Oct 17, 2026
The gap isn't specific to
StringSet<T>. It's that no core wrapper arity checks that an element's text reads back as the same element. Measured on main (c4f0272), using the array literal (ToString/Parse):StringSet<Decorated>:ToString→CUST-123,Parseaccepts digits only{CUST-123}FormatExceptionfrom the element. The data can't be read backStringSet<Lenient>:ToString→CUST-123,Parsekeeps any text{CUST-123}{CUST-CUST-123},Equalsfalse. Silent, and the prefix grows on every round tripInt32Set<OffByOne>:ToString→V + 1,Parse→V{124}Equalsfalse. SilentThe integer arity only refuses decoration that isn't numeric, because the
intgrammar catches it. A wrong number goes through.The NodaTime wrapper arities already have the general check: their EF bridge sets
requireRoundTrip, and the element's ISO text has to read back as that same element. Applying the same check (Parse(Format(x))equalsx) to every core arity on write would turn all three rows into a loud error naming the contract. Applying it on read as well (Format(Parse(text))equalstext) would catch theLenientrow in data that was written before the fix.[-]StringSet<T> writes a decorated wrapper element instead of refusing it (text-form contract)[/-][+]Wrapper set arities don't check that an element's text form round-trips[/+]test: skip project-less test folders in CI and register the round-tri…
CaffeinatedCoder commentedon Oct 17, 2026
Oracle check (main at 3ecfa10): would catch.
DecorativeElementContractTestsfails on five rows:StringSet<DecoratedText>at the JSON write, the SQL literal and the bound parameter,Int32Set<OffByOne>: literal round trip changed the set(wrote {124}, read back [125]) andStringSet<Lenient>: literal round trip changed the set(read back [CUST-CUST-123]).Verified by a run on 3ecfa10, where the defect is unfixed. All five rows report
SKIPPED [known candidate …, #53]fromcandidates.json, and each will fail ascandidate fixedonce the fix lands. The seedbridge-swallows-unparsable-elementshows the class can fail. It was found by this oracle (found-by/oracle).fix: check that a wrapper set element's text form reads back as the s…
CaffeinatedCoder commentedon Oct 18, 2026
Fixed in #165 (stacked on #164), per the decision here: every core wrapper arity checks the round trip on write and on read.
WrapperElementholds the check once —Parse(Format(x)) == x, on the element rather than the text, so an element that normalizes on parse (trims, lowercases, as the documentedAccessRight) still passes while a lenient parse of a decorated text fails because what it yields formats as something else.FormatValuethrowsInvalidOperationExceptionnaming the contract (JsonExceptionfrom the JSON converters),ParseValuethrowsFormatException(soTryParseisfalse), which also catches data written before the check. The EF bridge checks the same contract through the store primitive:requireRoundTripon every core family (and on the Instant and LocalTime NodaTime families for consistency), the string bridge on its own, andFromPrimitivethrough the family'sParseValue. One case the bridge and the literal answer differently, documented in the migration guide: aDateTimeelement withKind.Localat a wall clock the zone skips — its"O"text carries the zone's offset, which the element's BCLParseapplies, so the literal refuses it while the store (which drops the offset) keeps the wall clock as 9.0.0 fixed. Two seeds (string and integer arity) are proven caught. Proven by reverting the fix.