TypeDescriptor-related trimming supportType descriptor

[steveharter]

Fixes #101202.

A minor API TypeDescriptionProvider change from the approved API; this now matches the existing pattern of 2 methods that forward to a virtual. A mail was sent to shiproom for:

public abstract class TypeDescriptionProvider
{
    public ICustomTypeDescriptor? GetTypeDescriptorFromRegisteredType(Type objectType); // already approved
+   public ICustomTypeDescriptor? GetTypeDescriptorFromRegisteredType(object instance);
+   public virtual ICustomTypeDescriptor? GetTypeDescriptorFromRegisteredType(Type objectType, object instance); 
}

Todos:

• The doc in this PR is minimal; update doc afterwards + sync XML doc to docs repo
• There will likely be a follow-up PR for passing attributes to various methods for filtering properties, etc. and any other asks.

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[dotnet-policy-service]

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

[LakshanF]

@sbomer, do we still need this? PublishTrimmed apps should have this property set to false automatically.

[LakshanF]

I might have missed this scenario below but asking to make sure - do we have tests for registering a parent type and asking for derived type members?

[steveharter]

The child properties work the same as they did before - see https://github.com/dotnet/runtime/pull/102094/files#diff-3e07e1115d281208825274551ed27b42fcadbe3832f148953b603a6e696b9d16R212-R213.

However, this does not support calling TypeDescriptor.GetPropertiesFromRegisteredType<T>() on a base T - you'll get InvalidOperationException. Basically, the previous code is kept as-is w.r.t registering base classes - the properties from base classes are included and trim-safe, but base type themselves are not automatically registered. We could add support to loop through the base classes and register them with the underlying provider, but I'm not sure if this is necessary.

Also, I can update the trimming tests to add a base class to make this support obvious.

[steveharter]

See the latest commit; I added some tests to clarify this and assert the current behavior.

[LakshanF]

Is this switch only used for the the existing APIs (without the FromRegisteredType suffix)? If yes, I assume the new APIs (with the FromRegisteredType suffix) will always check that the type is registered first.

[steveharter]

The "FromRegisteredType" APIs enforce the registration without the switch, at least for the normal cases like GetPropertiesFromRegisteredType() where the check more or less comes for free. Some "FromRegisteredType" APIs, to avoid an unnecessary perf hit, don't (see this).

When the switch is on:

• The existing APIs (e.g. TypeDescriptor.GetProperties()) check to see if the type is registered for the internal reflection provider. This allows existing code to basically remain as-is (for compat with code that may not be owned) but does require registering the type. This avoids issues where there are likely suppressed trim warning previously. Previously we discussed always throwing for these existing methods, but I thought that was too extreme.
• The base class implementation of "FromRegisteredType()" APIs check the bool? RequiresRegistration property which defaults to return null. These will now throw NotImplementedException when the switch is on. For example, see this. This enforces that providers need to be updated to declare whether or not they require registration if the "FromRegisteredType" APIs are used - most or all providers will return false; true is only needed to be returned if reflection is used at some point on that type. Existing APIs will not throw NotImplementedException.

[buyaa-n]

Left some NIT, overall LGTM