From 7d1ea38ff47faea7db37a5cb577930f57d471e61 Mon Sep 17 00:00:00 2001 From: Paul Trampert Date: Thu, 1 Oct 2026 13:12:41 -0400 Subject: [PATCH 1/5] Add an internal Reflection.Emit patch class builder EmitPatchClassBuilder builds the same patch class as PatchClassBuilder from PatchClassModel, but emits it as IL into a dynamic assembly per source type. The assembly grants itself [IgnoresAccessChecksTo] for every assembly the patch class names, so it isn't limited to public types. Nothing uses it yet. PatchClassBuilderTest now runs against both builders. The Roslyn-only cases (its cache and its public-only restriction) move to their own fixture. A new case checks that the patch doesn't read target.P when it sets P. Co-Authored-By: Claude Opus 5.5 --- .../PatchClassBuilderTest.cs | 150 +++----- .../PatchClassBuilders.cs | 15 + .../RoslynPatchClassBuilderTest.cs | 90 +++++ .../TestObjects/ReadTrackingTestObject.cs | 23 ++ .../EmitPatchClassBuilder.cs | 349 ++++++++++++++++++ 5 files changed, 525 insertions(+), 102 deletions(-) create mode 100644 PTrampert.SimplePatch.Test/PatchClassBuilders.cs create mode 100644 PTrampert.SimplePatch.Test/RoslynPatchClassBuilderTest.cs create mode 100644 PTrampert.SimplePatch.Test/TestObjects/ReadTrackingTestObject.cs create mode 100644 PTrampert.SimplePatch/EmitPatchClassBuilder.cs diff --git a/PTrampert.SimplePatch.Test/PatchClassBuilderTest.cs b/PTrampert.SimplePatch.Test/PatchClassBuilderTest.cs index 233b668..35a5bb5 100644 --- a/PTrampert.SimplePatch.Test/PatchClassBuilderTest.cs +++ b/PTrampert.SimplePatch.Test/PatchClassBuilderTest.cs @@ -4,12 +4,22 @@ namespace PTrampert.SimplePatch.Test; -public class PatchClassBuilderTest +// Runs every case against each way of building a patch class, so the two builders can't drift apart. +// Cases that only one builder supports, or that test its caching, are in that builder's own fixture. +[TestFixtureSource(typeof(PatchClassBuilders), nameof(PatchClassBuilders.All))] +public class PatchClassBuilderTest(Func getPatchClassFor) { + private Type GetPatchClassFor(Type type) => getPatchClassFor(type); + + // Deserializes straight into this fixture's patch class. Deserializing IPatchObject would + // go through PatchJsonConverterFactory, which always uses PatchClassBuilder.Instance. + private IPatchObject DeserializePatchObject(string json, JsonSerializerOptions options) => + (IPatchObject)JsonSerializer.Deserialize(json, GetPatchClassFor(typeof(T)), options)!; + [Test] public void GetPatchClassFor_CopiesThePropertiesAsOptionals() { - var optionalsType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(OptionalsBuilderTestObject)); + var optionalsType = GetPatchClassFor(typeof(OptionalsBuilderTestObject)); Assert.Multiple((Action)(() => { @@ -37,7 +47,7 @@ public void DynamicOptionalsClass_CanBeCreatedAndUsed() DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, }; options.Converters.Add(new OptionalJsonConverterFactory()); - var optionalsType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(OptionalsBuilderTestObject)); + var optionalsType = GetPatchClassFor(typeof(OptionalsBuilderTestObject)); var instance = JsonSerializer.Deserialize(json, optionalsType, options) as IPatchObject; @@ -63,7 +73,7 @@ public void DynamicOptionalsClass_CanBeCreatedAndUsed() [TestCase(nameof(JsonIgnoreConditionsTestObject.WhenWritingDefault), true)] public void GetPatchClassFor_ExcludesOnlyPropertiesIgnoredOnRead(string propertyName, bool patchable) { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(JsonIgnoreConditionsTestObject)); + var patchType = GetPatchClassFor(typeof(JsonIgnoreConditionsTestObject)); Assert.That(patchType.GetProperty(propertyName), patchable ? Is.Not.Null : Is.Null, "Only [JsonIgnore(Condition = Always)] stops System.Text.Json from deserializing a property."); @@ -83,7 +93,7 @@ public void Patch_AppliesPropertiesWithConditionalJsonIgnore() var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; options.AddSimplePatchConverters(); - var patch = JsonSerializer.Deserialize>(json, options)!; + var patch = DeserializePatchObject(json, options)!; var patched = patch.Patch(new JsonIgnoreConditionsTestObject { Always = "old", @@ -101,66 +111,13 @@ public void Patch_AppliesPropertiesWithConditionalJsonIgnore() })); } - [Test] - public void GetPatchClassFor_SharesGeneratedTypesAcrossBuilders() - { - // Deliberately the obsolete constructor: the point of this test is that separately - // constructed builders still share one cache, for as long as that constructor exists. -#pragma warning disable CS0618 - var first = new PatchClassBuilder().GetPatchClassFor(typeof(OptionalsBuilderTestObject)); - var second = new PatchClassBuilder().GetPatchClassFor(typeof(OptionalsBuilderTestObject)); -#pragma warning restore CS0618 - - Assert.Multiple((Action)(() => - { - Assert.That(second, Is.SameAs(first), - "Every builder should resolve a source type to one generated patch type, rather than each emitting its own dynamic assembly for it."); - Assert.That(PatchClassBuilder.Instance.GetPatchClassFor(typeof(OptionalsBuilderTestObject)), Is.SameAs(first)); - })); - } - - [Test] - public void GetPatchClassFor_GeneratesOnceUnderConcurrentFirstUse() - { - const int threadCount = 16; - var sourceType = typeof(ConcurrentFirstUseTestObject); - var results = new Type[threadCount]; - using var barrier = new Barrier(threadCount); - var threads = Enumerable.Range(0, threadCount) - .Select(i => new Thread(() => - { - barrier.SignalAndWait(); - results[i] = PatchClassBuilder.Instance.GetPatchClassFor(sourceType); - })) - .ToList(); - - threads.ForEach(t => t.Start()); - threads.ForEach(t => t.Join()); - - // Every generation loads its own in-memory assembly, so count the loaded types that - // patch the source type: a discarded duplicate would still show up here. - var patchInterface = typeof(IPatchObject<>).MakeGenericType(sourceType); - var generatedTypes = AppDomain.CurrentDomain.GetAssemblies() - .Where(a => !a.IsDynamic && string.IsNullOrEmpty(a.Location)) - .SelectMany(a => a.GetTypes()) - .Where(patchInterface.IsAssignableFrom) - .ToList(); - - Assert.Multiple((Action)(() => - { - Assert.That(results, Has.All.SameAs(results[0])); - Assert.That(generatedTypes, Is.EquivalentTo(new[] { results[0] }), - "Concurrent first use should generate the patch class once, not once per racing thread."); - })); - } - [Test] public void GetPatchClassFor_SupportsSourceTypesInTheGlobalNamespace() { var globalNamespaceType = typeof(GlobalNamespaceTestObject); Assert.That(globalNamespaceType.Namespace, Is.Null, "Guard: this test object must stay in the global namespace."); - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(globalNamespaceType); + var patchType = GetPatchClassFor(globalNamespaceType); Assert.That(patchType.GetProperty(nameof(GlobalNamespaceTestObject.Name)), Is.Not.Null); } @@ -168,7 +125,7 @@ public void GetPatchClassFor_SupportsSourceTypesInTheGlobalNamespace() [Test] public void GetPatchClassFor_UsesTheMostDerivedDeclarationOfAHiddenProperty() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(HiddenPropertyTestObject)); + var patchType = GetPatchClassFor(typeof(HiddenPropertyTestObject)); var valueProperties = patchType.GetProperties() .Where(p => p.Name == nameof(HiddenPropertyTestObject.Value)) @@ -183,7 +140,7 @@ public void Patch_SetsTheMostDerivedDeclarationOfAHiddenProperty() var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; options.AddSimplePatchConverters(); - var patch = JsonSerializer.Deserialize>("""{ "value": "new" }""", options); + var patch = DeserializePatchObject("""{ "value": "new" }""", options); var patched = patch!.Patch(new HiddenPropertyTestObject { Value = "old" }); // FakeStringConverter proves the converter lookup resolved the hiding property too. @@ -193,7 +150,7 @@ public void Patch_SetsTheMostDerivedDeclarationOfAHiddenProperty() [Test] public void GetPatchClassFor_LeavesOutPropertiesWithoutAPublicSetter() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(NonPublicSetterTestObject)); + var patchType = GetPatchClassFor(typeof(NonPublicSetterTestObject)); Assert.Multiple((Action)(() => { @@ -212,7 +169,7 @@ public void DynamicOptionalsClass_PatchesTypesWithNonPublicSetters() { var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; options.Converters.Add(new OptionalJsonConverterFactory()); - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(NonPublicSetterTestObject)); + var patchType = GetPatchClassFor(typeof(NonPublicSetterTestObject)); var patch = (IPatchObject)JsonSerializer.Deserialize( """{ "name": "New Name" }""", patchType, options)!; @@ -252,18 +209,18 @@ public void Patch_SupportsPropertiesNamedAfterKeywords() Is.EqualTo(new KeywordPropertiesTestObject { @class = "New", @event = "Ignored", Other = "Kept" })); } - private static IPatchObject Deserialize(string json) + private IPatchObject Deserialize(string json) { var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; options.Converters.Add(new OptionalJsonConverterFactory()); - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(T)); + var patchType = GetPatchClassFor(typeof(T)); return (IPatchObject)JsonSerializer.Deserialize(json, patchType, options)!; } [Test] public void GetPatchClassFor_LeavesOutStaticProperties() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(StaticPropertyTestObject)); + var patchType = GetPatchClassFor(typeof(StaticPropertyTestObject)); var patch = DeserializePatch("""{ "Name": "New" }""", patchType); Assert.Multiple((Action)(() => @@ -278,7 +235,7 @@ public void GetPatchClassFor_LeavesOutStaticProperties() [Test] public void GetPatchClassFor_LeavesOutIndexers() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(IndexerTestObject)); + var patchType = GetPatchClassFor(typeof(IndexerTestObject)); var patch = DeserializePatch("""{ "Name": "New" }""", patchType); Assert.Multiple((Action)(() => @@ -290,30 +247,12 @@ public void GetPatchClassFor_LeavesOutIndexers() })); } - [Test] - public void GetPatchClassFor_ThrowsNotSupportedForInternalTypes() - { - var ex = Assert.Throws( - (Action)(() => PatchClassBuilder.Instance.GetPatchClassFor(typeof(InternalTestObject)))); - - Assert.That(ex!.Message, Does.Contain(typeof(InternalTestObject).FullName).And.Contain("must be public")); - } - - [Test] - public void GetPatchClassFor_ThrowsNotSupportedForPrivateNestedTypes() - { - var ex = Assert.Throws( - (Action)(() => PatchClassBuilder.Instance.GetPatchClassFor(typeof(PrivateNestedTestObject)))); - - Assert.That(ex!.Message, Does.Contain(typeof(PrivateNestedTestObject).FullName).And.Contain("must be public")); - } - [Test] public void Patch_PassesConstructorParameters_FromThePatchOrTheTarget() { var optionsWithOptionals = new JsonSerializerOptions(JsonSerializerDefaults.Web); optionsWithOptionals.Converters.Add(new OptionalJsonConverterFactory()); - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(ConstructorTestObject)); + var patchType = GetPatchClassFor(typeof(ConstructorTestObject)); var target = new ConstructorTestObject("Old Name", "Old Secret") { Color = "Red" }; var renamed = (IPatchObject)JsonSerializer.Deserialize( @@ -341,7 +280,7 @@ public void Patch_PrefersTheJsonConstructor_OverTheParameterlessOne() { var optionsWithOptionals = new JsonSerializerOptions(JsonSerializerDefaults.Web); optionsWithOptionals.Converters.Add(new OptionalJsonConverterFactory()); - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(JsonConstructorTestObject)); + var patchType = GetPatchClassFor(typeof(JsonConstructorTestObject)); var target = new JsonConstructorTestObject(1) { Name = "Old Name" }; var patch = (IPatchObject)JsonSerializer.Deserialize( @@ -359,7 +298,7 @@ public void Patch_PrefersTheJsonConstructor_OverTheParameterlessOne() [Test] public void Patch_BindsGetOnlyConstructorProperties_ButLeavesOutPrivateSetters() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(ConstructorAndPrivateSetterTestObject)); + var patchType = GetPatchClassFor(typeof(ConstructorAndPrivateSetterTestObject)); var patch = DeserializePatch("""{ "Name": "New" }""", patchType); var result = patch.Patch(new ConstructorAndPrivateSetterTestObject("Old") { Color = "Red" }); @@ -377,7 +316,7 @@ public void Patch_BindsGetOnlyConstructorProperties_ButLeavesOutPrivateSetters() public void GetPatchClassFor_RejectsAmbiguousConstructors() { Assert.Throws( - (Action)(() => PatchClassBuilder.Instance.GetPatchClassFor(typeof(AmbiguousConstructorTestObject)))); + (Action)(() => GetPatchClassFor(typeof(AmbiguousConstructorTestObject)))); } private static IPatchObject DeserializePatch(string json, Type patchType) @@ -399,7 +338,7 @@ private static JsonSerializerOptions CreatePatchOptions() [Test] public void Patch_PositionalRecord_ReplacesOnlyTheSentProperties() { - var patch = JsonSerializer.Deserialize>( + var patch = DeserializePatchObject( """{ "count": 5 }""", PatchOptions)!; var result = patch.Patch(new PositionalRecordTestObject("Old Name", 1)); @@ -410,7 +349,7 @@ public void Patch_PositionalRecord_ReplacesOnlyTheSentProperties() [Test] public void Patch_Record_KeepsGetOnlyProperties() { - var patch = JsonSerializer.Deserialize>( + var patch = DeserializePatchObject( """{ "name": "New Name" }""", PatchOptions)!; var result = patch.Patch(new PositionalRecordTestObject("Old Name", 1)); @@ -426,7 +365,7 @@ public void Patch_Record_KeepsGetOnlyProperties() [Test] public void Patch_Record_KeepsTheTargetsDerivedRuntimeType() { - var patch = JsonSerializer.Deserialize>( + var patch = DeserializePatchObject( """{ "count": 5 }""", PatchOptions)!; var result = patch.Patch(new DerivedPositionalRecordTestObject("Name", 1, "Extra")); @@ -437,8 +376,8 @@ public void Patch_Record_KeepsTheTargetsDerivedRuntimeType() [Test] public void Patch_RecordWithConstructorSetGetOnlyProperty_KeepsItFromTheTarget() { - var patchType = PatchClassBuilder.Instance.GetPatchClassFor(typeof(ConstructorRecordTestObject)); - var patch = JsonSerializer.Deserialize>( + var patchType = GetPatchClassFor(typeof(ConstructorRecordTestObject)); + var patch = DeserializePatchObject( """{ "name": "New Name" }""", PatchOptions)!; var result = patch.Patch(new ConstructorRecordTestObject("Old Name", "C1")); @@ -454,7 +393,7 @@ public void Patch_RecordWithConstructorSetGetOnlyProperty_KeepsItFromTheTarget() [Test] public void Patch_NonRecordClass_StillBuildsANewInstance() { - var patch = JsonSerializer.Deserialize>( + var patch = DeserializePatchObject( """{ "name": "New Name" }""", PatchOptions)!; var target = new PlainClassTestObject { Name = "Old Name", IgnoredProp = "Kept" }; @@ -468,13 +407,20 @@ public void Patch_NonRecordClass_StillBuildsANewInstance() })); } - private class PrivateNestedTestObject + [Test] + public void Patch_DoesNotReadTheTargetPropertyThePatchSets() { - public string? Name { get; set; } - } -} + var patch = DeserializePatchObject("""{ "name": "New" }""", PatchOptions); + var target = new ReadTrackingTestObject { Name = "Old", Other = "Kept" }; -internal class InternalTestObject -{ - public string? Name { get; set; } + var result = patch.Patch(target); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Name, Is.EqualTo("New")); + Assert.That(result.Other, Is.EqualTo("Kept")); + Assert.That(target.NameReads, Is.Zero, + "The patch sets Name, so the target's Name should not be read."); + })); + } } diff --git a/PTrampert.SimplePatch.Test/PatchClassBuilders.cs b/PTrampert.SimplePatch.Test/PatchClassBuilders.cs new file mode 100644 index 0000000..e8adf12 --- /dev/null +++ b/PTrampert.SimplePatch.Test/PatchClassBuilders.cs @@ -0,0 +1,15 @@ +namespace PTrampert.SimplePatch.Test; + +/// +/// The ways of building a patch class, as fixture arguments for tests that run against each one. +/// +public static class PatchClassBuilders +{ + public static IEnumerable All() + { + yield return new TestFixtureData((Func)PatchClassBuilder.Instance.GetPatchClassFor) + .SetArgDisplayNames("Roslyn"); + yield return new TestFixtureData((Func)EmitPatchClassBuilder.GetPatchClassFor) + .SetArgDisplayNames("Emit"); + } +} diff --git a/PTrampert.SimplePatch.Test/RoslynPatchClassBuilderTest.cs b/PTrampert.SimplePatch.Test/RoslynPatchClassBuilderTest.cs new file mode 100644 index 0000000..ddd59d0 --- /dev/null +++ b/PTrampert.SimplePatch.Test/RoslynPatchClassBuilderTest.cs @@ -0,0 +1,90 @@ +using PTrampert.SimplePatch.Test.TestObjects; + +namespace PTrampert.SimplePatch.Test; + +// Cases specific to the Roslyn builder behind PatchClassBuilder.Instance: its cache, and the +// public-only restriction that comes from compiling C#. Cases it shares with the Emit builder are +// in PatchClassBuilderTest. +public class RoslynPatchClassBuilderTest +{ + [Test] + public void GetPatchClassFor_SharesGeneratedTypesAcrossBuilders() + { + // Deliberately the obsolete constructor: the point of this test is that separately + // constructed builders still share one cache, for as long as that constructor exists. +#pragma warning disable CS0618 + var first = new PatchClassBuilder().GetPatchClassFor(typeof(OptionalsBuilderTestObject)); + var second = new PatchClassBuilder().GetPatchClassFor(typeof(OptionalsBuilderTestObject)); +#pragma warning restore CS0618 + + Assert.Multiple((Action)(() => + { + Assert.That(second, Is.SameAs(first), + "Every builder should resolve a source type to one generated patch type, rather than each emitting its own dynamic assembly for it."); + Assert.That(PatchClassBuilder.Instance.GetPatchClassFor(typeof(OptionalsBuilderTestObject)), Is.SameAs(first)); + })); + } + + [Test] + public void GetPatchClassFor_GeneratesOnceUnderConcurrentFirstUse() + { + const int threadCount = 16; + var sourceType = typeof(ConcurrentFirstUseTestObject); + var results = new Type[threadCount]; + using var barrier = new Barrier(threadCount); + var threads = Enumerable.Range(0, threadCount) + .Select(i => new Thread(() => + { + barrier.SignalAndWait(); + results[i] = PatchClassBuilder.Instance.GetPatchClassFor(sourceType); + })) + .ToList(); + + threads.ForEach(t => t.Start()); + threads.ForEach(t => t.Join()); + + // Every generation loads its own in-memory assembly, so count the loaded types that + // patch the source type: a discarded duplicate would still show up here. + var patchInterface = typeof(IPatchObject<>).MakeGenericType(sourceType); + var generatedTypes = AppDomain.CurrentDomain.GetAssemblies() + .Where(a => !a.IsDynamic && string.IsNullOrEmpty(a.Location)) + .SelectMany(a => a.GetTypes()) + .Where(patchInterface.IsAssignableFrom) + .ToList(); + + Assert.Multiple((Action)(() => + { + Assert.That(results, Has.All.SameAs(results[0])); + Assert.That(generatedTypes, Is.EquivalentTo(new[] { results[0] }), + "Concurrent first use should generate the patch class once, not once per racing thread."); + })); + } + + [Test] + public void GetPatchClassFor_ThrowsNotSupportedForInternalTypes() + { + var ex = Assert.Throws( + (Action)(() => PatchClassBuilder.Instance.GetPatchClassFor(typeof(InternalTestObject)))); + + Assert.That(ex!.Message, Does.Contain(typeof(InternalTestObject).FullName).And.Contain("must be public")); + } + + [Test] + public void GetPatchClassFor_ThrowsNotSupportedForPrivateNestedTypes() + { + var ex = Assert.Throws( + (Action)(() => PatchClassBuilder.Instance.GetPatchClassFor(typeof(PrivateNestedTestObject)))); + + Assert.That(ex!.Message, Does.Contain(typeof(PrivateNestedTestObject).FullName).And.Contain("must be public")); + } + + private class PrivateNestedTestObject + { + public string? Name { get; set; } + } +} + +internal class InternalTestObject +{ + public string? Name { get; set; } +} diff --git a/PTrampert.SimplePatch.Test/TestObjects/ReadTrackingTestObject.cs b/PTrampert.SimplePatch.Test/TestObjects/ReadTrackingTestObject.cs new file mode 100644 index 0000000..f0af7c2 --- /dev/null +++ b/PTrampert.SimplePatch.Test/TestObjects/ReadTrackingTestObject.cs @@ -0,0 +1,23 @@ +namespace PTrampert.SimplePatch.Test.TestObjects; + +// Counts reads of Name, to show the patch class only reads the target's value when the patch +// leaves the property out. +public class ReadTrackingTestObject +{ + private string? name; + + public string? Name + { + get + { + NameReads++; + return name; + } + set => name = value; + } + + public string? Other { get; set; } + + // A private setter keeps this out of the patch class, so patching never reads it either. + public int NameReads { get; private set; } +} diff --git a/PTrampert.SimplePatch/EmitPatchClassBuilder.cs b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs new file mode 100644 index 0000000..34e590b --- /dev/null +++ b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs @@ -0,0 +1,349 @@ +using System.Collections.Concurrent; +using System.Reflection; +using System.Reflection.Emit; +using System.Runtime.Loader; +using System.Text.Json.Serialization; + +namespace PTrampert.SimplePatch; + +/// +/// Generates classes that implement by emitting IL with +/// Reflection.Emit, rather than compiling C# with Roslyn as does. +/// Unlike that builder, it supports source types that aren't public. +/// +/// +/// Roslyn checks accessibility when it compiles, so the separate assembly it builds can't name a +/// non-public type. IL has no compile-time accessibility check, and the runtime skips its own +/// checks for the assemblies a dynamic assembly lists in [IgnoresAccessChecksTo]. The class +/// emitted here has the same shape as the one compiles: both are +/// built from . +/// +internal static class EmitPatchClassBuilder +{ + private const string GlobalNamespaceFallback = "PTrampert.SimplePatch.Generated"; + + // The runtime recognizes this attribute by its full name alone, and the BCL doesn't ship a + // public one, so each dynamic assembly defines its own. + private const string IgnoresAccessChecksToAttributeName = + "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute"; + + // Separate from PatchClassBuilder's cache, so each builder hands out only the types it built. + // Lazy for the same reason as there: concurrent first use should emit one assembly, not one per thread. + private static readonly ConcurrentDictionary> PatchClasses = new(); + + /// + /// Gets or creates the patch class for . + /// + /// + /// can't model , or one of its patched + /// properties has no getter. + /// + public static Type GetPatchClassFor(Type type) + { + return PatchClasses.GetOrAdd(type, t => new Lazy(() => CreatePatchClass(t))).Value; + } + + private static Type CreatePatchClass(Type type) + { + var model = PatchClassModel.For(type); + + // Each source type gets its own assembly, so the patch type's name can't collide with + // another and needs neither a random suffix nor cleaning up into a C# identifier. + // Load it where the source type lives, as PatchClassBuilder does with its compiled assembly. + using var contextScope = AssemblyLoadContext.EnterContextualReflection(type.Assembly); + var assembly = AssemblyBuilder.DefineDynamicAssembly( + new AssemblyName($"PTrampert.SimplePatch.Emitted.{Guid.NewGuid():N}"), AssemblyBuilderAccess.Run); + var module = assembly.DefineDynamicModule(assembly.GetName().Name!); + + // The grant has to be in place before the patch type is created, because the runtime checks + // access when it loads the type (the interface it implements names the source type). + var ignoresAccessChecksTo = DefineIgnoresAccessChecksToAttribute(module); + foreach (var referencedAssembly in GetReferencedAssemblies(model)) + { + assembly.SetCustomAttribute(new CustomAttributeBuilder( + ignoresAccessChecksTo, [referencedAssembly.GetName().Name!])); + } + + var namespaceRoot = string.IsNullOrEmpty(type.Namespace) ? GlobalNamespaceFallback : type.Namespace; + var patchInterface = typeof(IPatchObject<>).MakeGenericType(type); + var typeBuilder = module.DefineType( + $"{namespaceRoot}.Optionals.{type.Name}_Optionals", + TypeAttributes.Public | TypeAttributes.Sealed | TypeAttributes.Class, + typeof(object), + [patchInterface]); + typeBuilder.DefineDefaultConstructor(MethodAttributes.Public); + + var fields = new Dictionary(); + foreach (var optionalProperty in model.OptionalProperties) + { + fields[optionalProperty.Property] = DefineOptionalProperty(typeBuilder, model, optionalProperty); + } + + DefinePatchMethod(typeBuilder, patchInterface, model, fields); + + return typeBuilder.CreateType(); + } + + /// + /// Defines an IgnoresAccessChecksToAttribute(string assemblyName) in + /// and returns its constructor. + /// + private static ConstructorInfo DefineIgnoresAccessChecksToAttribute(ModuleBuilder module) + { + var attributeBuilder = module.DefineType( + IgnoresAccessChecksToAttributeName, + TypeAttributes.NotPublic | TypeAttributes.Sealed | TypeAttributes.Class, + typeof(Attribute)); + attributeBuilder.SetCustomAttribute(new CustomAttributeBuilder( + typeof(AttributeUsageAttribute).GetConstructor([typeof(AttributeTargets)])!, + [AttributeTargets.Assembly], + [typeof(AttributeUsageAttribute).GetProperty(nameof(AttributeUsageAttribute.AllowMultiple))!], + [true])); + + var constructor = attributeBuilder.DefineConstructor( + MethodAttributes.Public, CallingConventions.Standard, [typeof(string)]); + var il = constructor.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Call, typeof(Attribute).GetConstructor( + BindingFlags.NonPublic | BindingFlags.Instance, Type.EmptyTypes)!); + il.Emit(OpCodes.Ret); + + return attributeBuilder.CreateType().GetConstructor([typeof(string)])!; + } + + /// + /// The assemblies whose types the patch class names in its signatures or its Patch + /// method: the source type and the types it is built from, and every patched property's type + /// ( names it) and declaring type. + /// + private static HashSet GetReferencedAssemblies(PatchClassModel model) + { + var assemblies = new HashSet(); + var visited = new HashSet(); + + void Visit(Type? type) + { + if (type == null || !visited.Add(type)) + { + return; + } + + assemblies.Add(type.Assembly); + Visit(type.DeclaringType); + if (type.HasElementType) + { + Visit(type.GetElementType()); + } + + if (type.IsGenericType) + { + foreach (var argument in type.GetGenericArguments()) + { + Visit(argument); + } + } + } + + Visit(model.SourceType); + foreach (var property in model.OptionalProperties.Select(p => p.Property) + .Concat(model.ConstructorProperties) + .Concat(model.IgnoredProperties)) + { + Visit(property.PropertyType); + Visit(property.DeclaringType); + } + + return assemblies; + } + + /// + /// Defines the backing field and property for one source property, + /// with the attributes gives it, and returns the field. + /// + private static FieldBuilder DefineOptionalProperty( + TypeBuilder typeBuilder, PatchClassModel model, OptionalPropertyModel optionalProperty) + { + var property = optionalProperty.Property; + var optionalType = typeof(Optional<>).MakeGenericType(property.PropertyType); + var field = typeBuilder.DefineField($"_{property.Name}", optionalType, FieldAttributes.Private); + var propertyBuilder = typeBuilder.DefineProperty(property.Name, PropertyAttributes.None, optionalType, null); + const MethodAttributes accessorAttributes = + MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.SpecialName; + + var getter = typeBuilder.DefineMethod($"get_{property.Name}", accessorAttributes, optionalType, Type.EmptyTypes); + var il = getter.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldfld, field); + il.Emit(OpCodes.Ret); + propertyBuilder.SetGetMethod(getter); + + var setter = typeBuilder.DefineMethod($"set_{property.Name}", accessorAttributes, typeof(void), [optionalType]); + il = setter.GetILGenerator(); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldarg_1); + il.Emit(OpCodes.Stfld, field); + il.Emit(OpCodes.Ret); + propertyBuilder.SetSetMethod(setter); + + if (optionalProperty.HasConverter) + { + propertyBuilder.SetCustomAttribute(new CustomAttributeBuilder( + typeof(OptionalConverterAttribute).GetConstructor([typeof(Type), typeof(string)])!, + [model.SourceType, property.Name])); + } + + if (optionalProperty.JsonPropertyName is { } jsonPropertyName) + { + propertyBuilder.SetCustomAttribute(new CustomAttributeBuilder( + typeof(JsonPropertyNameAttribute).GetConstructor([typeof(string)])!, + [jsonPropertyName])); + } + + foreach (var validator in optionalProperty.Validators) + { + propertyBuilder.SetCustomAttribute(new CustomAttributeBuilder( + typeof(OptionalValidationAttribute).GetConstructor([typeof(Type), typeof(int)])!, + [validator.ValidatorType, validator.Index])); + } + + return field; + } + + /// + /// Emits Patch(T target). It builds the result as 's C# + /// does: a with clone for a record, otherwise the chosen constructor followed by the + /// setters for the remaining properties. + /// + private static void DefinePatchMethod( + TypeBuilder typeBuilder, Type patchInterface, PatchClassModel model, Dictionary fields) + { + var type = model.SourceType; + var method = typeBuilder.DefineMethod( + nameof(IPatchObject.Patch), + MethodAttributes.Public | MethodAttributes.Final | MethodAttributes.HideBySig + | MethodAttributes.NewSlot | MethodAttributes.Virtual, + type, + [type]); + method.DefineParameter(1, ParameterAttributes.None, "target"); + typeBuilder.DefineMethodOverride(method, patchInterface.GetMethod(nameof(IPatchObject.Patch))!); + var il = method.GetILGenerator(); + + // As in the C# PatchClassBuilder generates, the clone made by `with` already carries the + // ignored properties over, so only a newly constructed instance has to copy them. + var assigned = model.OptionalProperties.Select(p => p.Property) + .Concat(model.IsRecord ? [] : model.IgnoredProperties) + .Except(model.ConstructorProperties) + .ToList(); + + if (model.IsRecord) + { + // target with { P = ... }: clone, then call each init accessor on the clone. + il.Emit(OpCodes.Ldarg_1); + il.Emit(OpCodes.Callvirt, type.GetMethod("$", BindingFlags.Public | BindingFlags.Instance)!); + il.Emit(OpCodes.Castclass, type); + foreach (var property in assigned) + { + il.Emit(OpCodes.Dup); + EmitPatchedValue(il, model, fields, property); + il.Emit(OpCodes.Callvirt, GetSetter(property)); + } + } + else if (!type.IsValueType) + { + // new T(...) { P = ... } + foreach (var property in model.ConstructorProperties) + { + EmitPatchedValue(il, model, fields, property); + } + + il.Emit(OpCodes.Newobj, model.Constructor!); + foreach (var property in assigned) + { + il.Emit(OpCodes.Dup); + EmitPatchedValue(il, model, fields, property); + il.Emit(OpCodes.Callvirt, GetSetter(property)); + } + } + else + { + // A struct is built in a local, because its setters need the address of the instance. + var result = il.DeclareLocal(type); + if (model.Constructor is { } constructor) + { + foreach (var property in model.ConstructorProperties) + { + EmitPatchedValue(il, model, fields, property); + } + + il.Emit(OpCodes.Newobj, constructor); + il.Emit(OpCodes.Stloc, result); + } + else + { + il.Emit(OpCodes.Ldloca, result); + il.Emit(OpCodes.Initobj, type); + } + + foreach (var property in assigned) + { + il.Emit(OpCodes.Ldloca, result); + EmitPatchedValue(il, model, fields, property); + il.Emit(OpCodes.Call, GetSetter(property)); + } + + il.Emit(OpCodes.Ldloc, result); + } + + il.Emit(OpCodes.Ret); + } + + /// + /// Pushes the patched value of : + /// _P.HasValue ? _P.Value : target.P, or just target.P if the patch class has no + /// for it. target.P is only read when the patch doesn't set P. + /// + private static void EmitPatchedValue( + ILGenerator il, PatchClassModel model, Dictionary fields, PropertyInfo property) + { + if (!fields.TryGetValue(property, out var field)) + { + EmitTargetValue(il, model.SourceType, property); + return; + } + + var useTarget = il.DefineLabel(); + var end = il.DefineLabel(); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldflda, field); + il.Emit(OpCodes.Call, field.FieldType.GetProperty(nameof(Optional.HasValue))!.GetMethod!); + il.Emit(OpCodes.Brfalse, useTarget); + il.Emit(OpCodes.Ldarg_0); + il.Emit(OpCodes.Ldflda, field); + il.Emit(OpCodes.Call, field.FieldType.GetProperty(nameof(Optional.Value))!.GetMethod!); + il.Emit(OpCodes.Br, end); + il.MarkLabel(useTarget); + EmitTargetValue(il, model.SourceType, property); + il.MarkLabel(end); + } + + /// Pushes target.P. + private static void EmitTargetValue(ILGenerator il, Type type, PropertyInfo property) + { + var getter = property.GetMethod ?? throw new NotSupportedException( + $"Cannot create a patch class for '{type.FullName}' because property '{property.Name}' has no getter, " + + "so a patch that leaves it out can't keep the target's value."); + if (type.IsValueType) + { + il.Emit(OpCodes.Ldarga_S, (byte)1); + il.Emit(OpCodes.Call, getter); + } + else + { + il.Emit(OpCodes.Ldarg_1); + il.Emit(OpCodes.Callvirt, getter); + } + } + + // PatchClassModel only assigns properties through a public setter or init accessor. + private static MethodInfo GetSetter(PropertyInfo property) => property.SetMethod!; +} From 4218be671b23b2668e074335ca9248fc14fbb9d4 Mon Sep 17 00:00:00 2001 From: Paul Trampert Date: Thu, 1 Oct 2026 13:14:20 -0400 Subject: [PATCH 2/5] Test the Emit builder with non-public source types Covers an internal class (explicit null, [JsonPropertyName], init accessor, [Range] validation, an internal [JsonConverter]), a private nested class, a private positional record, an internal primary-constructor class, an internal struct, and an internal property type from a second assembly. The second assembly is a new, unpacked test-helper project, PTrampert.SimplePatch.Test.External. Co-Authored-By: Claude Opus 5.5 --- .../ExternalInternalColor.cs | 9 ++ ...PTrampert.SimplePatch.Test.External.csproj | 16 ++ .../EmitPatchClassBuilderTest.cs | 143 ++++++++++++++++++ .../PTrampert.SimplePatch.Test.csproj | 1 + .../TestObjects/NonPublicTestObjects.cs | 52 +++++++ PTrampert.SimplePatch.sln | 14 ++ 6 files changed, 235 insertions(+) create mode 100644 PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs create mode 100644 PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj create mode 100644 PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs create mode 100644 PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs diff --git a/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs b/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs new file mode 100644 index 0000000..7ac14a3 --- /dev/null +++ b/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs @@ -0,0 +1,9 @@ +namespace PTrampert.SimplePatch.Test.External; + +// Internal, so a patch class that names it needs an access grant for this assembly as well as for +// the assembly of the source type that uses it. +internal enum ExternalInternalColor +{ + Red, + Blue, +} diff --git a/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj b/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj new file mode 100644 index 0000000..e6ffff4 --- /dev/null +++ b/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj @@ -0,0 +1,16 @@ + + + + + net8.0 + enable + enable + false + + + + + + + diff --git a/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs b/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs new file mode 100644 index 0000000..4548b28 --- /dev/null +++ b/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs @@ -0,0 +1,143 @@ +using System.ComponentModel.DataAnnotations; +using System.Text.Json; +using PTrampert.SimplePatch.Test.External; +using PTrampert.SimplePatch.Test.TestObjects; + +namespace PTrampert.SimplePatch.Test; + +// Cases only the Emit builder supports: source types that aren't public. Cases it shares with the +// Roslyn builder are in PatchClassBuilderTest. +public class EmitPatchClassBuilderTest +{ + private static readonly JsonSerializerOptions Options = CreateOptions(); + + private static JsonSerializerOptions CreateOptions() + { + var options = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase }; + options.AddSimplePatchConverters(); + return options; + } + + private static IPatchObject Deserialize(string json) => + (IPatchObject)JsonSerializer.Deserialize(json, EmitPatchClassBuilder.GetPatchClassFor(typeof(T)), Options)!; + + [Test] + public void GetPatchClassFor_ReturnsTheSameTypeEachTime() + { + var first = EmitPatchClassBuilder.GetPatchClassFor(typeof(InternalClassTestObject)); + + Assert.That(EmitPatchClassBuilder.GetPatchClassFor(typeof(InternalClassTestObject)), Is.SameAs(first)); + } + + [Test] + public void Patch_InternalClass_AppliesSetAndExplicitNullValues() + { + var patch = Deserialize( + """{ "display_name": null, "initOnly": "New", "converted": "value" }"""); + + var result = patch.Patch(new InternalClassTestObject + { + Name = "Old", InitOnly = "Old", Rating = 3, Converted = "Old", + }); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Name, Is.Null, "An explicit null should be applied."); + Assert.That(result.InitOnly, Is.EqualTo("New")); + Assert.That(result.Rating, Is.EqualTo(3), "A property the patch leaves out should keep its value."); + Assert.That(result.Converted, Is.EqualTo("Internal:value"), + "The internal [JsonConverter] on the source property should be used."); + })); + } + + [Test] + public void Validate_InternalClass_RunsTheSourceValidatorsOnSetProperties() + { + var invalid = Deserialize("""{ "rating": 20 }"""); + var omitted = Deserialize("""{ "initOnly": "x" }"""); + var invalidResults = new List(); + var omittedResults = new List(); + + var invalidIsValid = Validator.TryValidateObject(invalid, new ValidationContext(invalid), invalidResults, true); + var omittedIsValid = Validator.TryValidateObject(omitted, new ValidationContext(omitted), omittedResults, true); + + Assert.Multiple((Action)(() => + { + Assert.That(invalidIsValid, Is.False); + Assert.That(invalidResults.Select(r => r.ErrorMessage), + Is.EqualTo(new[] { "The field Rating must be between 1 and 10." })); + Assert.That(omittedIsValid, Is.True, "Validation should skip a property the patch leaves out."); + })); + } + + [Test] + public void Patch_PrivateNestedClass() + { + var patch = Deserialize("""{ "name": "New" }"""); + + var result = patch.Patch(new PrivateNestedTestObject { Name = "Old", Other = "Kept" }); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Name, Is.EqualTo("New")); + Assert.That(result.Other, Is.EqualTo("Kept")); + })); + } + + [Test] + public void Patch_PrivatePositionalRecord_KeepsTheTargetsOtherValues() + { + var patch = Deserialize("""{ "count": 5 }"""); + + var result = patch.Patch(new PrivatePositionalRecordTestObject("Old", 1)); + + Assert.That(result, Is.EqualTo(new PrivatePositionalRecordTestObject("Old", 5))); + } + + [Test] + public void Patch_InternalPrimaryConstructorClass_BindsGetOnlyPropertiesThroughTheConstructor() + { + var patch = Deserialize("""{ "name": "New" }"""); + + var result = patch.Patch(new InternalPrimaryConstructorTestObject("Old") { Color = "Red" }); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Name, Is.EqualTo("New")); + Assert.That(result.Color, Is.EqualTo("Red")); + })); + } + + [Test] + public void Patch_InternalStruct() + { + var patch = Deserialize("""{ "count": 2 }"""); + + var result = patch.Patch(new InternalStructTestObject { Name = "Kept", Count = 1 }); + + Assert.That(result, Is.EqualTo(new InternalStructTestObject { Name = "Kept", Count = 2 })); + } + + [Test] + public void Patch_PropertyOfANonPublicTypeFromAnotherAssembly() + { + var patch = Deserialize("""{ "color": 1 }"""); + + var result = patch.Patch(new ExternalPropertyTypeTestObject { Color = ExternalInternalColor.Red, Other = "Kept" }); + + Assert.Multiple((Action)(() => + { + Assert.That(result.Color, Is.EqualTo(ExternalInternalColor.Blue)); + Assert.That(result.Other, Is.EqualTo("Kept")); + })); + } + + private class PrivateNestedTestObject + { + public string? Name { get; set; } + + public string? Other { get; set; } + } + + private record PrivatePositionalRecordTestObject(string Name, int Count); +} diff --git a/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj b/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj index bbc0137..071029b 100644 --- a/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj +++ b/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj @@ -22,6 +22,7 @@ + diff --git a/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs b/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs new file mode 100644 index 0000000..f422a6b --- /dev/null +++ b/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs @@ -0,0 +1,52 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using PTrampert.SimplePatch.Test.External; + +namespace PTrampert.SimplePatch.Test.TestObjects; + +// Non-public patch source types, which only the Emit builder supports. Private nested ones have to +// be nested in the test fixture itself, so they are in EmitPatchClassBuilderTest. + +internal class InternalClassTestObject +{ + [JsonPropertyName("display_name")] + public string? Name { get; set; } + + public string? InitOnly { get; init; } + + [System.ComponentModel.DataAnnotations.Range(1, 10)] + public int Rating { get; set; } + + [JsonConverter(typeof(InternalFakeStringConverter))] + public string? Converted { get; set; } +} + +internal class InternalFakeStringConverter : JsonConverter +{ + public override string Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => + $"Internal:{reader.GetString()}"; + + public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => + writer.WriteStringValue(value); +} + +internal class InternalPrimaryConstructorTestObject(string name) +{ + public string Name { get; } = name; + + public string? Color { get; set; } +} + +internal struct InternalStructTestObject +{ + public string? Name { get; set; } + + public int Count { get; set; } +} + +internal class ExternalPropertyTypeTestObject +{ + public ExternalInternalColor Color { get; set; } + + public string? Other { get; set; } +} diff --git a/PTrampert.SimplePatch.sln b/PTrampert.SimplePatch.sln index 04a7853..9382da1 100644 --- a/PTrampert.SimplePatch.sln +++ b/PTrampert.SimplePatch.sln @@ -16,6 +16,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PTrampert.SimplePatch.OpenA EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PTrampert.SimplePatch.Schema", "PTrampert.SimplePatch.Schema\PTrampert.SimplePatch.Schema.csproj", "{B887D919-9F17-40EA-B6AC-16A4AE157C6E}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PTrampert.SimplePatch.Test.External", "PTrampert.SimplePatch.Test.External\PTrampert.SimplePatch.Test.External.csproj", "{30E6042F-FA0C-410E-9751-406483D739AF}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -122,6 +124,18 @@ Global {B887D919-9F17-40EA-B6AC-16A4AE157C6E}.Release|x64.Build.0 = Release|Any CPU {B887D919-9F17-40EA-B6AC-16A4AE157C6E}.Release|x86.ActiveCfg = Release|Any CPU {B887D919-9F17-40EA-B6AC-16A4AE157C6E}.Release|x86.Build.0 = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|Any CPU.Build.0 = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|x64.ActiveCfg = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|x64.Build.0 = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|x86.ActiveCfg = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Debug|x86.Build.0 = Debug|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|Any CPU.ActiveCfg = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|Any CPU.Build.0 = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|x64.ActiveCfg = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|x64.Build.0 = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|x86.ActiveCfg = Release|Any CPU + {30E6042F-FA0C-410E-9751-406483D739AF}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE From 44e5eed13f4c51adabf4f843584efc2b38e210e0 Mon Sep 17 00:00:00 2001 From: Paul Trampert Date: Thu, 1 Oct 2026 13:14:20 -0400 Subject: [PATCH 3/5] Describe the Emit builder and the test-helper project in AGENTS.md Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 2139d10..61da0b8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,7 @@ omitted apart from one that was explicitly set to `null` or to a value. A contro | `PTrampert.SimplePatch.Swashbuckle` | net8.0 | Swashbuckle schema filter | | `PTrampert.SimplePatch.OpenApi` | net10.0 | `Microsoft.AspNetCore.OpenApi` schema transformer. It is net10.0 only because it needs `GetOrCreateSchemaAsync`. | | `*.Test` | match their subject | NUnit test projects, one per shipped package (except `Schema`, which the integration tests cover) | +| `PTrampert.SimplePatch.Test.External` | net8.0 | A second assembly for the core tests, holding non-public types they use from another assembly. Not packed. | | `PTrampert.SimplePatch.Sample` | net8.0 | Sample web API, not packed | Docs are built with docfx (`docfx.json`, `index.md`, `docs/`). The API reference is generated @@ -37,6 +38,10 @@ public face of the library on nuget.org. - Source property attributes are carried over: `[JsonConverter]` becomes `[OptionalConverter]`, `[JsonPropertyName]` is copied, and each `ValidationAttribute` becomes an `[OptionalValidation(type, index)]` that runs only when the property is present. +- The internal `EmitPatchClassBuilder` builds the same class from the same `PatchClassModel` with + Reflection.Emit, one dynamic assembly per source type. The assembly declares + `[IgnoresAccessChecksTo]` for every assembly the class names, so it also supports non-public + source types. `PatchClassBuilderTest` runs against both builders. - `JsonOptionsExtensions.AddSimplePatchConverters` registers `OptionalJsonConverterFactory` and `PatchJsonConverterFactory`. - The OpenAPI packages build the patch schema from the **source model's** schema, not from the From e412f5e45852c680efdc85fe040ada53ca4f8895 Mon Sep 17 00:00:00 2001 From: Paul Trampert Date: Fri, 2 Oct 2026 18:57:15 -0400 Subject: [PATCH 4/5] Link the IgnoresAccessChecksTo docs issue and runtime test from the code comment Co-Authored-By: Claude Opus 5.5 --- PTrampert.SimplePatch/EmitPatchClassBuilder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/PTrampert.SimplePatch/EmitPatchClassBuilder.cs b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs index 34e590b..3c5085d 100644 --- a/PTrampert.SimplePatch/EmitPatchClassBuilder.cs +++ b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs @@ -23,7 +23,10 @@ internal static class EmitPatchClassBuilder private const string GlobalNamespaceFallback = "PTrampert.SimplePatch.Generated"; // The runtime recognizes this attribute by its full name alone, and the BCL doesn't ship a - // public one, so each dynamic assembly defines its own. + // public one, so each dynamic assembly defines its own. It has no Microsoft Learn page + // (https://github.com/dotnet/runtime/issues/37875), but the runtime's own DispatchProxy relies + // on it the same way, and dotnet/runtime tests it in + // src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs. private const string IgnoresAccessChecksToAttributeName = "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute"; From 78593922ac42f37e6b9de97398a788beae75ef45 Mon Sep 17 00:00:00 2001 From: Paul Trampert Date: Sat, 3 Oct 2026 18:14:48 -0400 Subject: [PATCH 5/5] Require InternalsVisibleTo instead of IgnoresAccessChecksTo IgnoresAccessChecksToAttribute isn't officially supported (dotnet/runtime#37875), so take Castle DynamicProxy's approach: every emitted assembly has the fixed name PTrampert.SimplePatch.Emitted, and an assembly whose internal types are patched grants it InternalsVisibleTo. Before emitting, check that every type and getter the patch class uses is accessible, and throw NotSupportedException naming the assembly that needs the grant. Private nested types and private getters are no longer supported, because InternalsVisibleTo doesn't reach them. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 7 +- .../ExternalInternalColor.cs | 4 +- ...PTrampert.SimplePatch.Test.External.csproj | 3 +- .../EmitPatchClassBuilderTest.cs | 50 +++--- .../PTrampert.SimplePatch.Test.csproj | 5 + .../TestObjects/NonPublicTestObjects.cs | 12 ++ .../EmitPatchClassBuilder.cs | 151 +++++++++++------- 7 files changed, 140 insertions(+), 92 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 61da0b8..948d102 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,9 +39,10 @@ public face of the library on nuget.org. `[OptionalConverter]`, `[JsonPropertyName]` is copied, and each `ValidationAttribute` becomes an `[OptionalValidation(type, index)]` that runs only when the property is present. - The internal `EmitPatchClassBuilder` builds the same class from the same `PatchClassModel` with - Reflection.Emit, one dynamic assembly per source type. The assembly declares - `[IgnoresAccessChecksTo]` for every assembly the class names, so it also supports non-public - source types. `PatchClassBuilderTest` runs against both builders. + Reflection.Emit, one dynamic assembly per source type. Every such assembly is named + `PTrampert.SimplePatch.Emitted`, so it also supports internal source types whose assembly declares + `[InternalsVisibleTo("PTrampert.SimplePatch.Emitted")]`, as Castle DynamicProxy does. Private + nested types aren't supported. `PatchClassBuilderTest` runs against both builders. - `JsonOptionsExtensions.AddSimplePatchConverters` registers `OptionalJsonConverterFactory` and `PatchJsonConverterFactory`. - The OpenAPI packages build the patch schema from the **source model's** schema, not from the diff --git a/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs b/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs index 7ac14a3..ab8540b 100644 --- a/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs +++ b/PTrampert.SimplePatch.Test.External/ExternalInternalColor.cs @@ -1,7 +1,7 @@ namespace PTrampert.SimplePatch.Test.External; -// Internal, so a patch class that names it needs an access grant for this assembly as well as for -// the assembly of the source type that uses it. +// Internal, and this assembly grants no [InternalsVisibleTo] to EmitPatchClassBuilder's generated +// assemblies, so a patch class can't name it even when the source type's own assembly grants one. internal enum ExternalInternalColor { Red, diff --git a/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj b/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj index e6ffff4..48a855a 100644 --- a/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj +++ b/PTrampert.SimplePatch.Test.External/PTrampert.SimplePatch.Test.External.csproj @@ -1,7 +1,8 @@ + non-public property type from an assembly that, unlike the test project, grants no access + to the generated patch classes. --> net8.0 enable diff --git a/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs b/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs index 4548b28..4c4300b 100644 --- a/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs +++ b/PTrampert.SimplePatch.Test/EmitPatchClassBuilderTest.cs @@ -5,8 +5,9 @@ namespace PTrampert.SimplePatch.Test; -// Cases only the Emit builder supports: source types that aren't public. Cases it shares with the -// Roslyn builder are in PatchClassBuilderTest. +// Cases only the Emit builder supports: internal source types, which this assembly grants to the +// generated assemblies, and the errors for types it can't reach. Cases it shares with the Roslyn +// builder are in PatchClassBuilderTest. public class EmitPatchClassBuilderTest { private static readonly JsonSerializerOptions Options = CreateOptions(); @@ -71,27 +72,31 @@ public void Validate_InternalClass_RunsTheSourceValidatorsOnSetProperties() } [Test] - public void Patch_PrivateNestedClass() + public void Patch_InternalGenericArgument() { - var patch = Deserialize("""{ "name": "New" }"""); + var patch = Deserialize("""{ "items": [{ "count": 2 }] }"""); - var result = patch.Patch(new PrivateNestedTestObject { Name = "Old", Other = "Kept" }); + var result = patch.Patch(new InternalListPropertyTestObject()); - Assert.Multiple((Action)(() => - { - Assert.That(result.Name, Is.EqualTo("New")); - Assert.That(result.Other, Is.EqualTo("Kept")); - })); + Assert.That(result.Items, Is.EqualTo(new[] { new InternalStructTestObject { Count = 2 } })); } - [Test] - public void Patch_PrivatePositionalRecord_KeepsTheTargetsOtherValues() + [TestCase(typeof(PrivateNestedTestObject))] + [TestCase(typeof(PrivatePositionalRecordTestObject))] + public void GetPatchClassFor_PrivateNestedType_Throws(Type type) { - var patch = Deserialize("""{ "count": 5 }"""); + var ex = Assert.Throws(() => EmitPatchClassBuilder.GetPatchClassFor(type)); - var result = patch.Patch(new PrivatePositionalRecordTestObject("Old", 1)); + Assert.That(ex!.Message, Does.Contain($"'{type.FullName}', which the generated assembly can't access")); + } - Assert.That(result, Is.EqualTo(new PrivatePositionalRecordTestObject("Old", 5))); + [Test] + public void GetPatchClassFor_PrivateGetter_Throws() + { + var ex = Assert.Throws( + () => EmitPatchClassBuilder.GetPatchClassFor(typeof(PrivateGetterTestObject))); + + Assert.That(ex!.Message, Does.Contain("the getter of 'Name' isn't accessible")); } [Test] @@ -119,17 +124,14 @@ public void Patch_InternalStruct() } [Test] - public void Patch_PropertyOfANonPublicTypeFromAnotherAssembly() + public void GetPatchClassFor_NonPublicPropertyTypeFromAnAssemblyWithoutTheGrant_ThrowsNamingThatAssembly() { - var patch = Deserialize("""{ "color": 1 }"""); - - var result = patch.Patch(new ExternalPropertyTypeTestObject { Color = ExternalInternalColor.Red, Other = "Kept" }); + var ex = Assert.Throws( + () => EmitPatchClassBuilder.GetPatchClassFor(typeof(ExternalPropertyTypeTestObject))); - Assert.Multiple((Action)(() => - { - Assert.That(result.Color, Is.EqualTo(ExternalInternalColor.Blue)); - Assert.That(result.Other, Is.EqualTo("Kept")); - })); + Assert.That(ex!.Message, Does.Contain(typeof(ExternalInternalColor).FullName) + .And.Contain($"[assembly: InternalsVisibleTo(\"{EmitPatchClassBuilder.AssemblyName}\")]") + .And.Contain("'PTrampert.SimplePatch.Test.External'")); } private class PrivateNestedTestObject diff --git a/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj b/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj index 071029b..e2b5554 100644 --- a/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj +++ b/PTrampert.SimplePatch.Test/PTrampert.SimplePatch.Test.csproj @@ -20,6 +20,11 @@ + + + + + diff --git a/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs b/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs index f422a6b..20d0662 100644 --- a/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs +++ b/PTrampert.SimplePatch.Test/TestObjects/NonPublicTestObjects.cs @@ -50,3 +50,15 @@ internal class ExternalPropertyTypeTestObject public string? Other { get; set; } } + +internal class InternalListPropertyTestObject +{ + public List? Items { get; set; } +} + +internal class PrivateGetterTestObject +{ + public string? Name { private get; set; } + + public string? Other { get; set; } +} diff --git a/PTrampert.SimplePatch/EmitPatchClassBuilder.cs b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs index 3c5085d..1a3315d 100644 --- a/PTrampert.SimplePatch/EmitPatchClassBuilder.cs +++ b/PTrampert.SimplePatch/EmitPatchClassBuilder.cs @@ -1,6 +1,7 @@ using System.Collections.Concurrent; using System.Reflection; using System.Reflection.Emit; +using System.Runtime.CompilerServices; using System.Runtime.Loader; using System.Text.Json.Serialization; @@ -9,26 +10,29 @@ namespace PTrampert.SimplePatch; /// /// Generates classes that implement by emitting IL with /// Reflection.Emit, rather than compiling C# with Roslyn as does. -/// Unlike that builder, it supports source types that aren't public. +/// Unlike that builder, it supports internal source types, provided their assembly grants +/// [InternalsVisibleTo] to . /// /// /// Roslyn checks accessibility when it compiles, so the separate assembly it builds can't name a -/// non-public type. IL has no compile-time accessibility check, and the runtime skips its own -/// checks for the assemblies a dynamic assembly lists in [IgnoresAccessChecksTo]. The class -/// emitted here has the same shape as the one compiles: both are -/// built from . +/// non-public type. The runtime still checks access when it loads emitted IL, so the emitted +/// assembly needs a grant too. It gets one the way Castle DynamicProxy's does: every assembly it +/// emits has the same fixed name, which the consuming assembly names in +/// [InternalsVisibleTo]. The undocumented [IgnoresAccessChecksTo] would need no +/// grant, but it isn't officially supported (https://github.com/dotnet/runtime/issues/37875). +/// [InternalsVisibleTo] doesn't reach private or protected members, so private +/// nested source types aren't supported. The class emitted here has the same shape as the one +/// compiles: both are built from . /// internal static class EmitPatchClassBuilder { - private const string GlobalNamespaceFallback = "PTrampert.SimplePatch.Generated"; + /// + /// The name of every assembly this builder emits. An assembly whose internal types are patched + /// declares [assembly: InternalsVisibleTo("PTrampert.SimplePatch.Emitted")]. + /// + public const string AssemblyName = "PTrampert.SimplePatch.Emitted"; - // The runtime recognizes this attribute by its full name alone, and the BCL doesn't ship a - // public one, so each dynamic assembly defines its own. It has no Microsoft Learn page - // (https://github.com/dotnet/runtime/issues/37875), but the runtime's own DispatchProxy relies - // on it the same way, and dotnet/runtime tests it in - // src/tests/reflection/RefEmit/EmittingIgnoresAccessChecksToAttributeIsRespected.cs. - private const string IgnoresAccessChecksToAttributeName = - "System.Runtime.CompilerServices.IgnoresAccessChecksToAttribute"; + private const string GlobalNamespaceFallback = "PTrampert.SimplePatch.Generated"; // Separate from PatchClassBuilder's cache, so each builder hands out only the types it built. // Lazy for the same reason as there: concurrent first use should emit one assembly, not one per thread. @@ -38,8 +42,9 @@ internal static class EmitPatchClassBuilder /// Gets or creates the patch class for . /// /// - /// can't model , or one of its patched - /// properties has no getter. + /// can't model , one of its patched + /// properties has no getter, or the patch class would name a type or getter that the emitted + /// assembly can't access. /// public static Type GetPatchClassFor(Type type) { @@ -49,23 +54,16 @@ public static Type GetPatchClassFor(Type type) private static Type CreatePatchClass(Type type) { var model = PatchClassModel.For(type); + EnsureAccessible(model); // Each source type gets its own assembly, so the patch type's name can't collide with - // another and needs neither a random suffix nor cleaning up into a C# identifier. + // another and needs neither a random suffix nor cleaning up into a C# identifier. The + // assemblies all share one name, because that name is what [InternalsVisibleTo] grants. // Load it where the source type lives, as PatchClassBuilder does with its compiled assembly. using var contextScope = AssemblyLoadContext.EnterContextualReflection(type.Assembly); var assembly = AssemblyBuilder.DefineDynamicAssembly( - new AssemblyName($"PTrampert.SimplePatch.Emitted.{Guid.NewGuid():N}"), AssemblyBuilderAccess.Run); - var module = assembly.DefineDynamicModule(assembly.GetName().Name!); - - // The grant has to be in place before the patch type is created, because the runtime checks - // access when it loads the type (the interface it implements names the source type). - var ignoresAccessChecksTo = DefineIgnoresAccessChecksToAttribute(module); - foreach (var referencedAssembly in GetReferencedAssemblies(model)) - { - assembly.SetCustomAttribute(new CustomAttributeBuilder( - ignoresAccessChecksTo, [referencedAssembly.GetName().Name!])); - } + new System.Reflection.AssemblyName(AssemblyName), AssemblyBuilderAccess.Run); + var module = assembly.DefineDynamicModule(AssemblyName); var namespaceRoot = string.IsNullOrEmpty(type.Namespace) ? GlobalNamespaceFallback : type.Namespace; var patchInterface = typeof(IPatchObject<>).MakeGenericType(type); @@ -88,50 +86,37 @@ private static Type CreatePatchClass(Type type) } /// - /// Defines an IgnoresAccessChecksToAttribute(string assemblyName) in - /// and returns its constructor. - /// - private static ConstructorInfo DefineIgnoresAccessChecksToAttribute(ModuleBuilder module) - { - var attributeBuilder = module.DefineType( - IgnoresAccessChecksToAttributeName, - TypeAttributes.NotPublic | TypeAttributes.Sealed | TypeAttributes.Class, - typeof(Attribute)); - attributeBuilder.SetCustomAttribute(new CustomAttributeBuilder( - typeof(AttributeUsageAttribute).GetConstructor([typeof(AttributeTargets)])!, - [AttributeTargets.Assembly], - [typeof(AttributeUsageAttribute).GetProperty(nameof(AttributeUsageAttribute.AllowMultiple))!], - [true])); - - var constructor = attributeBuilder.DefineConstructor( - MethodAttributes.Public, CallingConventions.Standard, [typeof(string)]); - var il = constructor.GetILGenerator(); - il.Emit(OpCodes.Ldarg_0); - il.Emit(OpCodes.Call, typeof(Attribute).GetConstructor( - BindingFlags.NonPublic | BindingFlags.Instance, Type.EmptyTypes)!); - il.Emit(OpCodes.Ret); - - return attributeBuilder.CreateType().GetConstructor([typeof(string)])!; - } - - /// - /// The assemblies whose types the patch class names in its signatures or its Patch - /// method: the source type and the types it is built from, and every patched property's type - /// ( names it) and declaring type. + /// Throws if the patch class would name a type or call a getter that the emitted assembly + /// can't access. Otherwise the runtime would only fail when it loads the type or first runs + /// Patch, with an error that doesn't say how to fix it. /// - private static HashSet GetReferencedAssemblies(PatchClassModel model) + /// + /// The patch class names the source type and the types it is built from, and every patched + /// property's type ( names it) and declaring type. Setters and + /// constructors need no check, because only uses public ones. + /// + private static void EnsureAccessible(PatchClassModel model) { - var assemblies = new HashSet(); var visited = new HashSet(); void Visit(Type? type) { - if (type == null || !visited.Add(type)) + if (type == null || type.IsGenericParameter || !visited.Add(type)) { return; } - assemblies.Add(type.Assembly); + // An array or constructed generic type is accessible if its parts are, and they are + // visited below. + var definition = type.IsGenericType ? type.GetGenericTypeDefinition() : type; + if (!type.HasElementType && !IsAccessible(definition)) + { + throw new NotSupportedException( + $"Cannot create a patch class for '{model.SourceType.FullName}' because it uses " + + $"'{type.FullName}', which the generated assembly can't access. " + + DescribeFix(type.Assembly, "Make the type public, or internal")); + } + Visit(type.DeclaringType); if (type.HasElementType) { @@ -154,11 +139,53 @@ void Visit(Type? type) { Visit(property.PropertyType); Visit(property.DeclaringType); + + // Patch reads the target's value of any property the patch leaves out. + if (property.GetMethod is { } getter && !IsAccessible(getter)) + { + throw new NotSupportedException( + $"Cannot create a patch class for '{model.SourceType.FullName}' because the getter of " + + $"'{property.Name}' isn't accessible to the generated assembly. " + + DescribeFix(getter.Module.Assembly, "Make the getter public, or internal")); + } + } + } + + private static string DescribeFix(Assembly assembly, string makeItAccessible) => + $"{makeItAccessible} with [assembly: InternalsVisibleTo(\"{AssemblyName}\")] in " + + $"'{assembly.GetName().Name}'."; + + /// + /// Whether code in the emitted assembly can name : it is public, or it + /// and every type it is nested in are at least internal, in an assembly that grants + /// [InternalsVisibleTo] to . + /// + private static bool IsAccessible(Type type) + { + if (type.IsVisible) + { + return true; } - return assemblies; + for (var current = type; current != null; current = current.DeclaringType) + { + if (current.IsNested + && !(current.IsNestedPublic || current.IsNestedAssembly || current.IsNestedFamORAssem)) + { + return false; + } + } + + return GrantsInternalsAccess(type.Assembly); } + private static bool IsAccessible(MethodInfo method) => + method.IsPublic || ((method.IsAssembly || method.IsFamilyOrAssembly) && GrantsInternalsAccess(method.Module.Assembly)); + + private static bool GrantsInternalsAccess(Assembly assembly) => + assembly.GetCustomAttributes() + .Any(a => new System.Reflection.AssemblyName(a.AssemblyName).Name == AssemblyName); + /// /// Defines the backing field and property for one source property, /// with the attributes gives it, and returns the field.