[clang-format] Add SpaceBeforeCaseColon option

[HazardyKnusperkeks]

Summary

If `false`, spaces will be removed before case colon.

true:                                   false
switch (x) {                    vs.     switch (x) {
  case 1 : break;                         case 1: break;
}                                       }

[MyDeveloperDay]

I generally don't have the same aversion to new options than others have, but can you point out a style guide that might want this option. (that is kind of the process)

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

12115

maybe add a default example with {}

try not to change existing tests just add your own new ones

12158

Add new test leave the old one alone

[HazardyKnusperkeks]

In D93240#2454268, @MyDeveloperDay wrote:

I generally don't have the same aversion to new options than others have, but can you point out a style guide that might want this option. (that is kind of the process)

Mostly: My own style :) But I was really surprised that this is not in already, when nearly all other colons can be given a space and actually are in the default.

But I can show something I got from the web:

1. Uncrustify has the Option, so there was the need https://github.com/uncrustify/uncrustify/blob/1d3d8fa5e81bece0fac4b81316b0844f7cc35926/etc/uigui_uncrustify.ini#L1013
2. On this tutorial it is actually using a space: https://www.tutorialspoint.com/cplusplus/cpp_switch_statement.htm
3. And the cppreference uses both in its article on switch https://en.cppreference.com/w/cpp/language/switch

clang/unittests/Format/FormatTest.cpp

12115

First one: Will do
Second one: See below

12158

Do you mean a whole new function? I would strongly disagree, because this function is the right one since its called ConfigurableSpaceBeforeColon. And without it I would not have got the wrong assignment of the colon of a default statement.

Or do you mean the function is okay, but I should not touch this string (and probably the other strings too)? I would mildly disagree because I all to have to have the same unformatted text. But this one I'm willing to revert if that's what it's takes.

[MyDeveloperDay]

Nit: Please just separate your test from old test, apart from that I think its fine

I noticed support for this on uncrustify, (but couldn't find many github references who didn't set it to remove!)

I think adding this is fine

clang/unittests/Format/FormatTest.cpp

12158

