Update the nullable presentation for locals/parameters in the IDE.

[LWChris]

Problem

Consider this code:

#nullable enable
public void Foo()
{
    var val1 = p.Count; // int
    var val2 = p.Max; // int?
    var ref1 = p.GetOrCreate(1); // TheType
    var ref2 = p.TryGet(2); // TheType?

    // val1 = null;
    val2 = null;
    ref1 = null;
    ref2 = null;
}

With #nullable enable, the line ref1 = null causes a warning, because the type of ref1 was inferred from GetOrCreate whose return type isn't nullable. The line val1 = null would obviously not even compile.

In order to have all variables nullable, I'd either have to change the return type of GetOrCreate and Count to nullable types (which is bad because both will never return null, so why declare otherwise), or I have to use explicit type, which can range from
int? val1 = p.Count;
to
TheTypeThatRequiresAnImport<WithGenerics, WhichRequire, EvenMoreImports>? ref1 = p.GetOrCreate(1);

Proposal

If var ref1 can be resolved to TheType ref1, then var? ref1 could be resolved to TheType? ref1.

Considerations

This syntax can theoretically be allowed for reference types and value types. For both cases, var? will ensure that the declared variable is nullable, regardless of the right-hand side's nullablity (for value types) respectively nullable annotation (for reference types).

For value types, the behavior will be:

public void Foo()
{
    var? val1 = p.Count; // int? val1 = new Nullable<int>(p.Count);
    var? val2 = p.Max; // int? val2 = p.Max;
}

which is equivalent to

public void Foo()
{
    var val1 = MakeNullable(p.Count);
    var val2 = MakeNullable(p.Max);
}

private static T? MakeNullable<T>(T value) where T : struct => new T?(value);
private static T? MakeNullable<T>(T? value) where T : struct => value;

For reference types, the question mark behaves the same like the one in TheType? obj:

var? ref1 = p.GetOrCreate(1); // TheType? ref1 = p.GetOrCreate(1);
var? ref2 = p.TryGet(2); // TheType? ref2 = p.TryGet(2);
#nullable disable
var? ref3 = p.GetOrCreate(3); // Warning: The annotation for nullable reference types should only be used in code within a '#nullable' annotations context.

[HaloFour]

This will no longer produce a warning in C# 9.0 as the language team has decided to always treat var as nullable and to rely on flow analysis to determine when to warn. Assigning null will no longer warn, but trying to deference after that assignment will.

Example

[LWChris]

1. Your example shows a warning for me at ref1 = null. Do I have to set the language to something like "C# 9.0 Preview" somewhere on the page?
2. Apart from that I thought solving the question of whether something will ever be null is equivalent to solving the halting problem, I think "always treat var as nullable" sounds dangerously close to "always treat references as nullable". To me, this looks like a step in the opposite direction of where the whole concept of "non-nullable-references" was aiming at. In my opinion, you should treat var as what it is - that is "the type of the right-hand side", including the nullability.

[HaloFour]

@LWChris

Your example shows a warning for me at ref1 = null. Do I have to set the language to something like "C# 9.0 Preview" somewhere on the page?

If the selected branch is "master" then it should be using the C# 9.0 bits. I do not see the warnings there, but I do see the warning if I change the branch back to "Default".

I think "always treat var as nullable" sounds dangerously close to "always treat references as nullable". To me, this looks like a step in the opposite direction of where the whole concept of "non-nullable-references" was aiming at.

The flow analysis of NRTs is pretty good at determining when it's safe to dereference the variable and it's already a core aspect of the experience. The behavior of the variable will remain the same. If the method returns a non-nullable, the compiler will consider the variable to not be null and will not warn on dereferencing. If you assign null to the variable the compiler will not warn at the point of assignment, but it will warn if you attempt to dereference the variable again without first checking for null:

string SomeMethod() => "";

var s = SomeMethod();
var length = s.Length; // does not warn
s = null; // does not warn
length = s.Length; // warns
if (s != null) {
    length = s.Length; // does not warn
}

The nullability annotations aren't that important on locals, the compiler will infer from usage. It's at the boundary where the annotations are much more important because the compiler can't know the expected state of the value crossing that boundary.

In my opinion, you should treat var as what it is - that is "the type of the right-hand side", including the nullability.

That is what the language team thought, too. But the experience and feedback from that decision is that the behavior is suboptimal, especially around existing code, so the team decided to reverse that decision. The warning on assignment isn't particularly helpful if that variable isn't dereferenced again without checking for nullability.

Anyway, here's the meeting notes from when the language team already considered the addition of var? and decided on this behavior for C# 9.0 instead:

