Skip to content

Enable trim/AOT analyzers on System.Text.Json source generator tests - #130132

Merged
eiriktsarpalis merged 4 commits into
mainfrom
eiriktsarpalis-stj-sourcegen-aot-warnings
Jul 3, 2026
Merged

Enable trim/AOT analyzers on System.Text.Json source generator tests#130132
eiriktsarpalis merged 4 commits into
mainfrom
eiriktsarpalis-stj-sourcegen-aot-warnings

Conversation

@eiriktsarpalis

@eiriktsarpaliseiriktsarpalis commented Jul 2, 2026

Copy link
Copy Markdown
Member

Enables the trim/AOT analyzers on the System.Text.Json source generator test projects so that AOT/trim regressions are caught at PR build time, instead of only surfacing in the Native AOT outerloop runs that don't execute on regular PRs.

This is a follow up to #129965.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR re-enables trim/AOT analyzers for the System.Text.Json source generator test projects and updates shared test code to be analyzer-clean (primarily by avoiding reflection-based serialization paths and annotating reflection usage explicitly).

Changes:

  • Re-enable trim/AOT analyzers for the source-generator Roslyn test projects (remove per-project opt-outs).
  • Update tests to prefer JsonTypeInfo-based APIs (e.g., options.GetTypeInfo<T>(), GetTypeInfo<T>()) and add Requires* / DynamicallyAccessedMembers annotations where reflection remains intentional.
  • Add missing attribute sources for non-NetCoreAppCurrent / .NET Framework builds so new annotations compile.

