Replacing obsolete type with var shouldn't suppress deprecation warning.

[ronnygunawan]

Version Used: 16.4.0 Preview 5

Steps to Reproduce:

[Obsolete]
class A {
}

class B {
#pragma warning disable CS0612
    public A A { get; } = new A();
#pragma warning restore CS0612
}

class C {
    public void Test() {
        var b = new B();
        var a1 = b.A;
        A a2 = b.A;
    }
}

Expected Behavior:
Both var to the left of a1 and A to the left of a2 produce warning: 'A' is obsolete.

Actual Behavior:

• var to the left of a1 doesn't produce the warning.
• A to the left of a2 produces the warning.

[YairHalberstadt]

I'll give this a go

[YairHalberstadt]

same problem in VB

Imports System

<Obsolete>
Class A
End Class

Class B
    #Disable Warning
    Public ReadOnly Property A As A = New A()
    #Enable Warning
End Class

Class C
    Public Sub Test()
        Dim b = New B()
        Dim a1 = b.A
        Dim a2 As A = b.A
    End Sub
End Class

I'm interested in whether this actually a bug in VB. If it is then the location of the warning should be moved to the DIM.

[YairHalberstadt]

@jaredpar

I'm interested in getting your assessment as to whether this is even a bug, and if so what the bug actually is.

Consider the following three code examples:

A a = b.A; // should there be a warning on the first A
return a.Prop;
...
var a = b.A; // should there be a warning on var
return a.Prop;
...
return b.A.Prop; // should there be a warning on b.A?

It feels like they should be consistent. Making a temporary local shouldn't introduce a warning, and changing from var to a named type shouldn't introduce a warning.

I feel like the most consistent approach is not to have a warning here at all.

cc @jnm2

[YairHalberstadt]

I think a good rule is:

There should be an error on usage of obsolete types in signatures.
There should be an error on usage of obsolete types in imports.
There should be an error on construction of obsolete types.
There should be an error on usage of obsolete types as TypeArguments, and as arguments to nameof, typeof, and default.

[YairHalberstadt]

We have a similar difference between default(A) and default:

using System;

[Obsolete]
class A {
}

class B {
#pragma warning disable CS0612
    public static void M(A a){}
#pragma warning restore CS0612
}

class C {
    public void Test() {
        B.M(default);
        B.M(default(A));
    }
}

I'm not sure this is a bug, so leaving this for now till a decision is made.

[jnm2]

It's not about whether A is statically inferable; it's about the direct cause that is introducing A.

With Foo(default(A)) or A a = b.A;, you have a problem located in the syntax: no matter how Foo or b.A may change, this particular line of code is still left demanding to bring in A. Therefore, this this line must also change to avoid using the type A.

With Foo(default) or var a = b.A;, this particular line of code is not necessarily doing anything that it needs to change. To avoid using the type A, Foo or b.A need to change by being obsoleted or by choosing a new parameter type and return type. It may not even be possible for the immediate line of code to avoid introducing A.

Reflecting the causal structure of the code seems super important to me. Accountability should match control. This is the same kind of thing that bothered me so much about #38638 (comment) (now fixed).

Therefore, every example I've seen from the C# compiler so far seems right to me.

[jnm2]

Also, consider something similar to the var situation which also behaves consistently:

using System;

class C<TFoo> where TFoo : A // CS0612 here
{
    public C(Func<TFoo> getFoo)
    {
        // No CS0612 here
        TFoo foo = getFoo();

        // Or here
        var foo2 = getFoo();
    }
}

[Obsolete]
class A { }

[gafter]

I think the rule is that naming an obsolete type in source triggers the diagnostic. Inferring it does not. In other words, I suspect we should keep the current behavior.