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.07k stars 4.04k forks source link

Roslyn ignores [DisallowNull] attribute for uninitialized non-nullable members #48203

Open TessenR opened 4 years ago

TessenR commented 4 years ago

Version Used:

Branch master (29 Sep 2020)
Latest commit cb0344b by Shen Chen:
Merge pull request #48144 from dotnet/merges/release/dev16.8-to-master

Merge release/dev16.8 to master

Steps to Reproduce:

Compile and run the following code

#nullable enable
using System.Diagnostics.CodeAnalysis;
class C
{
  [NotNull, DisallowNull] public string? Prop { get; set; } // no warnings

  public static void Main()
  {
    new C().Prop.ToString(); // runtime crash
  }
}

Expected Behavior: Warning for not initialized property with [DisallowNull] annotation

Actual Behavior: No warnings. The program crashes at runtime with a NullReferenceException

Notes Roslyn uses [AllowNull] annotation to suppress not initialized non-nullable fields warnings but doesn't respect its counterpart [DisallowNull]

e.g.:

#nullable enable
using System.Diagnostics.CodeAnalysis;
#pragma warning disable CS0649
class C
{
  [AllowNull] public string F2; // no warnings
  [DisallowNull] public string? F3; // no warnings either
}
TessenR commented 4 years ago

Actually it's the same with [MaybeNull] / [NotNull] Roslyn respects the former and ignores the latter

huoyaoyuan commented 4 years ago

The behavior should be expected. Specifically, [NotNull][DisallowNull] behaves very close to = null!.

TessenR commented 4 years ago

The behavior should be expected. Specifically, [NotNull][DisallowNull] behaves very close to = null!.

Could you please elaborate on why it should be expected?

I understand that it currently behaves close to = null! because Roslyn ignores the initialization rules for such members but I think this is a bug because the semantics should be very different.

= null! uses the suppression operator to ignore all compiler safety rules and assign an invalid value to a member while attributes are designed to help the compiler enforce these rules.

Specifically [NotNull] informs the compiler that some member should never return null even if its type allows it. E.g. Array.Resize<T>([NotNull] ref T[]? array, int newSize) tells the compiler that after the call the array returned from this parameter won't be nullable even though it can be according to its type.

[DisallowNull] informs the compiler that some member doesn't accept null input even if its type allows it e.g. IEqualityComparer<T>.GetHashCode([DisallowNull] T obj) will have a warning on the following invocation: ((IEqualityComparer<string?>) comparer).GetHashCode(null); even though the T in GetHashCode(T obj) is nullable string

Therefore these attributes can alter top-level nullability of a type member and are useful when you can't express it with just types e.g.

class C<T> { [NotNull, DisallowNull] public T Generic { get; set; } }

Due to the attributes the compiler will know that it should treat T Generic as not nullable and warn on usages like this: new C<string?>().Generic = default; because they assign invalid value to the property And not warn on usages like this because Generic is known to be not nullable: new C<string?>().Generic.ToString();

Currently the compiler assumes that a [NotNull] property is not nullable but fails to enforce it during object creation resulting in runtime exceptions that nullable reference types and these attributes are specifically designed to prevent. I don't think that should be expected