Avoid MissingMethodException on generic classes with init property setters - #1137

Does that make it impossible to deserialize normal records as well?

No, records can typically deserialize just find. positional records have a constructor that can be used, and for non-positional records (where the properties are written out explicitly) they work as well as classes. So this really has nothing to do with records. It's just any generic class with an explicitly written out init property accessor that doesn't have a deserializing constructor that replaces the need to access that property setter.

How about throwing an exception for generics and isInit?

I could totally go that way. In fact an earlier iteration of my change did just that. I'll rework this.

@neuecc I've made the change you requested.

Thank you, I see that the fields used in the ctor are not to be reassigned.
If its behavior is correct, good.
But I think it's a story that occurred in another PR, but it's a breaking change from the current release version.
Was this change intended and OK?

using MessagePack;
using MessagePack.Resolvers;
using System;

[MessagePackObject(true)]
public class My
{
    public int MyProperty { get; set; } // set or init

    public My(int myProperty) /* dummy */
    {
    }
}

class Program
{
    static void Main(string[] args)
    {
        var m1 = new My(999) { MyProperty = 100 };
        var bin = MessagePackSerializer.Serialize(m1);
        var m2 = MessagePackSerializer.Deserialize<My>(bin);

        // 2.2.60 is `100`
        // AArnott:fix1134 is `0`
        Console.WriteLine(m2.MyProperty);
    }
}

This wasn't intended to have any impact on the scenario you identified, so I'll look more closely. But FWIW I would have expected the "new" behavior. Why would we set the value twice?

First design generate simply

return new T(/* constructor parameters */)
{
   // all public writable members
}

It will be set member in ctor(user defined) -> set field from generated code.

• If you have a parameter in ctor, it is usually a private set, therefore, twice set does not occur
• There is no guarantee to set the parameters received in ctor (as the above example is)

However, init makes parameter in ctor with public set(in reflection view).

Maybe after this PR changed this behaviour ##1095

return new T(/* constructor parameters */)
{
    // all public writable members except used in constructor
}

This change is okay, I think.
But we had to be aware that there was a breaking changes.

By the way, what about detecting ThrowIfNotWritable in the try-catch and falling back to DynamicMethod?

TypeInfo formatterTypeInfo = null;
try
{
    formatterTypeInfo = DynamicObjectTypeBuilder.BuildType(DynamicAssembly.Value, typeof(T), false, false);
}
// TODO: use perticular exception
catch (NotSupportedException e) when (e.Message.StartsWith("`init` property accessor"))
{
    Formatter = (IMessagePackFormatter<T>)DynamicObjectTypeBuilder.BuildFormatterToDynamicMethod(typeof(T), false, false, false);
    return;
}

if (formatterTypeInfo == null)
{
    return;
}

Formatter = (IMessagePackFormatter<T>)Activator.CreateInstance(formatterTypeInfo.AsType());

which slows down the generation slightly but better thant throw.

In this case, we fix new EmittableMember(allowPrivate) because above becomes allowPrivate:false and dynamicMethod:true.

I added a breaking change notice for the upcoming v2.3 release here: https://github.com/neuecc/MessagePack-CSharp/releases/edit/untagged-f7b8032af5f20c994b47
I think it's a good change in behavior given the reasons I include in that doc.
But I agree it predates this PR.

I will update this PR after an attempt to add the fallback path as you suggest.

@neuecc It's done. Please take another look.

sorry for delayed response, I'll check soon.