Question about switch expression with ?? operator

[sungam3r]

See https://app.codecov.io/gh/graphql-dotnet/parser/blob/master/src/GraphQLParser/ParserContext.Parse.cs
All 4 usages of ExpectOneOf (lines 862, 1101, 1114, 1353) are partilly covered though there are tests that call ExpectOneOf with null and non-null values. Why so?

[daveMueller]

We have to look into it, could be a bug. Thanks for reporting this. 👍

It's related to null-coalescing operator inside method argument.

[daveMueller]

I took a quick peek and it really seems to be a bug. I created a simplified repro here: https://github.com/daveMueller/CoverletIssue1292 and I will start working on this.

[daveMueller]

OK seems to be a really special case. When I change the default value to just underscore coverage works.

[sungam3r]

What if you try string??

[daveMueller]

Thanks, but it's the same with string?. This is getting way more complicated than I thought it would be 😄. For switch cases the compiler generates multiple branch point. Depending on how complex the switch expression is the amount of those generated branch points can vary. In the simplest form the issue here looks like this (not related to ?? at all):

I suspect the second branch is generated as the else branch to the default case. If the switch expression doesn't have a defined default case, the compiler generates an exception.

Nevertheless I have to look into which of the generated branch point we already exclude and which have to be excluded in addition.

[sungam3r]

You are right, interesting 🤔 . From here https://stackoverflow.com/questions/56068105/what-happens-if-my-c-sharp-switch-expression-is-non-exhaustive:

The compiler will warn you if your switch expression is not exhaustive, but it's not an error and it'll compile and run anyway.

But I see no warning in my code 😕 . Yes, after reading this I tried to pass null intentionally from OneOf and indeed got SwitchExpressionException at runtime. So my switch expression was not exhaustive because of null branch. And no compiler warnings...

Fully covered now! https://app.codecov.io/gh/graphql-dotnet/parser/compare/238/diff

[sungam3r]

So actually it may be a question to Roslyn team.

[daveMueller]

Yes I also started to think that this most likely isn't an issue with coverlet. Because you can fully cover the code when you know that it can throw an exception. E.g. the function

public static bool DoSomething(string? foo)
{
   return foo switch
   {
      "hello" => true
   };
}

Can be fully covered with

[Fact]
public void Test1()
{
   Assert.True(Class1.DoSomething("hello"));
   Assert.Throws<SwitchExpressionException>(() => Class1.DoSomething(""));
}

When I looked into your code I also thought you handled all the bad cases up front (e.g. with ??) and thats why there is no warning anymore. So I also don't really understand why the compiler still generates the exception. Maybe this really is a Roslyn issue.

Nevertheless I don't think that coverlet has an issue here anymore.

[sungam3r]

There was no case for null in my switch.

[daveMueller]

Yes you are right.

[daveMueller]

Closing this because coverlet works as designed. Feel free to reopen.