Proposal: Attribute to Specify Return Value Effect on Related Property/Field Non-Nullability

[mikernet]

I've found myself running into this a few times as I port my libs to NRTs. It would be useful to be able to specify that a particular property or method return value means that a related property or field will not be null. It could be marked in either "direction" and the second example is probably easier to implement in the NRT flow analyzer, though the first example reads much nicer to me and can be implemented with overloaded constructors on the existing attributes:

public class Path
{
    public bool IsFilePath { get; }

    public bool IsDirectoryPath => !IsFilePath;

    // I think this is much nicer, overload NotNullWhen:
    [NotNullWhen(nameof(IsFilePath), true)]
    [NotNullWhen(nameof(IsDirectoryPath), false)]
    public Path? ParentDirectory { get; }
}

public class Path
{    
    // This direction feels a bit more awkward and probably requires new attributes
    // to feel somewhat sensible:
    [IndicatesNotNullWhen(true, nameof(ParentDirectory))]
    public bool IsFilePath { get; }

    [IndicatesNotNullWhen(false, nameof(ParentDirectory))]
    public bool IsDirectoryPath => !IsFilePath;

    public Path? ParentDirectory { get; }
}

Path path = GetPath();

if (path.IsFilePath)
{
    // Do not warn that ParentDirectory may be null:
    Console.WriteLine("Parent directory: " + path.ParentDirectory.ToString());
}
else
{
    // Warning that ParentDirectory may be null if ? or ! is not used:
    Console.WriteLine("Parent directory: " + (path.ParentDirectory?.ToString() ?? "[none]"));
}

This should/could also work for fields:

public class CollectionOfStuff
{
    [NotNullWhen(nameof(IsEmpty), false)]
    private Stuff[]? _stuff;

    public bool IsEmpty => _stuff == null || _stuff.Length == 0;

    public void DoSomething()
    {
        if (!IsEmpty)
            Array.Reverse(_stuff); // Don't warn that _stuff may be null
    }
}

I played around with various ways to name the [IndicatesNotNullWhen] annotation and arrange the parameters but I couldn't really come up with something that felt right. That said, both annotations could be useful since the [IndicatesNotNullWhen] variation could be used on return values of methods as well:

public class PersonEntity
{
    public string? Name { get; set; }

    [return: IndicatesNotNullWhen(true, nameof(Name))]
    public bool IsValid()
    {
        return !string.IsNullOrWhiteSpace(Name);
    }
}

var person = GetPerson();

if (person.IsValid())
{
    string[] nameParts = person.Name.Split(' '); // Don't warn that Name might be null
}

[CyrusNajmabadi]

Yes, this comes up in Roslyn apis as well. For example SyntaxNodeOrToken's IsToken/IsNode and AsToken()/AsNode().

Seems reasonably common to have some some property available that allows you to know when somethin else will/won't be null.

[yaakov-h]

I seem to recall this being raised somewhere for Task.Exception/Task.IsFaulted, but I can't find that issue.

[SirUppyPancakes]

This would allow for a dead-simple safe implementation of an Option<T> type (for those of us who need more explicit representation of optional values at compile-time and run-time):

public struct Option<T>
{
    public readonly bool HasValue;

    [NotNullWhen(nameof(HasValue), true)]
    public readonly T Value;

    public Option<T>(T value) => (HasValue, Value) = (true, value);

    // equality and friends omitted
}

Is this sort of functionality on the roadmap?

[CyrusNajmabadi]

That Option<T> doesn't look right. new Option<string>(null) would be .HasValue == true, but Value == null.

[SirUppyPancakes]

Hmmm you're right. The "pure" implementation of Option, which must allow null in order to be a proper monad over all possible C# values, would need more of an "uninitialized context" rather than a "null context".

Something along the lines of...

public class Test
{
    public int A;
    public int? B;
    public string C;
    public string? D;
}

Each of these members would have an uninitialized error/warning, and so you'd have to initialize them in the constructor. It would still let you initialize them to null or default or whatever you want. This would be basically identical to how the null context works, except it wouldn't care about the types since it's concerned with initialization.

This is definitely a bit too complicated to even propose though, especially given how complicated the null context stuff already is, and this wouldn't even be able to replace that since initialization does not affect nullability. Not to mention that initialization can be a little hard to pin down in every scenario (just like nullability is proving to be).

I suppose an impure implementation of Option where you could simply coerce all null values to .HasValue = false would work well enough. It could still meet the monad laws, just so long as you exclude null from the domain of possible values.

[SirUppyPancakes]

Actually, even if you coerced null values to None, you would still need to flow "default context" for value types, though it seems something along the lines this was proposed here: #146

[Joe4evr]

@SirUppyPancakes out parameters to the rescue! 😛

public bool HasValue([MaybeNullWhen(false)] out T value)

I've been using this in my own version of Option<T> (and friends) and it works like a charm. It also entirely avoids the need to do a double access.

[SirUppyPancakes]

Yeah that's definitely the cleanest approach at the moment, though there is still the problem of handling value types:

Option<int> x = new Option<int>(123);

if (x.HasValue(out int value))
{
    // accessing value here has no warning
}
{
    // accessing value here has no warning, but would be nice if it warned
}

If the default context attributes and flow analysis is introduced, it would warn in the else block.

Having a default(int) slip through into the domain is definitely less dramatic than having a null slip through into the domain though.

[Joe4evr]

That's true, but as you already noted, that requires 146 and Defaultable analysis (which hopefully isn't too hard to piggy-back off of the compiler's infrastructure for NRT analysis).

[jnm2]

You can also close the gap with a custom analyzer if you're willing to put in 1000x the effort compared to carrying around a code snippet.

[HaloFour]

@CyrusNajmabadi

That Option<T> doesn't look right. new Option<string>(null) would be .HasValue == true, but Value == null.

Not all languages consider None and Some(null) to be the same thing (or for the latter to be invalid). F# allows it, so does Scala.

Nevermind, I shouldn't be posting before coffee.

[YairHalberstadt]

Closing as now implemented in C# 9 using MemberNotNullWhenAttribute.

e.g.

public class CollectionOfStuff
{
    private Stuff[]? _stuff;

    [MemberNotNullWhen(false, nameof(_stuff))]
    public bool IsEmpty => _stuff == null || _stuff.Length == 0;

    public void DoSomething()
    {
        if (!IsEmpty)
            Array.Reverse(_stuff); // Don't warn that _stuff may be null
    }
}