clang-format Additional Indent for class blocks

[dpayne]

Summary

Hi,

This is another attempt at fixing the issue describe here https://reviews.llvm.org/D22505. Some code bases will have an extra indent for member variables and functions, while the access modifiers are only indented once. For example,

class MyClass
{
    public:            // 1 level of indent
        int value2;    // 2 levels of indent
    private:           //1 level of indent
        int value3;    // 2 levels of indent
};

The current solution is to use a combination of increasing indent width and setting AccessModifierOffset to a offset. While this might work for some code bases, it has the downside of changing the indent level across the whole project. To fix this I added AdditionalIndentClassBlock, which when true will indent everything within a class block twice. This then could be used with AccessModifierOffset to achieve the desired formatting. By default it is off.

Going off the previous ticket, my goal here was to come up with a way to get this kind of formatting with the least amount of changes to the rest of the formatting rules.

[dpayne]

Adding unit tests to check indents, these were mostly taken from the original patch https://reviews.llvm.org/D22505.

[dpayne]

Adding the missing Style member variable.

[dpayne]

@djasper @klimek

Are there any comments here about this approach?

docs/ClangFormatStyleOptions.rst

* ``WebKit``
     A style complying with `WebKit's style guide
     <http://www.webkit.org/coding/coding-style.html>`_
 
 .. START_FORMAT_STYLE_OPTIONS
 
 **AccessModifierOffset** (``int``)
   The extra indent or outdent of access modifiers, e.g. ``public:``.
 
+**AdditionalIndentClassBlock** (``bool``)
+  If ``true``, adds an additional level of indention for class blocks.
+
 **AlignAfterOpenBracket** (``BracketAlignmentStyle``)
   If ``true``, horizontally aligns arguments after an open bracket.
 
   This applies to round brackets (parentheses), angle brackets and square
   brackets.
 
   Possible values:
 

include/clang/Format/Format.h

 std::error_code make_error_code(ParseError e);
 
 /// The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
   /// The extra indent or outdent of access modifiers, e.g. ``public:``.
   int AccessModifierOffset;
 
+  /// If ``true``, adds an additional level of indention for class blocks
+  bool AdditionalIndentClassBlock;
+
   /// Different styles for aligning after open brackets.
   enum BracketAlignmentStyle {
     /// Align parameters on the open bracket, e.g.:
     /// \code
     ///   someLongFunction(argument1,
     ///                    argument2);
     /// \endcode
     BAS_Align,
enum UseTabStyle {
     /// one tab stop to the next one.
     UT_Always
   };
 
   /// The way to use tab characters in the resulting file.
   UseTabStyle UseTab;
 
   bool operator==(const FormatStyle &R) const {
-    return AccessModifierOffset == R.AccessModifierOffset &&
+    return AdditionalIndentClassBlock == R.AdditionalIndentClassBlock &&
+           AccessModifierOffset == R.AccessModifierOffset &&
            AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
            AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
            AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
            AlignEscapedNewlines == R.AlignEscapedNewlines &&
            AlignOperands == R.AlignOperands &&
            AlignTrailingComments == R.AlignTrailingComments &&
            AllowAllParametersOfDeclarationOnNextLine ==
                R.AllowAllParametersOfDeclarationOnNextLine &&

lib/Format/Format.cpp

if (!IO.outputting()) {
       IO.mapOptional("IndentFunctionDeclarationAfterType",
                      Style.IndentWrappedFunctionNames);
       IO.mapOptional("PointerBindsToType", Style.PointerAlignment);
       IO.mapOptional("SpaceAfterControlStatementKeyword",
                      Style.SpaceBeforeParens);
     }
 
     IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
+    IO.mapOptional("AdditionalIndentClassBlock", Style.AdditionalIndentClassBlock);
     IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
     IO.mapOptional("AlignConsecutiveAssignments",
                    Style.AlignConsecutiveAssignments);
     IO.mapOptional("AlignConsecutiveDeclarations",
                    Style.AlignConsecutiveDeclarations);
     IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
     IO.mapOptional("AlignOperands", Style.AlignOperands);
     IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
static FormatStyle expandPresets(const FormatStyle &Style) {
   }
   return Expanded;
 }
 
 FormatStyle getLLVMStyle() {
   FormatStyle LLVMStyle;
   LLVMStyle.Language = FormatStyle::LK_Cpp;
   LLVMStyle.AccessModifierOffset = -2;
+  LLVMStyle.AdditionalIndentClassBlock = false;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
   LLVMStyle.AlignOperands = true;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;

lib/Format/UnwrappedLineParser.h

public:
 
   void parse();
 
 private:
   void reset();
   void parseFile();
   void parseLevel(bool HasOpeningBrace);
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
-                  bool MunchSemi = true);
+                  bool MunchSemi = true, bool ClassBlock = false);
   void parseChildBlock();
   void parsePPDirective();
   void parsePPDefine();
   void parsePPIf(bool IfDef);
   void parsePPElIf();
   void parsePPElse();
   void parsePPEndIf();
   void parsePPUnknown();

lib/Format/UnwrappedLineParser.cpp

size_t UnwrappedLineParser::computePPHash() const {
   for (const auto &i : PPStack) {
     hash_combine(h, size_t(i.Kind));
     hash_combine(h, i.Line);
   }
   return h;
 }
 
 void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
-                                     bool MunchSemi) {
+                                     bool MunchSemi, bool ClassBlock) {
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
          "'{' or macro block token expected");
   const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
   FormatTok->BlockKind = BK_Block;
 
   size_t PPStartHash = computePPHash();
 
   unsigned InitialLevel = Line->Level;
