Skip to content

(MINOR): Add IPatchClassBuilder, with PatchClassBuilder delegating to an internal RoslynPatchClassBuilder - #133

Merged
PaulTrampert merged 2 commits into
mainfrom
feature/ipatchclassbuilder
Oct 3, 2026
Merged

(MINOR): Add IPatchClassBuilder, with PatchClassBuilder delegating to an internal RoslynPatchClassBuilder#133
PaulTrampert merged 2 commits into
mainfrom
feature/ipatchclassbuilder

Conversation

@PaulTrampert

@PaulTrampert PaulTrampert commented Oct 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Why

The library has two ways to build a patch class: the Roslyn builder, which lives inside the public PatchClassBuilder, and the internal EmitPatchClassBuilder. They share no abstraction, so callers that choose between them, such as the shared test fixture, pass method groups around as Func<Type, Type>. PatchClassBuilder is also both the public entry point and the Roslyn implementation. #95 (opt-in Emit builder) and #126 (remove the Roslyn builder) both need the entry point to stay put while the implementation behind it changes.

What changed

  • New public interface IPatchClassBuilder with Type GetPatchClassFor(Type type).
  • New internal sealed class RoslynPatchClassBuilder : IPatchClassBuilder holds the CodeDom/Roslyn generation and its static cache, moved verbatim from PatchClassBuilder. It has a private constructor and a static Instance.
  • PatchClassBuilder implements IPatchClassBuilder and is now a thin facade. It keeps its type, Instance, the obsolete public constructor, and the GetPatchClassFor signature and docs. GetPatchClassFor delegates to RoslynPatchClassBuilder.Instance. Because the cache is static, instances made with the obsolete constructor still share it.
  • EmitPatchClassBuilder changes from an internal static class to an internal sealed class that implements the interface, with a private constructor and a static Instance. Its cache, AssemblyName and the generation helpers are unchanged.
  • Tests:
    • The shared PatchClassBuilderTest fixture takes an IPatchClassBuilder and runs against RoslynPatchClassBuilder.Instance and EmitPatchClassBuilder.Instance.
    • The Roslyn-specific tests call RoslynPatchClassBuilder directly.
    • The shared-cache test also asserts that PatchClassBuilder returns the Roslyn builder's types.

No behaviour changes. The only public API change is the new interface, which is additive, so this is MINOR.

Alternatives rejected

Tests

  • dotnet build: the warning set is identical to main's.
  • dotnet test: core 124/124, Swashbuckle 17/17 and OpenApi 11/11 pass. The net8.0 projects were run with DOTNET_ROLL_FORWARD=Major because only the .NET 10 runtime is installed locally.

Supersedes #134, which was stacked on this PR and is now folded into it.

🤖 Generated with Claude Code

@PaulTrampert @claude
Add IPatchClassBuilder and implement it on both builders
f1275d5 
PatchClassBuilder implements the new public IPatchClassBuilder interface.
EmitPatchClassBuilder changes from a static class to a sealed singleton
that implements it too, so the shared test fixture takes the builder
itself rather than a Func<Type, Type>. No behaviour changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@PaulTrampert PaulTrampert mentioned this pull request Oct 3, 2026
Merged
@PaulTrampert @claude
Move the Roslyn builder into RoslynPatchClassBuilder
1f7bb62 
PatchClassBuilder becomes a thin public facade over a new internal
RoslynPatchClassBuilder, which holds the CodeDom/Roslyn implementation
and its static cache. PatchClassBuilder keeps its Instance, its obsolete
constructor and its GetPatchClassFor signature, so nothing breaks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@PaulTrampert PaulTrampert changed the title (MINOR): Add IPatchClassBuilder and implement it on both patch class builders (MINOR): Add IPatchClassBuilder, with PatchClassBuilder delegating to an internal RoslynPatchClassBuilder Oct 3, 2026
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

✅ PR Title Formatted Correctly

The title of this PR has been updated to match the correct format. Thank you!

