Differentiate flavours of "obsolete" in .NET

[IS4Code]

Based on past discussions and observations from the use of ObsoleteAttribute in .NET, I feel that a single flavour/kind/degree of obsoleteness is not enough, as its use varies from "safe but meaningless" to "not working at all", thus having a potential to give false impression to anyone who needs to deal with an occurrence of this attribute in a larger quantity. The DiagnosticId property is nice to filter out various areas, but it often does not distinguish between varying degrees of obsoleteness.

I have tried to search for usable custom attributes, but there are not that many of them:

• ObsoleteAttribute itself, with or without IsError (just a diagnostic instruction, without any additional semantics). This is being used as an umbrella attribute for most situations.
• ExperimentalAttribute and RequiresPreviewFeaturesAttribute specify a certain degree of instability, but they are used at the other end of the chronological spectrum, and are expected to mark APIs that eventually transition to being stable, obsolete, or removed.
• There is a mention of DeprecatedAttribute but I could not find any actual documentation. Considering it was proposed alongside ExperimentalAttribute which did make it, I am left to assume that DeprecatedAttribute was, well, deprecated.
• EditorBrowsableAttribute has some precedent for being used alongside the other attributes, but again, it is mostly just an instruction, the only semantics being carried by EditorBrowsableState.Advanced.
• The various platform-based attributes, such as ObsoletedOSPlatformAttribute or UnsupportedOSPlatformAttribute, which are good to take inspiration from, but differentiate between platforms, whereas this should be purely runtime-based.

I have tried to come up with more levels that should be distinguished and preferably give the programmer a better indication of the status of an API:

Meaningless

The API is there for compatibility or completeness, and is going to stay supported, but it does not offer anything at all to its consumers, and as such its use might be an indication that it was misunderstood, thus justifying a warning.

New API should ideally never start in this state, and if existing API transitions to this state, it should also receive [EditorBrowsable(EditorBrowsableState.Never)].

A good candidate for this status is the MarshalByRefObject class since its usefulness in .NET Core is extremely limited, likely only to give a stronger sense of "identity" to classes deriving from it (without any real consequences whatsoever), or compatibility with ICustomFactory.CreateInstance.

Another candidate would be String.Clone, since it literally does not do anything (for a good reason, but it breaks the contract of ICloneable.Clone and should really have been just an explicit interface implementation from the beginning). Likewise for Enum.ToString(IFormatProvider). There is no point in calling any of those methods explicitly.

Dangerous

The API is useful in some situations, and is likely stable, but the consumer must absolutely be aware of the situations in which it works reliably, to ensure security, stability, or memory safety of the application. Ideally, a code analyzer should exist to help ensuring proper use of any such API.

New API may start in this state. Note that this status is for the contract, not the implementation, i.e. the API is simply inherently dangerous, always has been, and always will be, and it is not something the runtime can "fix" other than by providing a more restricted API. This should be used alongside [EditorBrowsable(EditorBrowsableState.Advanced)].

This covers everything from BinaryFormatter (and other formatters that allow data to run arbitrary code, including flags in other libraries enabling the same), to ControlledExecution (somewhat paradoxically starting as "obsolete") and most members of Unsafe, MemoryMarshal, and similar.

There may be a desire to make this status "contagious", i.e. to mark any method using a dangerous API dangerous itself, unless suppressed.

Archaic

Such an API is meaningful to use, but there are objectively better ways to achieve the same result. This serves more as a suggestion than a warning, with the intention to point the consumer to a better alternative. While the API works and should continue to work, it is best to upgrade to a newer one.

New API should (obviously) not start in this state, and when a transition to this state occurs, there should really be an alternative that covers the use cases of the original API. Personally I would not ascribe any [EditorBrowsable] attribute here, since there is no inherent necessity to hide such APIs (but such can be decided on a per-case basis).

A good example for this status are most collections in System.Collections.Specialized, which are fine but dated, since System.Collections.Generic is preferred. ICloneable and IConvertible likely fall within this category as well, and the whole marshalling infrastructure probably too.

Deprecated

