Use System.Text.Json source generation to remove some trim/AOT warnings by charlesroddie · Pull Request #4683 · dotnet/SqlClient

charlesroddie · 2026-09-13T09:10:19Z

Description

Replaces reflection-based JsonSerializer calls with an internal JsonSerializerContext (SqlClientJsonSerializerContext), removing the IL2026/IL3050 warnings they raise under trimming and Native AOT:

SqlVector<T>.GetString() – serializes ReadOnlyMemory<float> (the constructor already restricts T to float).
SqlParameter string-to-vector coercion – deserializes float[].
HostGuardianServiceEnclaveProvider.MakeRequest – deserializes List<byte>.

Trim/AOT warnings with -p:IsAotCompatible=true (unique by location and code):

TFM	Before	After
net8.0	50	44
net9.0	52	46

No public API or behavioral changes; JSON output is unchanged.

Issues

Part of #1947.

Testing

Built net462, net8.0 and net9.0 with -p:IsAotCompatible=true; none of the touched files emit trim/AOT warnings.
Existing SqlVector/SqlParameter unit tests (including exact GetString() output) pass on net462, net8.0 and net10.0.
The string-to-vector and HGS attestation paths are covered only by tests requiring SQL Server / HGS and were not run locally.

🤖 Generated with Claude Code

Replaces reflection-based JsonSerializer calls in SqlVector, SqlParameter and HostGuardianServiceEnclaveProvider with a JsonSerializerContext, removing IL2026/IL3050 warnings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

azure-pipelines · 2026-09-13T09:10:28Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR removes trim/NativeAOT warnings by switching JSON (de)serialization from reflection-based JsonSerializer APIs to System.Text.Json source-generated metadata via a new internal SqlClientJsonSerializerContext.

Changes:

Added SqlClientJsonSerializerContext with [JsonSerializable] entries for the JSON types used.
Updated SqlVector<T>.GetString() to serialize using source-generated JsonTypeInfo.
Updated SqlParameter vector coercion and VirtualSecureModeEnclaveProvider.MakeRequest to deserialize using source-generated JsonTypeInfo.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs	Uses source-generated type info for vector JSON serialization
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/VirtualSecureModeEnclaveProvider.cs	Uses source-generated type info for HGS response deserialization
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs	Uses source-generated type info for string-to-vector JSON coercion
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientJsonSerializerContext.cs	Introduces the source-generation context and registered serializable types

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

azure-pipelines · 2026-09-13T15:53:44Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

cheenamalhotra · 2026-09-14T20:10:32Z

/azp run

azure-pipelines · 2026-09-14T20:11:04Z

Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions.

codecov · 2026-09-14T21:39:08Z

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 64.60%. Comparing base (f2310e9) to head (7a2392c).

Files with missing lines	Patch %	Lines
...Data/SqlClient/VirtualSecureModeEnclaveProvider.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4683      +/-   ##
==========================================
- Coverage   66.27%   64.60%   -1.67%     
==========================================
  Files         290      284       -6     
  Lines       45062    68015   +22953     
==========================================
+ Hits        29865    43943   +14078     
- Misses      15197    24072    +8875

