Inconsistent diagnostics for uninitialized `[AllowNull]` and `[DisallowNull]` members

[TessenR]

Version Used:

Branch master (19 Dec 2020)
Latest commit 80e9227 by msftbot[bot]:
Merge pull request #50073 from dotnet/merges/release/dev16.9-to-master

Merge release/dev16.9 to master

Steps to Reproduce:

#nullable enable
using System.Diagnostics.CodeAnalysis;
#pragma warning disable CS0649 // field is never assigned
class C1
{
  [AllowNull] public string f1;
  [DisallowNull] public string? f2;
}

class C2
{
  [AllowNull] public string f1;
  [DisallowNull] public string? f2;

  public C2() { }
}

Expected Behavior: Either warnings for uninitialized state of non-nullable f1 fields at the end of initialization / constructor or warnings for uninitialized states of null-disallowing f2 fields.

Actual Behavior: No warnings at all

Notes Currently the compiler recognized [AllowNull] members as members that can contain null values hence not requiring initialization of [AllowNull] string f1 despite its non-nullable type. However, it doesn't handle [DisallowNull] similarly and just ignores this attribute, using the actual type of [DisallowNull] string? f2 to determine whether the member can be nullable after the initialization logic.

According to this comment the planned solution is to change [AllowNull] to match [DisallowNull] (i.e. ignore it) and warn for f1 fields in the example above: https://github.com/dotnet/roslyn/pull/49911#discussion_r544620374

[Joe4evr]

FWIW, [DisallowNull] public string? f2; should not generate a warning. The intent that is conveyed here is "reads of this field (i.e. the initial state) can be nullable, but any writes to it may not".

[TessenR]

To be clear the handling of [DisallowNull] here doesn't introduce any problems since the output contract of the containing type is not violated. [AllowNull] however introduces a hole in the NRT safety net by allowing the following code to compile without warnings and crash at runtime with a NullReferenceException

#nullable enable
using System.Diagnostics.CodeAnalysis;

new C1().f1.ToString();

class C1
{
  [AllowNull] public string f1;
}

[RikkiGibson]

I would be interested in adding a diagnostic for any fields or auto-implemented properties where the input can be null but the output is not-null. I think this would not involve any changes to the nullability flow analysis.

I believe at one point dotnet/runtime used this attribute as a method of suppressing initialization diagnostics, but I reviewed their usages of it recently and it doesn't look like they do that any more.

I'd like to introduce this warning in wave 6 (i.e. .NET 6). /cc @jaredpar