# Nullability improvements working group meeting for October 24th, 2022

The *nullability improvements* working group met to look at issues which were raised during LDM triage and/or have received significant community engagement.

## Task<T> covariance
https://github.com/dotnet/csharplang/issues/3950

The general scenario that people have complained about is:
```cs
using System.Threading.Tasks;

#nullable enable

public class C
{
   public Task<string?> M() // not async
   {
       return Task.FromResult(""); // type of return value is Task<string>, which warns when converting to Task<string?>.
   }
}
```

We saw that this was reported on a few different occasions. We think this is a fairly annoying papercut and that it would be nice to be able to make it "just work".

A few options we considered to that end:

### Option 0: special-case Task<T>

IEquatable is already "special" in the compiler in that it is "nullable-contravariant", not fully contravariant: `IEquatable<string?>` is convertible to `IEquatable<string>`. This limited form of contravariance is primarily present for compat reasons, though there are design reasons one might want it to work this way as well.

```cs
IEquatable<string?> e1 = ...;
IEquatable<string> e2 = e1; // ok
IEquatable<string?> e3 = e2; // warning CS8619: Nullability of reference types in value of type 'IEquatable<string>' doesn't match target type 'IEquatable<string?>'.
```

Because only the same few types have been used in the samples where people complain about *nullable-invariance*, we think it might make sense to just expand on it. Let the compiler assume that `Task<T>` and `ValueTask<T>` are *nullable-covariant*. We wouldn't check the implementations of these well-known types to decide if they are safely handling the variant type parameter.

We could also extend this to say that anything [task-like](https://github.com/dotnet/csharplang/blob/main/proposals/csharp-7.0/task-types.md) is assumed to be *nullable-covariant*.

### Option 1: [NullableOut] and [NullableIn]?

Introduce special attributes which denote that type parameters are *nullable-covariant* or *nullable-contravariant*.

```cs
interface IEquatable<[NullableIn] T> { }
class Task<[NullableOut] T> { }
struct ImmutableArray<[NullableOut] T> { }
```

We can imagine other interfaces which might want the *nullable-contravariance* used by `IEquatable`, such as `IEqualityOperators`, where `TSelf` is meant to be constrained to exactly the type which implements the interface, but the type argument may still vary on its nullability for incidental reasons (i.e. due to other constraints which happen to be applied to handle all the things a type parameter needs to be able to do).

```cs
public interface IEqualityOperators<TSelf, TOther, TResult>
        where TSelf : IEqualityOperators<TSelf, TOther, TResult>?
{
    static abstract TResult operator ==(TSelf? left, TOther? right);
    static abstract TResult operator !=(TSelf? left, TOther? right);
}
```

However, we think that a scenario where someone will actually be inconvenienced by this limitation is rare--to the point that we did not take the effort to actually sketch out such a scenario.

We think that if we added these attributes to the platform, we would want to add a practical amount of enforcement to ensure that nulls do not sneak in unexpectedly. For example, when `[NullableOut]` is used on a type parameter, we might want a warning if inputs of that type might be non-nullable. We're not sure how extensive this enforcement would need to be. In practice the amount of effort needed to provide a good user experience with these attributes could be high.

```cs
class Task<[NullableOut] T>
{
    private T result; // maybe this needs a warning?
    internal void SetResult1(T t) // warning: input of type 'T' should be nullable because the type parameter is nullable-covariant.
    {
        result = t; // ok
    }

    internal void SetResult1(T? t) // ok
    {
        result = t; // warning: possible null reference assignment.
    }
}
```

Additional concerns we have with this approach:
1. It may not be obvious for API authors when they need to add this attribute. Pretty much anything that exposes a read-only value of a parameterized type could be a good "candidate" for nullable covariance, for example.
2. Nullable-oblivious types, or types the end user doesn't control in general, won't have this attribute, and users will continue to get warnings on variant conversions in those cases.
    - To elaborate: Types whose original definitions are oblivious are still nullable-invariant in a nullable-enabled context. The constructed type `ObliviousGeneric<NullableAware>` is not convertible to `ObliviousGeneric<NullableAware?>`.

### Option 2: `<out T>` in class types

Eric Lippert's [StackOverflow answer](https://stackoverflow.com/questions/2733346/why-isnt-there-generic-variance-for-classes-in-c-sharp-4-0/2734070#2734070) sheds some light on why type parameter variance was not implemented for class types originally. Since then we have added `readonly struct` and have considered `readonly class`, and it might be interesting in the future to extend readonly types to permit covariant type parameters, for example.

However, to address the case people are actually complaining about, it feels a bit like using a wrecking ball to hit a nail. `Task<T>` is not readonly, and any change to actually make it covariant at the language level would be more of a hack.

### Option 3: status quo (suppression needed or explicit type arguments)

The two workarounds currently recommended for the original scenario are to either suppress the conversion warning with `!` or to provide an explicit type argument when creating the object.
```cs
Task<string> x1 = ...;
Task<string?> x2 = x1!; // could be doc'ed usage of suppression
x2 = Task.FromResult<string?>(""); // ok
```

The current articles for `!` emphasize its use to assert that a variable is *not_null*. There's little coverage that the operator will suppress these conversion warnings as well. Adding this information, especially linked to the F1 service for the specific warning generated.

### Conclusion

**Option 0 (special-casing)** would be most palatable, but we're not sure yet whether to prioritize making any change here. It will depend on how the cost/benefit of addressing this compares to other nullable issues on the docket.

Ultimately, we think the main issue here is that when users encounter this, they will naturally think the compiler is telling them they're doing something wrong. It may take some research for users to conclude that actually, the conversion is fine, and they should just suppress the warning or provide the additional type information on the input side. Any way that we can make that process better (i.e. documentation, samples, editor support) would also help here.

### Postscript

After meeting I wondered if it would be possible to reduce warnings in some cases by using the target-type of an expression as an input to the nullable reinference process. This is slightly similar to what is planned for `IList<string?> e = ["a", "b", "c"]` in [collection literals](https://github.com/dotnet/csharplang/issues/5354).

Essentially, in `Task<string?> task = Task.FromResult("value")`, the type of `task` would be contributed to the inference of `TResult` in `Task<TResult> Task.FromResult<TResult>(TResult)`.
- This step would probably only be done for nullable reference types, and would essentially be completely new, so it's difficult to say whether the design and implementation effort needed would be appropriate for the magnitude of the problem.
- It's possible this would introduce a cyclic dependency in the reinference process which makes the approach unviable.
- If it *did* work, it would address "factory method" issues in general, i.e. `ImmutableArray.Create<T>()`.

## Next

Our plan for the next working group is to do a higher-level overview of more nullable issues, and then dive into further depth in future sessions.
- `[MemberNotNullWhenComplete]`: https://github.com/dotnet/csharplang/discussions/5657
- LINQ: https://github.com/dotnet/roslyn/issues/37468
- Defaultable value types: https://github.com/dotnet/csharplang/discussions/5337
