Avoid `MissingMethodException` on generic classes with `init` property setters

Fixes #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`.

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.

src/MessagePack.UnityClient/Assets/Scripts/MessagePack/Resolvers/DynamicObjectResolver.cs

@@ -611,7 +611,7 @@ namespace MessagePack.Internal
         private static Dictionary<ObjectSerializationInfo.EmittableMember, FieldInfo> BuildCustomFormatterField(TypeBuilder builder, ObjectSerializationInfo info, ILGenerator il)
         {
             Dictionary<ObjectSerializationInfo.EmittableMember, FieldInfo> dict = new Dictionary<ObjectSerializationInfo.EmittableMember, FieldInfo>();
-            foreach (ObjectSerializationInfo.EmittableMember item in info.Members.Where(x => x.IsReadable || x.IsWritable))
+            foreach (ObjectSerializationInfo.EmittableMember item in info.Members.Where(x => x.IsReadable || x.IsActuallyWritable))
             {
                 MessagePackFormatterAttribute attr = item.GetMessagePackFormatterAttribute();
                 if (attr != null)
@@ -977,7 +977,7 @@ namespace MessagePack.Internal
             }
 
             Label? memberAssignmentDoneLabel = null;
-            var intKeyMap = infoList.Where(x => x.MemberInfo != null && x.MemberInfo.IsWritable).ToDictionary(x => x.MemberInfo.IntKey);
+            var intKeyMap = infoList.Where(x => x.MemberInfo != null && x.MemberInfo.IsActuallyWritable).ToDictionary(x => x.MemberInfo.IntKey);
             for (var key = 0; key <= maxKey; key++)
             {
                 if (!intKeyMap.TryGetValue(key, out var item))
@@ -1043,7 +1043,7 @@ namespace MessagePack.Internal
             for (var i = 0; i < infoList.Length; i++)
             {
                 var item = info.Members[i];
-                if (canOverwrite && item.IsWritable)
+                if (canOverwrite && item.IsActuallyWritable)
                 {
                     infoList[i] = new DeserializeInfo
                     {
@@ -1077,7 +1077,7 @@ namespace MessagePack.Internal
             {
                 if (intKeyMap.TryGetValue(i, out var member))
                 {
-                    if (canOverwrite && member.IsWritable)
+                    if (canOverwrite && member.IsActuallyWritable)
                     {
                         infoList[i] = new DeserializeInfo
                         {
@@ -1375,7 +1375,7 @@ namespace MessagePack.Internal
             var t = member.Type;
             var emitter = tryEmitLoadCustomFormatter(index, member);
 
-            if (member.IsWritable)
+            if (member.IsActuallyWritable)
             {
                 if (localResult.LocalType.IsClass)
                 {
@@ -1439,7 +1439,7 @@ namespace MessagePack.Internal
             }
 
             il.MarkLabel(storeLabel);
-            if (member.IsWritable)
+            if (member.IsActuallyWritable)
             {
                 member.EmitStoreValue(il);
             }
@@ -1739,7 +1739,7 @@ namespace MessagePack.Internal
                             MethodInfo getMethod = property.GetGetMethod(true);
                             MethodInfo setMethod = property.GetSetMethod(true);
 
-                            member = new EmittableMember
+                            member = new EmittableMember(allowPrivate)
                             {
                                 PropertyInfo = property,
                                 IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic,
@@ -1759,7 +1759,7 @@ namespace MessagePack.Internal
                                 continue;
                             }
 
-                            member = new EmittableMember
+                            member = new EmittableMember(allowPrivate)
                             {
                                 FieldInfo = field,
                                 IsReadable = allowPrivate || field.IsPublic,
@@ -1817,7 +1817,7 @@ namespace MessagePack.Internal
                     MethodInfo getMethod = item.GetGetMethod(true);
                     MethodInfo setMethod = item.GetSetMethod(true);
 
-                    var member = new EmittableMember
+                    var member = new EmittableMember(allowPrivate)
                     {
                         PropertyInfo = item,
                         IsReadable = (getMethod != null) && (allowPrivate || getMethod.IsPublic) && !getMethod.IsStatic,
@@ -1942,7 +1942,7 @@ namespace MessagePack.Internal
                         continue;
                     }
 
-                    var member = new EmittableMember
+                    var member = new EmittableMember(allowPrivate)
                     {
                         FieldInfo = item,
                         IsReadable = allowPrivate || item.IsPublic,
@@ -2219,6 +2219,15 @@ namespace MessagePack.Internal
                 break;
             }
 
+            // Under a certain combination of conditions, throw to draw attention to the fact that we cannot set a property.
+            if (!allowPrivate)
+            {
+                // A property is not actually problematic if we can set it via the type's constructor.
+                var problematicProperties = membersArray
+                    .Where(m => m.IsProblematicInitProperty && !constructorParameters.Any(cp => cp.MemberInfo == m));
+                problematicProperties.FirstOrDefault()?.ThrowIfNotWritable();
+            }
+
             return new ObjectSerializationInfo
             {
                 Type = type,
@@ -2318,6 +2327,13 @@ namespace MessagePack.Internal
 
         public class EmittableMember
         {
+            private readonly bool dynamicMethod;
+
+            internal EmittableMember(bool dynamicMethod)
+            {
+                this.dynamicMethod = dynamicMethod;
+            }
+
             public bool IsProperty
             {
                 get { return this.PropertyInfo != null; }
@@ -2330,6 +2346,11 @@ namespace MessagePack.Internal
 
             public bool IsWritable { get; set; }
 
+            /// <summary>
+            /// Gets a value indicating whether the property can only be set by an object initializer, a constructor, or another `init` member.
+            /// </summary>
+            public bool IsInitOnly => this.PropertyInfo?.GetSetMethod(true)?.ReturnParameter.GetRequiredCustomModifiers().Any(modifierType => modifierType.FullName == "System.Runtime.CompilerServices.IsExternalInit") ?? false;
+
             public bool IsReadable { get; set; }
 
             public int IntKey { get; set; }
@@ -2367,6 +2388,22 @@ namespace MessagePack.Internal
             /// </summary>
             public bool IsExplicitContract { get; set; }
 
+            /// <summary>
+            /// Gets a value indicating whether a dynamic resolver can write to this property,
+            /// going beyond <see cref="IsWritable"/> by also considering CLR bugs.
+            /// </summary>
+            internal bool IsActuallyWritable => this.IsWritable && (this.dynamicMethod || !this.IsProblematicInitProperty);
+
+            /// <summary>
+            /// Gets a value indicating whether this member is a property with an <see langword="init" /> property setter
+            /// and is declared on a generic class.
+            /// </summary>
+            /// <remarks>
+            /// <see href="https://github.com/neuecc/MessagePack-CSharp/issues/1134">A bug</see> in <see cref="MethodBuilder"/>
+            /// blocks its ability to invoke property init accessors when in a generic class.
+            /// </remarks>
+            internal bool IsProblematicInitProperty => this.PropertyInfo is PropertyInfo property && property.DeclaringType.IsGenericType && this.IsInitOnly;
+
             public MessagePackFormatterAttribute GetMessagePackFormatterAttribute()
             {
                 if (this.IsProperty)
@@ -2415,6 +2452,17 @@ namespace MessagePack.Internal
                 }
             }
 
+            internal void ThrowIfNotWritable()
+            {
+                if (this.IsProblematicInitProperty && !this.dynamicMethod)
+                {
+                    throw new NotSupportedException(
+                        $"`init` property accessor {this.PropertyInfo.SetMethod.DeclaringType.FullName}.{this.PropertyInfo.Name} found in generic type, " +
+                        $"which is not supported with the DynamicObjectResolver. Use the AllowPrivate variety of the resolver instead. " +
+                        $"See https://github.com/neuecc/MessagePack-CSharp/issues/1134 for details.");
+                }
+            }
+
             ////public object ReflectionLoadValue(object value)
             ////{
             ////    if (IsProperty)

src/MessagePack.UnityClient/Assets/Scripts/Tests/ShareTests/DynamicObjectResolverTests.cs

@@ -314,11 +314,46 @@ namespace MessagePack.Tests
 
 #if NET5_0
         [Fact]
-        public void RoundtripGenericClass()
+        public void RoundtripGenericClass_StandardResolverThrowsOnInitProperty()
         {
             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;
+            var exception = Assert.Throws<MessagePackSerializationException>(() =>
+            {
+                byte[] msgpack = MessagePackSerializer.Serialize(person, options);
+                var deserialized = MessagePackSerializer.Deserialize<GenericPerson<int>>(msgpack, options);
+                ////Assert.Equal(person.Name, deserialized.Name);
+            });
+            Assert.Contains("https://github.com/neuecc/MessagePack-CSharp/issues/1134", exception.ToString());
+        }
+
+        [Fact]
+        public void RoundtripNonGenericClass_StandardResolverWorksWithInitPropertySetter()
+        {
+            var person = new Person { Name = "bob" };
+            var options = StandardResolver.Options;
+            byte[] msgpack = MessagePackSerializer.Serialize(person, options);
+            var deserialized = MessagePackSerializer.Deserialize<Person>(msgpack, options);
+            Assert.Equal(person.Name, 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
@@ -610,12 +645,29 @@ namespace MessagePack.Tests
         }
 
 #if NET5_0
+        [MessagePackObject]
+        public class Person
+        {
+            [Key(0)]
+            public string Name { get; init; }
+        }
+
         [MessagePackObject]
         public class GenericPerson<T>
         {
             [Key(0)]
             public string Name { get; init; }
         }
+
+        [MessagePackObject]
+        public class GenericPersonWithCtor<T>
+        {
+            [SerializationConstructor]
+            public GenericPersonWithCtor(string name) => this.Name = name;
+
+            [Key(0)]
+            public string Name { get; init; }
+        }
 #endif
     }
 }

tests/MessagePack.Tests/DynamicObjectResolverRecordsTests.cs

@@ -21,6 +21,12 @@ namespace MessagePack.Tests
             this.AssertRoundTrip(new PersonPositional("bob", "smith"));
         }
 
+        [Fact]
+        public void RoundtripGenericPositionalRecord()
+        {
+            this.AssertRoundTrip(new GenericPersonPositional<int>("bob", "smith"));
+        }
+
         [Fact]
         public void RoundtripDerivedRecord()
         {
@@ -41,6 +47,9 @@ namespace MessagePack.Tests
             return deserializedValue;
         }
 
+        [MessagePackObject]
+        public record GenericPersonPositional<T>([property: Key(0)] string FirstName, [property: Key(1)] string LastName);
+
         [MessagePackObject]
         public record PersonPositional([property: Key(0)] string FirstName, [property: Key(1)] string LastName);