roslyn: CS8509 The switch expression does not handle all possible inputs when it does

To me enums are a kind of safety feature that limit the amount of values you can use, you can technically circumvent such limitation but you generally shouldn’t. (In my opinion)

Let’s say I write this code:

public enum Operation
{
    Plus,
    Minus,
    Multiply
}

public static string OperationToString(Operation op)
{
    return op switch
    {
        Operation.Plus => "+",
        Operation.Minus => "-",
    };
}

Now I get this warning:

CS8509 The switch expression does not handle all possible values of its input type (it is not exhaustive). For example, the pattern 'Program.Operation.Multiply' is not covered.

This is a very nice warning telling me precisely that I’m not handling Operation.Multiply. This warning has prevented a bug, great!

But when I add the code to handle this case the warning doesn’t go away, instead it only changes the example from Program.Operation.Multiply to a cast like (Program.Operation)3.

I almost never do casts like this and honestly don’t mind a SwitchExpressionException when someone takes off the safety feature and it goes wrong… But I don’t like warnings so I’ll add a pattern that handles cases like this:

_ => throw new ArgumentOutOfRangeException(nameof(op)),

Great, the warning is gone, but _ handles every possible value… so now when I add a new operation (i.e. Operation.Divide) I don’t get a new warning! but of course I’d really like to get such a warning!, since this is now a real bug.

I don’t think I’m the only person that uses enums as if they can only contain values defined in the enum declaration. The closed enum types proposal is great, but I don’t think it’ll get added to C# anytime soon, and there’s already a lot of existing code that uses enums this way.

If the enum cast vs. defined enum value missing pattern warnings got separate warning codes people could decide for themselves how they’d like to handle cases like this by enabling/disabling the appropriate warnings.

It considers only the domain of the type in the expression, not using flow analysis to limit it to a smaller domain of values.

It is incredibly counter-intuitive that listing all enum values is not enough to make the switch expression exhaustive. Other major languages with similar feature do not require this dummy catch-all case.

In fact, there is a first-class citizen language in the .net ecosystem that handles this scenario very well. The following is valid F# code and does not generate any warning or error.

type E = A | B | C

let f = function
    | A -> "a"
    | B -> "b"
    | C -> "c"

[<EntryPoint>]
let main argv =
    printfn "%s" (f A)
    0

Outside of the .net land, Haskell is another good example.

data E = A | B | C

f :: E -> String
f A = "a"
f B = "b"
f C = "c"

main :: IO ()
main = print $ (f A)

In the JVM land, Scala is yet another example.

sealed trait E
case object A extends E
case object B extends E
case object C extends E

def f(s: E) = s match {
    case A => "a"
    case B => "b"
    case C => "c"
}

println(f(A))

And many others.

All snippets above compile without warnings because the compiler understands that the pattern matching expressions are exhaustive. Also, a warning pops up if one of the cases is removed, as expected.

(In Haskell, the warning requires the compiler flag -Wincomplete-patterns)

The fact C# behaves differently is surprising and annoying.

As of dotnet version 3.1.300, the following C# code generates a warning.

enum E
{
    A, B, C
}

class Program
{
    static string f(E x) => x switch
    {
        E.A => "a",
        E.B => "b",
        E.C => "c",
    };

    static void Main(string[] args)
    {
        Console.WriteLine(f(E.A));
    }
}

The compiler could be modified to issue a different warning when a switch expression is only incomplete because of the possibility of an unnamed enum value being part of the input. That would be a reasonable feature request. I suggest you file that request as a separate issue.

All great reasons to regret C#'s C heritage. There is a proposal to permit declaring enums that don’t have unnamed values. See https://github.com/dotnet/csharplang/issues/3179

@CyrusNajmabadi Thanks for the analyzer suggestion, I definitely should to take a look at them. The reason I’m commenting here is because I truly think this warning has the potential to prevent a lot of bugs, but that its effectiveness is severely lessened due to the fact that it encourages people to add a catch all pattern, thereby guaranteeing the warning will never again trigger for that code.

A function call that fails when you pass it a named enum value is fundamentally different to me than a function call that fails due to passing an unnamed enum value. That’s why I think these cases should be reflected by different warnings.

But I might be the only one with this opinion… maybe one day I’ll write an analyzer and convince the whole world to use it 😅 .

Great, the warning is gone, but _ handles every possible value… so now when I add a new operation (i.e. Operation.Divide) I don’t get a new warning! but of course I’d really like to get such a warning!,

@dextercd You can accomplish this with an analyzer. Then you can have whatever rules you want here. For example, you could run that rule for all enums, only your own enums, only your own enums taht have a certain attribute on them, etc. etc.

A good rule of thumb: if you want the language to be more restrictive in what it allows, but otherwise it’s all the same lang syntax and features, then just go write an analyzer. That’s their primary use case 😃

Thank you for the encouragement! I’ve opened a new issue: https://github.com/dotnet/roslyn/issues/47066.

That’s why I think these cases should be reflected by different warnings

Sure. We made a call on which to align with. With the understanding that the otehr side could always supplant the problem with an analyzer. i.e. even if we made the language not warn if you missed the _ case, tehre would be some who would want that. So it was either have them write the analyzer, or you. We chose you 😃

@miloush that type of flow analysis is expensive. Our work on nullable reference types really drove hom the cost of doing this correctly. At the time I don’t think it’s worth the necessary language design and compiler implementation cost. Closing but keeping on the backlog of future possible improvements if enough scenarios / asks came up.