Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
731 stars 488 forks source link

Apply nullable reference type & attributes to public APIs #2665

Open chwarr opened 3 years ago

chwarr commented 3 years ago

Is your feature request related to a problem? Please describe.

When coding, I would like to know when I should be expected to handle null values from the SDK or when I can pass null in.

Describe the solution you'd like

Annotate all public APIs with nullable reference types and related annotations.

Describe alternatives you've considered

Reading the documentation. Often, it is not documented when something will or will not be null, however, so I have to write tests or read the source code.

For example, Response<T>.Resource doesn't say if it could ever be null. From experience, the ItemResponse<T> that Container.DeleteItemStreamAsync returns has a null Resource.

Additional context

It looks like an initial foray for this was started in PR #1324, Nullable Enable. It now needs to be done to the rest of the SDK.

This doesn't need to be done in a single commit or a single release either. It can be added progressively.

I've retorfitted these annotations into an existing code base progressively by:

  1. Adding <Nullable>enable</Nullable> in Directory.Build.props
  2. Adding to the top of every existing file:
    #nullable disable annotations // do not treat RefType as a non-nullabe reference
    #nullable enable warnings // enable compiler null analysis
  3. Fixing any bugs the compiler reports.

New code should be written without the #nullable directives. Over time, existing files can be annotated correctly and their #nullable directives can be removed.

This may not seem like it adds much to the SDK, as it doesn't enable anything new, but it will improve developer productivity quite a bit. Please give this due consideration.

j82w commented 3 years ago

@chwarr can you also make the suggestion against the Azure SDK team? https://github.com/Azure/azure-sdk-for-net/tree/main/sdk/core/Azure.Core

chwarr commented 3 years ago

I see some existing issues that I think covers this. If they don't, can you please be more explicit about what I should be asking the Azure SDK team for? (I don't know how Cosmos DB and the Azure SDK relate the each other.) Thanks.

kirankumarkolli commented 2 years ago

Tracking it for V4 (future track2)