clang-format: add options to merge empty record body

[Typz]

Summary

This patch introduces a few extra BraceWrapping options, similar to
SplitEmptyFunction, to allow merging empty 'record' bodies (e.g.
class, struct, union and namespace):

• SplitEmptyClass
• SplitEmptyStruct
• SplitEmptyUnion
• SplitEmptyNamespace

The SplitEmptyFunction option name has also been simplified/
shortened (from SplitEmptyFunctionBody).

These options are helpful when the correspond AfterXXX option is
enabled, to allow merging the empty record:

class Foo
{};

In addition, this fixes an unexpected merging of short records, when
the AfterXXXX options are used, which caused to be formatted like
this:

class Foo
{ void Foo(); };

This is now properly formatted as:

class Foo
{
   void Foo();
};

[Typz]

Enable merging records for Mozilla style

[Typz]

ping?

[djasper]

Do you know of a style guide that would actually want to handle class, structs and unions differently? In most of Clang, they are handled as "records" and fundamentally, they are so alike that I'd hope that people always want the same behavior for all of them.

[Typz]

I don't know if some style would want different styles, and I agree with you on principle; but since the brace wrapping is already configured for each kind of record, I choose to keep things consistent and have flags for each kind of record.
But I can merge the options, and keep only SplitEmptyRecord and SplitEmptyNamespace, if you really think it is better.

[djasper]

Yes merge them into those two, please. I think we introduced the others because of some linux style, but generally lets try not to introduce options that people aren't going to use.

[Typz]

Merge SplitEmptyClass/Struct/Union options into a single SplitEmptyRecord option.

[djasper]

Looks good.

include/clang/Format/Format.h

struct BraceWrappingFlags {
     /// set, and the function could/should not be put on a single line (as per
     /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
     /// \code
     ///   int f()   vs.   inf f()
     ///   {}              {
     ///                   }
     /// \endcode
     ///
-    bool SplitEmptyFunctionBody;
+    bool SplitEmptyFunction;
+    /// \brief If ``false``, empty class body can be put on a single line.
+    /// This option is used only if the opening brace of the class has
+    /// already been wrapped, i.e. the `AfterClass` brace wrapping mode is
+    /// set.
+    /// \code
+    ///   class Foo   vs.  class Foo
+    ///   {}               {
+    ///                    }
+    /// \endcode
+    ///
+    bool SplitEmptyClass;
+    /// \brief If ``false``, empty struct body can be put on a single line.
+    /// This option is used only if the opening brace of the struct has
+    /// already been wrapped, i.e. the `AfterStruct` brace wrapping mode is
+    /// set.
+    /// \code
+    ///   struct Foo   vs.  struct Foo
+    ///   {}                {
+    ///                     }
+    /// \endcode
+    ///
+    bool SplitEmptyStruct;
+    /// \brief If ``false``, empty union can be put on a single line.
+    /// This option is used only if the opening brace of the union has
+    /// already been wrapped, i.e. the `AfterUnion` brace wrapping mode is
+    /// set.
+    /// \code
+    ///   union Foo   vs.  union Foo
+    ///   {}               {
+    ///                    }
+    /// \endcode
+    ///
+    bool SplitEmptyUnion;
+    /// \brief If ``false``, empty namespace body can be put on a single line.
+    /// This option is used only if the opening brace of the namespace has
+    /// already been wrapped, i.e. the `AfterNamespace` brace wrapping mode is
+    /// set.
+    /// \code
+    ///   namespace Foo   vs.  namespace Foo
+    ///   {}                   {
+    ///                        }
+    /// \endcode
+    ///
+    bool SplitEmptyNamespace;
   };
 
   /// \brief Control of individual brace wrapping cases.
   ///
   /// If ``BreakBeforeBraces`` is set to ``BS_Custom``, use this to specify how
   /// each individual brace case should be handled. Otherwise, this is ignored.
   BraceWrappingFlags BraceWrapping;
 

lib/Format/Format.cpp