I mean your own unique verifyFormat() but keeping it in the same function is ok, (sorry I wasn't clear)

The real problem is that clang-format has so many interactions with the code that is around it I personally like to keep the old tests and new tests separate as much as possible to ensure we don't regress anything, but also so I know you are not changing behaviour

Whilst in this case you don't really change anything this test was testing a case and default with no } before the default

I don't mind the repetition in the test if in my view its testing a slightly difference scenario (however slight)

There are so many billions of lines of code which are now formatted throughout all the software project in the world , the thought of introducing regressions is a little scary.

I like to use the "Beyonce Rule" when it comes to tests, so from my perspective that means no changes to existing tests unless absolutely necessary or the test is in fact wrong.

12179

I guess you shouldn't need this right? if false is the default then you could just add

EXPECT_EQ(NoSpaceStyle.SpaceBeforeCaseColon,false);

[HazardyKnusperkeks]

Test case adapted.

[sstwcw]

A goto label isn't affected by this option. Is it intentional?

[MyDeveloperDay]

In D93240#4254439, @sstwcw wrote:

A goto label isn't affected by this option. Is it intentional?

why would it?

[HazardyKnusperkeks]

In D93240#4254439, @sstwcw wrote:

A goto label isn't affected by this option. Is it intentional?

Of course, because it's only for case.

clang/docs/ClangFormatStyleOptions.rst

**SpaceBeforeAssignmentOperators** (``bool``)
   If ``false``, spaces will be removed before assignment operators.
 
   .. code-block:: c++
 
      true:                                  false:
      int a = 5;                     vs.     int a= 5;
      a += 42;                               a+= 42;
 
+**SpaceBeforeCaseColon** (``bool``)
+  If ``false``, spaces will be removed before case colon.
+
+  .. code-block:: c++
+
+    true:                                   false
+    switch (x) {                    vs.     switch (x) {
+      case 1 : break;                         case 1: break;
+    }                                       }
+
 **SpaceBeforeCpp11BracedList** (``bool``)
   If ``true``, a space will be inserted before a C++11 braced list
   used to initialize an object (after the preceding identifier or type).
 
   .. code-block:: c++
 
      true:                                  false:
      Foo foo { bar };               vs.     Foo foo{ bar };

clang/docs/ReleaseNotes.rst

- Experimental Support in clang-format for concepts has been improved, to
   aid this the follow options have been added
 
 - Option ``IndentRequires`` has been added to indent the ``requires`` keyword
   in templates.
 - Option ``BreakBeforeConceptDeclarations`` has been added to aid the formatting of concepts.
 
 - Option ``IndentPragmas`` has been added to allow #pragma to indented with the current scope level. This is especially useful when using #pragma to mark OpenMP sections of code.
 
+- Option ``SpaceBeforeCaseColon`` has been added to add a space before the
+  colon in a case or default statement.
+
 
 libclang
 --------
 
 - ...
 
 Static Analyzer
 ---------------

clang/include/clang/Format/Format.h

   /// If ``false``, spaces will be removed before assignment operators.
   /// \code
   ///    true:                                  false:
   ///    int a = 5;                     vs.     int a= 5;
   ///    a += 42;                               a+= 42;
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
+  /// If ``false``, spaces will be removed before case colon.
+  /// \code
+  ///   true:                                   false
+  ///   switch (x) {                    vs.     switch (x) {
+  ///     case 1 : break;                         case 1: break;
+  ///   }                                       }
+  /// \endcode
+  bool SpaceBeforeCaseColon;
+
   /// If ``true``, a space will be inserted before a C++11 braced list
   /// used to initialize an object (after the preceding identifier or type).
   /// \code
   ///    true:                                  false:
   ///    Foo foo { bar };               vs.     Foo foo{ bar };
   ///    Foo {};                                Foo{};
   ///    vector<int> { 1, 2, 3 };               vector<int>{ 1, 2, 3 };
   ///    new int[3] { 1, 2, 3 };                new int[3]{ 1, 2, 3 };
return AccessModifierOffset == R.AccessModifierOffset &&
                R.PenaltyBreakTemplateDeclaration &&
            PointerAlignment == R.PointerAlignment &&
            RawStringFormats == R.RawStringFormats &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
            SpaceAfterLogicalNot == R.SpaceAfterLogicalNot &&
            SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
            SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+           SpaceBeforeCaseColon == R.SpaceBeforeCaseColon &&
            SpaceBeforeCpp11BracedList == R.SpaceBeforeCpp11BracedList &&
            SpaceBeforeCtorInitializerColon ==
                R.SpaceBeforeCtorInitializerColon &&
            SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
            SpaceBeforeParens == R.SpaceBeforeParens &&
            SpaceAroundPointerQualifiers == R.SpaceAroundPointerQualifiers &&
            SpaceBeforeRangeBasedForLoopColon ==
                R.SpaceBeforeRangeBasedForLoopColon &&

clang/lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle &Style) {
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
     IO.mapOptional("SortUsingDeclarations", Style.SortUsingDeclarations);
     IO.mapOptional("SpaceAfterCStyleCast", Style.SpaceAfterCStyleCast);
     IO.mapOptional("SpaceAfterLogicalNot", Style.SpaceAfterLogicalNot);
     IO.mapOptional("SpaceAfterTemplateKeyword",
                    Style.SpaceAfterTemplateKeyword);
     IO.mapOptional("SpaceBeforeAssignmentOperators",
                    Style.SpaceBeforeAssignmentOperators);
+    IO.mapOptional("SpaceBeforeCaseColon", Style.SpaceBeforeCaseColon);
     IO.mapOptional("SpaceBeforeCpp11BracedList",
                    Style.SpaceBeforeCpp11BracedList);
     IO.mapOptional("SpaceBeforeCtorInitializerColon",
                    Style.SpaceBeforeCtorInitializerColon);
     IO.mapOptional("SpaceBeforeInheritanceColon",
                    Style.SpaceBeforeInheritanceColon);
     IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
     IO.mapOptional("SpaceAroundPointerQualifiers",
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
   LLVMStyle.SpacesInLineComments = {/*Minimum=*/1, /*Maximum=*/-1u};
   LLVMStyle.SpaceAfterCStyleCast = false;
   LLVMStyle.SpaceAfterLogicalNot = false;
   LLVMStyle.SpaceAfterTemplateKeyword = true;
   LLVMStyle.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Default;
+  LLVMStyle.SpaceBeforeCaseColon = false;
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.BitFieldColonSpacing = FormatStyle::BFCS_Both;

clang/lib/Format/TokenAnnotator.cpp

case tok::colon:
               Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
           ++Contexts.back().FirstObjCSelectorName->ObjCSelectorNameParts;
         }
       } else if (Contexts.back().ColonIsForRangeExpr) {
         Tok->setType(TT_RangeBasedForLoopColon);
       } else if (CurrentToken && CurrentToken->is(tok::numeric_constant)) {
         Tok->setType(TT_BitFieldColon);
       } else if (Contexts.size() == 1 &&
-                 !Line.First->isOneOf(tok::kw_enum, tok::kw_case)) {
+                 !Line.First->isOneOf(tok::kw_enum, tok::kw_case,
+                                      tok::kw_default)) {
         FormatToken *Prev = Tok->getPreviousNonComment();
         if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept))
           Tok->setType(TT_CtorInitializerColon);
         else if (Prev->is(tok::kw_try)) {
           // Member initializer list within function try block.
           FormatToken *PrevPrev = Prev->getPreviousNonComment();
           if (PrevPrev && PrevPrev->isOneOf(tok::r_paren, tok::kw_noexcept))
             Tok->setType(TT_CtorInitializerColon);
if (Right.is(TT_InheritanceColon) && !Style.SpaceBeforeInheritanceColon)
     return false;
   if (Right.is(TT_RangeBasedForLoopColon) &&
       !Style.SpaceBeforeRangeBasedForLoopColon)
     return false;
   if (Left.is(TT_BitFieldColon))
     return Style.BitFieldColonSpacing == FormatStyle::BFCS_Both ||
            Style.BitFieldColonSpacing == FormatStyle::BFCS_After;
   if (Right.is(tok::colon)) {
-    if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
-        !Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))
+    if (Line.First->isOneOf(tok::kw_default, tok::kw_case))
+      return Style.SpaceBeforeCaseColon;
+    if (!Right.getNextNonComment() || Right.getNextNonComment()->is(tok::semi))
       return false;
     if (Right.is(TT_ObjCMethodExpr))
       return false;
     if (Left.is(tok::question))
       return false;
     if (Right.is(TT_InlineASMColon) && Left.is(tok::coloncolon))
       return false;
     if (Right.is(TT_DictLiteral))

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, ConfigurableSpaceBeforeColon) {
   verifyFormat("for (auto a : b) {\n}");
   verifyFormat("int x = a ? b : c;");
   verifyFormat("{\n"
                "label0:\n"
                "  int x = 0;\n"
                "}");
   verifyFormat("switch (x) {\n"
                "case 1:\n"
+               "case 2: {\n"
+               "  break;\n"
+               "}\n"
                "default:\n"
+               "  break;\n"
                "}");
 
   FormatStyle CtorInitializerStyle = getLLVMStyleWithColumns(30);
   CtorInitializerStyle.SpaceBeforeCtorInitializerColon = false;
   verifyFormat("class Foo : public Bar {};", CtorInitializerStyle);
   verifyFormat("Foo::Foo(): foo(1) {}", CtorInitializerStyle);
   verifyFormat("for (auto a : b) {\n}", CtorInitializerStyle);
   verifyFormat("int x = a ? b : c;", CtorInitializerStyle);
   verifyFormat("{\n"
                "label1:\n"
                "  int x = 0;\n"
                "}",
                CtorInitializerStyle);
   verifyFormat("switch (x) {\n"
                "case 1:\n"
+               "case 2: {\n"
+               "  break;\n"
+               "}\n"
                "default:\n"
+               "  break;\n"
                "}",
                CtorInitializerStyle);
   CtorInitializerStyle.BreakConstructorInitializers =
       FormatStyle::BCIS_AfterColon;
   verifyFormat("Fooooooooooo::Fooooooooooo():\n"
                "    aaaaaaaaaaaaaaaa(1),\n"
                "    bbbbbbbbbbbbbbbb(2) {}",
                CtorInitializerStyle);
TEST_F(FormatTest, ConfigurableSpaceBeforeColon) {
   verifyFormat("int x = a ? b : c;", InheritanceStyle);
   verifyFormat("{\n"
                "label2:\n"
                "  int x = 0;\n"
                "}",
                InheritanceStyle);
   verifyFormat("switch (x) {\n"
                "case 1:\n"
+               "case 2: {\n"
+               "  break;\n"
+               "}\n"
                "default:\n"
+               "  break;\n"
                "}",
                InheritanceStyle);
   InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_AfterColon;
   verifyFormat("class Foooooooooooooooooooooo:\n"
                "    public aaaaaaaaaaaaaaaaaa,\n"
                "    public bbbbbbbbbbbbbbbbbb {\n"
                "}",
                InheritanceStyle);
   InheritanceStyle.BreakInheritanceList = FormatStyle::BILS_BeforeComma;
TEST_F(FormatTest, ConfigurableSpaceBeforeColon) {
   verifyFormat("int x = a ? b : c;", ForLoopStyle);
   verifyFormat("{\n"
                "label2:\n"
                "  int x = 0;\n"
                "}",
                ForLoopStyle);
   verifyFormat("switch (x) {\n"
                "case 1:\n"
+               "case 2: {\n"
+               "  break;\n"
+               "}\n"
                "default:\n"
+               "  break;\n"
                "}",
                ForLoopStyle);
 
+  FormatStyle CaseStyle = getLLVMStyle();
+  CaseStyle.SpaceBeforeCaseColon = true;
+  verifyFormat("class Foo : public Bar {};", CaseStyle);
+  verifyFormat("Foo::Foo() : foo(1) {}", CaseStyle);
+  verifyFormat("for (auto a : b) {\n}", CaseStyle);
+  verifyFormat("int x = a ? b : c;", CaseStyle);
+  verifyFormat("switch (x) {\n"
+               "case 1 :\n"
+               "case 2 : {\n"
+               "  break;\n"
+               "}\n"
+               "default :\n"
+               "  break;\n"
+               "}",
+               CaseStyle);
+
   FormatStyle NoSpaceStyle = getLLVMStyle();
+  NoSpaceStyle.SpaceBeforeCaseColon = false;
   NoSpaceStyle.SpaceBeforeCtorInitializerColon = false;
   NoSpaceStyle.SpaceBeforeInheritanceColon = false;
   NoSpaceStyle.SpaceBeforeRangeBasedForLoopColon = false;
   verifyFormat("class Foo: public Bar {};", NoSpaceStyle);
   verifyFormat("Foo::Foo(): foo(1) {}", NoSpaceStyle);
   verifyFormat("for (auto a: b) {\n}", NoSpaceStyle);
   verifyFormat("int x = a ? b : c;", NoSpaceStyle);
   verifyFormat("{\n"
                "label3:\n"
                "  int x = 0;\n"
                "}",
                NoSpaceStyle);
   verifyFormat("switch (x) {\n"
                "case 1:\n"
+               "case 2: {\n"
+               "  break;\n"
+               "}\n"
                "default:\n"
+               "  break;\n"
                "}",
                NoSpaceStyle);
+
+  FormatStyle InvertedSpaceStyle = getLLVMStyle();
+  InvertedSpaceStyle.SpaceBeforeCaseColon = true;
+  InvertedSpaceStyle.SpaceBeforeCtorInitializerColon = false;
+  InvertedSpaceStyle.SpaceBeforeInheritanceColon = false;
+  InvertedSpaceStyle.SpaceBeforeRangeBasedForLoopColon = false;
+  verifyFormat("class Foo: public Bar {};", InvertedSpaceStyle);
+  verifyFormat("Foo::Foo(): foo(1) {}", InvertedSpaceStyle);
+  verifyFormat("for (auto a: b) {\n}", InvertedSpaceStyle);
+  verifyFormat("int x = a ? b : c;", InvertedSpaceStyle);
+  verifyFormat("{\n"
+               "label3:\n"
+               "  int x = 0;\n"
+               "}",
+               InvertedSpaceStyle);
+  verifyFormat("switch (x) {\n"
+               "case 1 :\n"
+               "case 2 : {\n"
+               "  break;\n"
+               "}\n"
+               "default :\n"
+               "  break;\n"
+               "}",
+               InvertedSpaceStyle);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceAroundPointerQualifiers) {
   FormatStyle Style = getLLVMStyle();
 
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.SpaceAroundPointerQualifiers = FormatStyle::SAPQ_Default;
   verifyFormat("void* const* x = NULL;", Style);
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
   CHECK_PARSE_BOOL(SpaceAfterCStyleCast);
   CHECK_PARSE_BOOL(SpaceAfterTemplateKeyword);
   CHECK_PARSE_BOOL(SpaceAfterLogicalNot);
   CHECK_PARSE_BOOL(SpaceBeforeAssignmentOperators);
+  CHECK_PARSE_BOOL(SpaceBeforeCaseColon);
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
   CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
   CHECK_PARSE_BOOL(UseCRLF);
 
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);