Introduce support for nullable reference types

[lailabougria]

CSharp 8 introduced nullable reference types. Developers of libraries and frameworks can enable nullability and annotate their types and methods with nullable attributes to make sure consumers of their APIs can compiler enforced warnings and hints.

It is possible to support nullable reference types even if you are targetting netstandard 2.0 as long as you are careful of only using the csharp 8 features that are supported like nullable reference types. Either way, CSharp 8 is enabled almost everywhere. For projects targeting .NET Framework, t is possible to bring the nullable attributes as well.

It is possible to gradually introduce nullable reference types on a "component by component"-base and even a "file by file"-base by adding #nullable enable to the file and from there start addressing the nullability issues until everything is in place to enable nullable on the project directly.

This can be done in a non-breaking manner. You can expose the attributes, as attributes don't need to be public like the actual types you expose in APIs do. You can declare them as internal when on the NS2.0 TFM, use them in all your public APIs, and consumers on any TFM will still benefit from them as you'd expect.

When this is picked up, please document guidance for downstream components to follow in the future. The work can be split into components (in which case issues should be opened for the components lacking support), or taken on as a whole.

[SimonCropp]

this would be great to see in V8. Or at least decorate the public APIs with nullable attributes

[SimonCropp]

bump

[andreasohlund]

We have started to introduce this starting with 8.1, the initial focus was the most used public APIs

https://github.com/Particular/NServiceBus/pull/6757

We have also added some approval tests to make sure that we add nullable on new types and the plan is to chip away at the list as we work with NServiceBus core going forwards.

[DavidBoike]

As part of NServiceBus 8.1.0 we started to introduce nullable types, focusing on common public APIs, as that would give the most benefit to NServiceBus users.

We also added a NullableAnnotations approval test that interrogates the public, non-obsolete API surface for nullability annotations and outputs the types that don't have it. The goal is for this list to decrease to zero over time. No new types should be added that don't have nullable annotations, and it should also hopefully serve to ensure we don't have any regressions in types that are annotated.

We did hit a few bumps along the way though, resulting in burning 8.1.0 and shipping 8.1.1 instead. The saga mapping expressions for ToMessage and ToSaga are expressed as (for example) Expression<Func<TSagaType, object>>. When the annotations were added to the file where those definitions lived, everything was fine. However during smoke testing we discovered that you can't have a saga data class with non-null members, as those members would need to be non-null before the constructor exits, and the user isn't even responsible for instantiating the saga data instance.

So your saga data is going to look like this (at minimum):

class MyData : ContainSagaData
{
    public string? CorrelationId { get; set; }
}

Which means a mapping expression .ToSaga(saga => saga.CorrelationId) gets flagged as a "could return null" warning. (In NServiceBus code, with TreatWarningsAsErrors active, this becomes a build failure.)

Rather than rip out all the nullable annotations, we applied the 80/20 rule and created the NullableApiUsages "test"…which means there are no assertions in the test, it's just set up as a file with a bunch of API usages and #nullable enabled just to make sure it all compiles correctly. The code inside should never be executed, but just tries to illustrate as much idiomatic usage of public Core APIs as possible to attempt to ensure there are no other "surprises" like the saga mapping expressions.

The task force wants to stress that this is not ideal. In retrospect, it would be far better to have the entire unit test project observe <Nullable>enable</Nullable> which would give far greater certainty that any nullable annotation added in the future doesn't create a warning when using NServiceBus APIs normally.

The task force feels that this strategy should be used in other component repositories as well: first enable nullable on the unit tests project, then start to convert the component itself.