static void mapping(IO &IO, FormatStyle::BraceWrappingFlags &Wrapping) {
     IO.mapOptional("AfterFunction", Wrapping.AfterFunction);
     IO.mapOptional("AfterNamespace", Wrapping.AfterNamespace);
     IO.mapOptional("AfterObjCDeclaration", Wrapping.AfterObjCDeclaration);
     IO.mapOptional("AfterStruct", Wrapping.AfterStruct);
     IO.mapOptional("AfterUnion", Wrapping.AfterUnion);
     IO.mapOptional("BeforeCatch", Wrapping.BeforeCatch);
     IO.mapOptional("BeforeElse", Wrapping.BeforeElse);
     IO.mapOptional("IndentBraces", Wrapping.IndentBraces);
-    IO.mapOptional("SplitEmptyFunctionBody", Wrapping.SplitEmptyFunctionBody);
+    IO.mapOptional("SplitEmptyFunction", Wrapping.SplitEmptyFunction);
+    IO.mapOptional("SplitEmptyClass", Wrapping.SplitEmptyClass);
+    IO.mapOptional("SplitEmptyStruct", Wrapping.SplitEmptyStruct);
+    IO.mapOptional("SplitEmptyUnion", Wrapping.SplitEmptyUnion);
+    IO.mapOptional("SplitEmptyNamespace", Wrapping.SplitEmptyNamespace);
   }
 };
 
 template <> struct MappingTraits<FormatStyle::IncludeCategory> {
   static void mapping(IO &IO, FormatStyle::IncludeCategory &Category) {
     IO.mapOptional("Regex", Category.Regex);
     IO.mapOptional("Priority", Category.Priority);
   }
std::string ParseErrorCategory::message(int EV) const {
   llvm_unreachable("unexpected parse error");
 }
 
 static FormatStyle expandPresets(const FormatStyle &Style) {
   if (Style.BreakBeforeBraces == FormatStyle::BS_Custom)
     return Style;
   FormatStyle Expanded = Style;
   Expanded.BraceWrapping = {false, false, false, false, false, false,
-                            false, false, false, false, false, true};
+                            false, false, false, false, false, true,
+                            true,  true,  true,  true};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:
     Expanded.BraceWrapping.AfterClass = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterNamespace = true;
     break;
   case FormatStyle::BS_Mozilla:
     Expanded.BraceWrapping.AfterClass = true;
     Expanded.BraceWrapping.AfterEnum = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterStruct = true;
     Expanded.BraceWrapping.AfterUnion = true;
-    Expanded.BraceWrapping.SplitEmptyFunctionBody = false;
+    Expanded.BraceWrapping.SplitEmptyFunction = false;
+    Expanded.BraceWrapping.SplitEmptyClass = false;
+    Expanded.BraceWrapping.SplitEmptyStruct = false;
+    Expanded.BraceWrapping.SplitEmptyUnion = false;
     break;
   case FormatStyle::BS_Stroustrup:
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.BeforeCatch = true;
     Expanded.BraceWrapping.BeforeElse = true;
     break;
   case FormatStyle::BS_Allman:
     Expanded.BraceWrapping.AfterClass = true;
     Expanded.BraceWrapping.AfterControlStatement = true;
     Expanded.BraceWrapping.AfterEnum = true;
     Expanded.BraceWrapping.AfterFunction = true;
     Expanded.BraceWrapping.AfterNamespace = true;
     Expanded.BraceWrapping.AfterObjCDeclaration = true;
     Expanded.BraceWrapping.AfterStruct = true;
     Expanded.BraceWrapping.BeforeCatch = true;
     Expanded.BraceWrapping.BeforeElse = true;
     break;
   case FormatStyle::BS_GNU:
     Expanded.BraceWrapping = {true, true, true, true, true, true,
-                              true, true, true, true, true, true};
+                              true, true, true, true, true, true,
+                              true, true, true, true};
     break;
   case FormatStyle::BS_WebKit:
     Expanded.BraceWrapping.AfterFunction = true;
     break;
   default:
     break;
   }
   return Expanded;
FormatStyle getLLVMStyle() {
   LLVMStyle.AlwaysBreakBeforeMultilineStrings = false;
   LLVMStyle.AlwaysBreakTemplateDeclarations = false;
   LLVMStyle.BinPackArguments = true;
   LLVMStyle.BinPackParameters = true;
   LLVMStyle.BreakBeforeBinaryOperators = FormatStyle::BOS_None;
   LLVMStyle.BreakBeforeTernaryOperators = true;
   LLVMStyle.BreakBeforeBraces = FormatStyle::BS_Attach;
   LLVMStyle.BraceWrapping = {false, false, false, false, false, false,
-                             false, false, false, false, false, true};
+                             false, false, false, false, false, true,
+                             true,  true,  true,  true};
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanceComma = false;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
   LLVMStyle.CompactNamespaces = false;
   LLVMStyle.ConstructorInitializerAllOnOneLineOrOnePerLine = false;

lib/Format/FormatToken.h

struct FormatToken {
 
   /// \brief Same as opensBlockOrBlockTypeList, but for the closing token.
   bool closesBlockOrBlockTypeList(const FormatStyle &Style) const {
     if (is(TT_TemplateString) && closesScope())
       return true;
     return MatchingParen && MatchingParen->opensBlockOrBlockTypeList(Style);
   }
 
+  /// \brief Return the actual namespace token, if this token starts a namespace
+  /// block.
+  const FormatToken *getNamespaceToken() const {
+    const FormatToken *NamespaceTok = this;
+    if (is(tok::comment))
+      NamespaceTok = NamespaceTok->getNextNonComment();
+    // Detect "(inline)? namespace" in the beginning of a line.
+    if (NamespaceTok && NamespaceTok->is(tok::kw_inline))
+      NamespaceTok = NamespaceTok->getNextNonComment();
+    return NamespaceTok && NamespaceTok->is(tok::kw_namespace) ? NamespaceTok
+                                                               : nullptr;
+  }
+
 private:
   // Disallow copying.
   FormatToken(const FormatToken &) = delete;
   void operator=(const FormatToken &) = delete;
 
