VB -> C#: Duplicate cases should be discarded

[mrmonday]

VB allows Case in a Select Case to have duplicate labels.

Input code

Imports System

Friend Module Module1
    Sub Main()
        Select Case 1
            Case 1
                Console.WriteLine("a")

            Case 1
                Console.WriteLine("b")

        End Select
        Console.ReadLine()
    End Sub
End Module

This compiles, and outputs a.

Erroneous output

using System;

namespace ConsoleApp3
{
    internal static class Module1
    {
        public static void Main()
        {
            switch (1)
            {
                case 1:
                    {
                        Console.WriteLine("a");
                        break;
                    }

                case 1:
                    {
                        Console.WriteLine("b");
                        break;
                    }
            }
            Console.ReadLine();
        }
    }
}

Expected output

using System;

namespace ConsoleApp3
{
    internal static class Module1
    {
        public static void Main()
        {
            switch (1)
            {
                case 1:
                    {
                        Console.WriteLine("a");
                        break;
                    }
            }
            Console.ReadLine();
        }
    }
}

Details

Error:

error CS0152: The switch statement contains multiple cases with the label value '1'

I wonder if it's worth leaving the code there, but commented. Or even leaving the error - the behaviour changes, since it won't compile, but I can't think of a case where this wouldn't be a bug.

Version: master @ 3ffc597

[GrahamTheCoder]

Thanks for figuring out the actual behaviour here. I've just found the relevant bit in the docs too.

I've updated the contributing doc to explain how I try to weigh up decisions on what the output should be in these complicated cases.

So from those, if we are confident we can generate compiling code that behaves the same, we should do so. So let's not leave a compile error (or insert an error pragma).

The next applicable point is "code that resembles the input code". This suggests we should try to keep the information that was in the VB source. I like your suggestion of keeping it in a comment.

Having put it as a comment, we might confuse the user, and in a sense we've generated non-idiomatic code. So the comment should explain where it comes from, or be accompanied by a warning pragma. The ideal is to make what we do clear enough that someone wouldn't feel the need to look back at the VB to be sure they could safely delete the code.

[BrianFreemanAtlanta]

I've been working on this. I have the code to not convert the duplicate. How is this for a comment in place of the duplicate case:

    #warning Cannot convert CaseBlockSyntax - see comment for details
                    /* Cannot convert CaseBlockSyntax, System.NotSupportedException: Duplicate Case label commented out
    
                    Input: 
    
                                Case 1
                                    Console.WriteLine("b")
    
                     */

[GrahamTheCoder]

Cool. I think that warning leads me somewhat to the conclusion that the converter has failed, which it hasn't really since the converted code will behave identically. How about something like:

    #warning Unreachable case label
                    /* This case was not executable prior to code conversion, but is preserved here in case it helps troubleshoot a bug
                                case 1:
                                    Console.WriteLine("b");
                     */

In my example I've made the commented code the conversion result. The benefit I'm thinking of is quick comparison to the case which is executed.as converted code. If it's easy to do I think this would be nice, otherwise just the input code in its place is fine (and possibly reference it like "...the original VB is preserved here. in case..."

One other thought on a possible conversion. Here's a C# program that has an unreachable case statement:

        switch (e.GetHashCode())
        {
            case 3:
            break;
            #warning Unreachable case label
            case object x when object.Equals(x, 3):
            break;
        }

So we could actually keep it as executable code that way, but put the warning pragma just above it as a handy hint. Note that there's already some code for generating things a bit like case object x when object.Equals(x, 3) which I think is used for non-literals if you go down this path.