https://github.com/dotnet/csharplang/blob/master/meetings/2019/LDM-2019-12-18.md#var

[sharwell]

@HaloFour the behavior you link to is enabled for C# 8 as well, provided the compiler version is new enough.

[LWChris]

s = null; // does not warn I was really hoping this would change once and for all when I saw non-nullable references found their way into C#. IMO, it should. It's a pity, but oh well. C# is a language, not my language, so it's not my choice to make.

I'd rather opt into being pedantic about forcing you to express what you wanted by using var or var?, than relying on inherently weaker local context analysis. It's one thing to not mess with old code just for convenience (protected virtual event NotifyCollectionChangedEventHandler? CollectionChanged;), but another to mess with it to prevent future errors, and when the "fix" is merely adding the ? to the var keyword a few lines earlier. Even more so, if adding that one character doesn't get rid of all warnings immediately, you apparently just identified a risk you never thought about. Sounds promising, not annoying to me.

Thanks a lot for the link though.

[CyrusNajmabadi]

I was really hoping this would change once and for all when I saw non-nullable references found their way into C#. IMO, it should. It's a pity, but oh well. C# is a language, not my language, so it's not my choice to make.

You can always write an analyzer yourself to find and report thsi if you want that extra restriction :)

I'd rather opt into being pedantic about forcing you to express what you wanted

We looked into this, and it was effectively awful. So much code that was null safe and was written in idiomatic C# was then flagged as needing additional work.

This violates my own goal for the feature which is that good, existing, coding practices should not feel like something that is now punishing you.

[LWChris]

You can always write an analyzer yourself to find and report thsi if you want that extra restriction :)

Not every programmer has the skill or time to write custom analyzers to mitigate shortcomings of the native compiler. ;)

So much code that was null safe and was written in idiomatic C# was then flagged as needing additional work.

I understand your point and see where you're coming from. But quite honestly - with this behavior for #nullable enable, the C# language team might as well consider to remove the whole feature again. After all the point of non-nullable-references is to introduce references that cannot be null. Now they can be null, and we still have to rely on flow analysis. But flow analysis can be done with or without #nullable enable, right? So what will be the point of having #nullable enable scopes in the first place?

If x = null is an error for int x, then it should be an error for SomeReferenceType x when #nullable enable is defined.

If it bothers me that I must write var? x = Foo(); x = null; instead of var x = Foo(); x = null;, because I know I have always checked for null when I use x later on, then why did I even enable the feature in the first place? I might as well disable it and rely on flow analysis anyway, since apparently flow analysis is "good enough" to catch all issues.

And maybe, after killing #nullable, C# may introduce #nonnullable, this time not with SomeReferenceType? vs. SomeReferenceType, but with SomeReferenceType vs. SomeReferenceType! or so.

[HaloFour]

@LWChris

The goal of NRTs is to avoid NREs, not nulls. A null assigned to a local that is never dereferenced is innocuous. You can argue that this makes annotating locals with their nullability is pointless if flow analysis can make this determination, and I agree with you. The annotation makes sense at the boundary where flow analysis can't track what happens. But I disagree that this decision somehow weakens NRTs or makes them irrelevant. NRTs were never intended to be a change to the type system, they're meant to be guardrails, and they need to be a very pragmatic solution so that can be practically applied to tons and tons of existing code.

[333fred]

But flow analysis can be done with or without #nullable enable, right?

No, that is not how the design works. You must, at minimum, turn on the flow analysis with #nullable enable warnings. Nullable does not run in disabled locations.

[LWChris]

#nullable enable

public class C {
    readonly string S = "s";
    
    public void A() {
        string s = S;
        bool flag = s == null; // 1
        int length = s.Length; // 2
    }
}

Question 1: What is the point of having a wrong warning "Dereference of a possibly null reference." in line 2 instead of a correct warning "Expression is always false." in line 1 with #nullable enable?
Question 2: If we establish that showing the warnings as described above was somehow useful, then what is the point of not showing them without #nullable enable?

In other words: if NRT are meant to be "guardrails", then why are those rails installed in a way that it is easy to make them ineffective (by adding a useless null-check), and if we deemed them sensible nevertheless, why would I have to explicitly ask for the guardrails to be installed?

[HaloFour]

@LWChris

The guardrails exist to steer you away from NREs. That may warn spuriously, but it won't NRE. More interesting would be non-pathological cases where the compiler doesn't warn and can result in an NRE.

[333fred]

Consider instead a public API: It's annotated as not accepting null, but of course someone could be calling you from a nullable disabled context or they could put a ! on it because they're pretty sure they're not passing null. However, you still want to check the input for null, and if you do so, we should make sure you actually handle it. Or perhaps the return type of a function is not annotated, or the person who annotated it was wrong. Nullable is not perfect: it's designed to take your input into account. We assume that you know what you're doing, and that if you check for null then there's a real reason to do so.

