New API: MemberNotNullAttribute and MemberNotNullWhenAttribute

[jcouv]

As part of the nullability feature C# 8, we have shipped a number of attributes already (NotNull, MaybeNull, NotNullWhen, MaybeNullWhen, DoesNotReturn, DoesNotReturnIf, NotNullWhenNotNull, listed here with some details).

As part of enhancements to nullability analysis for C# 9, some more attributes will be introduced. We're working documenting an explicit backlog, but we already know one of the top scenarios we're interested in addressing: dependent calls.

Let's start with two illustrations to motivate the problem:

class C
{
    string field;
    C()
    {
        // nullability analysis currently cannot tell which fields the Init method will initialize
        // but if it were annotated with `[MemberNotNull(nameof(field))]` then we would know
        // that `this.field` is initialized and not-null (so the compiler would not complain about `field`)
        Init(); 
    }
    void Init()
    {
        ...
    }
}

class D
{
     bool HasValue { get { ... } }
     // `Value` is not-null if `HasValue` is true
     string? Value { get { ... } }

    void M()
    {
        if (HasValue)
        {
             // nullability analysis currently complains, but if `HasValue` were annotated
             // with `[MemberNotNullWhen(returnValue: true, nameof(Value))]` then we would know
             // that this property is not-null in this branch of the `if`
             Value.ToString(); 
        }
    }
}

The attribute definitions that were discussed in C# LDM are:

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = true)]
    public sealed class MemberNotNullAttribute : Attribute
    {
        public MemberNotNullAttribute(params string[] members) { }
    }
}

and

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = true)]
    public sealed class MemberNotNullWhenAttribute : Attribute
    {
        public MemberNotNullWhenAttribute(bool when, params string[] members) { }
    }
}

Note that unlike other nullability attributes, both of these new attributes allow multiple instances:

• [MemberNotNull(nameof(field)), MemberNotNull(nameof(Property))] is the same as [MemberNotNull(nameof(field), nameof(Property))].
• [MemberNotNullWhen(true, nameof(field1)), MemberNotNullWhen(false, nameof(field2))] is also valid.

LDM notes for 2/5/2020: https://github.com/dotnet/csharplang/blob/master/meetings/2020/LDM-2020-02-05.md#nullability

Note that some other scenarios were also discussed in that LDM (declaring that nested fields/properties of a parameter or return value are not-null after a method returns, possibly with a new overload of the NotNullAttribute and NotNullWhenAttribute constructors), but we decided to reduce the scope.

[CyrusNajmabadi]

Should there be a MemberNull attribute for methods that might reset members back to their null state?

[jcouv]

Should there be a MemberMaybeNull [jcouv corrected attribute name] attribute for methods that might reset members back to their null state?

We know multiple scenarios (like constructor helpers) where [MemberNotNull] will be useful, but we have not identified a scenario where a [MemberMaybeNull] attribute would be useful.
Any use cases would be appreciated.

[terrajobst]

Video

• It seems they should be marked as Inherited = false, because that's what we did for the other annotations. But it seems the behavior of this is odd when the method it's applied to is overriden because the inherited method doesn't need to call the base. @jcouv: should the compiler when the the base isn't called?
• Can we make them AllowMultiple = false? The code gen argument seems odd; it might make sense for MemberNotNullWhenAttribute for true and false having different list of members but that feels contrived.

namespace System.Diagnostics.CodeAnalysis
{
    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = true, Inherited = false)]
    public sealed class MemberNotNullAttribute : Attribute
    {
        public MemberNotNullAttribute(params string[] members) { }
    }

    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = true, Inherited = false)]
    public sealed class MemberNotNullWhenAttribute : Attribute
    {
        public MemberNotNullWhenAttribute(bool when, params string[] members) { }
    }
}

[jcouv]

@terrajobst
Confirmed with LDM that AllowMultiple=false is acceptable.

I think that Inherited=false makes sense because putting [MemberNotNull("field")] on Base.Init() has a different meaning than putting it on Derived.Init(). The former refers to Base.field and the latter refers to Derived.field (could be new, or we'll produce a warning). So although the contacts accumulate through overrides (notes), they are not "inherited" in the sense that attributes can be inherited.

[jcouv]

Opened PR for adding these attributes: #33567
I'm hoping to get the compiler change into 16.6p2 (ie. next 2 or 3 days) and the attributes in a similar timeframe.

[RussKie]

The examples provide motivation for the new API ("before state") but do not provide the "after state" examples. Is it possible to provide examples with decorations?

[stephentoub]

Is it possible to provide examples with decorations?

e.g.

If you have:

public class C
{
    private string _s;

    public C()
    {
        Init();
    }

    private void Init()
    {
        _s = "Hello";
    }
}

Today you might use = null! to workaround the nullability warnings:

public class C
{
    private string _s = null!;

    public C()
    {
        Init();
    }

    private void Init()
    {
        _s = "Hello";
    }
}

whereas with MemberNotNull you do:

public class C
{
    private string _s;

    public C()
    {
        Init();
    }

    [MemberNotNull(nameof(_s))]
    private void Init()
    {
        _s = "Hello";
    }
}

[Joe4evr]

@jcouv @stephentoub Is it too late to ask for a revision around [MemberNotNullWhen]? Is there some way that instead of only a bool condition, checks against any constant value (primarily enum values) can inform the flow analysis?

My use-case is that I have a tri-state enum that gives a bit more info around related state, with one state guaranteed that another property that isn't null:

public enum CurrentlyPlaying
{
    None = 0,
    ThisGame = 1,
    DifferentGame = 2
}
public abstract partial class GameModuleBase<TGame>
{
    // What I'd like to write here:
    [MemberNotNullWhen(CurrentlyPlaying.ThisGame, nameof(Game))]
    protected CurrentlyPlaying GameInProgress { get; }

    protected TGame? Game { get; }
}

So my users can write their checks like GameInProgress == CurrentlyPlaying.ThisGame without any additional fuss.

[stephentoub]

We discussed that for Maybe/NotNullWhen as well back when they were first introduced. The use case certainly comes up, but it was deemed not widespread enough to meet the bar for the compiler. @jcouv can comment on whether we can revisit it. If we were going to do this, I'd want to do that for all of the When attributes, not just MemberNotNullWhen.

[jnm2]

I would like this for all of the When attributes. I think I had a few enum-returning methods with nullable out parameters.

[jcouv]

I would like to support enums as well, but unfortunately, that would require some significant changes to dataflow/nullability analysis.
For some basics, here's a short write-up on basics of nullability analysis. Currently, we split the state (ie. start tracking a state-when-true and a state-when-false) when encountering a boolean expression, and either collapse it back in unconditional statements, or analyze the two branches separately in conditional ones.

If we want to analyze based on enum values, now we need to split the state multiple ways, ranging from 2 to (potentially large) N and need to known which states correspond to which enum values (it's no longer just true and false alternatives).

This is technically feasible, but requires significant change that go deeper than nullability analysis (affect the common dataflow analysis infrastructure). That does not seem worthwhile when a simple pattern can solve the problem: add an instance or extension method that does the test, is annotated with the attribute and returns a boolean.
Then if (GameInProgress.IsThisGame()) ... will behave as desired.