dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.99k stars 4.03k forks source link

Missing nullability warnings caused by incorrect processing of `default` literals in conditional,null coalescing and switch expressions #47935

Open TessenR opened 4 years ago

TessenR commented 4 years ago

Version Used:

Branch master (20 Sep 2020)
Latest commit d148f06 by Sam Harwell:
Merge pull request #47537 from sharwell/non-null-compilation

Update SyntaxNodeAnalysisContext.Compilation to not return null

Steps to Reproduce:

Compile and run the following code:

#nullable enable
class C
{
  public static void Main() => M(false);

  public static void M(bool b)
  {
    (string foo, string bar) = b ? ("foo", "bar") : default;
    foo.ToString();
  }
}

Expected Behavior: Warning for assigning non-nullable variable foo with possible null value Warning for foo.ToString() as the deconstructed tuple can be default and return null values from its deconstruction

Actual Behavior: No warnings. The code crashes with a NullReferenceException at runtime

Notes Same problem here, the t ?? default expression can return default tuple with null components but Roslyn doesn't produce any warnings. This code also crashes at runtime with a NullReferenceException

#nullable enable
class C
{
  public static void Main() => M(null);

  public static void M((string, string)? t)
  {
    (string foo, string bar) = t ?? default;
    foo.ToString();
  }
}

Same problem with switch expressions:

#nullable enable
class C
{
  public static void Main() => M(null);

  public static void M((string, string)? t)
  {
    (string foo, string bar) = t switch { { } => t.Value, _ => default };
    foo.ToString();
  }
}
RikkiGibson commented 4 years ago

Is this a scenario where we need to be forcing slots for all the struct fields when converting a default literal of a struct type?

RikkiGibson commented 4 years ago

manually adding untriaged label so we can put this in a milestone :)