[CyrusNajmabadi]

Not every programmer has the skill or time to write custom analyzers to mitigate shortcomings of the native compiler

I disagree. The cost to doing something like this is extremely small. So either this is a significant issue for you, in which case writing hte analyzer is a cheap and easy way to solve the problem. Or it isn't an significant issue, in which case it doesn't really matter if it solved :)

After all the point of non-nullable-references is to introduce references that cannot be null.

This was never the point. The point was to diminish the quantity of null reference exceptions, and to introduce a system that had a low amount of false positives and false negatives.

in line 2 instead of a correct warning "Expression is always false." in line 1 with #nullable enable?

It would be fine to introduce an analyzer to give such a warning. No one is stopping that.

[LWChris]

This was never the point.

If that is so, this is the root cause of my problems with that behavior, and apparently some others. Because everything that has been named, said, documented or marketed about this feature promises exactly what you say "was never the point".

Just read the introduction of the official documentation:

C# 8.0 introduces nullable reference types and non-nullable reference types that enable you to make important statements about the properties for reference type variables:

• A reference isn't supposed to be null. When variables aren't supposed to be null, the compiler enforces rules that ensure it's safe to dereference these variables without first checking that it isn't null:
  • The variable must be initialized to a non-null value.
  • The variable can never be assigned the value null.
• A reference may be null. When variables may be null, the compiler enforces different rules to ensure that you've correctly checked for a null reference:
  • The variable may only be dereferenced when the compiler can guarantee that the value isn't null.
  • These variables may be initialized with the default null value and may be assigned the value null in other code.

[...]
With the addition of nullable reference types, you can declare your intent more clearly.

• The term "non-nullable reference types" is misleading, because they are still considered nullable enough to allow assignment without warning, even if you explicitly declared otherwise at every possible location.
• "that enable you to make important statements about the properties for reference type variables", except that these statements are going to be ignored at the next best opportunity.
• "you can declare your intent more clearly", but the compiler is very eager to ignore your declaration and replace it with its own assumptions.
• "the compiler enforces rules that ensure it's safe to dereference these variables without first checking that it isn't null", which turns out to be "you can shatter that enforcement by writing x == null somewhere" and then we will urge you to introduce more checks instead of allowing you to get rid of unneccesary checks.
• "The variable can never be assigned the value null.", except you wrote x == null somewhere, or you used the var keyword like advised. And it's not just "not an error", it's not even a warning anymore.

What's even worse, the documentation continues to compare it to the "earlier" behavior:

This new feature provides significant benefits over the handling of reference variables in earlier versions of C# where the design intent can't be determined from the variable declaration. The compiler didn't provide safety against null reference exceptions for reference types:

• A reference can be null. The compiler doesn't issue warnings when a reference type is initialized to null, or later assigned to null. The compiler issues warnings when these variables are dereferenced without null checks.
• A reference is assumed to be not null. The compiler doesn't issue any warnings when reference types are dereferenced. The compiler issues warnings if a variable is set to an expression that may be null.

Comparing this to the current behavior:

• "earlier versions of C# where the design intent can't be determined from the variable declaration" - now I can seemingly declare design intent at declaration, but actually somewhere the code I may accidentially change the believed intent, so we're back at "you have to read all the code to know if a variable is considered "not null" or "nullable".

Statements about when a reference "can be null":

• "The compiler doesn't issue warnings when a reference type is [...] later assigned to null." - yup, got that behavior back.
• "The compiler issues warnings when these variables are dereferenced without null checks." - yup, got that behavior back as well.

Statement about when a refereces "is assumed to be not null" (in fact, the feature was marketed to get rid of "assume" and introduce "declare", which is stronger):

• "The compiler doesn't issue any warnings when reference types are dereferenced." - you may now see warnings even if you declare "not null", so this got even worse.
• "The compiler issues warnings if a variable is set to an expression that may be null." - not if you used the var keyword, or checked for null anywhere above. Got worse as well.

I hope you can see what a gigantic let down this feature appears to have become, compared to what was promised. I want you to take a step back and not look at why a decision was made, but look at the bigger picture of what the feature now brings to the table in comparison of what is said it brings to the table.

Optimizing the behavior under the premise of "show as few warnings as possible" and "signal 'everything is fine, you don't have to change a thing' as long as possible" is creating a meaningless tool. NRT/NNRT promised and is marketed to allow me to get rid of vague assumptions and replace those with rigid declarations. But everything that has been done ever since were softening those "declarations" back to "assumptions". These new assumptions may be smarter than before, which is a fine thing in itself, but why even introduce "better algorithms" as a new feature, not as improvement of the already existing feature.

