Avoid MissingMethodException on generic classes with init propert…

…y setters Fixes MessagePack-CSharp#1134 as much as we can while .NET has the underlying bug. When .NET 6 ships with the fix, we can add a .NET 6 target that re-allows setting `init` property setters from the `DynamicObjectResolver`.
AArnott · Nov 23, 2020 · 30ee4f8 · 30ee4f8
1 parent 908ac7b
commit 30ee4f8
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 5 deletions.
diff --git a/README.md b/README.md
@@ -404,6 +404,8 @@ public struct Point
 }
 ```
 
+### C# 9 `record` types
+
 C# 9.0 record with primary constructor is similar immutable object, also supports serialize/deserialize.
 
 ```csharp
@@ -412,8 +414,26 @@ C# 9.0 record with primary constructor is similar immutable object, also support
 
 // use property: to set KeyAttribute
 [MessagePackObject] public record Point([property:Key(0)] int X, [property: Key(1)] int Y);
+
+// Or use explicit properties
+[MessagePackObject]
+public record Person
+{
+    [Key(0)]
+    public string FirstName { get; init; }
+
+    [Key(1)]
+    public string LastName { get; init; }
+}
 ```
 
+### C# 9 `init` property setter limitations
+
+When using `init` property setters in _generic_ classes, [a CLR bug](https://github.com/neuecc/MessagePack-CSharp/issues/1134) prevents our most efficient code generation from invoking the property setter.
+As a result, you should avoid using `init` on property setters in generic classes when using the public-only `DynamicObjectResolver`/`StandardResolver`.
+
+When using the `DynamicObjectResolverAllowPrivate`/`StandardResolverAllowPrivate` resolver the bug does not apply and you may use `init` without restriction.
+
 ## Serialization Callback
 
 Objects implementing the `IMessagePackSerializationCallbackReceiver` interface will received `OnBeforeSerialize` and `OnAfterDeserialize` calls during serialization/deserialization.

diff --git a/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs b/src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs
@@ -1739,11 +1739,14 @@ public static ObjectSerializationInfo CreateOrNull(Type type, bool forceStringKe
                             MethodInfo getMethod = property.GetGetMethod(true);
                             MethodInfo setMethod = property.GetSetMethod(true);
 
+                            // init property setters only work with DynamicMethods: https://github.com/neuecc/MessagePack-CSharp/issues/1134
+                            bool isInit = setMethod?.ReturnParameter.GetRequiredCustomModifiers().Any(modifierType => modifierType.FullName == "System.Runtime.CompilerServices.IsExternalInit") ?? false;
+
                             member = new EmittableMember
                             {
                                 PropertyInfo = property,
                                 IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic,
-                                IsWritable = (setMethod != null) && (allowPrivate || setMethod.IsPublic) && !setMethod.IsStatic,
+                                IsWritable = (setMethod != null) && (allowPrivate || (setMethod.IsPublic && !isInit)) && !setMethod.IsStatic,
                                 StringKey = firstMemberByName ? item.Name : $"{item.DeclaringType.FullName}.{item.Name}",
                             };
                         }
@@ -1817,11 +1820,14 @@ public static ObjectSerializationInfo CreateOrNull(Type type, bool forceStringKe
                     MethodInfo getMethod = item.GetGetMethod(true);
                     MethodInfo setMethod = item.GetSetMethod(true);
 
+                    // init property setters only work with DynamicMethods: https://github.com/neuecc/MessagePack-CSharp/issues/1134
+                    bool isInit = setMethod?.ReturnParameter.GetRequiredCustomModifiers().Any(modifierType => modifierType.FullName == "System.Runtime.CompilerServices.IsExternalInit") ?? false;
+
                     var member = new EmittableMember
                     {
                         PropertyInfo = item,
                         IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic,
-                        IsWritable = (setMethod != null) && (allowPrivate || setMethod.IsPublic) && !setMethod.IsStatic,
+                        IsWritable = (setMethod != null) && (allowPrivate || (setMethod.IsPublic && !isInit)) && !setMethod.IsStatic,
                     };
                     if (!member.IsReadable && !member.IsWritable)
                     {

diff --git a/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs b/src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs
@@ -314,11 +314,32 @@ public void DefaultValueIntKeyStructWithExplicitConstructorSetPropertyTest()
 
 #if NET5_0
         [Fact]
-        public void RoundtripGenericClass()
+        public void RoundtripGenericClass_StandardResolverConsidersInitPropertyReadOnly()
         {
             var person = new GenericPerson<int> { Name = "bob" };
-            byte[] msgpack = MessagePackSerializer.Serialize(person, MessagePackSerializerOptions.Standard);
-            var deserialized = MessagePackSerializer.Deserialize<GenericPerson<int>>(msgpack, MessagePackSerializerOptions.Standard);
+            var options = StandardResolver.Options;
+            byte[] msgpack = MessagePackSerializer.Serialize(person, options);
+            var deserialized = MessagePackSerializer.Deserialize<GenericPerson<int>>(msgpack, options);
+            Assert.Null(deserialized.Name);
+        }
+
+        [Fact]
+        public void RoundtripGenericClass_StandardResolverWorksWithDeserializingCtor()
+        {
+            var person = new GenericPersonWithCtor<int>("bob");
+            var options = StandardResolver.Options;
+            byte[] msgpack = MessagePackSerializer.Serialize(person, options);
+            var deserialized = MessagePackSerializer.Deserialize<GenericPersonWithCtor<int>>(msgpack, options);
+            Assert.Equal(person.Name, deserialized.Name);
+        }
+
+        [Fact]
+        public void RoundtripGenericClass_AllowPrivateStandardResolver()
+        {
+            var person = new GenericPerson<int> { Name = "bob" };
+            var options = StandardResolverAllowPrivate.Options;
+            byte[] msgpack = MessagePackSerializer.Serialize(person, options);
+            var deserialized = MessagePackSerializer.Deserialize<GenericPerson<int>>(msgpack, options);
             Assert.Equal(person.Name, deserialized.Name);
         }
 #endif
@@ -616,6 +637,15 @@ public class GenericPerson<T>
             [Key(0)]
             public string Name { get; init; }
         }
+
+        [MessagePackObject]
+        public class GenericPersonWithCtor<T>
+        {
+            public GenericPersonWithCtor(string name) => this.Name = name;
+
+            [Key(0)]
+            public string Name { get; init; }
+        }
 #endif
     }
 }