Roslyn: NRT doesn't detect nullability of field that hasn't been declared yet

[chucker]

Consider the following code:

#nullable enable

private class NullBecauseOfDeclarationOrder
{
    private static readonly string S1 = "Value A";
    private static readonly string[] Strings1 = new string[] { S1 };

    private static readonly string[] Strings2 = new string[] { S2 };
    private static readonly string S2 = "Value B";
}

At compile time, this doesn't produce any warnings. However, at _runtime_, Strings2 will contain null rathre than Value B.

That's because the produced IL actually _declares_ each field first, _then_ initializes them. When initializing Strings2, the value of S2 is still null, so that gets inserted into the array.

However, nullable reference types don't seem to discover this as a problem. The compiler insists inside the array that S2 is not null here, because it sees that

1. S2 is not marked as nullable, and
2. it is immediately set to a value

I believe this should yield either CS8618, or something similar:

Warning CS8618 Non-nullable field 'S2' is uninitialized. Consider declaring the field as nullable.

(A more correct warning would be "…is not yet initialized when accessed here".)

[jnm2]

I'd like a warning for accessing an uninitialized static field with an initializer when it's a nullable field, too.

[AartBluestoke]

The current implementation is not 'pit of success' (i learnt something from this ticket, having never had to learn this the hard way). I'd expect this to work in a manner consistent with some other language feature:

• similar to accessing static members cross-class, where they are initialized on demand
  -- having this trains you to act as if the above code is safe.
• similar to use in a local function where you can only use it after it is declared, so always has the initialized value
  -- it's nice the language is more permissive than this and allows statics to reference other statics.
• similar to a class members where you can't cross-reference and you have to be explicit in a constructor.
  -- this is how i have acted within my code, always declaring statics that are "parameters" up front for later use

Therefore my naive look at that code says "it is surprising that that runs but is null, nothing else in the language works that way" .

[moonbuffalo]

I came here to suggest something like this... similar but a little different.

public class MyClass 
{
        public Parameter param1 = new Parameter()
        {
            Name = param2.Name
        };

        public Parameter param2 = new Parameter()
        {
            Name = param1.Name
        };
}

param1 and param2 have an issue, which I expect. This is working as intended. Here are the error messages it tells us.

• A field initializer cannot reference the non-static field, method, or property 'MyClass.param2'
• A field initializer cannot reference the non-static field, method, or property 'MyClass.param1'

Now if I make them static fields....

public class MyClass 
{
        public static Parameter param1 = new Parameter()
        {
            Name = param2.Name
        };

        public static Parameter param2 = new Parameter()
        {
            Name = param1.Name
        };
}

param1 and param2 seem fine now. The error message goes away and the project builds fine. Then we get an error at runtime when we try to access either static field: "The type initializer for ' * ' threw an exception."

What I'd like is to get the same error I get for non-static field initializers.

[CyrusNajmabadi]

What I'd like is to get the same error I get for non-static field initializers.

This works be a breaking change as legal code would now cause errors, so we can't do that.

That said, you can easily write an analyzer yourself that will check for these things and issue diagnostics when matches are found.

[moonbuffalo]

This works be a breaking change as legal code would now cause errors, so we can't do that.

Makes sense. Good lesson for me. Only raises the question to me, how on earth do yall ever make any changes? seems like an impossible task. It's hard enough not to break the codebase at the small company I'm at so I can't imagine considering everyone else's codebase too.

That said, you can easily write an analyzer yourself that will check for these things and issue diagnostics when matches are found.

I will take a stab at this eventually. Gotta always upgrade my skils. Definitely not something that pays the bills but it would have saved me an afternoon.

[CyrusNajmabadi]

how on earth do yall ever make any changes

Carefully :-)

[RikkiGibson]

Good news: we now give a warning in this scenario.
Bad news: the public API is giving wrong nullability information, thus the IDE shows the following:

Since we at least are giving a warning now, I will close this issue. The remaining work is tracked in dotnet/roslyn#46424.

[RikkiGibson]

I consider this just a nullable bug, not really a language design or feature discussion, so will also move this to dotnet/roslyn.