dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.06k stars 4.04k forks source link

Inexplicable CS8978 'T' cannot be made nullable #64431

Open ploeh opened 2 years ago

ploeh commented 2 years ago

Version Used: Visual Studio 2022, .NET 6.0

Steps to Reproduce:

Create a new C# library in Visual Studio 2022. Add this class:

public sealed class Element<T>
{
    public Element(T key)
    {
        Key = key;
    }

    public T Key { get; }
    public Element<T>? Next { get; set; }

    public readonly static IEqualityComparer<Element<T>> Comparer = new ElementSnapshotComparer();

    public class ElementSnapshotComparer : IEqualityComparer<Element<T>>
    {
        public bool Equals(Element<T>? x, Element<T>? y)
        {
            return Equals(x?.Key, y is null ? null : y.Key) && Equals(x?.Next, y?.Next);
        }

        public int GetHashCode([DisallowNull] Element<T> obj)
        {
            return HashCode.Combine(obj.Key, obj.Next);
        }
    }
}

Expected Behavior:

The code compiles.

Actual Behavior:

The code doesn't compile. Instead, it fails to compile with this error message:

Error CS8978 'T' cannot be made nullable.

The offending expression is x?.Key, even though it was suggested by Visual Studio.

Notice the alternative null check made for y: y is null ? null : y.Key, which does compile. I left the two alternative versions side by side to be able to talk about the different compiler behaviour.

For the y is null ? null : y.Key expression, Visual Studio helpfully suggests IDE0031 Null check can be simplified, which, if followed, refactors the expression to y?.Key, which then also fails to compile.

I've tried to search for CS8978, but haven't found anything that looks relevant. While I'm aware of #59805, it doesn't look like the same issue to me.

It's possible that this is correct behaviour, and that I'm just too ignorant to understand that this is so. If so, however, may I suggest a more helpful error message? (I have 20 years of C# experience, so if this confuses me, it may confuse other people, too.)

HobbsCode commented 2 years ago

Constraining T to 'class' will make the error go away. The compiler is worried about T being a struct.

ploeh commented 2 years ago

Constraining T to 'class' will make the error go away. The compiler is worried about T being a struct.

Two reactions to that:

  1. One major use case is that I need T ~ int, so that is not a useful option.
  2. x and y, above, are not of the type T. They are of the type Element<T>, which is literally a class. If "The compiler is worried about T being a struct.", I have to admit that I understand nothing. Why is that even relevant?
RikkiGibson commented 2 years ago

The reason y is null ? null : y.Key works is because of the target typed conditional expression feature added in C# 9. This permits null and y.Key to independently convert to the target type of object in this context.

We don't have a similar concept of target typing a conditional access expression like y?.Key. This expression is generally equivalent to y is null ? (T)null : y.Key (simplifying), but since (T)null is not allowed (since we don't know how to convert null to T in the case it is a non-nullable value type), the standard defines this case as an error.

It might be reasonable to define a sort of target typing in this scenario, i.e. if a target type of the ?. expression is known, then treat it as y is null ? (Target)null : y.Key instead. But that would be a language design question best suited for a csharplang discussion.

ploeh commented 2 years ago

Thank you, that makes it a little clearer what's going on. It sounds like, then, that the IDE0031 Null check can be simplified suggestion is a false positive?

CyrusNajmabadi commented 2 years ago

Can you give her exact version of VS you're using? Thanks!

ploeh commented 2 years ago

Can you give her exact version of VS you're using? Thanks!

Microsoft Visual Studio Community 2022 Version 17.3.5 VisualStudio.17.Release/17.3.5+32922.545 Microsoft .NET Framework Version 4.8.04161

Installed Version: Community

ASP.NET and Web Tools 17.3.376.3011 ASP.NET and Web Tools

Azure App Service Tools v3.0.0 17.3.376.3011 Azure App Service Tools v3.0.0

Azure Functions and Web Jobs Tools 17.3.376.3011 Azure Functions and Web Jobs Tools

C# Tools 4.3.0-3.22470.13+80a8ce8d5fdb9ceda4101e2acb8e8eb7be4ebcea C# components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

CodeMaintainability 2022 1.2 Extension for tracking code maintainability of your methods. Instead of often performing report driven code analysis tools, you can use this extension to view in real-time maintainability score.

Common Azure Tools 1.10 Provides common services for use by Azure Mobile Services and Microsoft Azure Tools.

GitHub Copilot 1.49.0.1 (v1.49.0.1@3a5736ec4) GitHub Copilot is an AI pair programmer that helps you write code faster and with less work.

GitHub Copilot Agent 1.49.6911 (v1.49.0)

Microsoft JVM Debugger 1.0 Provides support for connecting the Visual Studio debugger to JDWP compatible Java Virtual Machines

NuGet Package Manager 6.3.0 NuGet Package Manager in Visual Studio. For more information about NuGet, visit https://docs.nuget.org/

Razor (ASP.NET Core) 17.0.0.2232702+e1d654e792aa2fe6646a6935bcca80ff0aff4387 Provides languages services for ASP.NET Core Razor.

SQL Server Data Tools 17.0.62207.04100 Microsoft SQL Server Data Tools

TypeScript Tools 17.0.10701.2001 TypeScript Tools for Microsoft Visual Studio

Visual Basic Tools 4.3.0-3.22470.13+80a8ce8d5fdb9ceda4101e2acb8e8eb7be4ebcea Visual Basic components used in the IDE. Depending on your project type and settings, a different version of the compiler may be used.

Visual F# Tools 17.1.0-beta.22363.4+1b94f89d4d1f41f20f9be73c76f4b229d4e49078 Microsoft Visual F# Tools

Visual Studio Inline Suggestions 2.2.2055.12395 Ghost text API for Visual Studio inline suggestions

Visual Studio IntelliCode 2.2 AI-assisted development for Visual Studio.

CyrusNajmabadi commented 2 years ago

Thanks. Def an issue for IDE side to fix.

CyrusNajmabadi commented 2 weeks ago

Would take a small community fix here.