Reviewed changes

Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/System.Text.Json.Tests.csprojAdd missing CodeAnalysis / CompilerServices attribute source files for .NET Framework build of the test project.
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/DomTests.csUse generic GetTypeInfo<T>() to avoid reflection-based GetTypeInfo(Type) + cast patterns.
src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonValueTests.csSwitch to generic GetTypeInfo<T>() usage for analyzer-friendliness.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/TestClasses.CustomConverters.csRoute converter serialization through options.GetTypeInfo<T>() APIs instead of reflection-based overloads.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Tests.targetsAdd missing attribute source files for non-current TFMs and configure source-gen test compilation.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Roslyn4.4.Tests.csprojRemove EnableTrimAnalyzer/EnableAotAnalyzer=false opt-out to re-enable analyzers.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/System.Text.Json.SourceGeneration.Roslyn3.11.Tests.csprojRemove EnableTrimAnalyzer/EnableAotAnalyzer=false opt-out to re-enable analyzers.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationLogicTests.csAdjust enum-converter usage to be compatible with analyzer expectations in source-gen contexts.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/PropertyVisibilityTests.csAdd DynamicallyAccessedMembers annotations and switch to GetTypeInfo-based APIs in source-gen tests.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/PolymorphicTests.csExpand [JsonSerializable] registrations needed for source-gen test coverage under analyzers.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/Serialization/JsonSerializerWrapper.SourceGen.csEnsure wrapper uses JsonTypeInfo-based serialize/deserialize overloads (trim/AOT-safe).
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/RealWorldContextTests.csUpdate enum converter instantiation to avoid analyzer warnings in source-gen tests.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSourceGenerationOptionsTests.csMark reflection-heavy assertions with RequiresUnreferencedCode.
src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/JsonSerializerContextTests.csAdd RequiresUnreferencedCode/RequiresDynamicCode where reflection-based APIs are intentionally exercised.
src/libraries/System.Text.Json/tests/Common/UnsupportedTypesTests.csReplace conditional compilation with explicit Requires* annotations for reflection-heavy test paths.
src/libraries/System.Text.Json/tests/Common/UnionTests.csRemove populateMissingResolver: true usage and switch to analyzer-friendlier patterns.
src/libraries/System.Text.Json/tests/Common/StructuralJsonTypeClassifierTests.csBase options on serializer-under-test defaults to align with source-gen / reflection-disabled configurations.
src/libraries/System.Text.Json/tests/Common/SerializerTests.csReplace reflection-based structural comparer with a JsonTypeInfo-based comparer helper.
src/libraries/System.Text.Json/tests/Common/RequiredKeywordTests.csGuard reflection-emit dependent test and annotate reflection/dynamic code requirements; reuse wrapper GetTypeInfo.
src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.Serialize.csAdd suppressed, guarded Newtonsoft baseline comparison helper to satisfy analyzers.
src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.IgnoreCycles.csAdd DynamicallyAccessedMembers annotations to reflection-based helper usage.
src/libraries/System.Text.Json/tests/Common/ReferenceHandlerTests/ReferenceHandlerTests.csSwitch comparisons to the guarded Newtonsoft baseline helper; add CodeAnalysis using.
src/libraries/System.Text.Json/tests/Common/PropertyVisibilityTests.NonPublicAccessors.csAdd DynamicallyAccessedMembers and adjust enum converter usage for analyzer compatibility.
src/libraries/System.Text.Json/tests/Common/PropertyVisibilityTests.InitOnly.csAdd DynamicallyAccessedMembers annotations on Type-driven tests.
src/libraries/System.Text.Json/tests/Common/PropertyVisibilityTests.csAdd DynamicallyAccessedMembers annotations on reflection-driven Type parameters.
src/libraries/System.Text.Json/tests/Common/PolymorphicTests.TypeClassifier.csUse Serializer.DefaultOptions to avoid reflection-resolver setup that conflicts with source-gen configurations.
src/libraries/System.Text.Json/tests/Common/PolymorphicTests.CustomTypeHierarchies.csSwitch equality comparers to JsonTypeInfo-based comparer; add guards for reflection-disabled configurations.
src/libraries/System.Text.Json/tests/Common/PolymorphicTests.csAvoid reflection-resolver setup; add DynamicallyAccessedMembers where Activator.CreateInstance is used.
src/libraries/System.Text.Json/tests/Common/NumberHandlingTests.csAnnotate reflection-heavy helpers and gate NativeAOT-incompatible test sections.
src/libraries/System.Text.Json/tests/Common/NullableAnnotationsTests.csAdd DynamicallyAccessedMembers annotations for Activator.CreateInstance(Type) usage.
src/libraries/System.Text.Json/tests/Common/MetadataTests.Options.csAvoid reflection-based CreateJsonTypeInfo path in favor of metadata services for a known value type.
src/libraries/System.Text.Json/tests/Common/MetadataTests.csAdd DynamicallyAccessedMembers / RequiresUnreferencedCode annotations around reflection-heavy validation.
src/libraries/System.Text.Json/tests/Common/JsonTestHelper.csAnnotate reflection-based options comparison helper with RequiresUnreferencedCode.
src/libraries/System.Text.Json/tests/Common/JsonSerializerWrapper.csAdjust MakeReadOnly behavior under BUILDING_SOURCE_GENERATOR_TESTS for analyzer compatibility.
src/libraries/System.Text.Json/tests/Common/JsonSchemaExporterTests.csGate reflection-heavy tests and switch to GetTypeInfo-based serialization in helper paths.
src/libraries/System.Text.Json/tests/Common/JsonCreationHandlingTests.Object.csAdd DynamicallyAccessedMembers and RequiresDynamicCode for reflection/dynamic-type construction.
src/libraries/System.Text.Json/tests/Common/ExtensionDataTests.csAdd DynamicallyAccessedMembers annotations for reflection-based property access.
src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.ParameterMatching.csAdd DynamicallyAccessedMembers to generic test helper to satisfy trimming analysis.
src/libraries/System.Text.Json/tests/Common/ConstructorTests/ConstructorTests.Cache.csAdd DynamicallyAccessedMembers annotations to reflection-heavy generic helpers.
src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.KeyValuePair.csAdd DynamicallyAccessedMembers annotations for reflection-based policy instantiation.
src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.Dictionary.csAdd Requires* annotations and adjust reflection patterns to be trim/AOT analyzer compatible.
src/libraries/System.Text.Json/tests/Common/CollectionTests/CollectionTests.AsyncEnumerable.csReplace reflection-based serialization baselines with wrapper/type-info based calls for source-gen compatibility.

