DynamicallyAccessedMembers with a string TypeName of a generic type doesn't appear to work

[eerhardt]

Annotating a string property that contains a generic TypeName doesn't appear to work with the ILLinker.

This appears to be the root of the issue reported in dotnet/runtime#39709. In dotnet/runtime we have a TypeDescriptionProviderAttribute, which can contain a string pointing to a Type. We annotated this attribute correctly to preserve the PublicParameterlessConstructor, since we are going to instantiate the Type using reflection.

However, one of the usages of this attribute on XElement and XAttribute points to a generic Type:

    [System.ComponentModel.TypeDescriptionProvider("MS.Internal.Xml.Linq.ComponentModel.XTypeDescriptionProvider`1[[System.Xml.Linq.XElement, System.Xml.Linq, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089]],System.ComponentModel.TypeConverter")]
    public class XElement : XContainer, IXmlSerializable
    {

This issue can be observed with the following stand-alone application. Run the following in an untrimmed application, and it will print

ConsoleApp1.TypeDescriptionProvider
ConsoleApp1.MyGenericTypeProvider`1[ConsoleApp1.MyTypeWithGeneric]

Then Publish with PublishTrimmed=true and TrimMode=link, and adding the application assembly as a TrimMode=link assembly. Run the published app and you will see:

ConsoleApp1.TypeDescriptionProvider
Unhandled exception. System.ArgumentNullException: Value cannot be null. (Parameter 'type')
   at System.Activator.CreateInstance(Type type, Boolean nonPublic, Boolean wrapExceptions)
   at System.Activator.CreateInstance(Type type, Boolean nonPublic)
   at System.Activator.CreateInstance(Type type)
   at ConsoleApp1.Program.GetDescriptionProvider(Object o) in F:\DotNetTest\TrimmingTest\Program.cs:line 20
   at ConsoleApp1.Program.Main() in F:\DotNetTest\TrimmingTest\Program.cs:line 14

(Note my app was named TrimmingTest. If you have a different name, you will need to use a different assembly name in the strings.)

using System;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

namespace ConsoleApp1
{
    class Program
    {
        static void Main()
        {
            var d = GetDescriptionProvider(new MyTypeWithNongeneric());
            Console.WriteLine(d.GetType());

            d = GetDescriptionProvider(new MyTypeWithGeneric());
            Console.WriteLine(d.GetType());
        }

        private static TypeDescriptionProvider GetDescriptionProvider(object o)
        {
            ProviderAttribute a = o.GetType().GetCustomAttribute<ProviderAttribute>();
            return Activator.CreateInstance(Type.GetType(a.TypeName)) as TypeDescriptionProvider;
        }
    }

    public class ProviderAttribute : Attribute
    {
        public ProviderAttribute([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string typeName)
        {
            TypeName = typeName;
        }

        [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
        public string TypeName { get; }
    }

    internal class TypeDescriptionProvider { }

    internal class MyGenericTypeProvider<T> : TypeDescriptionProvider { }

    [Provider("ConsoleApp1.MyGenericTypeProvider`1[[ConsoleApp1.MyTypeWithGeneric, TrimmingTest]], TrimmingTest")]
    public class MyTypeWithGeneric { }

    [Provider("ConsoleApp1.TypeDescriptionProvider, TrimmingTest")]
    public class MyTypeWithNongeneric { }
}

This may be related to #1387.

cc @marek-safar @vitek-karas @MichalStrehovsky

[MichalStrehovsky]

Yep, this is #1387.

[MichalStrehovsky]

The .NET Native treeshaker has a Type.GetType parser that has been factored out of the .NET Native runtime reflection stack that we could use to parse these properly.

[marek-safar]

This looks to me like quite a serious limitation in the newly added code in 5.0. Why is there no error or warning if we cannot parse the string format? I think this is worth addressing for 5.0.

@MichalStrehovsky could you work on that?

/cc @vitek-karas

[MichalStrehovsky]

This looks to me like quite a serious limitation in the newly added code in 5.0

AFAIK this is calling the same codepaths to parse the string that the Type.GetType pattern matcher that pre-dates dataflow analysis was calling. Was that also new in 5.0?

I won't have time to work on it, but I can point whoever works on this to the factored out type name parser we have in .NET Native. The part that needs to be converted from CCI to Cecil is pretty small.

Why is there no error or warning if we cannot parse the string format

Because it could just be a type name that doesn't exist. Reflection is used for lightups and the things the code is looking for often don't exist. We tried to be helpful like this in .NET Native ("hey the p/invoke you're doing doesn't call into any known Win32 APIs that are allowed in the Windows Store and will throw at runtime", or "hey, the IL in this method is invalid and will throw at runtime"). This pretty much always found unreachable code (the former in crossplatform libraries that had Unix p/invokes under condition checks, the latter pretty much always found unreachable code generated by Unity tooling). It was a support cost because people asked questions about this, or got distracted by this when they hit some unrelated issue.

[vitek-karas]

No warning: This is in sync with the rest of the data flow behavior - linker will not warn/fail on missing types/members even when specifically referenced in code via reflection. So for example:

typeof(MyType).GetMethod("ThisMethodDoesNotExist")

Will not cause a warning - the reasoning is that it's not "wrong" code in any sense. If the method doesn't exist the API will return null - and this will not change after linker. So linker doesn't change the code behavior in any way here.

It might not be what the author wanted, but linker is not in the business of recognizing the intent of the developer.

Data flow in this case is basically just an extension of the above rule, where the value does not come from a literal, but comes from annotated variable.

[marek-safar]

Closing as fixed