[clang-format] PR48569 clang-format fails to align case label with `switch` with Whitesmith Indentation

[MyDeveloperDay]

Summary

https://bugs.llvm.org/show_bug.cgi?id=48569

This is a tentative fix which addresses a PR raise regarding Case indentation when working with Whitesmiths Indentation

I could not find online any reference sources as to what the case indentation for Whitesmith's should be (or be allowed to be)

But according to the documentation, we don't obey the rules for Whitesmith's

In particular, the documentation states that this option is to "indent case labels one level from the switch statement. When false, use the same indentation level as for the switch statement."

The behaviour we add here is actually as the TODO in the tests used to state in D67627: Clang-format: Add Whitesmiths indentation style, but when D82016: [clang-format] [PR462254] fix indentation of default and break correctly in whitesmiths style was added and I brought these tests out from being TODO I realized I changed the indentation.

[HazardyKnusperkeks]

If that's how it's supposed to look, than this patch is fine.

[curdeius]

It would be great if we had some spec for this style, it gets a bit hard to verify if the formatting is actually what is expected.
Having said that, LGTM if the bug reporter confirms the behaviour is ok.

clang/lib/Format/UnwrappedLineParser.cpp

2248

Nit: the style is named Whitesmiths so I'd expect the variable to match that: small s in the middle, trailing s.

[MyDeveloperDay]

This is the closest I could find thus far http://bitsavers.informatik.uni-stuttgart.de/pdf/chromatics/CGC_7900_C_Programmers_Manual_Mar82.pdf

and the very few examples from the manual show the case is aligned with the switch

[curdeius]

In D93806#2471738, @MyDeveloperDay wrote:

This is the closest I could find thus far http://bitsavers.informatik.uni-stuttgart.de/pdf/chromatics/CGC_7900_C_Programmers_Manual_Mar82.pdf

and the very few examples from the manual show the case is aligned with the switch

Nice! Thank you.

clang/lib/Format/UnwrappedLineParser.cpp

 
 void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
     --Line->Level;
   if (LeftAlignLabel)
     Line->Level = 0;
+
+  bool RemoveWhiteSmithCaseIndent =
+      (!Style.IndentCaseBlocks &&
+       Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths);
+
+  if (RemoveWhiteSmithCaseIndent)
+    --Line->Level;
+
   if (!Style.IndentCaseBlocks && CommentsBeforeNextToken.empty() &&
       FormatTok->Tok.is(tok::l_brace)) {
-    CompoundStatementIndenter Indenter(this, Line->Level,
-                                       Style.BraceWrapping.AfterCaseLabel,
-                                       Style.BraceWrapping.IndentBraces);
+
+    CompoundStatementIndenter Indenter(
+        this, Line->Level, Style.BraceWrapping.AfterCaseLabel,
+        Style.BraceWrapping.IndentBraces || RemoveWhiteSmithCaseIndent);
     parseBlock(/*MustBeDeclaration=*/false);
     if (FormatTok->Tok.is(tok::kw_break)) {
       if (Style.BraceWrapping.AfterControlStatement ==
           FormatStyle::BWACS_Always) {
         addUnwrappedLine();
-        if (!Style.IndentCaseBlocks &&
-            Style.BreakBeforeBraces == FormatStyle::BS_Whitesmiths) {
+        if (RemoveWhiteSmithCaseIndent) {
           Line->Level++;
         }
       }
       parseStructuralElement();
     }
     addUnwrappedLine();
   } else {
     if (FormatTok->is(tok::semi))
       nextToken();
     addUnwrappedLine();
   }
   Line->Level = OldLineLevel;
   if (FormatTok->isNot(tok::l_brace)) {
     parseStructuralElement();
     addUnwrappedLine();
   }
 }
 
 void UnwrappedLineParser::parseCaseLabel() {
   assert(FormatTok->Tok.is(tok::kw_case) && "'case' expected");
+
   // FIXME: fix handling of complex expressions here.
   do {
     nextToken();
   } while (!eof() && !FormatTok->Tok.is(tok::colon));
   parseLabel();
 }
 
 void UnwrappedLineParser::parseSwitch() {

clang/unittests/Format/FormatTest.cpp

verifyFormat("void switchTest3(int a)\n"
                WhitesmithsBraceStyle);
 
   WhitesmithsBraceStyle.IndentCaseBlocks = false;
 
   verifyFormat("void switchTest4(int a)\n"
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 2:\n"
+               "  case 2:\n"
                "    {\n"
                "    }\n"
                "    break;\n"
                "    }\n"
                "  }\n",
                WhitesmithsBraceStyle);
 
   verifyFormat("void switchTest5(int a)\n"
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 0:\n"
+               "  case 0:\n"
                "    break;\n"
-               "    case 1:\n"
+               "  case 1:\n"
                "    {\n"
                "    foo();\n"
                "    break;\n"
                "    }\n"
-               "    case 2:\n"
+               "  case 2:\n"
                "    {\n"
                "    }\n"
                "    break;\n"
-               "    default:\n"
+               "  default:\n"
                "    break;\n"
                "    }\n"
                "  }\n",
                WhitesmithsBraceStyle);
 
   verifyFormat("void switchTest6(int a)\n"
                "  {\n"
                "  switch (a)\n"
                "    {\n"
-               "    case 0:\n"
+               "  case 0:\n"
                "    {\n"
                "    foo(x);\n"
                "    }\n"
                "    break;\n"
-               "    default:\n"
+               "  default:\n"
                "    {\n"
                "    foo(1);\n"
                "    }\n"
                "    break;\n"
                "    }\n"
                "  }\n",
                WhitesmithsBraceStyle);