Switch expressions assist conversion with OR removes comments

[FMorschel]

From an example posted by @pq in dart-lang/linter#3673 (comment):

switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  case 0x00A0: // No-break space.
  case 0x1680: // Ogham space mark.
  case 0x180E: // Mongolian vowel separator.
  case 0x2000: // En quad.
  case 0x2001: // Em quad.
  case 0x2002: // En space.
  case 0x2003: // Em space.
  case 0x2004: // Three-per-em space.
  case 0x2005: // Four-per-em space.
  case 0x2006: // Six-per-em space.
  case 0x2007: // Figure space.
  case 0x2008: // Punctuation space.
  case 0x2009: // Thin space.
  case 0x200A: // Hair space.
  case 0x200B: // Zero width space.
  case 0x2028: // Line separator.
  case 0x2029: // Paragraph separator.
  case 0x202F: // Narrow no-break space.
  case 0x205F: // Medium mathematical space.
  case 0x3000: // Ideographic space.
  case 0xFEFF: // Zero width no-break space.
    return something;

  default:
    return somethingElse;
}

Turns into the following losing all comments:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || 0x1680 || 0x180E || 0x2000 || 0x2001 || 0x2002 || 0x2003 || 0x2004 || 0x2005 || 0x2006 || 0x2007 || 0x2008 || 0x2009 || 0x200A || 0x200B || 0x2028 || 0x2029 || 0x202F || 0x205F || 0x3000 || 0xFEFF => // Zero width no-break space.
    1,

  _ => 2
};

After formatted:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 ||
  0x1680 ||
  0x180E ||
  0x2000 ||
  0x2001 ||
  0x2002 ||
  0x2003 ||
  0x2004 ||
  0x2005 ||
  0x2006 ||
  0x2007 ||
  0x2008 ||
  0x2009 ||
  0x200A ||
  0x200B ||
  0x2028 ||
  0x2029 ||
  0x202F ||
  0x205F ||
  0x3000 ||
  0xFEFF => // Zero width no-break space.
    1,
  _ => 2
};

A smaller example:

switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  case 0x00A0: // No-break space.
  case 0xFEFF: // Zero width no-break space.
    return 1;
  default:
    return 2;
}

Turns into:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || 0xFEFF => // Zero width no-break space.
    1,
  _ => 2
};

I believe it should become something along the lines of:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || // No-break space.
  0xFEFF => // Zero width no-break space.
    1,
  _ => 2,
};

But I think the result being alone is weird in all of the above so I suggest doing:

return switch (character) {
  /// See [General Punctuation]
  /// (http://www.unicode.org/charts/PDF/U2000.pdf).
  0x00A0 || // No-break space.
  0xFEFF // Zero width no-break space.
    => 1,
  _ => 2,
};

Currently, in the formatter, if you have the above, it formats the => in one line and the result in a new one. Will file an issue there and link here.

[dart-github-bot]

Summary: The Dart switch expression conversion with OR operator removes comments associated with individual cases, leading to code that is less readable and harder to understand. The user suggests preserving comments by placing them after the OR operator, but this results in inconsistent formatting.

[FMorschel]

Issue created dart-lang/dart_style#1551

[FMorschel]

This also inspired me to create dart-lang/linter#5078.

[FMorschel]

Also:

return switch (value) {
  0 || //case 0
  1 || //case 1
  2 //case 2
    =>
    2,
  _ => 3,
};

Becomes:

switch (value) {
  case 0 || //case 0
  1 || //case 1
  2:
    return 2;
  default:
    return 3;
}

So we are still losing the last comment here.

So if you pick the first example and convert it twice you lose every comment there. Really bad if dart-lang/linter#3673 ever happens.

[FMorschel]

Converting the following does keep the comments, but if converting back it loses the last comment as mentioned above:

switch (value) {
  case 0 || //case 0
        1 || //case 1
        2: // case 2
    return 2;
  default:
    return 3;
}

Just point it out so tests can also be added for this case.

[pq]

Thanks for all of these examples @FMorschel!

/fyi @scheglov (who fixed the associated #54567)

[FMorschel]

NP!

He had already seen this issue since he added the priority.

Also, something to keep in mind would be #56602 because as you can see in the first example, it doesn't format the output. I completely ignored it first since it is so easy to format in the IDE but my colleague pointed that when fixing via CLI with the possible future lint this would not be expected.