Add a feature that prevents types that inherit from specific types or interfaces from being trimmed

[pranavkm]

A common pattern in ASP.NET Core is to have well known types or interfaces such as IComponent, Controller or Hub, that a user derives from and the framework reflects over. For e.g.

void SetProperties(IComponent component, IDictionary<string, object> values)
{
    var properties = component.GetType().GetProperties(...);
    SetValues(properties, values);
}

InvokeController(Controller controller, string method)
{
   controller.GetMethod(method).Invoke();
}

In the component case, having the ability to have type level trimming would help - if the type is referenced, always preserve it's members. In the event of a Controller or a Hub, we want the ability to say always preserve any types that derive from these types since they're primarily discovered and accessed via reflection.

[pranavkm]

@eerhardt

[MichalStrehovsky]

Looking at the pull request in aspnetcore I think there are two orthogonal issues:

1. Make it possible to express "if a type inherits from a specific base, or implements a specific interface, make it so that accessing select members from it is safe". We could potentially do that with a new extension method.

   // Old code
   Type someType = GetTypeFromSomewhere();
   
   // Linker warns because it has no clue what `someType` is
   foreach (var prop in someType.GetProperties())
   {
       // ...
   }

   // New code
   Type someType = GetTypeFromSomewhere().AssumeImplementsInterface(typeof(IComponent));
   // AssumeImplementInterface validated at runtime that the type passed to it implements the interface
   // It also now gives a hint to the linker that the type in question implements the interface
   
   // Linker sees we're calling GetProperties on something that implements IComponent. It needs to
   // go over all IComponent-implementing types that were marked and keep their properties.
   foreach (var prop in someType.GetProperties())
   {
       // ...
   }

   It feels like this could fit nicely with the dataflow analysis in general.

2. Make it possible to express "keep all types that derive from a certain base or implement certain interface". It looks like this would be necessary to fix the "I would like to walk over all types in an assembly and find a type". The difference from 1 is that we want to keep the type even if it wasn't statically referenced anywhere in code. This is potentially a pretty big hammer.

[vitek-karas]

1. I kind of dislike the fact that it's very similar to IsAssignableTo (and other similar APIs) with a tiny twist (it throws and it returns the Type again). On the other hand, it's a pretty clean solution. The only other concern is (just like 2 below) it could lead to marking LOT of things if not used properly. But that's something we can work on later on (maybe some kind of warning or similar in the linker if it detects that the condition matches lot of types).

2. I do agree that it's potentially a big hammer. On the other hand it's definitely better than nothing - which typically leads to "don't trim the app code at all". I wonder if we should do this through an API as well (instead of attribute) - that would allow for potentially NOT marking all those things if the app uses different code path (source gen?). So something like:

   var controlTypes = TypeExtensions.GetTypesAssignableTo<IComponent>();
   foreach (var t in controlTypes)
   {
       // ...
   }

   Unlike Assembly.GetTypes linker would not warn on this, instead it would go and mark all types which implement IComponent. It can then be used in conjunction with 1 and also apply some additional requirements (like PublicProperties) on all those types.

[MichalStrehovsky]

Th only annoyance with combining 2 with dataflow analysis is that now we need to model collections and enumerators. Not unimaginable, but it's extra work.

Maybe to avoid someone being too lazy and doing .GetTypesAssignableTo<object>() or .AssumeImplementsInterface(typeof(IEnumerable)), we could require the type used as the base has a custom attribute (a new one we define for this purpose, I guess). It might also simplify things within illink since we won't have to keep track of all the interfaces/bases just in case one of them gets superpowers during scanning, and can limit to annotated types.

[pranavkm]

we could require the type used as the base has a custom attribute (a new one we define for this purpose, I guess)

That's certainly a workable strategy.

[vitek-karas]

Th only annoyance with combining 2 with dataflow analysis is that now we need to model collections and enumerators. Not unimaginable, but it's extra work.

Well - not really. The Type.GetTypesAssignableTo<IComponent> would only guarantee existence of all the types which implement IComponet - it would not guarantee anything else about them. So there's nothing to track (the existence is already "tracked" by the fact that there's a Type representing it).

Requiring attribute would work I guess - not sure we have to add it - it's basically just to guard against extreme behaviors.

Linker already tracks all of the interfaces/bases for other reasons (instantiation detection and virtual method removal), so I don't see this being a big change to how linker works.

[MichalStrehovsky]

@pranavkm It is now possible to place [DynamicallyAccessedMembers(...)] on types and interfaces. The effect is that if object.GetType() is called on something that derives from or implements an annotated type, the resulting System.Type can be assumed to have the members specified by the annotation preserved. Hopefully it should unblock some of dotnet/aspnetcore#29946.