Proposal: obsolete or hide TypeInfo/GetTypeInfo

[huoyaoyuan]

Background and Motivation

#53113
TypeInfo was introduced in .NET of Windows Store, for limited reflection. Nowadays in .NET Core, it has no feature difference with Type. To avoid confusing, we can hide or obsolete the type and tell people to use Type directly.

Proposed API

namespace System.Reflection
{
+    [Obsolete]
    public class TypeInfo { }
    public static class IntrospectionExtensions
    {
+        [EditorBrowsable(Never)]
        public static TypeInfo GetTypeInfo(this Type type);
    }
}

Usage Examples

N/A

Alternative Designs

N/A

Risks

There is also a public interface IReflectableType. Can there be custom implementation of it? Does it need to be hidden/obsoleted?

[joperezr]

cc @steveharter @GrabYourPitchforks thoughts?

[steveharter]

I agree with the proposal, especially given that TypeInfo derived from Type in every release of .NET except for .NET Portable which is essentially supplanted with .NET Core.

The original rationale for TypeInfo is explained here.

[GrabYourPitchforks]

I agree in spirit with the idea of obsoleting or hiding TypeInfo, but I fear this will be much more complicated than it appears. We already expose it through public type hierarchies, including the *Builder types through ref emit.

Since TypeInfo also does enjoy widespread use throughout the ecosystem, I don't know how good an idea it would be to hide the GetTypeInfo extension method. It could lead to difficulty for callers who have a Type, need to pass it to an API that takes a TypeInfo, and don't see any immediately obvious solution as to how to convert between the two.

[buyaa-n]

cc @jeffhandley @jkotas @stephentoub @terrajobst

[jeffhandley]

I appreciate the hesitation that @GrabYourPitchforks expressed. If the type is used in non-obsolete APIs, then we should not obsolete it. Therefore this proposal would also need to include identification of all such APIs in the .NET Libraries and assessing if it's feasible to add method overloads and replacement properties. Depending on the surface area affected by that, we'd either conclude it to be low enough impact to proceed, or too disruptive.

[terrajobst]

I suggest we start with a code fixer that replaces GetTypeInfo() with calls to GetType(), and occurrences of TypeInfo with Type. A heavy reflection code based that migrated to Windows 8/was written for the Windows 8 era, would have have widely used both. The key would be making it easier for folks to move and then pushing them. Only obsoleting it without a code fixer will likely result in folks just suppressing it.

[buyaa-n]

The usage of TypeInfo in source.dot.net is huge, and many internal/public reflection types derived from it. Can we change to derive from Type instead? I guess that could be a breaking change

[terrajobst]

The usage of TypeInfo in source.dot.net is huge, and many internal/public reflection types derived from it. Can we change to derive from Type instead? I guess that could be a breaking change

Yeah, we can't do that. External folks don't inherit from Type or TypeInfo, so all our own usage where we extend TypeInfo would need to remain and be suppressed.

[buyaa-n]

I suggest we start with a code fixer that replaces GetTypeInfo() with calls to GetType(), and occurrences of TypeInfo with Type

@terrajobst the analyzer proposal is ready for review #61122, we might want to mark this one ready for review too

[bartonjs]

Video

• Marking the type as [Obsolete] feels like overkill, since it will force anything already taking one to add a suppression.
• The things that produce a TypeInfo are the more likely candidate.
• But at this point we think we're probably fine with just hiding the methods, instead of Obsoleting them.
• And since it's the only method we see on IntrospectionExtensions, let's mark that type as EB-Never, too.
• Someone should update the docs for TypeInfo to call out it's now an obsolete/unnecessary indirection.

namespace System.Reflection
{
+   [EditorBrowsable(Never)]
    public static class IntrospectionExtensions
    {
+       [EditorBrowsable(Never)]
        public static TypeInfo GetTypeInfo(this Type type);
    }
}

[terrajobst]

• Marking the type as [Obsolete] feels like overkill, since it will force anything already taking one to add a suppression.

That's fair.

Couple of other questions:

1. Have we considered hiding TypeInfo as well?
2. What about all the other reflection additions we did for Win8, such as
   • System.Reflection.AssemblyExtensions
   • System.Reflection.CustomAttributeExtensions
   • System.Reflection.CustomAttributeExtensions
   • System.Reflection.EventInfoExtensions
   • System.Reflection.MemberInfoExtensions
   • System.Reflection.MethodInfoExtensions
   • System.Reflection.ModuleExtensions
   • System.Reflection.RuntimeReflectionExtensions
   • System.Reflection.TypeExtensions

[KalleOlaviNiemitalo]

Don't obsolete CustomAttributeExtensions. It defines GetCustomAttribute<T> and GetCustomAttributes<T> extension methods that are useful and do not exist as instance methods.

[steveharter]

Moving to 9; should be simple but there are the questions stated above about expanding the scope here around other types and extension methods.