This indicates a badly designed API, one that was or turned out to be fundamentally flawed, whose contract is not achievable or leads to more problems than it solves, so, in reality, the API should have never been released in the first place. In such a case, the status should also point to a better API that achieves the same, but properly. Nevertheless, the API may be usable when the specific requirements under which it works are known and upheld, so it is similar to a combination of archaic and dangerous.

Obviously new API should not be created with this status, and when used, it warrants [EditorBrowsable(EditorBrowsableState.Never)].

A nice example of this situation is Process.PrivateMemorySize, surpassed by Process.PrivateMemorySize64, and similar properties where the value range of the type is not enough to express the possible range of values.

Unimplemented

This API is really not implemented in any sensible way ‒ it just throws an exception, or similarly does not attempt to uphold its contract in any way. In some ways, it is all of the above: meaningless, dangerous, archaic, and deprecated.

Obviously [EditorBrowsable(EditorBrowsableState.Never)] should be used for any such API, and, possibly, its use could be an error by default. While new API should obviously not start with this status (unless marked experimental/preview), curiously there are methods like TypedReference.SetTypedReference which have never worked, but are nonetheless included (although it is not that hard to make that one work...).

This is also the tombstone status used for "removed" APIs that still need to exist for compatibility, but should never be called as there are no conditions under which they could work. Any method that just throws a NotSupportedException or similar should have this status (for example the BinaryFormatter and co.).

Unsupported

"All or any of the above." There is no guarantee that this API works or will continue to work. If you use it, you are on your own. If it breaks one day, don't be surprised. Any reasoning surrounding such an API can only be made with the full knowledge of the environment in which the code runs, e.g. the platform, runtime version, or other conditions that affect the compilation or execution.

In addition to[EditorBrowsable(EditorBrowsableState.Never)], it should be an error to use such an API by default, unless suppressed.

---

I have also decided to take a look throughout .NET for occurrences of [Obsolete], to give my thoughts on its use, grouped by the message:

"X has been deprecated and is not supported."

This is not helpful much. Either it has been deprecated and thus a better alternative is available (with or without making it also unimplemented), or it has become or always has been unsupported:

• It can be found on members of Microsoft.VisualBasic.CompilerServices.NewLateBinding and similar, where it looks like it was marked "obsolete" from the beginning and thus should have been unsupported as any API intended only for use by the compiler.
• AssociationAttribute and FilterUIHintAttribute are attributes, so there is really not much implementation to reason about. I was not looking into detail here, but archaic is probably the only applicable status here, possibly also meaningless in combination with a particular use of the attributes. Likewise for most other attributes.
• XmlDataDocument is an interesting case, looking like a bridge between XML and relational data. I am not sure if such a concept can be considered fundamentally flawed, but considering the implementation is still there, I would go just with archaic. Maybe unsupported once it is decided that class is ready to be removed.
• SslProtocols.Ssl2 and similar. This is more about the technology it identifies rather than the API, but in any case it should be more accurately described ‒ it is likely archaic (but possibly kept for compatibility with older systems) and dangerous (since there are security risks associated with using the older versions), and possibly also unimplemented (if it leads to an exception somewhere), meaningless (if another version is chosen anyway), or unsupported (if there are no guarantees for its functioning, or it is to become unimplemented in the future).
• Socket.UseOnlyOverlappedIO ‒ obsoleted by Obsolete Socket.UseOnlyOverlappedIO #47163, and based on the discussion, it is a good candidate for being meaningless ‒ not used by the implementation at all.
• Actual AppDomain-related members of AppDomain ‒ since there are no application domains on .NET Core, these members should be unimplemented (and potentially meaningless if the implementation is trivial).

