[API Proposal]: Make DynamicallyAccessedMembers valid for Type/string collection parameters

[koszeggy]

Background and motivation

[DynamicallyAccessedMembers] is valid for pure Type/string parameters, but not for collections, whose element type is Type or string.

Consider the following example:

// In safe mode polymorphic deserialization does not resolve type names in the serialization stream,
// even if they present with their assembly qualified name. All types that are not supported natively
// by the serializer must be enlisted in the expectedCustomTypes parameter.
// TRoot (along with its type arguments if it is a generic type) are automatically included in expectedCustomTypes
public TRoot DeserializeSafe<TRoot>(XmlReader reader, params Type[] expectedCustomTypes)
{
    // [...]
{

As of today, such an API cannot be made AOT-compatible, because I can apply [DynamicallyAccessedMembers] for TRoot only, but not for expectedCustomTypes:

warning IL2098: Parameter 'expectedCustomTypes' of method 'DeserializeSafe`1(XmlReader, params Type[])' has 'DynamicallyAccessedMembersAttribute', but that attribute can only be applied to parameters of type 'System.Type' or 'System.String'.

API Proposal

No actual change is needed for the DynamicallyAccessedMembersAttribute - the change affects the trimming only

API Usage

By accepting the proposal, this would become valid:

private const DynamicallyAccessedMemberTypes neededMembers = DynamicallyAccessedMemberTypes.AllConstructors
    | DynamicallyAccessedMemberTypes.PublicFields
    | DynamicallyAccessedMemberTypes.PublicProperties;

public TRoot DeserializeSafe<[DynamicallyAccessedMembers(neededMembers)]TRoot>( // nothing new here
  XmlReader reader,
  [DynamicallyAccessedMembers(neededMembers)]params Type[] expectedCustomTypes) // no IL2098 anymore
{
    // [...]
{

And then an AOT-friendly usage could use typeof() expressions passed to the expectedCustomTypes parameter:

var myObj = xmlSerializer.DeserializeSafe<MySerializableObject>(xmlStream, typeof(MyEmbeddedType1), typeof(MyEmbeddedType2));

Alternative Designs

• [API Proposal]: DynamicallyAccessedMembers for type arguments of the containing type #114044 is something similar, but for the generic arguments only. Implementing that one as well could simplify using a generic TRoot with custom types in the example, but it's not really necessary for this proposal, because we still can make this API work by enlisting also the type arguments in the expectedTypes parameter.
• [API Proposal]: Make DynamicallyAccessedMembersAttribute accept member names #129415 is also similar, but it intends to enlist explicit member names for a single type rather than multiple types.

Risks

?

[dotnet-policy-service]

Tagging subscribers to this area: @steveisok, @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

[MihuBot]

I'm a bot. Here is a possible related and/or duplicate issue (I may be wrong):

• Cannot pass list of DynamicallyAccessedMembers types #131407

[koszeggy]

The duplicate suggestion by @MihuBot is actually a bug report that could be solved by addressing this proposal.

[dotnet-policy-service]

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

[teo-tsirpanis]

You can encode the reflection annotations in a wrapper type. The following program maintains the same UX as params Type[] and does not produce trimming analyzer warnings:

public class Program
{
    public static void Main()
    {
        PrintMethodNames(typeof(string), typeof(int), typeof(DateTime));
    }

    public static void PrintMethodNames(params ReadOnlySpan<TypeWithAllMethods> types)
    {
        foreach (var type in types)
        {
            var value = type.Value;
            // Use the value as needed
            foreach (var method in value.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static))
            {
                // Do something with each method
                Console.WriteLine($"Method: {method.Name}");
            }
        }
    }
}

public readonly struct TypeWithAllMethods([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.AllMethods)] Type t)
{
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.AllMethods)]
    public Type Value => t;

    public static implicit operator TypeWithAllMethods([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.AllMethods)] Type t) => new TypeWithAllMethods(t);
}

Supporting applying [DynamicallyAccessedMembers] directly on collection arguments would non-trivial, since it would have to support C# 13's params collections feature, further increasing the trimmer's complexity (it would feel artificial to support only params Type[]).

I must say that having DeserializeSafe accept an array of types does not look ideal, even without taking trimmability into account, because you are restricted in how much information you can precompute across invocations (such as a hash set of allowed type names). My recommendation would be to refactor your API to follow a shape that can be handled by the trimmer's present capabilities (such as a SerializationContext type with an Add method that is properly annotated). This follows the approach System.Text.Json took, and can open the door to source-generated reflection-less implementations in the future.

[koszeggy]

@teo-tsirpanis: Thank you for the idea. Unfortunately I cannot remove the original API as it would be a breaking change (and because I target older frameworks as well with no span support), so such a new overload would not be picked for the existing typeof() collection calls, not even if I use the [OverloadResolutionPriority(-1)] attribute.

But at least it's a working workaround, it's just that the user must refactor their calls in AOT mode.

But now that the proposal got the Future milestone, I hope that it will be implemented eventually.

[teo-tsirpanis]

Unfortunately I cannot remove the original API as it would be a breaking change (and because I target older frameworks as well with no span support), so such a new overload would not be picked for the existing typeof() collection calls, not even if I use the [OverloadResolutionPriority(-1)] attribute.

My suggestion also works with params TypeWithAllMethods[], and the new overload gets preferred when using [OverloadResolutionPriority(-1)] on the old one:

[koszeggy]

You're right, my bad. When targeting multiple frameworks, I must double check which one is currently selected to get the right results. :D