Roslyn: Quick info says calling an oblivious method is non-null

[jasonmalinowski]

Expected: there is no message in quick info, since the flow state would be oblivious
Actual: we get a message, which implies that GetTypeInfo returned non-null as the flow state

[333fred]

I talked with @gafter briefly about this. This is intended behavior. An oblivious value is being assigned to an explicitly non-null variable. Therefore, we presume that the user knows what they're doing and we say that it's not null.

[jasonmalinowski]

@333fred My mental model was "the compiler doesn't really look at top level nullability of locals, it's always ignored and the flow state is always computed regardless". My mental model was wrong in the case of oblivious being assigned something? To be clear I'm not pushing back on the design, just trying to understand it.

[333fred]

Yes, the compiler tries to lose oblivious as soon as it's able to.

[sharwell]

This is a serious deficiency of Quick Info. Once an oblivious value is assigned to a variable, we can no longer show "'item' is not null here". It's acceptable to show "The nullability of 'item' is not known here", or to show nothing at all, but it's not fine to make claims about the nullability of APIs that contain no information about it.

[dpoeschl]

I'd like to +1 on revisiting this. I'm trying to write a classifier that shows the nullability status of a local as it travels through a method. I'm using the current APIs, green means NotNull and orange means MaybeNull:

I've pointed out where the API says it cannot be null, but where it of course can be null.

Regarding @333fred and @gafter 's response to the earlier version of this:

An oblivious value is being assigned to an explicitly non-null variable. Therefore, we presume that the user knows what they're doing and we say that it's not null.

In my case, oblivious is being assigned to an explicitly nullable variable, so it's even weirder that it comes back NotNull...

[jasonmalinowski]

I'm absolutely for changing this -- the original design strongly felt of an implementation detail leaking out of the API.

[odalet]

+1 for this issue! It nearly got me in code interacting with a non-"nullable enabled" third party lib...

Quoting @stephentoub:

Showing incorrect information is worse than showing nothing at all.
If it may or may not be null, it may be null.

I could not agree more! And by the way, maybe the target milestone should be updated: can this be fixed (or mitigated) for 16.6?

[gafter]

@odalet

And by the way, maybe the target milestone should be updated: can this be fixed (or mitigated) for 16.6?

If you think you could do this with a clean design we’d accept in the next week or so, yes.

[odalet]

Got it, apologies for being too pushy... Roslyn is quite the piece of code and I wouldn't know where to start... I suppose I'll just back down for now and hope someone with the know-how wil try to tackle the issue.

[dotMorten]

This is especially a big problems with using 3rd party libs that aren't annotated like WPF. For instance this intellelisense makes it clear how misleading it is. For example the Content property on the ContentPresenter:

[sharwell]

@dotMorten Note that even after this is fixed, it's possible for situations like the screenshot to occur. For example:

if (this.Content is not null)
{
  _ = this.Content; // Here 'Content' is known to not be null, but documentation will still say the default
}