Repository navigation
Make RequestIdSTJsonConverter public for source generation - #1557
Merged
Andrew Arnott (AArnott) merged 4 commits intoOct 2, 2026
Merged
Andrew Arnott (AArnott) merged 4 commits into
Andrew Arnott (AArnott) merged 4 commits into
Conversation
Expose the converter for consumer JsonSerializerContext implementations and validate typed JsonTypeInfo round trips, invalid tokens, and null writer handling. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The additive API change is documented, focused, and adequately covered by consumer-style source-generation tests.
Review effort: Balanced
Findings: None
What changed in this PR
Makes RequestIdSTJsonConverter publicly accessible for consumer source-generated serialization contexts.
Changes:
- Exposes and documents the converter while retaining its internal singleton.
- Adds explicit null-writer validation.
- Adds focused source-generation serialization tests.
| File | Description |
|---|---|
src/StreamJsonRpc/RequestIdSTJsonConverter.cs |
Exposes and documents the converter and validates the writer. |
test/StreamJsonRpc.Tests/RequestIdTests.cs |
Tests source-generated request ID serialization and validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Andrew Arnott (AArnott)
approved these changes
Oct 1, 2026
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Andrew Arnott (AArnott)
force-pushed
the
eerhardt-public-requestid-converter
branch
from
October 1, 2026 20:27
38f1d91 to
099fd55
Compare
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Andrew Arnott (AArnott)
enabled auto-merge (squash)
October 1, 2026 21:00
Eric Erhardt (eerhardt)
left a comment
Eric Erhardt (eerhardt)
left a comment
Member
Author
There was a problem hiding this comment.
I can't approve. :( But I can push a change and then Andrew Arnott (@AArnott) can approve.
auto-merge was automatically disabled
October 1, 2026 23:02
Head branch was pushed to by a user without write access
Andrew Arnott (AArnott)
approved these changes
Oct 2, 2026
Andrew Arnott (AArnott)
enabled auto-merge (squash)
October 2, 2026 04:42
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Eric Erhardt (eerhardt)
deleted the
eerhardt-public-requestid-converter
branch
October 2, 2026 18:47
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.
Summary
Make
RequestIdSTJsonConverterpublic so consumers can includeRequestIdin their source-generatedJsonSerializerContextimplementations without SYSLIB1220. Seal the converter, keep its singleton internal, document the source-generation scenario, and reject a null writer explicitly.Add focused tests using the generated, typed
JsonTypeInfo<RequestId>for numeric boundaries, string escaping, null and unspecified IDs, and invalid JSON token types, plus a null-writer guard test. Existing request ID tests are preserved.Addresses the converter-accessibility portion of #1407. Automatic formatter resolver registration is NOT in this PR and remains separate follow-up work. This change does not claim general AOT compatibility.
Validation
dotnet build test\StreamJsonRpc.Tests\StreamJsonRpc.Tests.csproj -c Release --no-restore --nologo --verbosity quiet— succeeded for all test target frameworks with zero warnings and errors.dotnet test --project test\StreamJsonRpc.Tests\StreamJsonRpc.Tests.csproj --no-build -c Release -- --filter-class RequestIdTests --filter-not-trait 'TestCategory=FailsInCloudTest'— 63 passed across net8.0, net9.0, and net472; no failures or skips.dotnet pack src\StreamJsonRpc\StreamJsonRpc.csproj -c Release --no-restore --nologo --verbosity quiet— succeeded, including package API compatibility checks, with zero warnings and errors.