String.Copy: "This API should not be used to create mutable strings. See https://go.microsoft.com/fwlink/?linkid=2084035 for alternatives." (#27515)

This is likewise not helpful much ‒ String.Copy is used for duplicating string instances, not for mutating a string. The real obsolete operation is mutating a string, not duplicating one, and this API complements nicely other methods like String.Intern etc. to control more the implementation details of strings. If anything, this should be just dangerous, informing the developer that mutating the resulting string is not a supported operation.

"Formatter-based serialization is obsolete and should not be used."

This is an umbrella message for everything in System.Runtime.Serialization and related members, but the consequences of using such are not as drastic, and definitely do not warrant the strong and authoritative "should not be used":

• Type.IsSerializable and similar ‒ there is no harm in finding out whether a type is serializable or not, so this should be just archaic. If the implementation is changed to always return false, then it becomes meaningless.
• IFormatterConverter, IObjectReference, ISerializationSurrogate, IFieldInfo, and other interfaces are mostly used alongside the standard formatters, but I could imagine them being used for other legitimate situations. On their own, they should have no obsoleteness at all, or perhaps just archaic on some, or meaningless (if there are no applications to them at all anymore).
• ObjectIDGenerator is associated with formatters but there is otherwise no harm at all to using it, sans from being slightly inefficient in some situations. Thus it may be only archaic (discussion here: Replacement for the obsolete ObjectIDGenerator #98168).
• SerializationInfo, SurrogateSelector, and other hard-core formatter infrastructure classes might be archaic or deprecated, and if their direct use has security issues, dangerous. If they are removed from .NET, they would transition to unsupported and then unimplemented.

"This API supports obsolete formatter-based serialization. It should not be called or extended by application code."

This is really used only on ISerializable.GetObjectData implementations and the (SerializationInfo, StreamingContext) constructor ‒ this API is not dangerous at all, and can be safely used to delegate the act of storing state of an object to the class itself, in an imperative style. Of course there are better, declarative-style alternatives, so this is still archaic. If the implementation of these methods is actually removed from .NET (which should not be the case though per the discussion in #98245), they may become meaningless. Otherwise the same reasoning holds as for the previous message.

"BinaryFormatter serialization is obsolete and should not be used. See https://aka.ms/binaryformatter for more information."

This is the actually dangerous API, but the wording "obsolete and should not be used" does not reflect at all the sheer difference between this and the previous ones. This (and similar serializers in .NET Framework) should have definitely started as dangerous, and then possibly deprecated (pointing to alternatives that work with the same format, just without constructing any arbitrary types) and remained as such.

"ReflectionOnly loading is not supported and throws PlatformNotSupportedException.", "Creating and unloading AppDomains is not supported and throws an exception.", "Code Access Security is not supported or honored by the runtime.", "This Remoting API is not supported and throws PlatformNotSupportedException.", "The Constrained Execution Region (CER) feature is not supported."

This is just missing technology and thus unimplemented or meaningless. In the case of ReflectionOnly, it should also be archaic and point to MetadataLoadContext.

"This API supports the .NET Framework infrastructure and is not intended to be used directly from your code."

Any API intended to be used just by the compiler should be unsupported, possibly also dangerous.

"Thread.Abort is not supported and throws PlatformNotSupportedException."

This is obviously unimplemented, but as an API it should also be deprecated and point to ControlledExecution.Run.

"The UTF-7 encoding is insecure and should not be used. Consider using UTF-8 instead."

It is interesting to see a text encoding being insecure, but from what I can tell, this encoding does not work anymore in .NET and thus this should be unimplemented (and previously potentially dangerous and archaic).

"Recovery from corrupted process state exceptions is not supported; HandleProcessCorruptedStateExceptionsAttribute is ignored.", "DisablePrivateReflectionAttribute has no effect in .NET 6.0+.", "SuppressIldasmAttribute has no effect in .NET 6.0+.", "ExecutionEngineException previously indicated an unspecified fatal error in the runtime. The runtime no longer raises this exception so this type is obsolete."

Just meaningless by definition.

"Marshal.X may be unavailable in future releases."

Based on the message, this should be already unsupported, or potentially dangerous and deprecated.

"WebRequest, HttpWebRequest, ServicePoint, and WebClient are obsolete. Use HttpClient instead."

HttpClient is surely nice, but WebClient and WebRequest are a part of a larger system that can be made to interoperate with any URI scheme, not just HTTP. If you want HTTP, then HttpClient is better than HttpWebRequest, and thus that one should be archaic, but the rest does not have a replacement. However, there might one day be a replacement for WebRequest.RegisterPrefix that is non-global, so that API could become deprecated.

"IHashCodeProvider has been deprecated. Use IEqualityComparer instead."

While an interesting piece of .NET history, this is just archaic. It is essentially IEqualityComparer minus Equals, which is not fundamentally flawed in itself, just not usable much when you also need equality comparison.

"HashCode is a mutable struct and should not be compared with other HashCodes. Use ToHashCode to retrieve the computed hash code.", "Equals() on Span will always throw an exception. Use the equality operator instead.", "GetHashCode() on Span will always throw an exception."

This is an interesting situation of overriding the standard System.Object methods just to make them obsolete. This is both meaningless and unimplemented, letting the consumer know that it does not make sense to use them, and that the implementation will simply not work just to make this obvious.

"IsolatedStorageFile.X has been deprecated because it is not CLS Compliant. To get the maximum size use IsolatedStorageFile.Y instead."

This is a strange case in regards to categorization per the criteria above. The fact that an API is not CLS-compliant is on its own not a deficiency (as many great APIs are not CLS-compliant anyway), but in this case it really is unnecessary ‒ the runtime uses signed integers for compatibility prevalently even in places where negative integers don't make sense, so this API should have been signed from the beginning, but then again, the unsigned versions work just as well as the signed ones, so it is really just a usability issue, thus this API should be only archaic (but with EditorBrowsableState.Never).

"The second data source of a binary operator must be of type System.Linq.ParallelQuery<T> rather than System.Collections.Generic.IEnumerable<T>. To fix this problem, use the AsParallel() extension method to convert the right data source to System.Linq.ParallelQuery<T>."

This is another sentinel method, this time warning against using a parallel API in a sequential fashion, thus it should be meaningless and unimplemented.

"Uri.EscapeUriString can corrupt the Uri string in some cases. Consider using Uri.EscapeDataString for query string components instead."

How about making it not corrupt the string then? Jokes aside, this method does well what it is supposed to do, and it has legitimate uses, but people just don't understand its purpose, so it could be just dangerous. It may be possible to appeal to its name as a bad API design, in which case it could also become deprecated, pointing to a better-named method.

CodeIdentifier constructor: "This class should never get constructed as it contains only static methods."

How about making the class static? Anyway just meaningless (not like someone would ever think to get something interesting from constructing it).

X509Certificate.Import: "X509Certificate and X509Certificate2 are immutable. Use the appropriate constructor to create a new certificate."

This is a good example of an actually deprecated API. Depending on the implementation, it could also be unimplemented.

Overlapped.Pack(IOCompletionCallback): "This overload is not safe and has been deprecated. Use Pack(IOCompletionCallback?, object?) instead."

It is not exactly helpful to state that an unsafe method is not safe. Interestingly enough, the dangerous overload really just calls the "safe" overload with a null argument, so the real danger is not really in the method, but in the way it is used ‒ the object? parameter should not be nullable since otherwise one may simply decide to "fix" it by using the overload with null, thinking it is safer.

[MichalPetryka]

• IsError (just a diagnostic instruction, without any additional semantics)

I'd say that making code fail to build is enough to be regarded as different semantics.

[IS4Code]

Well, in combination with [EditorBrowsable], there are 6 options, each with a possible interpretation:

	EditorBrowsableState.Always	EditorBrowsableState.Advanced	EditorBrowsableState.Never
IsError = false	archaic	dangerous	unsupported
IsError = true	deprecated	meaningless	unimplemented

This essentially divides the space into two dimensions of "could be used ‒ shouldn't be used" and "works ‒ does not work". I don't like this however, personally, as I don't feel it completely aligns with the categories, and it puts too much emphasis on [Obsolete] itself. It also does not shun adjustments to diagnostics, as archaic members should not really result in a warning, in most cases (just a message), and unsupported members should raise an error initially, unless suppressed.

[huoyaoyuan]

• There is a mention of DeprecatedAttribute but I could not find any actual documentation.

It's WinRT: DeprecatedAttribute

• ExperimentalAttribute and RequiresPreviewFeaturesAttribute specify a certain degree of instability, but they are used at the other end of the chronological spectrum, and are expected to mark APIs that eventually transition to being stable, obsolete, or removed.

Note that we did break compatibility for Experimental while transforming to stable, for Intrinsics and generic math.
Generally experimental stuff won't become obsolete, because we do not commit compatibility for them. They would be just removed instead.

Dangerous

This covers everything from BinaryFormatter (and other formatters that allow data to run arbitrary code, including flags in other libraries enabling the same), to ControlledExecution (somewhat paradoxically starting as "obsolete") and most members of Unsafe, MemoryMarshal, and similar.

It seems messed up in this category.
ControlledExecution is in the combination of unimplemented and meaningless. Because it's not implemented in .NET Core, it becomes no-op.
BinaryFormatter is dangerous for compatibility. It can't be made safe given it's shape.
Unsafe and MemoryMarshal are dangerous by design. They are for tasks that not allowed of verifiable in safe semantics. They should not be included as a flavor of "obsolete", but just "advanced".

Archaic

Such an API is meaningful to use, but there are objectively better ways to achieve the same result.

This is the most common area. We (and others) often create analyzers that suggest for alternates.

Unsupported

"All or any of the above." There is no guarantee that this API works or will continue to work.

Unsafes are supported, with semantics beyond the safe area.
Archaic stuff are still supported since they may be widely used for legacy components, like ArrayList used in .NET Framework 1.x era. We do support them to continue to work.

[IS4Code]

I think we disagree only on the use of dangerous, otherwise I agree with your observations (and I did not claim the contrary). You may have misunderstood what I meant by unsupported ‒ it's not that any of the previous categories are unsupported, it means that any unsupported member may also be in any other category without specifying what the category is, thus there is no possibility to tell if (when) it works or not.

ControlledExecution is in the combination of unimplemented and meaningless. Because it's not implemented in .NET Core, it becomes no-op.

Well I can run this code just fine, it is definitely implemented (and does what one would suppose):

var cancel = new CancellationTokenSource();
cancel.CancelAfter(1000);

ControlledExecution.Run(() => {
    for(int i = 0; i < Int32.MaxValue; i++)
    {
        Console.WriteLine(i);
    }
}, cancel.Token);

It would be unimplemented if it just threw an exception straight away (which it doesn't), and meaningless if it ran the code without any interruption (which it doesn't; it stops after 1 second). Of course given its nature, it may also be simply unsupported in addition to dangerous.

BinaryFormatter is dangerous for compatibility. It can't be made safe given it's shape.

Indeed, so dangerous is a good fit. The way it is designed makes it inherently dangerous (even if it was not designed to be dangerous) and there is no alternative that works the same way without being dangerous because it comes from the combination of its functions.

Unsafe and MemoryMarshal are dangerous by design. They are for tasks that not allowed of verifiable in safe semantics. They should not be included as a flavor of "obsolete", but just "advanced".

Of course, I was just pointing out that "obsolete" is at the moment used for members which work, but can be misused in some way that leads to issues, which for example holds true for ControlledExecution, BinaryFormatter (well not on .NET 9+), and String.Copy. So I say that if there is a need to warn the user about the usage of these APIs, it should also extend to Unsafe and MemoryMarshal which are obviously supported, but still dangerous by the same token.

[huoyaoyuan]

I'm just disagree about categorizing BinaryFormatter, ControlledExecution or Unsafe in a same category. In brief:

• BinaryFormatter is not recommended because of security risk. Users are asked to remove away from it, but they need to do some change in their code.
• ControlledExecution is not recommended because its whole family of feature (constrained execution) is not implemented. User should just replace it with direct method call, and implement the desired reliability at a totally different level.
• Unsafe is recommended, but just for advanced users.

[IS4Code]

They would not be in the same category ‒ BinaryFormatter can have, in addition to dangerous, any combination of archaic, deprecated, unimplemented, or unsupported. ControlledExecution can too be both dangerous and unsupported. They would not need to be just dangerous.

[Neme12]

I agree. It would be useful to have something like:

public sealed class ObsoleteAttribute
{
    public ObsoleteAttribute(string? message, DiagnosticLevel diagnosticLevel);
}

public enum DiagnosticLevel
{
    Suggestion,
    Warning,
    Error,
}

[rgroenewoudt]

I do would like an analyzer to warn me. For example I completely missed that SmtpClient shouldn't be used in new code as the warning is only shown in the documentation.