Flag	Coverage Δ
CI-SqlClient	`?`
PR-SqlClient-Project	`64.60% <66.66%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Copilot review overview

🟢 Approval recommended

The focused substitutions preserve existing serializer defaults and are covered by compilation and existing output tests.

Review effort: Balanced
Findings: None

Resolved since last review (2)

This cast goes through object, which will box the ReadOnlyMemory<T> value type and allocate on…
The call sites depend on auto-generated property names like SingleArray, ReadOnlyMemorySingle,…

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot

Copilot review overview

🟢 Approval recommended

The functional changes are localized and behavior-preserving; the remaining feedback is a minor source-encoding cleanup.

Review effort: Balanced
Findings: 1

Open (1)

Replace literal arrow characters with ASCII prose or Unicode escapes · New

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

paulmedynski · 2026-09-28T17:50:13Z

/azp run

Copilot

Copilot review overview

🟢 Approval recommended

The focused internal refactor preserves existing JSON contracts and has appropriate build and existing test validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

Replace literal arrow characters with ASCII prose or Unicode escapes

azure-pipelines · 2026-09-28T17:50:45Z

Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions.

Two conflicts, both where main replaced reflection-based JsonSerializer calls with the source-generated SqlClientJsonSerializerContext (#4683) in code this branch had changed. SqlVector.GetString on main assumes T is always float. That is no longer true here, so the Half branch is kept and now serialises its widened float array through the generated SingleArray metadata, with the float32 path taking main's ReadOnlyMemorySingle. The JSON string coercion in SqlParameter keeps CreateForConversion, which lets a wide float16 destination through, and takes main's source-generated deserialization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e17ed782-c576-4cb7-9b4b-7ad286d7a7d0

Copilot AI balanced review requested due to automatic review settings September 13, 2026 09:10

github-project-automation Bot added this to SqlClient Board Sep 13, 2026

github-project-automation Bot moved this to To triage in SqlClient Board Sep 13, 2026

Copilot AI reviewed Sep 13, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientJsonSerializerContext.cs

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlTypes/SqlVector.cs

charlesroddie mentioned this pull request Sep 13, 2026

NativeAOT support: get to zero warnings #1947

Open

charlesroddie marked this pull request as ready for review September 13, 2026 15:53

charlesroddie requested a review from a team as a code owner September 13, 2026 15:53

charlesroddie changed the title ~~Use System.Text.Json source generation to remove trim/AOT warnings~~ Use System.Text.Json source generation to remove some trim/AOT warnings Sep 13, 2026

cheenamalhotra added this to the 8.0.0-preview1 milestone Sep 14, 2026

This was referenced Sep 15, 2026

Remove DiagnosticSource.Write trim warnings in SqlDiagnosticListener #4688

Merged

Add an EnableAppConfig switch to remove trim warnings #4697

Open

mdaigle assigned mdaigle and cheenamalhotra Sep 18, 2026

cheenamalhotra moved this from To triage to In review in SqlClient Board Sep 23, 2026

paulmedynski requested a balanced review from Copilot September 28, 2026 12:52

Copilot started reviewing on behalf of paulmedynski September 28, 2026 12:53 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

paulmedynski requested changes Sep 28, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientJsonSerializerContext.cs

Document generated property names on SqlClientJsonSerializerContext

8fbc971

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings September 28, 2026 17:44

Copilot started reviewing on behalf of charlesroddie September 28, 2026 17:45 View session

Copilot AI reviewed Sep 28, 2026

View reviewed changes

Comment thread src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientJsonSerializerContext.cs Outdated

Use ASCII arrows in SqlClientJsonSerializerContext doc comment

801e771

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings September 28, 2026 17:48

Copilot started reviewing on behalf of charlesroddie September 28, 2026 17:48 View session

paulmedynski approved these changes Sep 28, 2026

View reviewed changes

Copilot AI reviewed Sep 28, 2026

View reviewed changes

cheenamalhotra assigned apoorvdeshmukh and unassigned cheenamalhotra Oct 1, 2026

mdaigle approved these changes Oct 1, 2026

View reviewed changes

mdaigle enabled auto-merge (squash) October 1, 2026 23:28

mdaigle merged commit 356123e into dotnet:main Oct 1, 2026
207 of 210 checks passed

Navigation Menu

Use System.Text.Json source generation to remove some trim/AOT warnings - #4683

Use System.Text.Json source generation to remove some trim/AOT warnings#4683
mdaigle merged 3 commits into
dotnet:mainfrom
charlesroddie:trim-json-source-gen

charlesroddie commented Sep 13, 2026 •

edited

Loading

Uh oh!

azure-pipelines Bot commented Sep 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

azure-pipelines Bot commented Sep 13, 2026

Uh oh!

cheenamalhotra commented Sep 14, 2026

Uh oh!

azure-pipelines Bot commented Sep 14, 2026

Uh oh!

codecov Bot commented Sep 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

paulmedynski commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

azure-pipelines Bot commented Sep 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

charlesroddie commented Sep 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Testing

Uh oh!

azure-pipelines Bot commented Sep 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

azure-pipelines Bot commented Sep 13, 2026

Uh oh!

cheenamalhotra commented Sep 14, 2026

Uh oh!

azure-pipelines Bot commented Sep 14, 2026

Uh oh!

codecov Bot commented Sep 14, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Uh oh!

paulmedynski commented Sep 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

azure-pipelines Bot commented Sep 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

charlesroddie commented Sep 13, 2026 •

edited

Loading