   template <typename A, typename... Ts>
   bool startsSequenceInternal(A K1, Ts... Tokens) const {
     if (is(tok::comment) && Next)

lib/Format/UnwrappedLineFormatter.cpp

private:
   int Offset = 0;
 
   /// \brief The current line's indent.
   unsigned Indent = 0;
 };
 
 bool isNamespaceDeclaration(const AnnotatedLine *Line) {
   const FormatToken *NamespaceTok = Line->First;
-  // Detect "(inline)? namespace" in the beginning of a line.
-  if (NamespaceTok->is(tok::kw_inline))
-    NamespaceTok = NamespaceTok->getNextNonComment();
-  return NamespaceTok && NamespaceTok->is(tok::kw_namespace);
+  return NamespaceTok && NamespaceTok->getNamespaceToken();
 }
 
 bool isEndOfNamespace(const AnnotatedLine *Line,
                       const SmallVectorImpl<AnnotatedLine *> &AnnotatedLines) {
   if (!Line->startsWith(tok::r_brace))
     return false;
   size_t StartLineIndex = Line->MatchingOpeningBlockLineIndex;
   if (StartLineIndex == UnwrappedLine::kInvalidIndex)
tryFitMultipleLinesInOne(LevelIndentTracker &IndentTracker,
     // If we already exceed the column limit, we set 'Limit' to 0. The different
     // tryMerge..() functions can then decide whether to still do merging.
     Limit = TheLine->Last->TotalLength > Limit
                 ? 0
                 : Limit - TheLine->Last->TotalLength;
 
     if (TheLine->Last->is(TT_FunctionLBrace) &&
         TheLine->First == TheLine->Last &&
-        !Style.BraceWrapping.SplitEmptyFunctionBody &&
+        !Style.BraceWrapping.SplitEmptyFunction &&
         I[1]->First->is(tok::r_brace))
       return tryMergeSimpleBlock(I, E, Limit);
 
+    // Handle empty record blocks where the brace has already been wrapped
+    if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
+        I != AnnotatedLines.begin()) {
+      bool EmptyBlock = I[1]->First->is(tok::r_brace);
+
+      const FormatToken *Tok = I[-1]->First;
+      if (Tok && Tok->is(tok::comment))
+        Tok = Tok->getNextNonComment();
+
+      if (Tok && Tok->getNamespaceToken())
+        return !Style.BraceWrapping.SplitEmptyNamespace && EmptyBlock
+            ? tryMergeSimpleBlock(I, E, Limit) : 0;
+
+      if (Tok && Tok->is(tok::kw_typedef))
+        Tok = Tok->getNextNonComment();
+      if (Tok) {
+        if (Tok->isOneOf(tok::kw_class, Keywords.kw_interface))
+          return !Style.BraceWrapping.SplitEmptyClass && EmptyBlock
+              ? tryMergeSimpleBlock(I, E, Limit) : 0;
+        if (Tok->is(tok::kw_struct))
+          return !Style.BraceWrapping.SplitEmptyStruct && EmptyBlock
+              ? tryMergeSimpleBlock(I, E, Limit) : 0;
+        if (Tok->is(tok::kw_union))
+          return !Style.BraceWrapping.SplitEmptyUnion && EmptyBlock
+              ? tryMergeSimpleBlock(I, E, Limit) : 0;
+      }
+    }
+
     // FIXME: TheLine->Level != 0 might or might not be the right check to do.
     // If necessary, change to something smarter.
     bool MergeShortFunctions =
         Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
         (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
          I[1]->First->is(tok::r_brace)) ||
         (Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_Inline &&
          TheLine->Level != 0);

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, PullInlineFunctionDefinitionsIntoSingleLine) {
   // SFS_Inline implies SFS_Empty
   verifyFormat("int f() {}", MergeInlineOnly);
   verifyFormat("class C {\n"
                "  int f() {}\n"
                "};",
                MergeInlineOnly);
 }
 
-TEST_F(FormatTest, SplitEmptyFunctionBody) {
+TEST_F(FormatTest, SplitEmptyFunction) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterFunction = true;
-  Style.BraceWrapping.SplitEmptyFunctionBody = false;
+  Style.BraceWrapping.SplitEmptyFunction = false;
   Style.ColumnLimit = 40;
 
   verifyFormat("int f()\n"
                "{}",
                Style);
   verifyFormat("int f()\n"
                "{\n"
                "  return 42;\n"
verifyFormat("int aaaaaaaaaaaaaa(int bbbbbbbbbbbbbb)\n"
                Style);
   verifyFormat("int aaaaaaaaaaaaaa(int bbbbbbbbbbbbbb)\n"
                "{\n"
                "  return 0;\n"
                "}",
                Style);
 }
 
+TEST_F(FormatTest, SplitEmptyClass) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.SplitEmptyClass = false;
+
+  verifyFormat("class Foo\n"
+               "{};",
+               Style);
+  verifyFormat("/* something */ class Foo\n"
+               "{};",
+               Style);
+  verifyFormat("template <typename X> class Foo\n"
+               "{};",
+               Style);
+  verifyFormat("class Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef class Foo\n"
+               "{\n"
+               "} Foo_t;",
+               Style);
+}
+
+TEST_F(FormatTest, SplitEmptyStruct) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.SplitEmptyStruct = false;
+
+  verifyFormat("struct Foo\n"
+               "{};",
+               Style);
+  verifyFormat("/* something */ struct Foo\n"
+               "{};",
+               Style);
+  verifyFormat("template <typename X> struct Foo\n"
+               "{};",
+               Style);
+  verifyFormat("struct Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef struct Foo\n"
+               "{\n"
+               "} Foo_t;",
+               Style);
+  //typedef struct Bar {} Bar_t;
+}
+
+TEST_F(FormatTest, SplitEmptyUnion) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.SplitEmptyUnion = false;
+
+  verifyFormat("union Foo\n"
+               "{};",
+               Style);
+  verifyFormat("/* something */ union Foo\n"
+               "{};",
+               Style);
+  verifyFormat("union Foo\n"
+               "{\n"
+               "  A,\n"
+               "};",
+               Style);
+  verifyFormat("typedef union Foo\n"
+               "{\n"
+               "} Foo_t;",
+               Style);
+}
+
+TEST_F(FormatTest, SplitEmptyNamespace) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterNamespace = true;
+  Style.BraceWrapping.SplitEmptyNamespace = false;
+
+  verifyFormat("namespace Foo\n"
+               "{};",
+               Style);
+  verifyFormat("/* something */ namespace Foo\n"
+               "{};",
+               Style);
+  verifyFormat("inline namespace Foo\n"
+               "{};",
+               Style);
+  verifyFormat("namespace Foo\n"
+               "{\n"
+               "void Bar();\n"
+               "};",
+               Style);
+}
+
+TEST_F(FormatTest, NeverMergeShortRecords) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("class Foo {\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef class Foo {\n"
+               "  Foo();\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("struct Foo {\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef struct Foo {\n"
+               "  Foo();\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("union Foo {\n"
+               "  A,\n"
+               "};",
+               Style);
+  verifyFormat("typedef union Foo {\n"
+               "  A,\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("namespace Foo {\n"
+               "void Bar();\n"
+               "};",
+               Style);
+
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = true;
+  Style.BraceWrapping.AfterStruct = true;
+  Style.BraceWrapping.AfterUnion = true;
+  Style.BraceWrapping.AfterNamespace = true;
+  verifyFormat("class Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef class Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("struct Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "};",
+               Style);
+  verifyFormat("typedef struct Foo\n"
+               "{\n"
+               "  Foo();\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("union Foo\n"
+               "{\n"
+               "  A,\n"
+               "};",
+               Style);
+  verifyFormat("typedef union Foo\n"
+               "{\n"
+               "  A,\n"
+               "} Foo_t;",
+               Style);
+  verifyFormat("namespace Foo\n"
+               "{\n"
+               "void Bar();\n"
+               "};",
+               Style);
+}
+
 TEST_F(FormatTest, UnderstandContextOfRecordTypeKeywords) {
   // Elaborate type variable declarations.
   verifyFormat("struct foo a = {bar};\nint n;");
   verifyFormat("class foo a = {bar};\nint n;");
   verifyFormat("union foo a = {bar};\nint n;");
 
   // Elaborate types inside function definitions.
   verifyFormat("struct foo f() {}\nint n;");
TEST_F(FormatTest, ParsesConfigurationBools) {
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterFunction);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterNamespace);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterObjCDeclaration);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterStruct);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterUnion);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeCatch);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, BeforeElse);
   CHECK_PARSE_NESTED_BOOL(BraceWrapping, IndentBraces);
-  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunctionBody);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyFunction);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyClass);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyStruct);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyUnion);
+  CHECK_PARSE_NESTED_BOOL(BraceWrapping, SplitEmptyNamespace);
 }
 
 #undef CHECK_PARSE_BOOL
 
 TEST_F(FormatTest, ParsesConfiguration) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE("AccessModifierOffset: -1234", AccessModifierOffset, -1234);