Roslyn: Tweak formatting for switch expressions

Created on 20 Mar 2019  Â·  11Comments  Â·  Source: dotnet/roslyn

Two questions:

  1. should we align the fat arrows? (illustrated below)
  2. where should the curly braces go?

Aligning fat arrows:
```C#
_ = e switch
{
null => 1,
_ => 2,
};


Current formatting:

_ = e switch
{
null => 1,
_ => 2,
};
```

Area-IDE Feature Request IDE-Formatter New Language Feature - Pattern Matching help wanted

Most helpful comment

I have all "Place open brace on new line for…" except "…types" uncheked in Visual Studio options, but switch expression formatted as:

static int Main(string[] args) {
  switch(args) { // OK
  case string[] oo when oo.Length > 10:
    return 1;
  }

  return args switch
  { // Wrong, moved to a new line
    string[] ee when ee.Length > 5 => ee.Length,
    _ => -1,
  };

  // Expression above ("open brace") should formatted as:
  return args switch {
    string[] ee when ee.Length > 5 => ee.Length,
    _ => -1,
  };
}

Is it the same issue or it is better to make a separate one?

All 11 comments

Feels like we might need an option here.

I'd say no aligning. We do not align case clauses of switch statement.
Do we align anywhere else in a similar situation?

I'd say main reason not to align is that a single line change in one case may result in mutli-line diff.

Do we align anywhere else in a similar situation?

The main problem is that we simply don't have any other similar situations :) We're in brand new territory :D

I'd say main reason not to align is that a single line change in one case may result in mutli-line diff.

A fair concern. Though one i would say is up to the individual dev/team to decide on. For example, the 'go' community pretty much mandates gofmt'ed code, and that aligns things all over the place. Turns out this is no biggie as you can trivially look at diffs in a whitespace insensitive manner.

I have a feeling (yes, it's just a gut feeling) that some reasonable subset of people will want alignment (and some will not). So my gut is coming down on this being an option.

That said, if we want to avoid an option, we should likely default to no-alignment.

I find the current formatting most consistent with the rest of our formatting features. Vertical alignment should be covered by the feature request to support formatting extensions. I'm not sure what the request here is for brace placement, but I would expect it to match the style and options for other expressions with braces (object and collection initializers, lambdas, and anonymous functions).

Just curious, @jcouv why isn't the syntax using : instead of =>? Seems like it would be more consistent with ternary ?: operator.

_ = e switch
{
    null: 1,
    _: 2,
};
_ = e == null ? 1 : 2;

@tmat Here are the notes I could find: https://github.com/dotnet/csharplang/blob/master/meetings/2018/LDM-2018-05-02.md#revisit-syntax-for-switch-expression
In short, lots of debate, not many winning arguments.

@gafter those notes said the syntax is only tentatively decided for prototype. Do you recall if we discussed it again and decided to keep it, or is it rather a de facto decision at this point?

Tentatively marking as By Design, but will update following design review.

Design review conclusion:

  1. Update the formatter to ignore more than one space between the switch arm expression and => (allows for manual alignment of =>)
  2. The current brace placement is fine for now (anchor to first token on line containing the first token of the switch expression)

Update the formatter to ignore more than one space between the switch arm expression and => (allows for manual alignment of =>)

Small suggested tweak: this makes sense in the absence of elastic trivia. in that case we're preserving intent. We probably want the 'hammer of thor' formatter, and the normal formatter to normalize on a single-space if there is elastic trivia.

I have all "Place open brace on new line for…" except "…types" uncheked in Visual Studio options, but switch expression formatted as:

static int Main(string[] args) {
  switch(args) { // OK
  case string[] oo when oo.Length > 10:
    return 1;
  }

  return args switch
  { // Wrong, moved to a new line
    string[] ee when ee.Length > 5 => ee.Length,
    _ => -1,
  };

  // Expression above ("open brace") should formatted as:
  return args switch {
    string[] ee when ee.Length > 5 => ee.Length,
    _ => -1,
  };
}

Is it the same issue or it is better to make a separate one?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

glennblock picture glennblock  Â·  3Comments

OndrejPetrzilka picture OndrejPetrzilka  Â·  3Comments

codingonHP picture codingonHP  Â·  3Comments

AdamSpeight2008 picture AdamSpeight2008  Â·  3Comments

orthoxerox picture orthoxerox  Â·  3Comments