@github-actions

This comment has been minimized.

@MichalStrehovskyMichalStrehovsky left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

…ectionEnabledByDefault
The test calls JsonSerializerOptions.GetConverter on options derived from
JsonSerializerOptions.Default, which requires the default reflection-based
resolver. Enabling the trim/AOT analyzer bakes IsReflectionEnabledByDefault=false
into the source-gen test project's runtimeconfig on all platforms (not just AOT),
so the previous IsNotNativeAot gate let it run on CoreCLR where it threw
NotSupportedException (EmptyJsonTypeInfoResolver). Gating on
JsonSerializer.IsReflectionEnabledByDefault correctly skips it in the source-gen
project while keeping it in the reflection project.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Copilot Code Review

Holistic Assessment

Motivation: Well-justified. Enabling trim/AOT analyzers on the source generator test projects closes a real gap where AOT/trim regressions would only surface in Native AOT outerloop runs (not part of regular PR builds). This directly follows up on #129965.

Approach: Sound and systematic. The PR resolves analyzer warnings using established patterns: options.GetTypeInfo<T>() for trim-safe serialization, [DynamicallyAccessedMembers] annotations where reflection is needed, [ConditionalFact/Theory] for platform-specific skips, [UnconditionalSuppressMessage] with runtime guards for justified suppressions, and JsonStringEnumConverter<TEnum>() over the non-generic variant.

Summary: ✅ LGTM. This is a well-executed, large-but-focused infrastructure improvement across 42 test files. All changes are confined to test code with no product code modifications. The patterns used are consistent with existing conventions. The only minor concern (early returns vs. SkipTestException) was already flagged by the previous inline review and is non-blocking.


Detailed Findings

Detailed Findings

✅ Analyzer Enablement — Core goal achieved correctly

Removing <EnableTrimAnalyzer>false</EnableTrimAnalyzer> and <EnableAotAnalyzer>false</EnableAotAnalyzer> from both Roslyn 3.11 and 4.4 .csproj files correctly enables the analyzers for the source generator test projects. The .targets file additions (DynamicallyAccessedMembersAttribute, RequiresUnreferencedCodeAttribute, RequiresDynamicCodeAttribute, UnconditionalSuppressMessageAttribute, CompilerLoweringPreserveAttribute) are needed for non-netcoreapp targets.

✅ Source-Gen Wrapper — Trim-safe overloads used correctly

JsonSerializerWrapper.SourceGen.cs consistently replaces JsonSerializer.Serialize<T>(value, options) and JsonSerializer.Deserialize<T>(json, options) calls with their JsonTypeInfo-based counterparts via options.GetTypeInfo<T>(). This is the canonical trim/AOT-safe pattern for System.Text.Json.

The #if BUILDING_SOURCE_GENERATOR_TESTS conditional in JsonSerializerWrapper.cs switching from MakeReadOnly(populateMissingResolver: true) to MakeReadOnly() is correct — in source-gen mode, all types are registered through the context and the reflection fallback should be disabled.

✅ Newtonsoft.Json Baseline Extraction — Clean and well-guarded

