Repository navigation
fix(validate): keep a csharp spec default when the value is null - #169
Merged
Merged
Conversation
typescript's getprop answers NONE for a null value, so validate keeps the
spec default for a null exactly as for an absent key, and still refuses
a null where a type or $EXACT is required. csharp kept defaults only for
an absent key: validate({retries: null}, {retries: 3}) threw "Expected
field retries to be integer, but found no value" where typescript gives
{retries: 3}, and the same for a null under a map or list default.
ValidateNullScalarDefaults pins the typescript answers for absent, null,
nested, list, map and override values, and the required and $EXACT
refusals; on the old condition it fails on every null case.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BoEed9CaF6sTNMcXxQaH6w
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
A $ONE alternative validates its value through a nested call whose data
is NONE when the key is absent or null, and csharp treated that NONE as a
present value: validate({a: null}, {a: [$ONE, 5, $STRING]}) refused it
where typescript gives {a: null}. The default step now treats NONE as it
treats an absent key or a null.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BoEed9CaF6sTNMcXxQaH6w
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.
The cause
typescript's
validatekeeps the spec default for a null value exactly as for an absent key, becausegetpropanswersNONEfor a null, and it still refuses a null where a type or$EXACTis required. csharp kept defaults only for an absent key (if (!exact && !cKeyExists)), with a comment saying typescript skips only an absent key, which is not whatgetpropdoes. Sovalidate({retries: null}, {retries: 3})threwExpected field retries to be integer, but found no valuewhere typescript gives{retries: 3}, and the same for a null under a map or list default. A$ONEalternative hit the same step with theNONEsentinel its nested call passes down for an absent or null value:validate({a: null}, {a: ['$ONE', 5, '$STRING']})was refused where typescript gives{a: null}.voxgig/sdkgen has carried a narrower version of this as a marked local patch in its vendored copy since 2026-09-23 (
PATCH (scalar defaults, pending upstream fix)), covering scalar defaults only. Landing it here lets the next sdk checkpoint carry the fix and sdkgen's resync drop the patch.Change
csharp/Struct.cs: a null value, and aNONE, keep the default as an absent key does (!cKeyExists || cval == null || ReferenceEquals(cval, NONE)). For a node default this matches typescript as well: a null map keeps its default map, and its required children are still checked. Exact mode is untouched.Tests
ValidateNullScalarDefaultsincsharp/tests/StructTest.cspins typescript's answers, each read offtypescript/dist:$ONEwith a plain default alternative: a null value ({a: null}), an absent key ({}), a null map holding one ({m: {}}) and a null slot in a list of them ({a: []});$STRING/$NUMBER/$BOOLEANabsent or null, a null map whose default has a required child, a null list slot under a required type, and$EXACTabsent or null.Controls
Struct.csthe test fails on the five null cases:null,nested,list,null-mapandnull-list.NONEone, it fails on the four$ONEcases.dotnet testpasses 91 of 91, the corpus suites included, andmake -C csharp lintis clean.Since review
A second Claude reviewer drove 154 shapes through typescript, csharp main and this branch: the change moves 58 of them, 57 to typescript's exact output (error messages included) and none away from it. It found the
NONEgap above, which predates this PR. AddingReferenceEquals(cval, NONE)fixed 9 more shapes with no other change, and f7888cc carries it, with the four$ONEcases. It also noted:.csfile, andmake -C csharp lintbuilds and formats the library project alone, so neither looks at the new test;structnull.test.ts: csharp'svalidateanswer for a null under a map default becomesdefault, as ts, js, go, py, perl and rb already answer.What it found outside this change is #170: php refuses a null under a default as csharp did, three more null cases where csharp differs from typescript, and the corpus's null rewrite that hides the class from every port. Codex reviewed bb4a4de and raised nothing.
🤖 Generated with Claude Code
https://claude.ai/code/session_01BoEed9CaF6sTNMcXxQaH6w