I'm genuinely disappointed. It was announced as "Declare if a reference can be null." and has devolved into "We try to hopefully figure out correctly most of the times whether a reference may be [allowed to be set to] null and will show you some warnings at some places where we assume they are appropiate, and if you think long enough about them you may even realize that the problem is not actually where we mark your code as problematic, but at an utterly different place in your code that has nuked our fragile heuristics that silently overrode your declared intents."

[LWChris]

In short:

• The goal was to create a feature that allows developers to escape the "might be null" situation.
• This would have enabled developers to code in a context where they don't have to worry about NREs, even without tedious null checks at every place.
• Using the premise "null checks are implicit declarations of nullability" instead of "a declaration of non-null is a declaration of non-null" creates a confusing behavior and inhibits the goal of creating a null-safe context, because now, checking for null on a non-null variable is not considered unnecessary, but instead destroys the whole null-safe context, re-introducing the necessity to check for nulls everywhere else.

[CyrusNajmabadi]

and have the compiler reject assignments of null so that it won't compile, unless I write my own analyzer.

Or you can make those warnings into errors.

[CyrusNajmabadi]

The current compiler however compiles string name = null; just fine, it only shows a warning. Therefore the documention does not match the feature.

The compiler enforces this by telling you about the issue. You can choose to have that be blocking or not. But we leave that choice to you, we do not want to enforce that on an astronomically large ecosystem with thousands and thousands of groups and projects all with varying needs in terms of how they can adopt this concept.

[CyrusNajmabadi]

Build started...
1>------ Build started: Project: ConsoleApp1, Configuration: Debug Any CPU ------
1>You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview
1>C:\Users\cyrusn\source\repos\ConsoleApp1\ConsoleApp1\Program.cs(23,20,23,24): error CS8600: Converting null literal or possible null value to non-nullable type.

[CyrusNajmabadi]

I don't think it means what you think it means.

I think you're confusing "blocking" with "enforcing". Our goal of NRT was to be non-blocking for users. Any reasonable sized project will take a fair amount of effort to move to NRT. In roslyn, we've been continuing to NRTify our whole codebase, and it's been taking months and months and months. It's imperative that people be able to make this sort of progress in an enforced, but non-blocking, fashion. That's what we provided and it was very intentional and necessary to be viable.

OUr defaults seem to not be what you want. However, you can change the defaults here to better suit you if you want.

[LWChris]

I am satisfied with this result, there are some quirks and it still sucks that var doesn't infer the nullability from the RHS, but you can mostly work around that.

However I really honestly do recommend to change the documentation to not use the phrase "enforce a rule". I'm convinced the majority of all readers will completey misunderstand that.

Ask 100 C# programmers who aren't involved in compiler development:

The compiler enforces that you cannot assign null to the variable name.
What will happen if you write name = null; anyway?

A) The code doesn't compile
B) The code compiles with a warning
C) The code compiles without a warning

I bet at least 90 will answer "A". 🙂
(Edit: I asked my colleagues, and they either said "A" or "I'd say A, but the fact that you ask me an obvious question makes me suspicious." 😄)

[jez9999]

I have to agree with @LWChris completely on this one. I didn't have a particularly strong opinion on the introduction of non-nullable types, but if you're going to do it, why make something as fundamental as the var keyword opt-out by default? The nomenclature around it really makes you as a programmer think that "turning on non-nullable types for the project" will make reference types non-nullable by default. The fact that var x = new Foo(); creates a nullable Foo in this context is unexpected, whether or not flow analysis still warns about null assignment. If flow analysis is so good, why bother with non-nullable types at all? If existing code gets warnings, why turn this feature on for existing code? It's opt-in.

[HaloFour]

@jez9999

why make something as fundamental as the var keyword opt-out by default?

NRT is opt-in specifically because it involves a migration for any existing codebase. The goal is to eventually get to 100% adoption where NRT is the default behavior, but that will take time. Given the vast impact NRTs has on existing codebases compromises had to be made so that it would be easier to adopt. When someone is evaluating opting-into NRTs for their codebase if they see thousands of warnings they will be less likely to consider the effort necessary to carry out the migration, so it's desirable to ensure that those warnings only appear in cases where NREs are more likely to occur.

The feedback from the first release is that inferring the nullability based on the RHS resulted in way too many extraneous warnings. While warning on assignment may help you identify where a null accidentally flowed into the local, the assignment itself is benign until that local is used. At that point NRTs would provide a warning, so the language is still providing that guard against NREs.

