
# C# Language Design Meeting for September 4, 2019

## Agenda

1. `[AllowNull]` on properties

## Discussion

[https://github.com/dotnet/roslyn/issues/37313](https://github.com/dotnet/roslyn/issues/37313)

### AllowNull and flow analysis

Some questions have come up concerning `AllowNull` and property analysis, like in the following example:

```C#
class Program
{
    private string _f = string.Empty;

    [AllowNull]
    public string P
    {
        get => _f;
        set => _f = value ?? string.Empty;
    }

    static void Main(string[] args)
    {
        var p = new Program();
        p.P = null; // No warning, as expected
        Console.Write(p.P.Length); // unexpected warning
    }
}
```

The tricky part here is that we are tracking flow state for properties, meaning if `null` is stored in a
property, we treat that property as having a null state. Although there is an attribute, the flow state
of the variable wins, meaning that we think there is a `null` inside `P`, even though the stated type
is `string`. Similarly, if you invert the attribute and use `[NotNull]` on a `string?`, the flow state
will win again.

There a couple ideas to address the problem, including

  1. Don't flow-track non-nullable fields/properties.
  2. Have `[AllowNull]` suppress both the warning, and suppress setting the flow state to `null`
  3. Have the `[NotNull]` attribute suppress the flow state (which it currently doesn't), and require
     the property be written with a nullable type (`string?`) and have `[NotNull]` on the getter.
  4. Stop tracking when any nullability attributes are present on properties.


2 and 3 are somewhat related and we could do both, in theory. The main problem is that it greatly
complicates the user's understanding of when flow tracking is enabled, and there are also potentially
downstream affects for type inference if we allow the rule for (2) to apply to parameters.

For (1) it seems plausible, since we would still have a warning on assigning a null to a non-nullable
member. This would remove the flow-based subsequent warnings, but the user would still be warned at
the point where the problem happens. However, it doesn't seem to solve the problem for generics, e.g.

```C#
class C<T> where T : new()
{
    private T _f = new T();

    [AllowNull]
    public T P
    {
        get => _f ?? throw new Exception();
        set
        {
            if (value is null)
            {
                throw new Exception();
            }
            return _f;
        }
    }
}
```

Here the generic property `P` cannot be marked nullable because it is unconstrained.

However, 2 & 3 also have a problem around `MaybeNull`. If a property is annotated `MaybeNull`, then
the attribute would override the state, meaning that a null check is useless. If you check for null
it would not matter, because when you read the property again, the attribute would override the state
and the result would still be maybe null.

An idea to address this is a combination of (4) and (3), where we have special attribute behavior
for properties, and in that case `NotNull` has precedence over nullable state, but nullable state
has precedence over `MaybeNull`. In addition, `AllowNull` modifies the state transition to use
the declared state if the input is null, while it otherwise uses the non-null state.

**Conclusion**

The proposal starting point is:

NotNull wins over tracked state, which wins over MaybeNull.
AllowNull transforms an incoming maybe null state to the declared state.

There's an action item to go investigate how this will play into the rest nullability, and an open
question of whether to treat fields like properties, or like local variables.