The AssertOutputEqualsNewtonsoft helper in ReferenceHandlerTests.Serialize.cs uses PlatformDetection.IsReflectionEmitSupported as a runtime guard (correct since Newtonsoft.Json's serialization APIs are reflection-based). The [UnconditionalSuppressMessage] suppressions are properly justified. The refactoring cleanly removes redundant JsonConvert.SerializeObject calls from individual tests without losing coverage.

✅ PolymorphicEqualityComparer Replacement — Correct structural improvement

Replacing the reflection-based PolymorphicEqualityComparer<T>.Instance (which iterated properties via GetProperties()) with CreateJsonEqualityComparer<T>() that serializes through JsonTypeInfo<T> is trim-safe. The compareRuntimeType parameter preserves the semantic of rejecting objects with differing runtime types.

✅ CustomPolymorphismResolver — Correct dual-mode resolver

The #if BUILDING_SOURCE_GENERATOR_TESTS conditional in CustomPolymorphismResolver that substitutes DefaultJsonTypeInfoResolver with the generated PolymorphicTestsContext_Metadata.Default is the correct approach for making the resolver work in both source-gen and reflection modes.

✅ MetadataTests.Options — Narrowed EmptyContext is sufficient

The EmptyContext.GetTypeInfo change from generic JsonTypeInfo.CreateJsonTypeInfo(type, Options) to only handling typeof(int) is correct for this test's purpose (testing that passing wrong options throws InvalidOperationException). The original implementation used a trim-unsafe API unnecessarily.

✅ StructuralJsonTypeClassifierTests — Instance options correct

Switching from static readonly options with new DefaultJsonTypeInfoResolver() to instance-level options derived from serializerUnderTest.DefaultOptions ensures tests use the correct resolver chain (source-gen or reflection) depending on configuration.

💡 Early Returns vs. SkipTestException — Minor (already flagged)

Some tests that cannot run in source-gen mode use return Task.CompletedTask; instead of throwing a skip exception (e.g., PolymorphicClass_CustomConfigWithNearestAncestorFallback_TestData_Serialization). This was already flagged by the previous inline review. Using throw new SkipException(...) would make test reports more explicit about what was skipped, but this is non-blocking.

Note

This review was generated by GitHub Copilot.

Generated by Code Review for issue #130132 · ● 67.7M ·

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-nativeaot-outerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

eiriktsarpalisand others added 2 commits July 3, 2026 12:47
Register the polymorphic derived types used by the CustomPolymorphismResolver
tests (Dictionary/SortedDictionary/IReadOnlyDictionary value hierarchies and
PolymorphicClass.DerivedAbstractClass) in PolymorphicTestsContext_Metadata so
the source-gen resolver resolves them without reflection. This fixes the
NotSupportedException seen on the net481 leg, where reflection is enabled but
these tests route resolution entirely through the source-gen context.
Also removes the now-redundant in-body 'if (!IsReflectionEnabledByDefault) return;'
guards from the nearest-ancestor and dictionary-interface polymorphic tests: they
are already gated via [ConditionalX(IsNotNativeAot)] and now pass under source-gen.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The BaseTypeFallback / NearestAncestorFallback / DictionaryInterface tests were
temporarily gated [ConditionalX(IsNotNativeAot)] while their derived types were
unregistered in the source-gen context. Now that those types are registered,
the tests resolve entirely through the source-gen resolver with no reflection,
so they run under NativeAOT again -- matching main and preserving the outerloop
coverage this PR exists to protect.
Validated: NativeAOT run executes 524 of these cases with 0 failures (overall
10203 pass / 55 skip / 0 fail); net11.0 reflection-off 10503 pass / 0 fail;
net481 + Roslyn3.11 build 0 warnings.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
CopilotAI review requested due to automatic review settings July 3, 2026 10:51

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

krwq
krwq approved these changes Jul 3, 2026

@krwqkrwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming nothing blocking comes up from comments but doesn't look like

@eiriktsarpalis
eiriktsarpalis merged commit e341991 into mainJul 3, 2026
86 of 89 checks passed
@eiriktsarpalis
eiriktsarpalis deleted the eiriktsarpalis-stj-sourcegen-aot-warnings branch July 3, 2026 14:46
@dotnet-milestone-botdotnet-milestone-botBot added this to the 11.0-preview7 milestone Jul 4, 2026
eiriktsarpalis added a commit that referenced this pull request Jul 15, 2026
…130132)
Enables the trim/AOT analyzers on the `System.Text.Json` source
generator test projects so that AOT/trim regressions are caught at PR
build time, instead of only surfacing in the Native AOT outerloop runs
that don't execute on regular PRs.
This is a follow up to #129965.
---------
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 4, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@eiriktsarpalis@krwq@MichalStrehovsky