size_t OpeningLineIndex =
       CurrentLines->empty()
           ? (UnwrappedLine::kInvalidIndex)
           : (CurrentLines->size() - 1 - NbPreprocessorDirectives);
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
   if (AddLevel)
     ++Line->Level;
+
+  if (Style.AdditionalIndentClassBlock && ClassBlock)
+    ++Line->Level;
   parseLevel(/*HasOpeningBrace=*/true);
 
   if (eof())
     return;
 
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
                  : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
   if (FormatTok->Tok.is(tok::l_brace)) {
     if (ParseAsExpr) {
       parseChildBlock();
     } else {
       if (ShouldBreakBeforeBrace(Style, InitialToken))
         addUnwrappedLine();
 
       parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
-                 /*MunchSemi=*/false);
+                 /*MunchSemi=*/false, /*ClassBlock*/ true);
     }
   }
   // There is no addUnwrappedLine() here so that we fall through to parsing a
   // structural element afterwards. Thus, in "class A {} n, m;",
   // "} n, m;" will end up in one unwrapped line.
 }
 
 void UnwrappedLineParser::parseObjCMethod() {

unittests/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(FormatTests
   CleanupTest.cpp
   FormatTest.cpp
+  FormatTestClassIndent.cpp
   FormatTestComments.cpp
   FormatTestJS.cpp
   FormatTestJava.cpp
   FormatTestObjC.cpp
   FormatTestProto.cpp
   FormatTestRawStrings.cpp
   FormatTestSelective.cpp
   FormatTestTextProto.cpp

unittests/Format/FormatTestClassIndent.cpp

+//===- unittest/Format/FormatTestAccess.cpp - Formatting unit tests
+//-------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Format/Format.h"
+
+#include "../Tooling/RewriterTestContext.h"
+#include "FormatTestUtils.h"
+
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+
+#define DEBUG_TYPE "format-test"
+
+namespace clang {
+namespace format {
+namespace {
+
+class FormatTestClassIndent : public ::testing::Test {
+protected:
+  std::string format(llvm::StringRef Code, const FormatStyle &Style) {
+    LLVM_DEBUG(llvm::errs() << "---\n");
+    LLVM_DEBUG(llvm::errs() << Code << "\n\n");
+    std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+    bool IncompleteFormat = false;
+    tooling::Replacements Replaces =
+        reformat(Style, Code, Ranges, "<stdin>", &IncompleteFormat);
+    EXPECT_FALSE(IncompleteFormat) << Code << "\n\n";
+    auto Result = applyAllReplacements(Code, Replaces);
+    EXPECT_TRUE(static_cast<bool>(Result));
+    LLVM_DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
+  }
+
+  FormatStyle getStyleWithAdditionalIndent() {
+    FormatStyle Style = getLLVMStyle();
+    Style.AdditionalIndentClassBlock = true;
+    return Style;
+  }
+
+  void verifyFormat(llvm::StringRef Code, const FormatStyle & Style = getLLVMStyle()) {
+    EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
+  }
+};
+
+TEST_F(FormatTestClassIndent, NoChangeWithoutAccess) {
+  verifyFormat("class A {\n"
+               "  int a1;\n"
+               "};");
+  verifyFormat("class B {\n"
+               "    int b1;\n"
+               "};",
+               getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, ChangeWithAccess) {
+  verifyFormat("class C {\n"
+               "  int c1;\n"
+               "public:\n"
+               "  int c2;\n"
+               "};");
+  verifyFormat("class D {\n"
+               "    int d1;\n"
+               "  public:\n"
+               "    int d2;\n"
+               "};",
+               getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, MultipleAccessLevels) {
+  verifyFormat("class E {\n"
+               "  int e1;\n"
+               "public:\n"
+               "  int e2;\n"
+               "private:\n"
+               "  int e3;\n"
+               "protected:\n"
+               "  int e4;\n"
+               "};");
+  verifyFormat("class F {\n"
+               "    int f1;\n"
+               "  public:\n"
+               "    int f2;\n"
+               "  private:\n"
+               "    int f3;\n"
+               "  protected:\n"
+               "    int f4;\n"
+               "};",
+               getStyleWithAdditionalIndent());
+}
+
+TEST_F(FormatTestClassIndent, NestedClass) {
+  verifyFormat("class G {\n"
+               "  int g1;\n"
+               "  class GGA {\n"
+               "    int gg1;\n"
+               "  public:\n"
+               "    int gg2;\n"
+               "  };\n"
+               "public:\n"
+               "  class GGB {\n"
+               "    int gg1;\n"
+               "  public:\n"
+               "    int gg2;\n"
+               "  };\n"
+               "  int g2;\n"
+               "private:\n"
+               "  int g3;\n"
+               "protected:\n"
+               "  int g4;\n"
+               "};");
+  verifyFormat("class H {\n"
+               "    int h1;\n"
+               "    class HHA {\n"
+               "        int hh1;\n"
+               "      public:\n"
+               "        int hh2;\n"
+               "    };\n"
+               "  public:\n"
+               "    class HHB {\n"
+               "        int hh1;\n"
+               "      public:\n"
+               "        int hh2;\n"
+               "    };\n"
+               "    int h2;\n"
+               "  private:\n"
+               "    int h3;\n"
+               "  protected:\n"
+               "    int h4;\n"
+               "};",
+               getStyleWithAdditionalIndent());
+}
+
+} // end namespace
+} // end namespace format
+} // end namespace clang