Non-Nullable reference types of static fields

[savahmel]

Following code samples compile without warnings on the VS2019 16.0.0 preview 1.1 and result in null reference exception

//Sample 1 - Fixed in VS2019 16.0 RTM

#nullable enable 
using System; 
class Program { 
    public static string x = null; 
    static void PrintLength(string s) { Console.WriteLine(s.Length); } 
    static void Main(string[] args) { PrintLength(x); } 
}

//Sample 2

#nullable enable
using System; 
class A { 
    static A() { Program.PrintLength(); } 
    public static string GetName() { return string.Empty; }
} 
class Program { 
    public static string name = A.GetName(); 
    public static void PrintLength() { Console.WriteLine(name.Length); } 
    static void Main(string[] args) { }
}

//Sample3 (thanks @PathogenDavid)

#nullable enable
using System;
class Program
{
    public static string name2 = name, name = string.Empty;
    static void Main(string[] args) { Console.WriteLine(name2.Length); }
}

//Sample4

#nullable enable
using System;
class A {
    static A() { Console.WriteLine(Program.a.Length); }
    public int Length => 0;
}
class Program {
    public static A a = new A();
    static void Main(string[] args) { Console.WriteLine(a.Length); }
}

I would expect to get a null-reference warning message for the following cases:

1. null initialization (or no initialization) of a static field

Usage of following fields without null check:

2. In a library, public static field
3. In a library, in a public non-sealed class, public or protected field
4. A static field whos initialization cannot be proven to happen before it is being accessed
5. In a library argument of any publicly available method

A lyrical digression, my personal regret about the language becoming less elegant :)
I am sure this was taken into account and found practical.
However, the feature the way it was designed could lead programmers towards omitting null checks and thus introducing new null reference exceptions, where previously they would not have happened.
The absence of compiler warnings is misleading. The compiler does not claim to ensure non-nullability, however, we are used to trusting it, and not to question the scope of its competency.
Provable non-nullability would have a run-time performance penalty, and I would rather opt into it when I prefer safety over speed.

[yaakov-h]

This sounds more like an issue for Roslyn.

However, I'm confused by sample 2. What in there should trigger a warning? Everything can be statically proven to be non-null.

[PathogenDavid]

Definitely an issue for the Roslyn repo. I believe it should already by covered by roslyn#26628 (initializing with null should warn) and roslyn#27014 (uninitialized static fields should warn).

---

I second the comment on sample 2. Did you mean to have A.GetName return null instead of String.Empty? If so, the warning will appear in A.GetName since it's the one returning a null for a non-nullable reference.

[khm1600]

What if sample 2 were

//A.dll
class A { 
    static A() { Program.PrintLength(); } 
    public static string GetName() { return string.Empty; }
} 
//Program.exe
using System; 
class Program { 
    public static string name = A.GetName(); 
    public static void PrintLength() { Console.WriteLine(name.Length); } 
    static void Main(string[] args) { }
}

[DavidArno]

@khm1600,

If sample 2 were that, then you'd never get it to compile as you are creating a circular dependency between the two assemblies.

[khm1600]

@DavidArno
Then how were System.dll, System.Xml.dll and System.Configuration.dll compiled?

[PathogenDavid]

Oops, I missed that the constructor was static in sample 2. In my opinion, that the compiler shouldn't try to detect that situation. Static initialization is already kinda funky, care should be taken when doing it. Methods used to initialize static members probably shouldn't even accessing other static fields as a rule.

---

@DavidArno You actually can do this with reference assembly shenanigans since the runtime will happily resolve circular assembly references like that even though the project system won't.

That example is pure evil though. If you're intentionally doing stuff like that, you'd better know what you're doing.

[DarthVella]

Relevant: roslyn#31440.

[YairHalberstadt]

I'm going to close this as it is an issue for roslyn. Some of these have been fixed, and some haven't, so I'm not going to ask this to be moved - instead I would suggest that you open new issues there.

Note that I'm not very hopeful on e.g. sample3 and sample4 being fixed. Nullable Analysis is at the moment intraprocedural (looks at only one method at a time). Changing that is infeasible, and in the general case impossible (due to the halting problem). So you would have to come up with a method that detects this case without forcing you to declare all static members as nullable.