2

I notice when using a switch expression which includes a null check as the range expression, the nullability state of the value in arms/cases isn't interpreted correctly in Visual Studio (16.7.6). For example:

string? value = GetSomeString();
var description = (value != null) switch
{
    true when value.Length == 0 => "Empty",
    true when value.Length == 1 => "Uno",
    true when value.Length == 3 => "Third time's a charm",
    _ => "null"
};

Here I expect to know within the switch block that all instances of value are not null, given the condition we're switching on. The last/default case handles the null case. Yet, Visual Studio thinks the first instance might be null:

enter image description here

Am I doing something wrong, is this a bug with the analyzer, or otherwise?

Noah Stahl
  • 6,905
  • 5
  • 25
  • 36
  • May be related: https://stackoverflow.com/q/59524568/284111 – Andrew Savinykh Oct 28 '20 at 00:51
  • An alternative is to do `value?.Length switch` which actually makes the code more terse as the branches would be shorter like `0 => "Empty"` – juharr Oct 28 '20 at 00:59
  • Thanks for the comments. I'm also asking this as an example of the general case where `value` is a class type, not just a string. – Noah Stahl Oct 28 '20 at 02:19

2 Answers2

5

You're switching on this condition: (value != null), which evaluate to true or false. Still, the variable value itself can still be null when inside the switch expression, so that hint is actually correct. Whenever the result of (value != null) is false, you'll still enter the switch body, and value will be null. In summary, the switch body will be executed regardless of whether value is null or not.

EDIT:

Scratch that, I got it wrong, you're right. value will not be null inside the specific conditions, though it might be null in the switch body. There's something funny with your Visual Studio, this is what I get (I'm using VS 16.4.5), no warnings:

enter image description here

Martin Ferrari
  • 186
  • 1
  • 7
  • Your screenshot shows a squiggle under the `string?`. Do you have `enable` in your project file? – Noah Stahl Oct 28 '20 at 02:18
  • No, I added it later but didn't update the image, but you're right, that's the cause for the squiggle. – Martin Ferrari Oct 28 '20 at 02:40
  • so, If I add enable or enclose the class or method inside #nullable enable and #nullable disable, I get the warning. *But* if I enclose just string? value = GetSomeString(); inside #nullable enable and #nullable disable, there's no warning at all – Martin Ferrari Oct 28 '20 at 02:53
  • Thanks for confirming. Since using the nullable warnings is the goal, disabling defeats the purpose. :) – Noah Stahl Oct 28 '20 at 14:07
0

I think that in your simple example we could all agree that once checked for null value will continue to retain that value, however neither the compiler or the runtime can make that guarantee.

It is fairly trivial to write another example in which the value of value could be set to null between the time that (value != null) is evaluated and any of the true when value.Length ... statements are.

Thus making the warning, 'value' may be null here, valid.

string? value = default;

Parallel.For(0, 4, _ =>
{
    while (true)
    {
        value = "Has a value";

        var description = (value != null) switch
        {
            true when value.Length == 0 => "Empty",
            true when value.Length == 1 => "Uno",
            true when value.Length == 3 => "Thrid time's a charm",
            _ => "null"
        };

        value = null; 
    }
});

enter image description here

Alternative ways to write such an expression would be

when value?.Length == 0 => "Empty",
when value?.Length == 1 => "Uno",
when value?.Length == 3 => "Thrid time's a charm",

or

var description = value?.Length switch
{
    0 => "Empty",
    1 => "Uno",
    3 => "Thrid time's a charm",
    _ => "null"
};
Jerry
  • 1,477
  • 7
  • 14
  • THanks. This seems contrived because you're introducing multiple threads modifying that variable. My question pertains only to what is shown with local variables. Are you saying there is some way value can change in my code example? – Noah Stahl Oct 29 '20 at 14:42
  • I'm saying that every time you dereference a nullable variable that it could be null. As I said in your simple example that would not happen but as code becomes more complex it definitely may, especially when multiple threads and lambda expressions that can take a reference to a local variable come in to play. – Jerry Oct 29 '20 at 16:52
  • I suppose its up to the programmer to ignore the warning, or disable it, if they are confident that there is no way possible the value can change. Or rewrite their code so there warning doesn't exist. – Jerry Oct 29 '20 at 17:01
  • "every time you dereference a nullable variable that it could be null" -- isn't the point of the nullable reference types annotations that the compiler can guarantee non-null via static analysis? If not, this seems like the effort of adding this feature would just be false impression of safety. – Noah Stahl Oct 29 '20 at 17:01
  • isn't the significant difference from OP here the fact that you've hoisted the declaration of value _outside the scope_ of the switch statement? In OP's code it still looks to me that value cannot change from it's non-null value because it a local variable. No other Thread even knows it exists. – Chris F Carroll Oct 16 '21 at 23:10