The perfect is the enemy of the good.

If flow analysis is so good, why bother with non-nullable types at all?

Flow analysis only really works well within the body of a method. It becomes infinitely more complicated across the boundaries between methods. Sure, the compiler could have inferred the metadata on the boundary based on use, but at that point the nullability becomes a part of the contract of that method, and the team is pretty consistent on the belief that said contract should be explicitly defined.

As for locals, I agree, having to explicitly notate which are nullable has a lot less value and could have been driven entirely by flow analysis.

[markm77]

I hope one day we get a v2 of non-nullable reference types with with real non-nullable types at runtime level. That would solve a host of confusions and complexities that arise because flow-analysis nullability is different from underlying nullability.

It would also remove the need for run-time null checks of non-nullables and mean var could infer non-nullable or nullable type based on the right hand side and stay consistent with the underlying type.

[jez9999]

@HaloFour Of course turning on nullable reference types for existing codebases is likely to create a bunch of warnings. But it's opt-in. I guess I misunderstood the purpose of this, because I assumed it would be for new code. Why not turn non-nullables on for new code, and use flow analysis on existing code?

[HaloFour]

@jez9999

I guess I misunderstood the purpose of this, because I assumed it would be for new code.

The goal is to make it work across all codebases and to make it relatively easy to adopt for maintainers of existing projects. Avoiding spurious warnings is intentional, especially if they arise in common cases.

[SRNissen]

Well I just spent a good hour or so of my time debugging and then googling why a set of circumstances resulted in a runtime null deref that should never have happened.

I would definitely prefer it if var t = FuncThatReturnsT() would infer t as type T rather than T? because T? might be null, which means I put in a null check, which means now it actually might be null which it couldn't before despite being T??

This is not what you might call "easily guessable"

EDIT: If changing var to (var and var?) is a non-starter due to legacy code, a change to (var and var!) would be adequate to my needs - assuming this includes compiler errors when (assigning null, possibly assigning null, or checking for null) of a var!-variable.

[SRNissen]

This is what I ultimately did

Instead of

Example 1
01  public class A
02  {
03      public List<int> list;
04  }
05  
06  var a = Database.Get<A>(key);

I now do a variation on

Example 2
07  public class A
08  {
09      public List<int> list;
10  }
11  
12  public class B
13  {
14      public List<int>? list;
15      public A MakeA()
16      {
17          if(list == null) throw
18          return new A(){list = list};
19      }
20  }
21  
22  var a = Database.Get<B>(key).MakeA();

But man what a drag. Would have loved it if the analyzer warned me on line 06 of example 1 than it couldn't guarantee anything about null states for an object acquired from Database.Get, and (since this is the case) I would also love it if future me could write code that looks a like:

Example 3
23  public class A
24  {
25      public List<int> list;
26  }
27  
28  var a = EnsureNoNullsOutsideOfNullableFieldsOf<A>((Database.Get<A>(key))); //throws if unsatisfied

[HaloFour]

Which deserializer are you using? My understanding is that some projects have at least offered options to consider nullable reference metadata. System.Text.Json I believe respects required now and you can use that to enforce that the list member at least has a value. Otherwise I might ask whatever deserialization library that you're using to consider respecting either NRT or required metadata and to enforce validation. You shouldn't have to write additional code around each deserialization like this.

[SRNissen]

This project interacts with three separate data sources and I honestly don't remember which of them it was, but it's the deserializer built into either graphql-dotnet or the one in CosmoDB, or it was just NewtonSoft directly.

EDIT:

Looking back into my test cases, git commits, and cross-referencing with my first comment in this discussion, it is probably CosmoDB.

But functionally, my takeaway lesson as a programmer cannot be "Blame CosmoDB," it has to be "every time I work on data from a supplier whose source code is outside of my control, I should write code to reassert NRT compliance" - this is, I have learned here, how the NRT concept fundamentally works.

[svick]

@LWChris

But this used to be perfectly fine code. How can it know the object will be used in a #nullable enable context?

The serializer can't know how the object will be used, but it can know how its type was declared. And I'd say it's one of the main design points of nullable reference types that "perfectly fine" code changes meaning (but not runtime behavior, at least not directly) depending on the nullability context.

Here is an example code showcasing that.

[LWChris]

@SRNissen

If NRTs are enabled, you would get a warning in line 3 of example 1 that list may be null. To fix that, one would declare list as nullable, that would yield the desired warning that a.list may cause a NRE. Right?

[SRNissen]

If the deserializer claims to return a non-null reference then the language will respect that.

Somewhere in the chain of custody, somebody is giving a guarantee they shouldn't give, and somebody else is believing that guarantee.

