Nullability analysis fails to warn on null coming from NameValueCollection. Why?

Question

Consider a .net core project with nullability analysis enabled in .csproj:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>netcoreapp3.1</TargetFramework>
        <RootNamespace>(...)</RootNamespace>
        <IsPackable>false</IsPackable>
        <LangVersion>latest</LangVersion>
        <Nullable>enable</Nullable>
        <WarningsAsErrors>nullable</WarningsAsErrors>
    </PropertyGroup>
(...)

Now consider this code:

public class NullabilityTests
{
    [Fact]
    public void Test()
    {
        var nameValueCollection = new NameValueCollection();
        string nullValue = nameValueCollection["I do not exist"];
        Foo(nullValue);
    }

    private void Foo(string shouldBeNotNull) => Assert.NotNull(shouldBeNotNull);
}

This code compiles and runs with no warnings, but the test fails on the Assert.NotNull. Thus the nullability analysis failed at detecting the null, or even warning about possible null. Why? If such situations happen, how can I know when to trust C# 8 nullability analysis?

Answer 1

The NameValueCollection type simply has not been updated yet. From Introducing Nullable Reference Types in C#:

If they add anotations after you, however, then the situation is more annoying. Before they do, you will “wrongly” interpret some of their inputs and outputs as non-null. You’ll get warnings you didn’t “deserve”, and miss warnings you should have had. You may have to use ! in a few places, because you really do know better.

(emphasis mine)

In particular, the type is declared as returning the type string, which when nullable reference types are enabled, means that the reference cannot be null. The compiler has no reason to think otherwise, so no warning can be emitted.

Since you know that null is a possible return value, it's up to you to enforce that. Which you can do by simply declaring the receiving variable with the correct type:

string? nullValue = (string?)nameValueCollection["I do not exist"];

(You need to explicitly cast, because otherwise the compiler's static analysis will still treat the now-nullable string? variable as non-null anyway.)

Normally, the ideal way around this would be to use a more modern type. For example, you could use Dictionary<string, string?> as an equivalent nullable-aware collection. However, you note that in this case you're getting the collection back from another .NET method (HttpUtility.ParseQueryString()).

If you want more robust protection, you can immediately copy the collection to Dictionary<string, string?>, or you could implement a wrapper for the NameValueCollection type that declares the indexer with the more-correct string? return type.

A possible example of the former would be something like this:

var nameValueDictionary = nameValueCollection
    .Cast<string>()
    .ToDictionary(k => k, k => (string?)nameValueCollection[k]);