Compiler doesn't show warning about not exhaustive switch expression

[sungam3r]

Hi. The issue is described in details here coverlet-coverage/coverlet#1292 so I will not repeat. The desired behavior - compiler shows warning about not exhaustive switch expression.

[CyrusNajmabadi]

I looked through the linked issue and I didn't get exactly what the issue is. Can you please restate it here with a clear code example, and an explanation of what behavior you are seeing and what behavior is expected (and why). Thanks!

[sungam3r]

public class Program
{
    public static void Main()
    {
        Test1(); // SwitchExpressionException
        Test2(); // SwitchExpressionException too
    }

    private static string? ExpectOneOf1()
    {
        return null;
    }

    private static string ExpectOneOf2()
    {
        return null!;
    }

    private static bool Test1()
    {
        return ExpectOneOf1() switch // CS8655
        {
            "a" => true,
            string _ => false
        };
    }

    private static bool Test2()
    {
        return ExpectOneOf2() switch // no warning
        {
            "a" => true,
            string _ => false
        };
    }
}

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

</Project>

I would like to see compiler warning in both cases.

[Youssef1313]

I think the compiler relies on the annotation for knowing if null is covered or not. So the problem is that ExpectOneOf2 is returning string. So this is pretty much by-design.

[sungam3r]

In both cases compiler emits default switch branch throwing SwitchExpressionException so I expect warning in both cases too. Does it make sense?

[sungam3r]

As I understood the warning necessary in order to show the presence of a default branch with runtime exception that exists when using not exhaustive switch expression. Since this branch exists in both cases, the warning should also be in both cases.

[Youssef1313]

@RikkiGibson What do you think?

Related to #52714

[CyrusNajmabadi]

I think this is by design. The switch case is there for actually badly behaving code. But we trust the annotation to not force the user to handle a pointless case in the majority of cases.

[sungam3r]

The switch case is there

Do you mean default case that throws SwitchExpressionException ?

Thanks. The explanation is clear, although I am not sure that the choice of such a design was the best solution.

[RikkiGibson]

It is by design for the same reason the following is by design

#nullable enable

var c = C.GetC();
c.M(); // no warning

public class C
{
    public static C GetC()
    {
        return null!;
    }
    
    public void M()
    {
    }    
}

Simply put. There is one place where the warning is "naturally" given. But the warning is suppressed by '!'. We don't use our analysis to encourage users to "distrust" methods which claim to return not null. We use it to ask methods which return not null to really return not null to the best of their knowledge--unless they shut us up.