In extremis, if the analyzer cannot guarantee that the object handed back by the deserializer actually conforms to the class' invariants (Here: All reference types have non-null values) it should not believe the deserializer's claim to have created such an object.

Alternatively: I, as the user, should not believe the compiler/analyzer's assurance that those non-nullable refrences don't contain null - but if those assurances aren't credible, I'd rather not receive them in the first place.

The actual solution I ended up with was marking all the references in the object as nullable - but I'd certainly have appreciated a warning or compile error instead.

[HaloFour]

Hence my desire for the compiler not to implicitly claim it knows.

There's nothing implicit happening here. You told the compiler that the property is non nullable.

[SRNissen]

I will leave aside a long pedantic argument about what "implicit" means and instead say:

You told the compiler that the property is non nullable.

Since such properties can be null anyway, that statement is always false. So why did the compiler believe my statement? Why does the language even allow me to make that false statement?

[CyrusNajmabadi]

In extremis

The problem there is that you would then have warnings on every line of code (including after performing a null check). That's because (again, in extremis) something in between could then change a value to be null.

The purpose of nrt is not to have perfect conservative analysis. If it did have that, the feature would be so onerous as to be unusable. The purpose of nrt is to allow users and libraries to provide enough information to give a high amount of true positives and a low amount of false positives. This then doesn't overwhelm the user with unnecessary issues to deal with, and instead helps call out real problems that should get attention.

However, a consequence of needing to prevent huge amounts of false positives is that you can occasionally get false negatives. This balance is intentional to actually ensure the feature is useful and non-onerous.

[HaloFour]

So why did the compiler believe my statement? Why does the language even allow me to make that false statement?

The compiler can only work with what it knows and what it can control. The compiler knows that you annotated that property as non-nullable. The compiler will warn if you attempt to assign null to that property from your own code. The compiler cannot control that some other assembly did reflective assignment which set the underlying value of that property to null, thus invalidating that contract. This is unfortunate, and it definitely puts the onus on developers writing model classes to understand the potential nullability of those properties. If your serialization library does support enforcing the nullability contracts you may want to consider enabling it. There didn't seem to be consensus as to whether that should happen due to the potential runtime impact of false negatives in existing code that would otherwise be "fine".

[CyrusNajmabadi]

Since such properties can be null anyway, that statement is always false. So why did the compiler believe my statement? Why does the language even allow me to make that false statement?

Addressed above, but I'll reiterate: because otherwise the feature would be useless.

Users would get null ref warnings on everything, making the system so onerous and painful it would never be used.

We do not attack these problems with any sort of intellectual persuot of purity. We attack then with a pragmatic understanding that the feature must be useful to be used by people. And that means making it so that:

1. Safe code normally doesn't warn (but rarely still might). And,
2. Unsafe code normally should warn (but rarely might not).

The practical benefit here is that the feature is both useful and used. And that null-ref-exceptions are greatly diminished (though not eliminated). However, it also means occasionally you might be in a scenario where you have to deal with an incorrect warning, or you do not get a warning.

Again, we need a feature that the billions of lines of code or the can actually adopt and thereby reduce reduce null-ref issues. We do not need a feature that only a tiny fraction of people would ever use.

[Liander]

I got here because (like many others) I was surprised by the seemingly changed behavior of var. Like most, I am expecting that it is interchangeable to use var instead of using explicit type declarations. But with NRT that is no longer true by design.

Now I am starting to read that people are abandoning the use of var into using explicit types instead in new projects. Not to draw too many parallels but with the targeted-typed new expression there could be a trend starting to not using var at all. Personally, I have got used to using var and feel it increases readability when you can easily infer the type yourself by assuming it will be inferred from the RHS. I really doubt it is the intention that we start to move away from using var.

I totally understand that you cannot have a RHS handling of var type when enabling NRT in old projects. In this thread I have learned that this was how nullability was designed and implemented at first, which turned out to be useless/impractical for legacy projects. But what about new projects where that argument does not exist? For new projects you might not need to loosen the nullability state at assignments to var declared variables. Has it been discussed to have yet another flag for the handling of var when using NRT in primarily new projects?

@CyrusNajmabadi Any thoughts on adding an additional flag for getting back the old behavior on variable assignments declared with var (i.e that it is interchangeable to declare a RHS type vs using var in the behavior)? Perhaps I am missing some aspect of it, but it feels like var-handling when using NRT can be treated in separation from NRT flow analysis and therefore deserves its own flag of preference and it is a flag to actually restore a pre-existing behavior, so there is no new experience introduced.

[Liander]

I don't think you need flow states in some of those cases.

