Null oblivious variables treated as non-null.

[qrjo]

Version Used:
.NET Core 3.1, Visual Studio 16.4.5

Steps to Reproduce:
Calling nullable disabled code from nullable enabled code can cause missing warnings about NRTs. The code below demonstrates this. I can assign null to Value without warning, yet I don't get warnings when I'm using Value as if it isn't nullable. When I hover over s, Visual Studio wrongly tells me that s is not null here. I can pass a null value to UseString without getting warnings, which results in that method throwing the exception, because s should never be null there unless warnings were ignored.

Changing string s to string? s has no effect. If I move x.Value = null; above the assignment to s, the warnings show up.

namespace TestApp
{
#nullable enable

    internal static class Program
    {
        private static void Main()
        {
            X x = GetX();
            string s = x.Value; // No warning, expected: CS8600 Converting null literal or possible null value to non-nullable type.
            UseString(s); // No warning, expected: CS8604 Possible null reference argument for parameter 's' in 'void Program.UseString(string s)'.
            x.Value = null; // No warning, correct.
        }

        private static X GetX()
        {
            return new X { Value = null };
        }

        private static void UseString(string s)
        {
            if (s == null)
            {
                throw new System.Exception("BUG"); // This exception should never be thrown if there are no CS8604 warnings, yet it is.
            }
        }
    }

#nullable disable

    public sealed class X
    {
        public string Value { get; set; }
    }
}

Expected Behavior:
Warnings when using Value as a non-nullable string.

Actual Behavior:
No warnings.

[YairHalberstadt]

I think this is by design. Null oblivious code is literally that - null oblivious. It should not warn if you dereference it, and you can assign null to it.

If you assigned null to a null oblivious value, and then dereferenced it, it warns, because it's updated the state from null-oblivious to maybe null:

x.Value = null; // No warning, correct.
x.Value.ToString(); // This warns

But in your case, your first using the value, and then assigning null to it.

[jcouv]

@YairHalberstadt is correct.
Null oblivious behaves by assuming the best. If you read from a null-oblivious API, you get back a value that is not-null. You write to a null-oblivious API, we assume the API can handle it.
The code snippet behaves in accordance to that design.

[qrjo]

The question is if assuming the best is the right thing to do. I think assuming the worst makes more sense. There is absolutely no reason to assume the null oblivious API returns a non-null value, because returning null in null oblivious code is a perfectly correct and useful thing to do. Also, as a programmer I have absolutely no indication that the value I'm using is null oblivious. The compiler doesn't tell me, nor does Visual Studio. In fact, Visual Studio tells me the value is not null, even if I assign it to string?.

In short, this design choice is a great source for nasty null reference bugs, which is exactly what NRTs were supposed to prevent. If you have NRTs enabled, you don't ever expect the exception in my code snippet to be thrown, unless you willfully ignore warnings or use the null-forgiving operator.

[YairHalberstadt]

@qrjo
Since the compiler is behaving according to spec, and you want to change the design, I would suggest moving the discussion to csharplang.

Note there's an existing issue with plenty of discussion on this topic at dotnet/csharplang#2244

[jcouv]

@qrjo The reasoning behind this decision is that a project doesn't control all the code it depends on. Referencing a null-oblivious library would yield lots of warnings, which would deter folks trying to migrate. There are many trade-offs involved in the nullability feature, and when in doubt we leaned towards less annoyance (the analysis is not the strictest it could be because we think that would be unbearable).

That said, you are correct that null-oblivious are a potential source of danger.
One could write an analyzer that warns for any use of an oblivious API.
We are also considering making uses of oblivious APIs more visible in the IDE (via QuickInfo and colorization).

[qrjo]

Well, all those errors are pretty easy to solve with the null-forgiving operator, right? The developer should be forced to think about whether to assign a value to a nullable variable or a non-nullable one.

[jcouv]

@qrjo
If the API is within your control, the best approach is to annotate it.
If the API is not within your control, then, sure, we could force you to suppress. The question is the amount of work versus the value it provided. Warning a lot on code that is likely already safe is not helping and risks turning adopters away.

We recognize that some folks want more strictness than the trade-off this feature was designed to provide. As @YairHalberstadt pointed out, this is good feedback and discussion to have on csharplang.

[SRNissen]

Warning a lot on code that is likely already safe is not helping and risks turning adopters away.

"likely already safe" is a lot of assumption about code you have neither seen nor reviewed

For reference, the google search that brought me here was "why am I not getting null warnings", and the reason I did that search is because I have a failing service that joyfully dereferences objects without null checks because the compiler would obviously warn me if those could have been null.

Well they could, and were, and it didn't.

[CyrusNajmabadi]

@SRNissen as mentioned already, it's a tradeoff. We found that there were far too many false positives, making the feature unpalatable for the majority of users in the majority of user cases. So we went with a design that was actually viable, at the cost of potentially introducing false negatives.