dotnet / csharplang

The official repo for the design of the C# programming language
11.55k stars 1.03k forks source link

Obsoletion warning is not shown for obsolete overrides of non-obsolete base members #2652

Open jhinder opened 5 years ago

jhinder commented 5 years ago

Version Used: VS 16.0 Preview 2

Steps to Reproduce:

abstract class Base
{
    public abstract void M();
}

class Derived : Base
{
    [Obsolete]
    public override void M() => throw null;

    public void M2()
    {
        M();
    }
}

Expected Behavior: The call to M will result in a warning about using an obsolete member.

Actual Behavior: QuickInfo will show "[deprecated]" for Derived.M(), but no warning is emitted for the call.

This is important for Span<T>, which overrides Equals and GetHashCode. Both of those are guaranteed to throw, and the obsoletion attribute of these methods contains important information about using these methods (e.g. to use == instead of Equals).

gafter commented 5 years ago

According to the language specification, the invocation binds to the original method, not the override. Since the original method is not obsolete, no warning is given.

YoshiRulz commented 2 weeks ago

This footgun is now at least documented correctly. But I would like to see the restriction lifted, per my earlier comment in the linked issue:

My use-case is to indicate that the "general" object.Equals(object?) overload shouldn't be used because, to preserve commutativity, it only mimics the IEquatable<TSelf>.Equals overload, and not the additional IEquatable<int>.Equals overload—that is, instance.Equals(1) may resolve to the wrong overload and always return false.

It's also already used in the BCL: for Span<T>/ReadOnlySpan<T>, HashCode, NonCryptographicHashAlgorithm, and this mock struct (all Equals and/or GetHashCode), and for PasswordDeriveBytes.GetBytes. It looks like VectorSpan<T>/ReadOnlyVectorSpan<T> copied Span<T> and will have it too.