Actually the biggest gain of all is if you display a nullable type on variables only when it is required and non-nullable otherwise. Then there are a lot of state cases disappearing. A non-null reference type on a variable is actually also a null state case, but one of being not-null and one that has been concluded not to change, so it tells you much more. The thing is the compiler knows these things, so why am I not being presented with it? Showing a nullable type and information from the flow analyzer such as "'s' is not null here and the variable is never reassigned with a nullable type" implies it actually is a non-nullable type, which is what the compiler knows. It would therefor be a much more succinct way of reporting it as such. Now the experience is much more complex than it needs to be.

Actually I think it is worse than that. I think by showing the declared type being always nullable type indicates it is a statically declared nullability which it is not. It teaches the user the wrong thing. What is statically declared is the underlying type and its nullability comes from the null state flow and since the nullability is shown together with the type it needs to be derived from the flow analyzer. But I should stop sounding like a broken record...

IMO the whole UX around this feature could probably do with some additional thought.

Yes. First, let's consider having the settings "Display inline type hints" (under Inline Hints) and "underline reassigned variables" (under Editor Help) then you get this screenshot:

So there is no point in showing a 'string?' on 'var1' since the compiler knows it is never being reassigned and thus it will always be a 'string'. It should be presented with what the compiler knows, which is that it is a 'string'.

In the case of 'var2' it is reassigned and the compiler knows the effective required type has changed to a nullable type in the state flow (i.e it needs dynamic null state representation) so in this case it makes sense to report a 'string?'. The underline at the declaration indicates that the inferred effective type cannot only be seen from the RHS, but is an effect from multiple assignments.