@PaulTrampert
PaulTrampert merged commit 537640f into main Oct 3, 2026
11 checks passed
@PaulTrampert
PaulTrampert deleted the feature/ipatchclassbuilder branch October 3, 2026 23:51
This was referenced Oct 4, 2026
Closed
Closed
Merged
PaulTrampert added a commit that referenced this pull request Oct 4, 2026
@PaulTrampert @claude
(MINOR): Add PatchClassBuilder.UseExperimentalDynamicClassBuilder (#136)
83b1db7 
## Why

`IPatchObject<T>` throws `NotSupportedException` for an `internal` `T`,
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 behind `IPatchClassBuilder`, with `PatchClassBuilder` as a
facade. This PR lets applications opt in to the Emit builder.

## What changed

- New `public static bool
PatchClassBuilder.UseExperimentalDynamicClassBuilder { get; set; }`, off
by default. `PatchClassBuilder.GetPatchClassFor` reads it on every call
and delegates to `EmitPatchClassBuilder.Instance` when it's on, or
`RoslynPatchClassBuilder.Instance` when it's off. `Instance` keeps its
type, so this is binary compatible.
- The flag is process-wide, so `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.
- Both builder classes stay internal. The flag is the only new public
API, so this is MINOR.
- The Roslyn builder's `NotSupportedException` for non-public types now
tells you to set the flag.
- Docs: the flag's XML docs, the `GetPatchClassFor` `<exception>` docs,
a new "Non-public write models (experimental)" section in `README.md`, a
short section in `docs/getting-started.md`, and `AGENTS.md` ("How it
works").

## Alternatives rejected

- **An `AddSimplePatchConverters(options, bool)` overload (#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.
- **Retyping `Instance` to `IPatchClassBuilder`.** This breaks binary
compatibility, so it's deferred to #135.
- **A one-way `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

- Core (`UseExperimentalDynamicClassBuilderTest`, `[NonParallelizable]`,
resets the flag in `TearDown`):
  - The flag defaults to off.
  - With the flag on, `Instance` returns the Emit builder's type.
- After the flag is turned back off, it returns the Roslyn builder's
type again (separate caches).
- With the flag off, an internal type throws an error that names the
flag.
- With the flag on, an internal type works end to end through
`AddSimplePatchConverters`: patching, explicit null, an omitted
property, and `[Range]` validation.
- Swashbuckle and OpenAPI: new tests generate a patch schema for an
internal model with the flag on. Each test project grants
`InternalsVisibleTo("PTrampert.SimplePatch.Emitted")`. I checked that
both tests fail with `NotSupportedException` when 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 with `DOTNET_ROLL_FORWARD=Major` because only
the .NET 10 runtime is installed locally.

Closes #95

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@PaulTrampert PaulTrampert mentioned this pull request Oct 4, 2026
Merged
PaulTrampert added a commit that referenced this pull request Oct 4, 2026
@PaulTrampert @claude
(MAJOR): Retype PatchClassBuilder.Instance to IPatchClassBuilder and …
1077cff 
…make PatchClassBuilder static (#141)

## Cause

#133 made `PatchClassBuilder` a facade over the internal
`RoslynPatchClassBuilder`, and #95 added
`UseExperimentalDynamicClassBuilder`, which the facade's
`GetPatchClassFor` read on every call. Both kept
`PatchClassBuilder.Instance` typed as `PatchClassBuilder`, because
changing a property's type breaks binary compatibility. This is the 2.0
change that #135 deferred.

Once `Instance` hands out the selected internal builder, nothing hands
out a `PatchClassBuilder` any more. Its instance surface (the obsolete
constructor, instance `GetPatchClassFor`, and its `IPatchClassBuilder`
implementation) only forwards to `Instance` and is dead weight (#143).
The two changes are one PR because a static class can't be a property
type, so #143 can't compile without #135.

## Fix

**Retype `Instance` (#135)**
- `public static IPatchClassBuilder Instance` returns the selected
builder itself: `EmitPatchClassBuilder.Instance` when the flag is on,
otherwise `RoslynPatchClassBuilder.Instance`. Both internal builders
already had static `Instance` singletons.
- `Instance` is a computed property (`=>`), not an initialised one, so
it honours the flag at read time. The flag is a settable static, and the
tests flip it back and forth.
- Call sites (`PatchJsonConverterFactory`, the Swashbuckle filter, the
OpenAPI transformer) already call
`PatchClassBuilder.Instance.GetPatchClassFor(...)` on each use, so they
compile and behave unchanged against the new type. None of them caches
the builder.

**Make `PatchClassBuilder` static (#143)**
- `public static class PatchClassBuilder` now holds only `Instance` and
`UseExperimentalDynamicClassBuilder`.
- The constructor, instance `GetPatchClassFor` and the
`IPatchClassBuilder` implementation are removed.
- The XML docs worth keeping moved. The description of the generated
class (its properties, its `Patch` method, sealed and public, the
`.Optionals` namespace) is now in the class remarks. The
`NotSupportedException` conditions are now in `Instance`'s remarks.
`IPatchClassBuilder.GetPatchClassFor` keeps its builder-neutral
contract. `RoslynPatchClassBuilder.GetPatchClassFor` had `<inheritdoc
cref="PatchClassBuilder.GetPatchClassFor"/>` and now has its own summary
and exception docs.
-
`RoslynPatchClassBuilderTest.GetPatchClassFor_SharesGeneratedTypesAcrossBuilders`
constructed `PatchClassBuilder` twice. It is now
`GetPatchClassFor_CachesTheGeneratedType`, which calls
`RoslynPatchClassBuilder.Instance` twice and still checks that
`PatchClassBuilder.Instance` agrees with it while the flag is off. The
internal builders have private constructors, so a second instance can't
be constructed.

**Docs:** `README.md` and `docs/getting-started.md` note that `Instance`
returns the selected builder, so you should read it where you use it and
not keep it. `AGENTS.md` describes the static class.

**Breaking:**
- Binaries compiled against `PatchClassBuilder
PatchClassBuilder.Instance { get; }` fail with `MissingMethodException`.
- `new PatchClassBuilder()` (obsolete since 1.x) and instance
`GetPatchClassFor` are gone.
- `PatchClassBuilder` no longer implements `IPatchClassBuilder`, and
can't be used as a variable, parameter or generic argument type.

## Coordination with sibling PRs into `release/2.0`

- **#139 (#75) makes the constructor internal.** This PR deletes the
constructor, which supersedes that change. When the two meet, resolve
the conflict by deleting the constructor.
- **#142 (#126)** makes Emit the only runtime builder and removes the
flag. Once it lands, `Instance` collapses to
`EmitPatchClassBuilder.Instance`.
- **#140 (#76)** changes TFMs. They are unchanged here.

## Alternatives rejected

- **Initialise `Instance` once (`{ get; } = ...`).** This would freeze
whatever builder the flag selected at type initialisation and ignore
later changes to the flag.
- **Keep returning the facade, typed as the interface.** This would
avoid the read-time caveat, but it keeps the per-call forwarding that
#135 asks to remove. It also leaves `Instance` as the one thing that
hands out a `PatchClassBuilder`.
- **Keep `PatchClassBuilder` non-static, with the constructor internal
(#75 alone).** Nothing would construct it, so its instance members would
be unreachable dead code.
- **Ship #143 as a separate PR stacked on this one.** The maintainer
chose to fold it in, because #143 can't compile without the retype.

## Tests

New tests in `UseExperimentalDynamicClassBuilderTest`:
- `Instance_IsTheRoslynBuilderWhenOff`
- `Instance_IsTheEmitBuilderWhenOn`
- `Instance_FollowsTheFlagWhenItIsTurnedBackOff`

Against the old `PatchClassBuilder.cs`, the NUnit analyzer rejects all
three with NUnit2020 (a `SameAs` that always fails because the types are
mutually exclusive), so they fail before the fix.

CI's build pipeline doesn't run on PRs that target a branch other than
`main`, so these results are local only:
- `dotnet build`: 0 errors, 21 warnings, the same count as the base
commit.
- `dotnet test`: all passed. Core 132/132, Swashbuckle 18/18, OpenApi
12/12. The net8.0 test hosts ran with `DOTNET_ROLL_FORWARD=Major`,
because only the .NET 10 runtime is installed locally.

Closes #135
Closes #143

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@dependabot dependabot Bot mentioned this pull request Oct 5, 2026
Merged
PaulTrampert pushed a commit to PaulTrampert/PacmanManager that referenced this pull request Oct 6, 2026
@dependabot
(PATCH) Bump the nuget group with 2 updates (#179)
5f576f8 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@PaulTrampert