# `field` keyword working group notes for August 7, 2024

The working group met to review the [Nullability analysis with the `field` keyword](FK-2024-08-07%20Nullability%20analysis%20with%20the%20`field`%20keyword.md) proposal to discuss the direction and come away ready to start on spec updates to present to the LDM.

## Takeaways

There was agreement on the changes laid out in the document. This included taking the proposed version of null-resilient getters, instead of the alternatives:

- We like the version of null-resilient getters that takes all nullability warnings into consideration, not just ensuring that exit points return non-null expressions. This should be very cheap to implement.

- We see no reason to stop halfway and apply only `[field: MaybeNull]` without `[field: AllowNull]`. The latter would just be harder to explain to users.

### Motivation takeaway

For the motivation to apply nullability automatically, we felt the motivation was much stronger than simply that it is too much code to write to manually specify `[field: MaybeNull]` in a common scenario. Rather, we feel that these nullability attributes are hard for users to understand and get right. It's much greater of a conceptual hurdle than we want for how commonplace it will be to write simple lazy or defaulting scenarios.

```cs
[field: MaybeNull]
public string Prop => field ?? GetDefault();

[field: MaybeNull]
public List<int> Prop => field ??= new();

[field: MaybeNull]
public SomeComponent Prop => LazyInitializer.EnsureInitialized(ref field);
```

### Additional NRT checking for setters

The [constructor initialization](FK-2024-08-07%20Nullability%20analysis%20with%20the%20`field`%20keyword.md#constructor-initialization) section proposes that "CS8618: Non-nullable property 'Prop' must contain a non-null value when exiting constructor" will be appeased by calling the property setter in the constructor, trusting that the property getter will return a non-null value after that call. This has precedent in C#.

This was accepted, but a suggestion was made to check the other half of this initialization scenario. The setter could be analyzed as though it had a `MemberNotNull` attribute guaranteeing that the backing field is not null by the time it exits.

Applying MemberNotNull-style analysis to the setter would provide useful warnings for two kinds of issues. One, where the field was null during the initial call to the property setter:

```cs
class C
{
    public C() { Prop = "..." };

    public string Prop
    {
        get;
        set
        {
            // CS8602 Dereference of a possibly null reference.
            Console.WriteLine(field.Length);
                              ~~~~~
            field = value;
        }
    }
}
```

And two, where the property setter can return without initializing the backing field:

```cs
class C
{
    public C() { Prop = "..." };

    public string Prop
    {
        get;
        set
        {
            if (condition) field = value;
        //  CS8774 Member 'field' must have a non-null value when exiting.
        }
        ~
    }
}
```

### Nullable backing field

We preferred the idea of making the backing field nullable rather than keeping it matching the property type while applying `[field: AllowNull, MaybeNull]`. Users are by far more familiar with using nullable types than working with the attributes. The null-resilient getter concept is stronger when its usage and explanation is simpler for users. Making the field nullable at the symbolic level is a difference that would be visible through the IDE, where the user would see a question mark after the type when hovering over the `field` keyword.

### `[AllowNull]` nullability hole

The proposal mentions that there is an existing nullability hole in the language when `[AllowNull]` is applied to a field, and that this hole will be inherited by `[field: AllowNull]`:

```cs
// C# 12
class C
{
    [AllowNull]
    private string resetIfSetToDefault;

    public string ResetIfSetToDefault
    {
        // Removed `?? GetDefault()` without warnings!
        get => resetIfSetToDefault;
        set => resetIfSetToDefault = value == GetDefault() ? null : value;
    }
}
```

The point was raised that when it comes to `[field: AllowNull]`, we might have an opportunity to close that gap in warnings because the backing field is scoped to the property. For instance, perhaps we could simply create a nullability warning if `[field: AllowNull]` is used without `[field: MaybeNull]`.

As mentioned elsewhere, `[field: AllowNull]` is likely only going to be specified in advanced scenarios where automatic backing field nullability (somewhat equivalent to AllowNull and MaybeNull together) isn't what the user wants.

### Open question: unrelated nullability warnings

We considered whether unrelated nullability warnings should affect the nullability of the backing field:

```cs
// Is this a null-resilient getter?
get
{
    string unrelated = null; // Warning
    return field ?? "";
}
```

Detecting the relatedness of a warning would likely be based on doing two nullability analysis passes, and considering the getter to be null-resilient if there are no _new_ warnings present in a maybe-null-field pass which were not present in a non-null-field pass. This diff would likely not be expensive to do. Any additional expense would also only kick in with new code, and even in new code, we expect complex getters to be quite rare. And in the event that a user needs an escape hatch to skip the null resilience evaluation, the user can apply nullability attributes manually.

There could be occasional scenarios with inferred generic type arguments where there is a different error in each pass; however, what would fall out in that case would be a "not null-resilient" detection, which is a safe place to land, still closer to the mark than doing only a single pass. Finally, it might be harder to explain and understand "are there new warnings caused by `field` becoming nullable" compared to understanding "are there any warnings when `field` is nullable."

We decided to implement only one pass for now and see how it does in practice during implementation. We can revisit this if it seems that the single pass is leaving something to be desired.

### Open question: interaction with manually-applied attributes

We decided that if any nullability attribute is applied using a `field:` target, including the preconditions AllowNull and DisallowNull, then null resilience will not be considered. The user has already entered the space of taking control over the nullability of the backing field. Someone specifying a nullability attribute is likely using it as an escape hatch because the null resilience check wasn't what they wanted.

## Next steps

We would like to bring three options to LDM:
1. No specific nullability support at all.  Users would be forced to initialize all non-nullable properties, regardless of their bodies.
2. Only support field-targeted attributes.  Users could suppress initialization warnings, through explicit addition of particular attributes for particular property bodies.
3. Support field-targeted attributes as well as null-resilient getters.  Users would normally not get initialization warnings and would not need to add attributes for safe coding patterns.

To get ready to present these options to LDM, a compiler team member will write the nullability spec changes and optionally do some implementation investigations.
