Roslyn: Strange CS0738 error when subclassing a parent using explicitly implemented interfaces and nullable reference types

[kevinchalet]

I recently took some time to add some nullable reference types love to my OpenIddict project. Sadly, I discovered that adding these annotations is now causing a quite-hard-to-isolate bug when mixing multiple projects, subclasses, nullable annotations and interfaces implemented explicitly.

Version Used: .NET 5.0.100-rc.1.20378.7 (also reproduces with older versions)

Steps to Reproduce:

_A complete repro solution can be found here_: https://github.com/kevinchalet/NullableReferenceTypesBug

1. Create an Abstractions .NET Standard 2.1 class library with <Nullable>enable</Nullable> and the following interface:

using System.Threading;
using System.Threading.Tasks;

namespace Abstractions
{
    public interface IMyNonGenericInterface
    {
        ValueTask<object?> GetAsync(string value, CancellationToken cancellationToken = default);
    }
}

1. Create a Core .NET Standard 2.1 class library referencing Abstractions with <Nullable>enable</Nullable> and the following class:

using System.Threading;
using System.Threading.Tasks;
using Abstractions;

namespace Core
{
    public class MyGenericClass<T> : IMyNonGenericInterface where T : class
    {
        public ValueTask<T?> GetAsync(string identifier, CancellationToken cancellationToken = default)
            => new ValueTask<T?>(result: null);

        async ValueTask<object?> IMyNonGenericInterface.GetAsync(string identifier, CancellationToken cancellationToken)
            => await GetAsync(identifier, cancellationToken);
    }
}

1. Create a ConsoleApp .NET 5.0 console referencing Core with the following main file (it doesn't need to have nullable reference types enabled):

using System;
using Abstractions;
using Core;

namespace ConsoleApp
{
    public class MySubclass<T> : MyGenericClass<T>, IMyNonGenericInterface where T : class
    {
    }

    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Hello World!");
        }
    }
}

Expected Behavior:

No compilation error.

Actual Behavior:

Program.cs(7,53): error CS0738: 'MySubclass<T>' does not implement interface member 'IMyNonGenericInterface.GetAsync(string, CancellationToken)'. 'MyGenericClass<T>.GetAsync(string, CancellationToken)' cannot implement 'IMyNonGenericInterface.GetAsync(string, CancellationToken)' because it does not have the matching return type of 'ValueTask<object?>'.

If you remove the nullable annotations and <Nullable>enable</Nullable> from the Abstractions class library, the error should go away.

[jaredpar]

@cston think this may be a metadata encoding issue. Can't construct a similar repro in a single project

https://sharplab.io/#v2:EYLgtghgzgLgpgJwD4AEBMBGAsAKBQBgAIUMA6AFQAsE4IATASwDsBzAblwOIwFYOcAxEwCuAG1ERgouIThNJ03LmbwEAMwgBjGQEkMhAN6FchU4QBqEUcLgoAbAB4A9sABWcTTAD8APkIBZAAoSIgAHDAAaQgBhCCZtcQgYBicmcicAazlCTwBKfgBfJTwAZmI0GIwHch8QQj1CAHdKRBlyQjr0QxMzS2tbR3JfAOCMMMiYuISJZNT0rKYcmFzCAF4/GGonRsIRcQBCfjMLKxt7ZzcPbz89UiCQwnCo2Pi4RNm0zOy8tY2tnb2okOuCKOFwQjEEikMkYUAUcE4ZS60TQ1VqlTRUQazVahHanTQRgKQA

[cston]

Similar issues occur with dynamic and with tuples with element names. For instance:
A.cs:
```C#
public struct S { }

public interface I
{
S F();
}

B.cs:
```C#
public class A<T> : I
{
    public S<T> F() { return default(S<T>); }
    S<dynamic> I.F() { return default(S<dynamic>); }
}

C.cs:
```C#
class B : A, I { }

Compile:

csc /t:library A.cs
csc /t:library /r:A.dll B.cs
csc /t:library /r:A.dll /r:B.dll C.cs

Result:

C.cs(1,20): error CS0738: 'B' does not implement interface member 'I.F()'.
'A.F()' cannot implement 'I.F()' because it does not have the matching return type of 'S'.
```
The pre-Roslyn C# compiler compiles this example without errors.

[kevinchalet]

@cston I wonder if the bug fixed in your PR could also explain why this snippet returns a CS0453 error when using unconstrained T? (with /LangVersion=preview):

public class C : I
{
    void I.M<T>(T? state) => throw null;
}

public interface I
{
    void M<T>(T? state);
}

CS0453  The type 'string' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'.

[cston]

I wonder if the bug fixed in your PR could also explain why this snippet returns a CS0453 error when using unconstrained T?

That is a distinct issue. When explicitly implementing (or overriding) a generic method with type parameter T, the compiler treats T? in the overridden method signature as Nullable<T> unless the overridden method includes an explicit type parameter constraint. That is for compatibility with code written before nullable reference type support was added in C#8.

The C#8 compiler allows an explicit where T : class constraint for cases where the type parameter is a reference type, and the C#9 compiler also allows a where T : default constraint for cases where the type parameter is unconstrained (as in your example above).

```C#
public class C : I
{
void I.M(T? state) where T : default => throw null;
}

public interface I
{
void M(T? state);
}
```

[kevinchalet]

@cston thanks for the detailed explanation! I must admit it was rather confusing, but it's now much clearer 😃

[cston]

I must admit it was rather confusing

@kevinchalet, thanks for the feedback. Yes, the error is confusing. We'll look at improving the error message for these cases: see https://github.com/dotnet/roslyn/issues/46458.

[kevinchalet]

@cston thanks a lot for fixing it. Question: the PR mentions the "Next" milestone while this ticket indicates "16.8". Do we know for sure when the fix will ship?

[cston]

@kevinchalet, ideally the fix will be included in 16.8. I believe the actual milestone will be updated in the PR when we branch for the release that contains the fix.

[kevinchalet]

@cston fantastic! Thanks again.