Roslyn: "Indent open and close braces" doesn't affect switch section block

[alrz]

When "Indent open and close braces" is unchecked:

Actual:

case 5: 
    {
    }

Expected:

case 5: 
{
}

[CyrusNajmabadi]

What is the following set to:

?

[alrz]

@CyrusNajmabadi That indents the whole section,

        switch (a)
        {
            case 1:
                {
                }
        }

vs.

        switch (a)
        {
        case 1:
            {
            }
        }

[CyrusNajmabadi]

Er, sorry:

[alrz]

It doesn't indent the block when unchecked but it also leaves statements unindented as well.

How can I have these at the same time:

case 5:
{
    break;
}
case 6:
    break;

[CyrusNajmabadi]

i don't think we have any facility for that. Tagging @heejaechang to see how hard it would be to have an option for this.

[heejaechang]

@basoundr actually added those options. so tagging him to see whether we have an option for it. about supporting what @alrz asking, It should be a matter of just adding one more option if it doesnt already exist.

[basoundr]

Yes, we cannot do something like that today with the available options. We will need to add an option to support something like this. Don't sure what a good name would be too. "Case Content without braces"?

[alrz]

Indent case blocks (defaults to true for backward-compat)?

[Pilchie]

Also reported https://developercommunity.visualstudio.com/content/problem/65219/visual-studio-2017-c-switch-case-braces-do-not-pre.html.

Are we sure this isn't a regression? It feels like braces used to be special cased there.

[miloush]

I am pretty sure the default settings have been always the way reported, i.e. indented with regards to case.

It would have been pretty awkward
C# case 1: blabla(); { foo(); } break;

unless you are thinking there was a special exception for when the whole case block is marked with braces only. Either way, if it helps, it behaves the same in VS 2010 - as reported.

[alrz]

unless you are thinking there was a special exception for when the whole case block is marked with braces only.

Exactly.

[alrz]

I was looking into it, as it turns out, trivia ain't trivial.

[tannergooding]

I logged an issue on this over a year ago, it is currently up for grabs:
https://github.com/dotnet/roslyn/issues/12304

[tannergooding]

There might even be a slightly abandoned PR that fixes this (haven't dug back into to validate that is the case, but my convo with @CyrusNajmabadi seems to indicate that was the case): https://github.com/dotnet/roslyn/pull/13264

[alrz]

Thanks! I think this doesn't really need another option so it should be fixed without public API changes.

[tannergooding]

Whether or not it gets a new option really depends on whether changing the formatting of existing users using one of the existing options is considered breaking (I imagine it may be).

[alrz]

Yeah, it probably is. But from that CR, it sounds like another option may not be desirable either. 😕

[tannergooding]

@Pilchie, could you chime in on the "appropriate" direction to take here?

Is adding a new option (like #13264) was doing appropriate? If so, should we ping @viceroypenguin and see if he wants to either pick it back up or hand it off to someone else to complete?

Otherwise, can we determine what does need to be done here so that we can move in the correct direction?

[CyrusNajmabadi]

It would definitely be breaking. We have a behavior today around these options and how blocks are handled in a switch. People may like this behavior and may not want it changed. I'm totally fine with a new option around this.

[CyrusNajmabadi]

I think this shoudl be pretty easy to implement. I'll try to whip up a solution here.

[CyrusNajmabadi]

open question, what if you say "i don't want blocks indented in case sections, but i do want normal statements indented", what happens with the following:

```c#
switch
{
case 0:
{
}
break;
}

Seems like that would be pretty ugly.  It feels like teh rule is effectively "don't indent if the first statement of a case is a block".  So you'd get:

```c#
switch
{
    case 0:
    {
    }
    break;
}

But if you had other statements first, it would be:

c# switch { case 0: Console.WriteLine(); { } break; }

[alrz]

I was thinking that we should not indent only if the whole section is a block. In your example, if we have more statements after block, it won't look quite right, specially if there is another case right after.

[tannergooding]

I'm not sure how frequently users have case statements that are a mix of blocks and not blocks, but "don't indent if the first statement of a case is a block" is probably the best behavior here. At least, I think it looks the most atheistically pleasing for the case where users are mixing (as you gave in your examples above).

Case statements are the only statement (at least that I can think of right now) where you don't have to "explicitly" define the block for multi-line statements. So, I think if the user doesn't define them (At least in my mind), they are effectively there "implicitly". If the user does define them, then we shouldn't implicitly define them ourselves

That is, the following case statements should indent "identically".
```C#
case 0:
Console.WriteLine();
{
}
break;

```C#
case 0:
{
    Console.WriteLine();
    {
    }
    break;
}

The only case where this breaks down is if the user doesn't define everything within the block. I think in that case, you don't indent as well, because it just looks bad otherwise:
```C#
case 0:
{
Console.WriteLine();
}
break;

case 0:
{
Console.WriteLine();
}
Console.WriteLine();
{
break;
}

```C#
case 0:
{
    Console.WriteLine();
}
    break;

case 0:
{
    Console.WriteLine();
}
    Console.WriteLine();
    {
        break;
    }

[CyrusNajmabadi]

PR ready. Working on tests right now.

[Pilchie]

Tagging @kuhlenh too. I generally hate adding options, but I do think this one would be nice.

I still believe that in VS2005 when I wrote formatting that I did special case blocks at the beginning of case statements without an option though :)

[CyrusNajmabadi]

Fixed with https://github.com/dotnet/roslyn/issues/20292