Roslyn: Possible bug in the flow analysis of nullable reference types

[johnknoop]

I've just enabled the new nullable reference types feature.

In this example, myObj is nullable. But shouldn't the compiler understand that myObj won't be null in this case, since I'm testing for it in the if clause?

I'm using .NET Core 3.0 in VS 2019.

[orthoxerox]

It should, but the compiler cannot infer everything. In 8.0 it recognizes the most common null-check patterns only. Does the warning go away if you replace the test with myObj != null && myObj.HasChanged?

Also, I think this should be in dotnet/roslyn, since C# spec doesn't mention which null check patterns must be recognized.

[YairHalberstadt]

You can report this on Roslyn, and they may choose to add support for this pattern.

[CyrusNajmabadi]

Note: i think this is a good place for the issue. I think ti would take an update to the language to define what sort of intra-expression patterns that are detected. Once it decides, the compiler can follow through.

IMO, patterns like this seem totally reasonable for the language/compiler to detect. So i'd like to see this done at some point :)

[theunrepentantgeek]

I agree that this is a pattern that the language/compiler should likely support.

I can [eproduce the warning using SharpLab.

Changing the condition from ?? false to == true has the same semantics, and doesn't generate a warning.

[alrz]

Relates to discussion at https://github.com/dotnet/csharplang/issues/2764

I still don't think it makes sense to pinpoint to such "patterns" inside the compiler, this is a byproduct of how flow analysis works. Ideally, it should be extended in a general manner to understand always-true/false/null/not null in a broader context.

R# has done a good job to infer such states in the local scope even when the code is not annotated at all.

[johnknoop]

Another thing I would expect to work is when you initialize a member using ??.

In the example below, I get this warning:

Non-nullable property 'MyProp' is uninitialized. Consider declaring the property as nullable.

```csharp
public class MyClass
{
public MyClass()
{
MyProp ??= Some.Value();
}

public string MyProp { get; private set; }

}
````

If I initialize it using = instead of ??=, the warning goes away.

[YairHalberstadt]

@johnknoop

Again, I think you can report that on roslyn. Can I ask why you would do that though?

[johnknoop]

When the class is initially created, there are a few properties that aren't being set (the value isn't produced yet). Hence they have to be nullable.

Later on, these values are added to the persisted MongoDB document representing the class, in a way that only uses expressions and never instantiates the class.

When loaded, I don't wanna do null-testing in all sites of usage, so I added the "if null" assignments to the constructor, so the values of the properties are either a real value or a suitable default value.

[Joe4evr]

@johnknoop dotnet/csharplang#2328

At the very least, what you can do today is put your model classes under a #nullable disable warnings so that you're not bothered by those.

[johnknoop]

@Joe4evr Absolutely, I've already suppressed the warnings, so this is nothing that really blocks us. I just wanted to let you guys know so you can make the new nullable feature even more awesome.

Also, I might be a bit spoiled when it comes to flow analysis since I work a lot in TypeScript and it's just great when it comes to inference.

[Joe4evr]

TypeScript's flow analysis has had a lot more time to mature, though. But I certainly expect the C# LDT to keep improving the analysis rules in the future, so that devs can convey even more intent while also letting the compiler help more on the consuming side.

[Korporal]

I've just enabled the new nullable reference types feature.

In this example, myObj is nullable. But shouldn't the compiler understand that myObj won't be null in this case, since I'm testing for it in the if clause?

I'm using .NET Core 3.0 in VS 2019.

@johnknoop

How is the compiler to know that the value of myObj cannot be asynchronously changed on some other thread? for example you might have a timer setup that manipulates myObj.

[Joe4evr]

@Korporal The compiler doesn't make any such assumptions.

[333fred]

How is the compiler to know that the value of myObj cannot be asynchronously changed on some other thread?

The nullable feature assumes that if you check it, it's not going to change on a different thread. We also make this assumption on other things, like class properties and fields.

This is a compiler bug, I'm going to move to dotnet/roslyn.

[YairHalberstadt]

@333fred

Is there any reason you linked to that comment on that issue? It doesn't seem relevant?

[333fred]

Uh, whoops. @YairHalberstadt fixed.

[jcouv]

This was fixed earlier in 16.5. Verified the scenario. Closing the issue now. Thanks