Annotate unsupported APIs in System.Reflection

[buyaa-n]

As part of #47228 i am running an analyzer to detect APIs throwing PNSE but not annotated with Obsolete/SupportedOSPlatform/UnsupportedOSPlatform attributes, we need to annotate them so that developers get warnings when they use them unexpectedly

For now, I have results only cross-platform and .Net 5.0 or higher builds, analysis of targeted builds or lower env are coming soon where more APIs could be detected

API	Comment	Suggestion	Location
'Assembly.ReflectionOnlyLoad(byte[])'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Reflection\Assembly.cs(376,73)
'Assembly.ReflectionOnlyLoad(string)'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Reflection\Assembly.cs(378,76)
'Assembly.ReflectionOnlyLoadFrom(string)'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Reflection\Assembly.cs(380,78)
'StrongNameKeyPair.StrongNameKeyPair(string)'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Reflection\StrongNameKeyPair.cs(33,13)
'StrongNameKeyPair.PublicKey.get'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Reflection\StrongNameKeyPair.cs(36,13)
'Type.ReflectionOnlyGetType(string, bool, bool)'	unconditionally throws on all platforms	add [Obsolete]	libraries\System.Private.CoreLib\src\System\Type.cs(544,110)
'Activator.CreateInstance(Type, BindingFlags, Binder?, object?[]?, CultureInfo?, object?[]?)'	throws with non platform related condition	consider changing the exception to NotSupportedException	libraries\System.Private.CoreLib\src\System\Activator.RuntimeType.cs(33,17)

Note: We are suggesting to add [Obsolete] with the corresponding message for APIs supported in .Net framework but not supported in .NetCore, if it is only supported on a specific platform(s) consider adding [SupportedOSPlatform("platformName")] or if it is unsupported on a specific platform(s) [UnsupportedOSPlatform("platformName")] attribute instead

Suggestions for changing the exception is optional

cc @jeffhandley @terrajobst @GrabYourPitchforks

[buyaa-n]

Notes need to consider:

If the API has an issue associated with future support we are not suggesting to add [Obsolete], we need to make sure that we are not gonna support the API ever and related issue should be closed, then we can add [Obsolete]
If the API is internal/private the annotation is not necessary, but they could point you to the public API needs annotation

[buyaa-n]

While annotating APIs in StrongNameKeyPair noticed that all APIs in that type is not supported in .NetCore and existing APIs only throwing PNSE or no-op. I am planning to update the no-op constructor overloads to throw PNSE and [Obsolete] the entire type. Which also causing to Obsolete AssemblyName.KeyPair property. This type and API doesn't have real usage in runtime repo but from https://source.dot.net/ we see it is referenced a lot from MSBuild repo (most likely not used for .NetCore build).

Please let me know if you have any issue/comment with Obsoleting StrongNameKeyPair and AssemblyName.KeyPair property @terrajobst @stephentoub @bartonjs

[bartonjs]

I am planning to update the no-op constructor overloads to throw PNSE and [Obsolete] the entire type.

Have we been obsoleting types, as opposed to just their ctors (and specific methods, when needed)? I thought we generally didn't like putting the marker on the type itself.

[jeffhandley]

We did obsolete some types in .NET 5, particularly in CAS.
https://docs.microsoft.com/en-us/dotnet/core/compatibility/syslib-warnings/syslib0003

[GrabYourPitchforks]

StrongNameKeyPair and AssemblyName.KeyPair are a bit of a weird special-case. Buyaa identified that nobody aside from msbuild seems to be using this property, and once the ctors are changed to be always-throwing the only thing they can actually pass via the property setter is nullptr. And if you can't ever instantiate a class, that seems like it'd be a good candidate to obsolete the entirety of the type.

[bartonjs]

So long as we're not doing something new, seems fine with me. Someone who has a utility method for working with one that wants it to stick around in both netstandard and net(core) compiles (to avoid surface area reduction problems) can just suppress it. They'd have to anyways, if all the members are also Obsolete.

In my head, the "nicest" thing is to obsolete the ctors (after making them throw) (and any property/method that returns one), and leave the type and methods as unannotated. You'll never get one in Core, so it doesn't matter that the member would throw, but it avoids cross-compile noise. But then it's annoying to us because the tool needs to understand that pattern, or except types on a one-off basis. Since it's not (to my knowledge) a type with any significant usage, the noise to the handful using it is probably fine.

[buyaa-n]

In my head, the "nicest" thing is to obsolete the ctors (after making them throw) (and any property/method that returns one), and leave the type and methods as unannotated. You'll never get one in Core, so it doesn't matter that the member would throw, but it avoids cross-compile noise. But then it's annoying to us because the tool needs to understand that pattern, or except types on a one-off basis. Since it's not (to my knowledge) a type with any significant usage, the noise to the handful using it is probably fine.

Yeah, that could be less breaking change, then probably leave AssemblyName.KeyPair as is without obsoleting

[GrabYourPitchforks]

Spoke with Buyaa offline. We may need to revert the change to the internal AssemblyName ctor since it has ripple effects throughout the native reflection stack. But that's doesn't impact the larger goal of getting these annotations visible to our users, and we can always do this cleanup later when we have some free cycles.