v2: make strong types work in configuration; remove the WPF package#123
Merged
Conversation
Without a TypeConverter, ConfigurationBinder cannot turn "5" into a strong
type. It does not fail: it treats the wrapper as a nested object graph and
leaves the property at its default, so `Positive<int> MaxRetries` silently
binds to 1 whatever appsettings.json says — and the offset encoding that
keeps `default` satisfying the invariant is what makes the wrong value look
plausible. Strong types were unusable in an options class.
Composite formatting has the same shape of failure: `$"{price:C}"` on a type
that is not IFormattable compiles, never throws, and drops the specifier and
the culture on the floor.
The numeric wrappers get IFormattable/ISpanFormattable/ISpanParsable by pure
delegation — INumber<T> already guarantees all of them on T — so the four
wrappers are covered by one generator change. The span interfaces buy little
on allocation (the implicit operator already routes to the BCL's fast paths,
and Create(int.Parse(slice)) was already zero-alloc); they earn their place
by letting callers pass wrappers to generic code constrained on them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CoverageLines: 4818 / 6364 (75.7%) Branches: 2514 / 3288 (76.5%) Files changed in this PR
StrongTypes — lines 92.6% (1942/2098), branches 87.8% (1245/1418)Booleans — lines 100.0% (14/14), branches 84.6% (22/26)
Collections — lines 90.2% (276/306), branches 85.1% (114/134)
ComponentModel — lines 89.5% (17/19), branches 68.8% (11/16)
Digits — lines 97.7% (85/87), branches 93.8% (30/32)
Emails — lines 97.4% (74/76), branches 95.7% (44/46)
Enums — lines 100.0% (58/58), branches 95.5% (21/22)
Exceptions — lines 78.9% (15/19), branches 50.0% (7/14)
Intervals — lines 96.5% (382/396), branches 95.5% (231/242)
Maybe — lines 92.1% (151/164), branches 83.9% (94/112)
Nullables — lines 100.0% (12/12), branches 83.3% (20/24)
Numbers — lines 100.0% (84/84), branches 94.4% (17/18)
Result — lines 98.5% (333/338), branches 95.6% (417/436)
Strings — lines 84.7% (133/157), branches 84.4% (54/64)
generated — lines 83.7% (308/368), branches 70.3% (163/232)
StrongTypes.Analyzers — lines 92.6% (522/564), branches 83.8% (238/284)(root) — lines 92.6% (522/564), branches 83.8% (238/284)
StrongTypes.Api — lines 97.3% (391/402), branches 86.3% (88/102)(root) — lines 100.0% (11/11), branches n/a (0/0)
Controllers — lines 97.0% (226/233), branches 86.3% (88/102)
Data — lines 100.0% (102/102), branches n/a (0/0)
Entities — lines 85.2% (23/27), branches n/a (0/0)
Models — lines 100.0% (29/29), branches n/a (0/0)
StrongTypes.AspNetCore — lines 89.3% (151/169), branches 84.5% (71/84)(root) — lines 89.3% (151/169), branches 84.5% (71/84)
StrongTypes.AspNetCore.TestApi — lines 90.7% (39/43), branches 92.5% (37/40)(root) — lines 100.0% (6/6), branches n/a (0/0)
Controllers — lines 89.2% (33/37), branches 92.5% (37/40)
StrongTypes.Configuration — lines 97.5% (79/81), branches 83.3% (40/48)(root) — lines 97.5% (79/81), branches 83.3% (40/48)
StrongTypes.Configuration.Tests.NullableDisabled — lines 100.0% (3/3), branches n/a (0/0)(root) — lines 100.0% (3/3), branches n/a (0/0)
StrongTypes.EfCore — lines 95.4% (227/238), branches 81.2% (78/96)(root) — lines 95.4% (227/238), branches 81.2% (78/96)
StrongTypes.FsCheck — lines 81.9% (127/155), branches 100.0% (2/2)(root) — lines 81.9% (127/155), branches 100.0% (2/2)
StrongTypes.OpenApi.Core — lines 91.4% (447/489), branches 83.4% (297/356)(root) — lines 91.9% (262/285), branches 86.1% (167/194)
Inlining — lines 90.7% (185/204), branches 80.2% (130/162)
StrongTypes.OpenApi.Microsoft — lines 36.4% (450/1236), branches 41.2% (177/430)(root) — lines 94.6% (246/260), branches 74.6% (97/130)
Binding — lines 98.1% (103/105), branches 78.6% (55/70)
Collections — lines 100.0% (19/19), branches 100.0% (8/8)
Digits — lines 100.0% (11/11), branches 100.0% (2/2)
Emails — lines 100.0% (11/11), branches 100.0% (2/2)
Inlining — lines 100.0% (7/7), branches 50.0% (1/2)
Intervals — lines 100.0% (24/24), branches 100.0% (6/6)
Maybe — lines 100.0% (12/12), branches 100.0% (2/2)
Numbers — lines 100.0% (8/8), branches 100.0% (2/2)
Strings — lines 100.0% (9/9), branches 100.0% (2/2)
obj/Debug/net10.0/Microsoft.AspNetCore.OpenApi.SourceGenerators/Microsoft.AspNetCore.OpenApi.SourceGenerators.XmlCommentGenerator — lines 0.0% (0/770), branches 0.0% (0/204)
StrongTypes.OpenApi.Swashbuckle — lines 94.7% (266/281), branches 79.1% (174/220)(root) — lines 98.7% (77/78), branches 93.4% (71/76)
Binding — lines 88.3% (106/120), branches 64.3% (72/112)
Collections — lines 100.0% (6/6), branches 100.0% (4/4)
Digits — lines 100.0% (10/10), branches 100.0% (4/4)
Emails — lines 100.0% (10/10), branches 100.0% (4/4)
Inlining — lines 100.0% (2/2), branches n/a (0/0)
Intervals — lines 100.0% (26/26), branches 100.0% (8/8)
Maybe — lines 100.0% (13/13), branches 75.0% (3/4)
Numbers — lines 100.0% (8/8), branches 100.0% (4/4)
Strings — lines 100.0% (8/8), branches 100.0% (4/4)
StrongTypes.OpenApi.TestApi.Microsoft — lines 41.0% (163/398), branches 31.6% (65/206)(root) — lines 100.0% (14/14), branches 100.0% (2/2)
obj/Debug/net10.0/Microsoft.AspNetCore.OpenApi.SourceGenerators/Microsoft.AspNetCore.OpenApi.SourceGenerators.XmlCommentGenerator — lines 38.8% (149/384), branches 30.9% (63/204)
StrongTypes.OpenApi.TestApi.Shared — lines 0.0% (0/196), branches n/a (0/0)(root) — lines 0.0% (0/196), branches n/a (0/0)
StrongTypes.OpenApi.TestApi.Swashbuckle — lines 100.0% (11/11), branches 100.0% (2/2)(root) — lines 100.0% (11/11), branches 100.0% (2/2)
|
The package existed to compensate for the core types not carrying [TypeConverter]. Its readme states the rationale: "the core package deliberately carries no UI dependency, so the wiring lives here." That premise is false — TypeConverter lives in System.ComponentModel, ships in the base framework, and its largest consumer is ConfigurationBinder. Core still has zero PackageReferences after taking it on. So the hole was never WPF-specific, and patching it per-host meant only WPF got patched: the same gap silently bound Positive<int> options to 1. One misconception, two symptoms. The whole public surface was ParsableTypeConverter<T> and an Application.UseStrongTypes() extension; the provider and descriptor were internal. All of it is superseded by the attribute, which is also strictly better: no global TypeDescriptor mutation, impossible to forget, and it cannot go stale as new strong types are added (the provider's type list was hardcoded, so BoundedInt<TBounds> would have silently missed it). It was also wrong. Its ConvertTo dropped the culture it was handed and formatted with the ambient one, so a de-DE binding rendered 1234.5 as "1234.5" and read it back as 12345. CultureBindingTests pins that. The binding tests stay, retargeted onto core with no registration call — they are now the only proof the attribute satisfies WPF's binding engine. Widened to 25: Digit, nullable struct and nullable reference wrappers, and culture round-trips. NullableStructBindingTests documents a pre-existing surprise found while writing it: clearing a TextBox bound to Positive<int>? raises a validation error rather than nulling, because WPF unwraps Nullable<T> and asks for the underlying converter, never consulting NullableConverter. Unchanged by this commit — the removed package didn't register against Nullable<T> either. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
These asserted a de-DE binding renders 1234.5 as "1234,5", but inherited the machine's culture. On a decimal-comma host — cs-CZ, de-DE — a converter that formatted with the ambient culture rather than the binding's passes anyway, so the tests only caught the bug on an en-US agent. Verified: run against the removed package they go green here and corrupt 1234.5 to 12345 on en-US. Pinning en-US makes the separators disagree, so the assertions can only hold if the binding's culture is the one used. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The suite only exercised IOptions and assumed ConfigurationBinder.Get<T>
matched. Every scenario now runs through both as a theory parameter, so the
equivalence is asserted rather than assumed. They do agree on every row.
Also pins what null and "" actually do, which is neither uniform nor
guessable:
- "" throws for every wrapper except a nullable struct one, where the BCL's
NullableConverter maps it to null before our converter is consulted. A
nullable reference wrapper gets no such treatment — NonEmptyString? is the
same runtime type as NonEmptyString — so "" reaches Parse and fails. Note
WPF differs here: it unwraps Nullable<T> itself, so the same "" is a
validation error there rather than null.
- An explicit null nulls even a non-nullable NonEmptyString: nullability is
erased before the binder runs, so it is no different from a string. The
invariant constrains every value the type can hold, not whether one is
assigned at all.
- An explicit null overwrites a property initialised in the options class;
an absent key leaves it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The binding tests used an options class with property initialisers, which hid the ordinary case: a real options class has none, so an unconfigured NonEmptyString is null. A wrapper's invariant constrains every value it can hold and cannot make the binder assign one — against a missing key it is no better than a string. ValidateOnStart() does not help: it forces eager binding, and binding of an absent key succeeds without assigning, so nothing is raised. The docs recommended it as though it were the guard for this. [Required] plus ValidateDataAnnotations() is what catches a missing value. And [Required] cannot catch a missing non-nullable struct wrapper at all: default(Positive<int>) is 1, a real invariant-satisfying value, so it never looks absent. Verified — the failures name Name and MaxRetriesOrUnset but not MaxRetries. A struct wrapper must be declared nullable for absence to be detectable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A wrapper's invariant constrains every value it can hold; it cannot make the binder assign one, because the binder reaches in through reflection and never calls Create. So an unconfigured NonEmptyString is null and an unconfigured Positive<int> is 1. Neither existing guard closes that. ValidateOnStart() only surfaces failures binding raises, and binding an absent key succeeds without assigning. [Required] finds the null NonEmptyString but cannot find MaxRetries at all — default(Positive<int>) is 1, an ordinary invariant-satisfying value. C#'s required keyword is compile-time only and the binder walks past it. BindStrongTypes asks the configuration whether the key is present rather than asking the bound object whether it looks null, so the struct case has an answer. Required-ness comes from the declaration already written — Positive<int> vs Positive<int>? — read via NullabilityInfoContext, so no attribute restates what the ? already said. Its own package: this needs Microsoft.Extensions.Options.ConfigurationExtensions and core has zero PackageReferences, which is worth keeping — a domain library referencing Kalicz.StrongTypes should not acquire the configuration stack. Only our wrappers are policed; plain string/int properties are left to whatever validation the caller already has. An options class compiled without nullable reference types declares no intent for a reference wrapper, so it is treated as optional rather than guessed at — pinned by a deliberately Nullable-disabled test project, since that degradation is a real wart and should not be folklore. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fires where a plain Bind/Configure binds an options type carrying a strong type that must be configured, and only where nothing else would notice: a struct wrapper, whose default is never null and so always satisfies [Required], or a reference wrapper with no [Required] at all. A reference wrapper already carrying [Required] is genuinely covered, so nagging there would be wrong. Like the EF Core and OpenAPI analyzers, this takes no dependency on what it detects — Options types resolve by metadata name from the consumer's compilation, and the analyzer no-ops when absent. Only the test project references the stack, to build a compilation containing it. Required-ness for a reference wrapper reads NullableAnnotation, mirroring the NullabilityInfoContext check BindStrongTypes makes at runtime; the test sources opt into #nullable enable because the harness does not enable it compilation-wide. The fix rewrites Bind to BindStrongTypes and is registered only when Kalicz.StrongTypes.Configuration is referenced — without it the rewrite would not compile. Configure<T> gets the diagnostic but no fix: that rewrite means restructuring to AddOptions<T>().BindStrongTypes(...), which is the caller's call. Source-editing fixes needed a new DocumentCodeFixTester; the existing CodeFixTester drives the csproj-writing package fixes, which are not observable as a document change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wrappers
Calling BindStrongTypes is the intent: this options class should be fully
configured. A missing string is exactly as silent as a missing NonEmptyString,
so policing only our wrappers made the guarantee arbitrary.
A property is required when it is not nullable and the options class gives it
no default of its own — read by probing a fresh instance, which separates a
real fallback from the `= null!` that only appeases nullable reference types.
Without that, `public string Endpoint { get; set; } = "https://x.test"` would
fail despite plainly stating its default, and the feature would be unusable.
It also fixes the same latent bug for wrappers.
Presence now asks Exists() rather than Value != null. A collection or nested
object is a section with children and no value of its own, so the old check
called a configured `Items: ["a","b"]` unconfigured — invisible while only
scalar wrappers were policed, a false failure the moment anything else is.
Two declarations cannot be read, and are documented rather than guessed:
a value type whose intended default is the CLR default (`bool Enabled = false`)
is indistinguishable from having no initialiser, so it is required — declare it
nullable; and a reference property in an assembly without nullable reference
types carries no annotation, so it stays optional.
Computing the required set per type in a static field also drops the lock
NullabilityInfoContext needed, since it now runs once on one thread.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collection and nested-object test proved nothing: both properties were
initialised, so they were optional and the presence check never ran on them.
Reverting the check to `Value is not null` left all 17 green. They are now
declared `= null!`, which makes them required — and that same revert now fails
two tests.
Pinning the real behaviour along the way, none of which was guessable:
- "Items": [] is configured. The provider records an empty array as an empty
value, so the key is present. The binder still leaves the property null
rather than building an empty list: presence and non-nullness are different
questions and this answers the first.
- "Nested": { } is not configured — neither value nor children.
- "Name": null is not configured, so a required property rejects it. That
closes the hole plain binding leaves, where an explicit null quietly nulls
a non-nullable NonEmptyString.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The requirement was wrong, and so was the reasoning behind it. Requiring every non-nullable property assumed an unconfigured value type is a problem. It is not: an unconfigured Positive<int> is 1 and an unconfigured bool is false — defaults, and values those types are perfectly happy to hold. Only null in something whose declaration says it can never be null is a contradiction, and only a reference can reach it. So "the case [Required] cannot catch" was never a case. Nothing needed catching there, and requiring configuration for a struct was a policy invented to justify the mechanism rather than a fix for anything. Asking the bound object for nulls instead of asking configuration for keys drops all of it: the Activator probe for declared defaults (a default is simply not null), the Exists() handling for sections that have children rather than a value (a bound collection is simply not null), and the empty-array puzzle it created. The validator is now half the size and answers the question that matters. ST0004 narrows to match: a struct wrapper has nothing to guard, so reporting it was noise. It now fires only for a non-nullable reference wrapper with no [Required]. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The skill reference still printed the old failure ("'Retry:MaxRetries' is
required but was not configured") against a value type that is no longer
checked, and named RetryOptions.Name in a failure the file's own class
declares nullable. Both are gone.
Pin the exact message in a test so the documented text is copied from a
passing assertion rather than read off the format string.
Add Kalicz.StrongTypes.Configuration to the package-layout diagrams, which
claimed to show every package and shipped without it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both readmes said the absent key leaves null "in the one type whose whole purpose is never being null". Under nullable reference types a non-nullable string promises exactly the same thing and the binder breaks it in exactly the same way, so the phrasing invites the wrong conclusion — that this is needed for wrappers and not for strings. The wrapper did not make the bug worse, only easier to notice: fifteen years of habit cushion a null string and nothing cushions a null NonEmptyString. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The check stopped at the first level, so a nested options class had none of
its own declaration enforced — proved by binding {"Nested": {"Unrelated": "x"}}
against a non-nullable NonEmptyString inside and watching it pass. Nesting a
section is routine, so the package under-delivered exactly where configuration
gets complicated enough to want it. That is a defect against its own stated
promise, which has no depth qualifier in it.
The walk stops where ConfigurationBinder stops: a type is a leaf when it has a
TypeConverter from string, which is the same question the binder asks to decide
between converting a scalar and recursing into an object graph. So the walk
covers exactly the graph the binder built, and the wrappers fall out as leaves
because of the converters added earlier in this branch.
Collections and dictionary values are walked too, reporting the index or key in
the path. Instances are visited once: a shared child would otherwise duplicate
every failure beneath it, and a constructor-made cycle would not terminate.
Reverting the recursion fails 5 of the 7 new tests and none of the old 14 —
which is also the evidence the old suite never touched depth.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The runtime walk now reaches every level, but the analyzer that tells you to use it still stopped at the first — so an options class whose wrappers all live in a nested section got no nudge toward the one thing that would catch them. It mirrors the runtime rule: recurse where ConfigurationBinder recurses, which in Roslyn is "no [TypeConverter], not a special type, not framework-owned". AnalyzerTester now fails a source that does not compile. It had been handing broken trees to the analyzer, which reads them without complaint and simply finds less — so a Silent_ test could pass precisely because its source was broken. The new false-positive guard was doing exactly that: TypeConverter the class is forwarded to an assembly the test compilation never referenced, while TypeConverterAttribute resolved from another, so the source failed to build and the test still went green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KaliCZ
commented
Jul 16, 2026
Comment on lines
+9
to
+41
| /// <summary>Binds <paramref name="section"/> to <typeparamref name="TOptions"/> and fails when a property declared non-nullable is null once bound.</summary> | ||
| /// <param name="builder">The options builder to bind.</param> | ||
| /// <param name="section">The configuration section to bind from.</param> | ||
| /// <returns><paramref name="builder"/>, for chaining — pair with <c>ValidateOnStart()</c> to fail the host rather than the first request that reads the options.</returns> | ||
| /// <remarks> | ||
| /// <para> | ||
| /// The binder assigns nothing for an absent key, so a property it never reaches keeps whatever | ||
| /// the options class gave it — and a <c>NonEmptyString</c> that was given nothing is <c>null</c>, | ||
| /// which is precisely what the type says it can never be. Nullable reference annotations already | ||
| /// state which properties that applies to, so no attribute has to repeat it: | ||
| /// </para> | ||
| /// <code> | ||
| /// public NonEmptyString Name { get; set; } = null!; // fails unless configured | ||
| /// public NonEmptyString? Nickname { get; set; } // nullable — fine | ||
| /// public string Endpoint { get; set; } = "https://x.test"; // has a default — never null | ||
| /// </code> | ||
| /// <para> | ||
| /// Every reference property is covered, not only Kalicz.StrongTypes wrappers: a <c>string</c> | ||
| /// declared non-nullable is as broken by a missing key as a <c>NonEmptyString</c> is. | ||
| /// </para> | ||
| /// <para> | ||
| /// A value type is not checked, because it has no invalid state to reach — an unconfigured | ||
| /// <c>Positive<int></c> is <c>1</c> and an unconfigured <c>bool</c> is <c>false</c>, both | ||
| /// values those types are happy to hold. If "not configured" has to be distinguishable from a | ||
| /// configured default, declare it nullable (<c>Positive<int>?</c>) and check for null | ||
| /// yourself. | ||
| /// </para> | ||
| /// <para> | ||
| /// This is about the key that is absent. A key that is present but invalid still fails while | ||
| /// binding, with the invariant's own message. | ||
| /// </para> | ||
| /// </remarks> | ||
| /// <exception cref="ArgumentNullException"><paramref name="builder"/> or <paramref name="section"/> is <c>null</c>.</exception> |
Owner
Author
There was a problem hiding this comment.
This is just horrible man. Can we make it shorter? Apply the comment rule for the entire repo
BindStrongTypes' XML doc was a 30-line essay in IntelliSense — the exact failure the repo's comment rule names: <remarks> explaining the mechanism and rationale, none of it something the caller can act on. Apply the rule across everything this PR added: XML that restates the member name or narrates internals is gone; // paragraphs are cut to the one-sentence why or deleted. Kept: the load-bearing one-liners — the culture round-trip in ConvertTo, the visit-once invariant in the walker, IsLeaf as the pivot that makes the walk match the binder — and one-line test summaries stating intent a reader can't get from the method name. Comments only; no code changed, all tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KaliCZ
commented
Jul 16, 2026
| /// ambient culture instead of the binding's would still pass every assertion below on a | ||
| /// de-DE — or cs-CZ — host, because the two separators happen to agree. | ||
| /// </summary> | ||
| private static readonly CultureInfo Host = CultureInfo.GetCultureInfo("en-US"); |
Owner
Author
There was a problem hiding this comment.
We only seem to be testing one host culture. We need to test multiple host cultures and multiple content cultures. Then we can call it well-tested
KaliCZ
commented
Jul 16, 2026
The fix lives in the core analyzer assembly, so nothing about it needs the Configuration package to exist — it was only gated behind a reference check. That hid it in exactly the case where a user most needs the nudge: they see ST0004 but no fix, with no hint that a package is the missing piece. Drop the gate. The rewrite won't compile until the package is added, and the fix title now names it, so the IDE's own missing-package flow takes it from there. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
One host culture proved little: WPF resolves a binding's culture as ConverterCulture ?? element.Language and ignores the thread culture entirely, so the single en-US host couldn't show the host was being ignored rather than merely matching. Run every case across en-US/de-DE/cs-CZ/ja-JP hosts crossed with the same set of binding cultures, and assert format, write-back, and round-trip all follow the binding culture. The no-ConverterCulture case now sets the element Language explicitly and checks the binding follows that, not the host. StaThread now shuts down each thread's Dispatcher: constructing a WPF element spins one up, and at this many cases the leftover foreground threads made the test host force-exit non-zero after every test had passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the ValidateDataAnnotations line: the options class has no annotations, so pointing at [Required] there was talking about something absent. Replace the missing-key essay with the two-block contrast it was describing — a plain Bind that silently leaves non-nullables null, then BindStrongTypes that fails on them, over an options class with a non-nullable string beside the wrapper so "even a plain string" is visible. Remove the "Changed in v2" note from the WPF section: the docs describe v2, and the change belongs in the changelog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The reviewer's question is a reader's question too: this package exists to keep core dependency-free, so a domain model or library using NonEmptyString doesn't inherit the options/configuration stack transitively. State it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KaliCZ
commented
Jul 16, 2026
Replace the computed host x binding matrix with concrete InlineData rows — (binding culture, text, expected) — so the data reads as a table and can carry negative cases: expected null means the input is invalid in that culture (not a number, or a value that breaks the Positive invariant), which must raise a validation error and leave the source unchanged. Positive and negative rows assert opposite sides of GetHasError on the same binding, so neither passes vacuously. Host independence still holds: each case runs under every host culture in turn, since WPF resolves ConverterCulture ?? element.Language and never the host. InlineData columns sidestep the object[]-vs-tuple question entirely. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The null/"" note sat right after the BindStrongTypes pitch but described plain ConfigurationBinder — so it read as though the check still let `"ApiName": null` null out a non-nullable property. It does not: an explicit null is caught exactly like a missing key (ExplicitNull_Fails proves it). Reword the readme to say so, and the only genuinely different input, "", throws during binding before validation runs. Cross-reference the skill reference's plain-Bind matrix bullet to BindStrongTypes so it can't mislead the same way. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The earlier negatives (not-a-number, invariant breach) fail in any culture, so they proved nothing about culture. The real culture case is a separator from the wrong culture, and it does not fail: "9876,5" under en-US binds to 98765, the comma swallowed as digit grouping — verified against the actual converter. That is silent corruption, so pin it: the culture must match the input or you get a wrong-but-valid value. NumberStyles.Number is deliberate (it matches decimal's own parse); a consumer wanting strict parsing controls it at the call site. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lead with the bad-value failure, and condense the plain Bind and BindStrongTypes examples into a single annotated code block, dropping the value-type, explicit-null, and matrix prose. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
KaliCZ
enabled auto-merge (squash)
July 16, 2026 17:39
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
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.
Breaking — targets v2. Closes the gap against what StronglyTypedId implements, makes strong types usable in
IOptions, and deletesKalicz.StrongTypes.Wpf, which existed only to work around the same root cause.One misconception produced every bug here.
StrongTypes.Wpf's readme states it: "the core package deliberately carries no UI dependency, so the wiring lives here."TypeConverteris not a UI dependency — it isSystem.ComponentModel, in the base framework, and its biggest consumer isConfigurationBinder. Proof: core still has zeroPackageReferences after taking it on. It cost oneusing. Core withheld the attribute believing it was UI-only; WPF patched it for WPF; nobody patched configuration.Bug 1: no
TypeConverter→ config silently ignoredConfigurationBinderconverts viaTypeDescriptor. With no converter it does not fail — it treats the wrapper as a nested object graph and leaves the property at its default. Measured againstmain:Never throws. The offset encoding that keeps
default(Positive<int>)satisfying the invariant is exactly what makes the wrong value look plausible. Now the invariant is the config rule:Bug 2: no
IFormattable→ formats and cultures dropped$"{price:C}"compiles, never throws, and discards the specifier.$"{price:N2}"1234.51,234.50$"{price:C}"1234.5$1,234.50string.Format(de, "{0}", price)1234.51234,5Numeric wrappers only —
stringis notIFormattable, soNonEmptyString/Emailcorrectly are not.Bug 3: the WPF converter corrupted decimals
Its
ConvertTodropped the culture it was handed and formatted with the ambient one. Verified in a real binding onmain:Nobody noticed because it only bites when the host and binding cultures disagree on the decimal separator — on a cs-CZ or de-DE box it is invisible. The WPF culture tests are now pinned to an en-US host so they cannot pass by coincidence.
Removing
Kalicz.StrongTypes.Wpf(breaking)Its entire public surface was
ParsableTypeConverter<T>andApplication.UseStrongTypes(); the provider and descriptor wereinternal. All superseded, and the attribute is strictly better: no globalTypeDescriptormutation, impossible to forget (the opt-in call failed open), and it cannot go stale as types are added — the provider's list was hardcoded, soBoundedInt<TBounds>(#79) would have silently missed it.Migration: drop the package reference and the
this.UseStrongTypes()call. Nothing replaces them. (EF Core / OpenAPIUseStrongTypes()are unrelated.) Verified before deleting: all WPF tests pass withUseStrongTypes()never called.New:
Kalicz.StrongTypes.ConfigurationThe
TypeConverterfixes the wrong value. This fixes the absent one — a different problem with a different answer.A wrapper's invariant constrains every value it can hold; it cannot make the binder assign one, and for an absent key it assigns nothing. So an unconfigured
NonEmptyStringisnull— the one thing that type says it can never be.ValidateOnStart()does not help (binding an absent key succeeds, it just does not assign), and C#'srequiredis a compile-time rule the binder's reflection walks past. All measured.It fails when a property declared non-nullable is null once bound. Intent comes from the annotation already written (
NonEmptyStringvsNonEmptyString?, viaNullabilityInfoContext), so no[Required]restates what the?said. Every reference property is covered, not just the wrappers — a non-nullablestringis as broken by a missing key.Value types are not checked, and that is the point rather than a limitation. An unconfigured
Positive<int>is1and an unconfiguredboolisfalse— defaults, and values those types are happy to hold. There is no contradiction to catch, so requiring configuration for them would be a policy invented to justify the mechanism. Onlynullin something that promised never to be null is a real contradiction, and only a reference can reach it.Its own package because it needs
Options.ConfigurationExtensions, and core's zero dependencies are worth keeping — a domain library should not acquire the configuration stack to useNonEmptyString.New: ST0004 + code fix
Flags a plain
Bind/Configurethat would leave a non-nullable reference wrapper null. Quiet for struct wrappers (nothing to catch) and for a property already carrying[Required](genuinely covered). It reports only this library's own wrappers, never a plainstring, so it sees less than the check it points you at.No dependency, per the house pattern — Options types resolve by metadata name from the consumer's compilation, like
MissingEfCorePackageAnalyzerdoes for EF Core. The fix rewritesBind→BindStrongTypes, registered only when the package is referenced so it cannot emit non-compiling code.What I deliberately left out
The UTF-8 pair. I had floated it as "nearly free" — measuring changed my mind. The JSON converters delegate to
JsonSerializer.Serialize(writer, underlying, options), so the BCL already takes the UTF-8 fast path.The allocation case for the span interfaces is weak, and the reason is our own design:
They are in for generic-constraint compatibility (
where T : ISpanParsable<T>/IFormattable), which has no workaround, and cost two members each on interfaces we already implement.Test coverage
Verified as real guards: removing
[TypeConverter]fromPositivealone fails 15 tests.ConfigurationBindingTests— every scenario through bothConfigurationBinder.Get<T>andIOptions<T>, asserted equivalent rather than assumed. Bound values asserted different from the default, sincedefaultis a plausible1and that is how the bug hid.UnconfiguredOptionsTests— no property initialisers, i.e. what a real options class looks like: unconfigured wrappers, and which guard catches which.BindStrongTypesTests—PlainBind_LeavesANonNullableWrapperNullsits besideMissingNonNullableWrapper_Failson the same config, so the gap is visible not described.ValueTypeProperties_AreNeverRequiredpins the deliberate non-check, andFailure_NamesTheConfigurationPathAndEveryWayOutasserts the message verbatim, so the text quoted above and in the docs is copied from a passing assertion. Plus a deliberately<Nullable>disable</Nullable>project pinning the NRT degradation.TypeConverterTests/NumericFormattingTests— culture round-trip (a real bug in my first draft), generic constraints.1274 unit / 14 configuration / 56 analyzer / 2 OpenAPI-core / 26 WPF green. Container-backed suites not run locally.
Documented, not folklore: clearing a TextBox bound to
Positive<int>?raises a validation error rather than nulling (WPF unwrapsNullable<T>and never consultsNullableConverter— pre-existing, verified identical onmain); and""in config binds a nullable struct wrapper tonullwhile throwing for everything else.🤖 Generated with Claude Code