I think that illustrates what I am after, even if you decide other ways to indicate the reassignments, perhaps more centered around when it is causing the 'effective' nullability of the inferred type to change. If that indication should always be shown as done with the underline-reassigned-variables setting or only when showing QI and when modified (as mentioned in #63959) is also UX thing to consider.

If it really is a nullable type then you should present the null state at various places where appropriate, but don't forget this non-nullable case. This also has the benefit of awarding good coding reducing nullable state, instead of always penalizing with state representation even in cases when it is not needed. And this is not just about QI which I think the above image shows.

[CyrusNajmabadi]

The thing is the compiler knows these things, so why am I not being presented with it?

Because no one has submitted a PR to do this. But contributions are welcome. No one has found this important enough to invest in though so far.

[Liander]

It was partly a rhetorical question since there doesn't seem to be an agreement of what the UX around this should be. I don't think my family would allow me to do a PR just to show... 😞 but even if I had the luxury to invest unpaid time doing it, it needs to be done with some kind of agreement, otherwise it is a just waist.

[CyrusNajmabadi]

it needs to be done with some kind of agreement,

The PR can help with that. By and large, it seems like it can easily fall out from the test cases. I would not anticipate contention or difficulty with determining what would be better behavior.

Indeed, this would be a case where TDD would be a good approach to do this work. Start with the tests showing what we'd want the experience to be in a multitude of cases. Once agreement is reached on that, the impl can simply follow. Thanks!

[CyrusNajmabadi]

It was partly a rhetorical question

Sure. But i wanted to make it clear: there is no explicit force requiring the presentation to be how it currently is. If there is interest in changing this, it can be changed. We accept PRs all the time in the IDE to improve a myriad of experiences. This would just be yet another case. So if there is interest, please go ahead and start something up, and we'll def help out. Thanks!

[LWChris]

I would like to chime in again after all this time to report what I think about it now:

The biggest trouble back when I wrote this came from the stark difference between what the documentation claimed NRTs would do, and how they actually worked. You know, statements like "The variable can never be assigned the value null", but in reality you absolutely can do that and it even compiles just fine with merely a warning. These statements are long-gone since the docs have been changed.

I was (and would still be) hyped for a way to get what was promised back then, i. e. "can never" means "can never successfully compile, like int x = "hello"; can never compile. So, "can never" as in the literal meaning to the words, not some niche "compiler-developer jargon" where "can never" means "will always at least show a warning". (Oh and before anyone mentions it: no, I am not going to sink months into writing my own analyzer).

As for the current state of NRTs - I think it works "well enough" for about 80% of my code.

The pain points are

• serialization (because if you annotate it nullable, you get annoying warning all the time when you're using the object, if you don't mark it, you have to initialize every propert with = default!; to keep your project from overflowing with warnings.
• Nullable<T> (e.g. if (max.HasValue) { return max.ToString(); } yields a warning)
• the lack of a MemberNotNullWhen with anything but bool results (e.g. an object that specifies with an enum which other properties are and aren't null)
• static initialization
• the amount of annotations flow state needs and how easy it is to write code where flow analysis loses track and needs new annotations
• having to fully qualify lengthy type names and having to include all using-statements for those types just to be able to declare a variable of a non-nullable reference type that would warn you of (possibly) assigning null (because var which saves me all that work unfortunately always is nullable)
• (in conjection with the previous) the availability of !; my co-workers tend to use this a lot to make the IDE shut up, because it's faster than annotating three dozen members with attributes

[CyrusNajmabadi]

You know, statements like "The variable can never be assigned the value null", but in reality you absolutely can do that and it even compiles just fine with merely a warning.

This is odd as that's not a statement we would have ever made. From the start, the feature was always a best effort, and was known that it could not and would not be able to enforce those restrictions.

As such, it's seems likely that that messaging came from different sources. But we can't ever practically control that. We can message accurately on our end, and attempt to correct it some people get it wrong, but ultimately incorrect information will spread.

[LWChris]

I had quoted it in an earlier reply of this issue. The text has been altered and complemented with an additional sentence. It now reads

The variable can never be assigned the value null. The compiler issues a warning when code assigns a maybe-null expression to a variable that shouldn't be null.

The second sentence is new. I would still prefer that the docs got rid of the "cannot/can never/enforce" terminology and instead used "should not/should never/checks". Like so:

Current text:

You use annotations that can declare whether a variable is a nullable reference type or a non-nullable reference type. These annotations make important statements about the null-state for variables:

• A reference isn't supposed to be null. The default state of a nonnullable reference variable is not-null. The compiler enforces rules that ensure it's safe to dereference these variables without first checking that it isn't null:
  • The variable must be initialized to a non-null value.
  • The variable can never be assigned the value null. The compiler issues a warning when code assigns a maybe-null expression to a variable that shouldn't be null.
• A reference may be null. The default state of a nullable reference variable is maybe-null. The compiler enforces rules to ensure that you've correctly checked for a null reference:
  • The variable may only be dereferenced when the compiler can guarantee that the value isn't null.
  • These variables may be initialized with the default null value and may be assigned the value null in other code.
  • The compiler doesn't issue warnings when code assigns a maybe-null expression to a variable that may be null.

Any reference variable that isn't supposed to be null has a null-state of not-null. Any reference variable that may be null initially has the null-state of maybe-null.

More accurate wording to what it actually can do:

You use annotations like ? or the NotNull attribute to declare whether a variable should be considered a nullable reference type or a non-nullable reference type. These annotations make important statements about the intended null-state for variables:

• A reference isn't supposed to be null. The default state of a non-nullable reference variable should be not-null. The compiler checks that this state is always kept, making it safe to dereference these variables without having to check for a null reference:
  • The compiler issues a warning when a variable isn't initialized to a non-null value.
  • The compiler issues a warning when code assigns null or a maybe-null expression.
  • The compiler doesn't issue a warning when code derefences a not-null variable.
• A reference may be null. The default state of a nullable reference variable is maybe-null. The compiler checks that you've correctly checked for a null reference before dereferencing a variable:
  • The compiler doesn't issue warnings when a variable is initialized with the default null value.
  • The compiler doesn't issue warnings when code assigns null or a maybe-null expression.
  • The compiler analyzes the code to infer whether a variable is maybe-null or not-null at any given place.
  • The compiler issues a warning when code dereferences a maybe-null variable, but not if it can infer that the variable is not-null.

Any reference variable that isn't supposed to be null has a null-state of not-null, and warnings are shown where assignments would change it to the maybe-null state. Any reference variable that may be null has initially the null-state of maybe-null, but can locally obtain the not-null state where this can be inferred from the code.

[DavidArno]

@LWChris,

Best bet with proposed changes to the docs like this is to have a read of the contributions guidelines and then open an issue to get the discussion going.

[sharwell]

I lean in favor of the text as it is currently written (considering it expresses the intent in definite terms), provided it is accompanied by documentation indicating that it is not always possible for the compiler to enforce the intent. Legacy code, and code which bypasses the language rules (e.g. serialization, reflection) can introduce states which invalidate the information known to the compiler.

[sharwell]

I [would still be] hyped for a way to get what was promised back then, i. e. "can never" means "can never successfully compile..."
...
The pain points are ...

The reality of the language is these two statements are in direct opposition to each other. The closer we move to one of these statements, the further we will move from the other.

[CyrusNajmabadi]

Migrating this discussion. The conversation has revealed that this is not a request for a language change, but rather suggestions around how the information should be presented in IDEs like VS and VSCode. As such, this should be continued over at roslyn. As per the above, if anyone here is interested in submitting PRs to improve the experience, they would be welcome. Thanks!