Repository navigation
(MINOR): Add PatchClassBuilder.UseExperimentalDynamicClassBuilder - #136
Merged
Merged
Conversation
A process-wide static flag, off by default, that makes PatchClassBuilder delegate to the internal Reflection.Emit builder instead of the Roslyn one, so internal write models can be patched. The Roslyn builder's NotSupportedException for non-public types now names the flag. Closes #95 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
✅ PR Title Formatted CorrectlyThe title of this PR has been updated to match the correct format. Thank you! |
PaulTrampert
added a commit
that referenced
this pull request
Oct 4, 2026
) Closes #126 ## Cause Since #130 and #136 the library has had two builders for the same patch class. One is the original builder, which generates C# with CodeDom and compiles it with Roslyn. The other is a Reflection.Emit builder, behind the experimental `PatchClassBuilder.UseExperimentalDynamicClassBuilder` flag that shipped in v1.5.0. The process-wide switch adds runtime complexity, and the Roslyn path has limits that the Emit path doesn't have. It can't patch internal write models, and it breaks under single-file publishing because it builds metadata references from `Assembly.Location`. ## Fix - `PatchClassBuilder.GetPatchClassFor` now always delegates to `EmitPatchClassBuilder.Instance`. Emit is the only builder used at runtime. - `PatchClassBuilder.UseExperimentalDynamicClassBuilder` is removed outright, and so are its tests. Its `GetPatchClassFor` `<exception>` docs now describe when Emit throws. - **`RoslynPatchClassBuilder` is kept as internal, unused, tested code, pending #144.** That issue decides whether it becomes a compile-time source generator. This departs from #126's scope, which asked for the Roslyn path to be deleted, because the maintainer wants it kept until #144 is decided. - The class remarks say it isn't used at runtime and is kept pending #144. - Its "not public" error no longer mentions the removed flag. - **The `Microsoft.CodeAnalysis.CSharp` and `System.CodeDom` package references remain, so consumers still get them transitively.** The `.csproj` has a comment explaining why. - Tests: - `PatchClassBuilderTest` is still parameterized over both builders through `PatchClassBuilders`, calling the internal builders directly, so the two can't drift apart. - `RoslynPatchClassBuilderTest` is kept. Its shared-cache test, which asserted that `PatchClassBuilder` hands out Roslyn types, is now a plain Roslyn cache test. - New `PatchClassBuilderDelegationTest` covers the public entry point: - `PatchClassBuilder` (including the obsolete constructor) hands out the Emit builder's types. - An internal model round-trips through `IPatchObject<T>` with validation. - A private nested type throws `NotSupportedException`. - `EmitPatchClassBuilderTest` gains a concurrent-first-use test. - The Swashbuckle and OpenApi `ExperimentalDynamicClassBuilderTest` files are renamed to `InternalWriteModelTest`, without the flag, `TearDown` or `[NonParallelizable]`. - Docs: - README: "Non-public write models (experimental)" is now "Internal write models", and there is a new "Deployment" section. - `docs/getting-started.md` matches the README. - `AGENTS.md` "How it works" describes Emit as the runtime builder, and the Roslyn builder as unused, tested code pending #144. ### Single-file publishing (investigated, as #126 asks) I published `PTrampert.SimplePatch.Sample` with `-r linux-x64 --self-contained false -p:PublishSingleFile=true` and sent a PATCH to `/People/1`: - **`release/2.0` (Roslyn):** HTTP 500. `CS0518: Predefined type 'System.Object' is not defined or imported` and `CS0234` for `IPatchObject<>`. Roslyn gets no metadata references because the bundled assemblies have no `Location`. - **This branch (Emit):** HTTP 200 with the patched person. I re-checked this after restoring the Roslyn builder. The Roslyn assemblies are still bundled, but nothing on the runtime path calls into them. The README's new Deployment section says single-file publishing is supported, and that Native AOT and trimming aren't. **I didn't add an automated single-file test or sample.** It would need a publish-and-run step in CI, which is a larger change than this PR should carry. ## Observable behaviour changes for consumers - `PatchClassBuilder.UseExperimentalDynamicClassBuilder` is gone. Code that sets it no longer compiles, and existing binaries fail with `MissingMethodException`. - Generated types change: - They are named `<Namespace>.Optionals.<Type>_Optionals`, without the random suffix. - They live in a dynamic assembly named `PTrampert.SimplePatch.Emitted`, so `Assembly.IsDynamic` is true and there is no `Location`. - Internal write models now work when their assembly declares `[InternalsVisibleTo("PTrampert.SimplePatch.Emitted")]`. Without that grant, they still throw `NotSupportedException`, and the message names the grant and the assembly that needs it. - Private and protected nested types still throw `NotSupportedException`. - If generation fails, the error surfaces from Reflection.Emit (`TypeLoadException` / `InvalidProgramException`), not as Roslyn diagnostics. - Single-file published apps now work. - **Unchanged:** `Microsoft.CodeAnalysis.*` and `System.CodeDom` still arrive transitively, until #144 is decided. ## Alternatives rejected - **Delete the Roslyn builder and its dependencies now, as #126 originally scoped.** I did this in this PR's first commit, then reverted it, because the maintainer wants the code kept until #144 decides whether it becomes a source generator. - **Keep `UseExperimentalDynamicClassBuilder` as an `[Obsolete]` no-op.** #126 rules this out. The removal ships in the same major as the other breaking changes, and a compile error is the clearest signal. - **Keep the flag so the Roslyn builder stays selectable.** That would keep the process-wide switch and the unsupported single-file path reachable. The dead code is exercised by tests instead. ## Coordination with parallel 2.0 PRs - **#135** (retype `PatchClassBuilder.Instance` to `IPatchClassBuilder`): this PR leaves `Instance` and its type alone. When both land, #135's `Instance` getter will need to resolve to `EmitPatchClassBuilder.Instance`. - **#75** (constructor visibility): this PR doesn't touch the obsolete public constructor. `PatchClassBuilderDelegationTest` calls it under `#pragma warning disable CS0618`, and will keep compiling if the constructor becomes internal, because the test project has `InternalsVisibleTo`. ## Test results - `dotnet build`: 0 errors, 21 warnings, the same count as `release/2.0`. - `dotnet test`: - `PTrampert.SimplePatch.Test` (net8.0): 128 passed. That is 129 on `release/2.0`, minus 5 flag tests, plus 4 new ones. - `PTrampert.SimplePatch.Swashbuckle.Test` (net8.0): 18 passed. - `PTrampert.SimplePatch.OpenApi.Test` (net10.0): 12 passed. - Only the .NET 10 runtime is installed locally, so I ran the net8.0 test hosts with `DOTNET_ROLL_FORWARD=Major`. - **CI won't run these tests on this PR.** The main pipeline only runs on PRs to `main`, so PRs into `release/2.0` get only the title check. The net8.0 suites will first run in CI when `release/2.0` is merged to `main`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
PaulTrampert
pushed a commit
to PaulTrampert/PacmanManager
that referenced
this pull request
Oct 6, 2026
Updated [PTrampert.SimplePatch](https://github.com/PaulTrampert/PTrampert.SimplePatch) from 1.3.6 to 2.0.3. <details> <summary>Release notes</summary> _Sourced from [PTrampert.SimplePatch's releases](https://github.com/PaulTrampert/PTrampert.SimplePatch/releases)._ ## 2.0.3 ## Changes - (PATCH): Drop the Roslyn patch class builder's own cache ([#154](PaulTrampert/PTrampert.SimplePatch#154)) ## 2.0.2 ## Changes - (PATCH): Route PatchClassBuilder.Instance through CachingPatchClassBuilder ([#155](PaulTrampert/PTrampert.SimplePatch#155)) ## 2.0.1 ## Changes - (PATCH): Add an internal CachingPatchClassBuilder decorator ([#153](PaulTrampert/PTrampert.SimplePatch#153)) - (PATCH): Document the release PR format for staged release branches ([#152](PaulTrampert/PTrampert.SimplePatch#152)) ## 2.0.0 ## Changes - (MAJOR): Release 2.0 ([#147](PaulTrampert/PTrampert.SimplePatch#147)) - (PATCH): Build PRs into release/** branches and document the release flow ([#146](PaulTrampert/PTrampert.SimplePatch#146)) ## 1.5.0 ## What's Changed * (MINOR): Add PatchClassBuilder.UseExperimentalDynamicClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#136 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.4.0...v1.5.0 ## 1.4.0 ## What's Changed * (MINOR): Add IPatchClassBuilder, with PatchClassBuilder delegating to an internal RoslynPatchClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#133 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.8...v1.4.0 ## 1.3.8 ## What's Changed * (PATCH): Add an internal Reflection.Emit patch class builder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#130 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.7...v1.3.8 ## 1.3.7 ## What's Changed * (PATCH): Bump the csharp-deps group with 6 updates by @dependabot[bot] in PaulTrampert/PTrampert.SimplePatch#81 * (PATCH): Add AGENTS.md, CLAUDE.md symlink and /implement-unblocked by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#125 * (PATCH): Extract the patch class model from PatchClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#129 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.6...v1.3.7 Commits viewable in [compare view](PaulTrampert/PTrampert.SimplePatch@v1.3.6...v2.0.3). </details> Updated [PTrampert.SimplePatch.Swashbuckle](https://github.com/PaulTrampert/PTrampert.SimplePatch) from 1.3.6 to 2.0.3. <details> <summary>Release notes</summary> _Sourced from [PTrampert.SimplePatch.Swashbuckle's releases](https://github.com/PaulTrampert/PTrampert.SimplePatch/releases)._ ## 2.0.3 ## Changes - (PATCH): Drop the Roslyn patch class builder's own cache ([#154](PaulTrampert/PTrampert.SimplePatch#154)) ## 2.0.2 ## Changes - (PATCH): Route PatchClassBuilder.Instance through CachingPatchClassBuilder ([#155](PaulTrampert/PTrampert.SimplePatch#155)) ## 2.0.1 ## Changes - (PATCH): Add an internal CachingPatchClassBuilder decorator ([#153](PaulTrampert/PTrampert.SimplePatch#153)) - (PATCH): Document the release PR format for staged release branches ([#152](PaulTrampert/PTrampert.SimplePatch#152)) ## 2.0.0 ## Changes - (MAJOR): Release 2.0 ([#147](PaulTrampert/PTrampert.SimplePatch#147)) - (PATCH): Build PRs into release/** branches and document the release flow ([#146](PaulTrampert/PTrampert.SimplePatch#146)) ## 1.5.0 ## What's Changed * (MINOR): Add PatchClassBuilder.UseExperimentalDynamicClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#136 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.4.0...v1.5.0 ## 1.4.0 ## What's Changed * (MINOR): Add IPatchClassBuilder, with PatchClassBuilder delegating to an internal RoslynPatchClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#133 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.8...v1.4.0 ## 1.3.8 ## What's Changed * (PATCH): Add an internal Reflection.Emit patch class builder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#130 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.7...v1.3.8 ## 1.3.7 ## What's Changed * (PATCH): Bump the csharp-deps group with 6 updates by @dependabot[bot] in PaulTrampert/PTrampert.SimplePatch#81 * (PATCH): Add AGENTS.md, CLAUDE.md symlink and /implement-unblocked by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#125 * (PATCH): Extract the patch class model from PatchClassBuilder by @PaulTrampert in PaulTrampert/PTrampert.SimplePatch#129 **Full Changelog**: PaulTrampert/PTrampert.SimplePatch@v1.3.6...v1.3.7 Commits viewable in [compare view](PaulTrampert/PTrampert.SimplePatch@v1.3.6...v2.0.3). </details> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.
Why
IPatchObject<T>throwsNotSupportedExceptionfor aninternalT, because the Roslyn builder compiles the patch class into a separate assembly that can only name public types (#94). #128 added an internal Reflection.Emit builder that doesn't have this limit, and #133 put both builders behindIPatchClassBuilder, withPatchClassBuilderas a facade. This PR lets applications opt in to the Emit builder.What changed
public static bool PatchClassBuilder.UseExperimentalDynamicClassBuilder { get; set; }, off by default.PatchClassBuilder.GetPatchClassForreads it on every call and delegates toEmitPatchClassBuilder.Instancewhen it's on, orRoslynPatchClassBuilder.Instancewhen it's off.Instancekeeps its type, so this is binary compatible.PatchJsonConverterFactory, the Swashbuckle filter and the OpenAPI transformer all follow it with no extra wiring. Its XML docs say to set it at startup, before first use: each builder keeps its own cache, but System.Text.Json and the OpenAPI generators cache the types they've already resolved.NotSupportedExceptionfor non-public types now tells you to set the flag.GetPatchClassFor<exception>docs, a new "Non-public write models (experimental)" section inREADME.md, a short section indocs/getting-started.md, andAGENTS.md("How it works").Alternatives rejected
AddSimplePatchConverters(options, bool)overload (Add an opt-in flag to use the Reflection.Emit patch class builder #95's original design). It hides a process-wide setting behind an API that looks per-options. It also needed a "can only be turned on" rule so that two option sets couldn't disagree.InstancetoIPatchClassBuilder. This breaks binary compatibility, so it's deferred to Retype PatchClassBuilder.Instance to IPatchClassBuilder #135.Enable…()method, or a flag that locks after first use. Both are harder to test and add API surface. Documenting "set at startup" is enough for an experimental opt-in.Tests
UseExperimentalDynamicClassBuilderTest,[NonParallelizable], resets the flag inTearDown):Instancereturns the Emit builder's type.AddSimplePatchConverters: patching, explicit null, an omitted property, and[Range]validation.InternalsVisibleTo("PTrampert.SimplePatch.Emitted"). I checked that both tests fail withNotSupportedExceptionwhen the flag isn't set.dotnet build: no new warnings, and none come from changed files.dotnet docfx: only the existing warnings.dotnet test: core 129/129, Swashbuckle 18/18, OpenApi 12/12. The net8.0 projects were run withDOTNET_ROLL_FORWARD=Majorbecause only the .NET 10 runtime is installed locally.Closes #95
🤖 Generated with Claude Code