You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(stjm)!: surface actionable trimming and NativeAOT warnings - #260
Migration-enabled applications can currently build without learning that discovery and runtime generic construction are unsupported under trimming or NativeAOT. This change reports actionable IL2026 and IL3050 warnings at public setup and registration calls, including callers using generated JSON metadata.
Changes
Annotate both AddJsonMigrationSupport overloads, all migrator registration methods, and the .NET 11 union classifier constructor. Messages name the unsupported behavior, corrective publish settings, and optional documentation URL.
Enable trim/AOT analysis for net10.0 and net11.0. Propagate requirements internally, preserve interface/override contracts with narrow justified callback suppressions, and retain service-provider activation without adding a new() constraint.
Add packed-package consumer checks for diagnostic IDs, real consumer source locations, message content, generated JSON metadata, generic/assembly registration, and union activation. Reject unexpected warnings and retain package hashes and diagnostic logs.
Document the supported untrimmed deployment and add CI checks for rooted trimmed and NativeAOT publishes on both frameworks.
No AOT-safe registration API or source generator is introduced; JSON source generation and warning-free library analysis are not presented as proof of AOT support.
Consumers treating trim/AOT warnings as errors may now fail their builds; the diagnostic change is marked as breaking in the commit history.
Validation
Baseline package fails the new check because IL2026 is missing at the setup call; the untrimmed control succeeds.
Release solution build: zero warnings/errors. Existing tests: 1,221 passed across both target frameworks.
Packed consumers: ordinary builds/runtime controls, trim/AOT diagnostic builds, rooted trimmed publishing, and NativeAOT compiler analysis pass on both frameworks. Controls include a DI-created external migrator without a parameterless constructor.
Documentation snippet generation and git diff --check pass.
Remaining limitations
Full rooted trimmed and NativeAOT publishes passed for net10.0 and net11.0 on Linux at commit bc8b137 (CI run). The packaged diagnostic checks, tests, documentation snippets, and package validation all passed. Local Windows native linking remains unverified because the C++ toolchain is unavailable; the local check used -Publish -NativeCompileOnly.
Previously measured aggregate branch coverage is 97.00% on .NET 10 and 97.99% on .NET 11. Several core classes remain below the repository's required 100%; this gate is not claimed as satisfied. The existing follow-up is #249; tracking it is not a waiver.
Standards: one finding—the documented core branch-coverage gate remains unmet. No other standards findings or maintainability smells were reported.
Spec: no actionable findings against the live issue. Subsequent CI verified full Linux NativeAOT publishing on both target frameworks.
Review remediation
At bc8b137, resolver constructors enforce the diagnostic boundary, callback suppressions share their rationale, and the consumer verifier checks warning explanations and validates package names. Source-location and reflection comments document their constraints; source-generation documentation points to the deployment rule. All 1,221 tests and local packed-consumer checks passed again. Independent Standards and Spec review found no new issues in the remediation delta. CI on this exact head passed tests, documentation snippets, package validation, and full Linux trimmed/NativeAOT publishing on both frameworks. All ten inline review threads were addressed and resolved.
Migration setup and registration now report standard trimming and NativeAOT diagnostics at consumer call sites, including configurations with generated JSON metadata. Messages explain the unsupported behavior and the deployment settings needed to avoid it.
Enable library analysis on both supported frameworks and verify the packed consumer contract without introducing a new registration API or changing service-provider activation.
The reason will be displayed to describe this comment to others. Learn more.
Two-axis review (Standards and Spec against #256). Nothing blocking: every public entry point that reaches discovery, reflection or MakeGenericType is annotated, the suppressions sit only on interface/override callbacks, and CI runs the packed-consumer check. The inline comments are suggestions.
Standards
Hard, not caused by this PR:Egil.SystemTextJson.Migration/AGENTS.md requires "Core components must maintain 100% branch coverage". The PR reports 97.00% (.NET 10) and 97.99% (.NET 11). This needs a tracking issue instead of an implicit waiver.
Consider a BREAKING CHANGE footer on the fix commit. Consumers who run trim/AOT analysis with warnings-as-errors will now fail to build.
MigrationCompatibility holds diagnostic message text, not compatibility logic. A name like TrimmingDiagnostics would say what it holds. This is a judgement call.
Spec
Partial: the script doesn't assert the explanation part of the warning messages (inline on verify-trimming-package.ps1:66-71).
Slightly beyond scope: CI installs clang and does a full NativeAOT native link, where #256 asks for publish analysis. It's useful evidence, but it costs CI time and depends on the runner. The PR correctly doesn't run the native output, which belongs to the follow-up issue.
Nit: the docs still say "compatible with System.Text.Json source generation" (inline).
Require trimming and dynamic-code diagnostics when constructing deferred resolvers, and verify that packaged warnings explain the unsupported behavior. Validate package inputs and document the source-location contract used by consumer checks.
BREAKING CHANGE: Migration setup and registration APIs now emit IL2026 and IL3050 when the corresponding analyzers are enabled. Consumers treating these warnings as errors must account for the unsupported deployment requirements.
…otes]
Separate serialization metadata support from deployment compatibility and explain the runtime limitations without duplicating the deployment rule.
The project policy requires 100% branch coverage for core components, but this PR explicitly leaves that gate unmet (97.00% on .NET 10 and 97.99% on .NET 11) and only points to open issue #249. Adding the trim/AOT verification here does not establish the required coverage disposition or an owner-approved exception, so this must be resolved before treating the CI validation as approval-ready.
Addressed and resolved all ten inline suggestions in faf9644/bc8b137. The 1,221 local tests and both-framework packed-consumer checks passed; CI on bc8b137 passed tests, docs, package validation, and full trimmed/NativeAOT publishing: https://github.com/egil/framework/actions/runs/36033651215. Independent Standards and Spec review found no new issues in the remediation delta.
The existing core-coverage gap is already tracked by #249, now linked from the PR; this remains an unmet gate, not a waiver. Added the BREAKING CHANGE footer and matching PR title for warnings-as-errors consumers. Kept the existing internal helper name to avoid an optional cross-file rename, and retained full Linux native linking as useful package evidence (the native output is not run).
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
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.
egil
left a comment
egil
changed the title
Migration-enabled applications can currently build without learning that discovery and runtime generic construction are unsupported under trimming or NativeAOT. This change reports actionable IL2026 and IL3050 warnings at public setup and registration calls, including callers using generated JSON metadata.
Changes
No AOT-safe registration API or source generator is introduced; JSON source generation and warning-free library analysis are not presented as proof of AOT support.
Consumers treating trim/AOT warnings as errors may now fail their builds; the diagnostic change is marked as breaking in the commit history.
Validation
Remaining limitations
Closes #256.
Independent review
Review remediation
At bc8b137, resolver constructors enforce the diagnostic boundary, callback suppressions share their rationale, and the consumer verifier checks warning explanations and validates package names. Source-location and reflection comments document their constraints; source-generation documentation points to the deployment rule. All 1,221 tests and local packed-consumer checks passed again. Independent Standards and Spec review found no new issues in the remediation delta. CI on this exact head passed tests, documentation snippets, package validation, and full Linux trimmed/NativeAOT publishing on both frameworks. All ten inline review threads were addressed and resolved.