[clang-format] Access Modifier Use Normal Indent

[HazardyKnusperkeks]

Summary

Access Modifiers (public/protected/private) causes standard indent.

class MyClass
{
    int value1;        // standard indent.
    public:            // access modifier (increases indent level)
        int value2;    // next item indent one more level 
    private:           // access modifier goes out a level
        int value3;    // next item indent one more level 
};

[djasper]

Before we continue with the actual code review and brainstorming how we could actually call this option, can you read through http://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options and provide feedback about why this option qualifies?

[LokiAstari]

Each new style option must ::

• be used in a project of significant size (have dozens of contributors)
• have a publicly accessible style guide
• have a person willing to contribute and maintain patches

Example:

Project:             https://github.com/openframeworks/openFrameworks
Style Guide:         https://github.com/openframeworks/openFrameworks/wiki/oF-code-style#indentation
Contributors:        220
Willing to maintain: Me (Loki Astari. I have more patches I want to contribute so I am not doing a drive bye :-)

[LokiAstari]

@djasper @klimek

Is that a sufficient example? Or do I need to do some more exhaustive search of github for projects?

[LokiAstari]

@djasper @klimek

Hi guys,
I am not sure how to move this PR forward.

I am not sure if you are uninterested or if this is two minor in comparison to other issues.
If there is some other task I need to do to move this forward I more than happy to do some leg work, but given the documentation I am not sure what else I need to do.

Loki.

[djasper]

Sorry, my bad. I should have replied sooner. I believe we can move further with this seems to be used widely enough (though it really doesn't make sense in my personal opinion ;) ).

My main concern is how we are going to configure this in the long run. This is mostly my bad as I was trying to be too clever with the AccessModifierOffset setting.

I see three options:

1. Roughly go ahead with what you are suggesting, although the option should not be called AccessModifierStandardIndent, as that carries no meaning and actually is far from the "standard" way. Reasonable names that spring to mind are: AccessModifiersCauseAdditionalIndent or (Additional)IndentAfterAccessModifiers.
2. Be even more "clever" with AccessModifierOffset (I can come up with various ways from negative numbers to special numbers, none of them really good).
3. Deprecate AccessModifierOffset and instead use an enum with a few dedicated settings.

Manuel, any thoughts?

[LokiAstari]

Hope you don't mind me chiming in:

Roughly go ahead with what you are suggesting, although the option should not be called AccessModifierStandardIndent, as that carries no meaning and actually is far from the "standard" way. Reasonable names that spring to mind are:

Though I personally disagree on the more standard technique :-) Since I am the newbie to this I am not going to fight on that point (though I suspect if we meet up for some beers we could have a spirited debate).

AccessModifiersCauseAdditionalIndent or (Additional)IndentAfterAccessModifiers.

How about "AccessModifiersUseNormaIndent" as we are indenting by a normal indent width. But I am not going to fight over the name when it comes down to it.

Be even more "clever" with AccessModifierOffset (I can come up with various ways from negative numbers to special numbers, none of them really good).

I don't think this is a good idea (but as the newbie no strong feelings). Unfortunately a lot of large projects have stated using this is a standard part of their formatting; got to this project too late to prevent that :-O

Deprecate AccessModifierOffset and instead use an enum with a few dedicated settings.

The issue with this is getting all projects to update their settings with the new version. Not sure this is a great user experience.

[LokiAstari]

@djasper @klimek

Just want to bring this to the top of your queue again :-)
Any further thoughts on the processes?

Loki

[klimek]

Perhaps call it AccessModifierIntroducesScope or something?

Side-track: I find it highly confusing that in

class C {
  int v1;
  private:
    int v2;
}

v1 & v2 have different indent, although they are in the same scope.

[djasper]

So you'd be for #1 of the three choices from my previous comment?

[klimek]

In D22505#503278, @djasper wrote:

So you'd be for #1 of the three choices from my previous comment?

Yes, because I think ultimately introducing scopes is different from simple outdents/indents.

[klimek]

(full disclosure: I'm also generally opposed to this change, but if there are really enough users using this it's probably a lost cause)

[djasper]

I generally agree. Note, however, that the mentioned style guide doesn't actually specify this. All it says is "The public, protected, and private keywords should be indented inside the class with the function declarations indented as well."

So I guess,

class C {
    int v1;
  private:
    int v2;
};

Would actually also be valid according to the style guide.

[LokiAstari]

I don't have a problem changing it so the default behaviour is:

class C {
    int v1;
  private:
    int v2;
};

But I would like to retain the functionality that if there is no explicit public/private/protected that it follows the original.
But if there is huge objection to that I am not going to fight over this one (I can live with the indention above).

Loki.

[klimek]

In D22505#503472, @LokiAstari wrote:

I don't have a problem changing it so the default behaviour is:

class C {
    int v1;
  private:
    int v2;
};

But I would like to retain the functionality that if there is no explicit public/private/protected that it follows the original.
But if there is huge objection to that I am not going to fight over this one (I can live with the indention above).

That should already be doable with a negative offset today, right?

[LokiAstari]

That should already be doable with a negative offset today, right?

Yes. So I don't need to add any changes. right?

[djasper]

No, I don't think so. Because you need a "double-indent" within classes.

[mithro]

I was wondering what the status of this feature was? Looks like it was last touched back in Aug 2 2016 -- do you know if a different or replacement patch was landed? I have a project (https://github.com/verilog-to-routing/vtr-verilog-to-routing) that I'm trying to convince to use clang-format but they use this style of indenting after the accessor modifier and convincing them to change style is a *much* bigger challenge then getting them to adopt clang-format usage :-)

[HazardyKnusperkeks]

This is dead, isn't it?

[MyDeveloperDay]

I think so, it came to the top because someone subscribed to it, I was wondering if this was something that is still needed or if our existing options cover this.

I recently came across a style like this. I don't know if there is still interest.

[HazardyKnusperkeks]

In D22505#3147501, @MyDeveloperDay wrote:

I think so, it came to the top because someone subscribed to it, I was wondering if this was something that is still needed or if our existing options cover this.

I recently came across a style like this. I don't know if there is still interest.

I definitely think there is interest, but is there someone willing to implement it? :)

[MyDeveloperDay]

I recently reviewed this item, but I wonder what the overlap with D94661: [clang-format] [PR19056] Add support for access modifiers indentation

Seeming the only difference here is the first element

e.g.

class B {
    int b1;
   
    public:
      int c1;
}

vs

class B {
  int b1;
   
    public:
      int c1;

}

[HazardyKnusperkeks]

Closing because of the long silence.

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:``.
 
+**AccessModifierStandardIndent** (``bool``)
+  Indent accesses modifier in the normal method.
+
 **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

 const std::error_category &getParseCategory();
 std::error_code make_error_code(ParseError e);
 
 /// \brief The ``FormatStyle`` is used to configure the formatting to follow
 /// specific guidelines.
 struct FormatStyle {
   /// \brief The extra indent or outdent of access modifiers, e.g. ``public:``.
   int AccessModifierOffset;
+  bool AccessModifierStandardIndent;
 
   /// \brief Different styles for aligning after open brackets.
   enum BracketAlignmentStyle {
     /// \brief Align parameters on the open bracket, e.g.:
     /// \code
     ///   someLongFunction(argument1,
     ///                    argument2);
     /// \endcode
enum UseTabStyle {
     UT_Always
   };
 
   /// \brief The way to use tab characters in the resulting file.
   UseTabStyle UseTab;
 
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
+           AccessModifierStandardIndent == R.AccessModifierStandardIndent &&
            AlignAfterOpenBracket == R.AlignAfterOpenBracket &&
            AlignConsecutiveAssignments == R.AlignConsecutiveAssignments &&
            AlignConsecutiveDeclarations == R.AlignConsecutiveDeclarations &&
            AlignEscapedNewlinesLeft == R.AlignEscapedNewlinesLeft &&
            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("AccessModifierStandardIndent",
+                   Style.AccessModifierStandardIndent);
     IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
     IO.mapOptional("AlignConsecutiveAssignments",
                    Style.AlignConsecutiveAssignments);
     IO.mapOptional("AlignConsecutiveDeclarations",
                    Style.AlignConsecutiveDeclarations);
     IO.mapOptional("AlignEscapedNewlinesLeft", Style.AlignEscapedNewlinesLeft);
     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.AccessModifierStandardIndent = false;
   LLVMStyle.AlignEscapedNewlinesLeft = false;
   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;
static bool affectsRange(ArrayRef<tooling::Range> Ranges, unsigned Start,
   }
   return false;
 }
 
 // Sorts a block of includes given by 'Includes' alphabetically adding the
 // necessary replacement to 'Replaces'. 'Includes' must be in strict source
 // order.
 static void sortCppIncludes(const FormatStyle &Style,
                             const SmallVectorImpl<IncludeDirective> &Includes,
                             ArrayRef<tooling::Range> Ranges, StringRef FileName,
                             tooling::Replacements &Replaces, unsigned *Cursor) {
   if (!affectsRange(Ranges, Includes.front().Offset,
                     Includes.back().Offset + Includes.back().Text.size()))
     return;
   SmallVector<unsigned, 16> Indices;
   for (unsigned i = 0, e = Includes.size(); i != e; ++i)
     Indices.push_back(i);
   std::stable_sort(
       Indices.begin(), Indices.end(), [&](unsigned LHSI, unsigned RHSI) {

lib/Format/TokenAnnotator.h

enum LineType {
   LT_PreprocessorDirective,
   LT_VirtualFunctionDecl
 };
 
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front().Tok), Level(Line.Level),
-        InPPDirective(Line.InPPDirective),
+        accessLevelMarker(Line.accessLevelMarker), InPPDirective(Line.InPPDirective),
         MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
         IsMultiVariableDeclStmt(false), Affected(false),
         LeadingEmptyLinesAffected(false), ChildrenAffected(false) {
     assert(!Line.Tokens.empty());
 
     // Calculate Next and Previous for all tokens. Note that we must overwrite
     // Next and Previous for every token, as previous formatting runs might have
     // left them in a different state.
public:
 
   FormatToken *First;
   FormatToken *Last;
 
   SmallVector<AnnotatedLine *, 0> Children;
 
   LineType Type;
   unsigned Level;
+  std::vector<char> accessLevelMarker;
   bool InPPDirective;
   bool MustBeDeclaration;
   bool MightBeFunctionDecl;
   bool IsMultiVariableDeclStmt;
 
   /// \c True if this line should be formatted, i.e. intersects directly or
   /// indirectly with one of the input ranges.
   bool Affected;

lib/Format/UnwrappedLineFormatter.cpp

 //===--- UnwrappedLineFormatter.cpp - Format C++ code ---------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "UnwrappedLineFormatter.h"
 #include "WhitespaceManager.h"
 #include "llvm/Support/Debug.h"
+#include <numeric>
 #include <queue>
 
 #define DEBUG_TYPE "format-formatter"
 
 namespace clang {
 namespace format {
 
 namespace {
public:
   /// \brief Update the indent state given that \p Line is going to be formatted
   /// next.
   void nextLine(const AnnotatedLine &Line) {
     Offset = getIndentOffset(*Line.First);
     // Update the indent level cache size so that we can rely on it
     // having the right size in adjustToUnmodifiedline.
     while (IndentForLevel.size() <= Line.Level)
       IndentForLevel.push_back(-1);
+    unsigned currentLevel = Line.Level;
+    if (Style.AccessModifierStandardIndent) {
+      currentLevel += std::accumulate(std::begin(Line.accessLevelMarker),
+                                      std::end(Line.accessLevelMarker), 0);
+    }
     if (Line.InPPDirective) {
-      Indent = Line.Level * Style.IndentWidth + AdditionalIndent;
+      Indent = currentLevel * Style.IndentWidth + AdditionalIndent;
     } else {
-      IndentForLevel.resize(Line.Level + 1);
-      Indent = getIndent(IndentForLevel, Line.Level);
+      std::size_t oldSize = IndentForLevel.size();
+      IndentForLevel.resize(currentLevel + 1);
+      std::fill(std::begin(IndentForLevel) + oldSize, std::end(IndentForLevel),
+                -1);
+      Indent = getIndent(IndentForLevel, currentLevel);
     }
     if (static_cast<int>(Indent) + Offset >= 0)
       Indent += Offset;
   }
 
   /// \brief Update the level indent to adapt to the given \p Line.
   ///
   /// When a line is not formatted, we move the subsequent lines on the same

lib/Format/UnwrappedLineParser.h

struct UnwrappedLine {
   UnwrappedLine();
 
   // FIXME: Don't use std::list here.
   /// \brief The \c Tokens comprising this \c UnwrappedLine.
   std::list<UnwrappedLineNode> Tokens;
 
   /// \brief The indent level of the \c UnwrappedLine.
   unsigned Level;
+  /// \brief If an accesses specifier has been used at a particular level
+  std::vector<char> accessLevelMarker;
 
   /// \brief Whether this \c UnwrappedLine is part of a preprocessor directive.
   bool InPPDirective;
 
   bool MustBeDeclaration;
 };
 
 class UnwrappedLineConsumer {
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();
struct UnwrappedLineNode {
   UnwrappedLineNode() : Tok(nullptr) {}
   UnwrappedLineNode(FormatToken *Tok) : Tok(Tok) {}
 
   FormatToken *Tok;
   SmallVector<UnwrappedLine, 0> Children;
 };
 
 inline UnwrappedLine::UnwrappedLine()
-    : Level(0), InPPDirective(false), MustBeDeclaration(false) {}
+    : Level(0), InPPDirective(false), MustBeDeclaration(false) {
+  accessLevelMarker.push_back(0);
+}
 
 } // end namespace format
 } // end namespace clang
 
 #endif

lib/Format/UnwrappedLineParser.cpp

for (unsigned i = 0, e = LBraceStack.size(); i != e; ++i) {
     if (LBraceStack[i]->BlockKind == BK_Unknown)
       LBraceStack[i]->BlockKind = BK_Block;
   }
 
   FormatTok = Tokens->setPosition(StoredPosition);
 }
 
 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;
 
   unsigned InitialLevel = Line->Level;
+  std::vector<char> InitialAccessUsed = Line->accessLevelMarker;
   nextToken();
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
     parseParens();
 
   addUnwrappedLine();
 
   ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
                                           MustBeDeclaration);
-  if (AddLevel)
+  if (AddLevel) {
+    if (ClassBlock) {
+      Line->accessLevelMarker.push_back(0);
+    }
     ++Line->Level;
+  }
   parseLevel(/*HasOpeningBrace=*/true);
 
   if (eof())
     return;
 
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
                  : !FormatTok->is(tok::r_brace)) {
     Line->Level = InitialLevel;
+    Line->accessLevelMarker = InitialAccessUsed;
     FormatTok->BlockKind = BK_Block;
     return;
   }
 
   nextToken(); // Munch the closing brace.
 
   if (MacroBlock && FormatTok->is(tok::l_paren))
     parseParens();
 
   if (MunchSemi && FormatTok->Tok.is(tok::semi))
     nextToken();
   Line->Level = InitialLevel;
+  Line->accessLevelMarker = InitialAccessUsed;
 }
 
 static bool isGoogScope(const UnwrappedLine &Line) {
   // FIXME: Closure-library specific stuff should not be hard-coded but be
   // configurable.
   if (Line.Tokens.size() < 4)
     return false;
   auto I = Line.Tokens.begin();
void UnwrappedLineParser::parseStructuralElement() {
   } while (!eof());
 }
 
 bool UnwrappedLineParser::tryToParseLambda() {
   if (Style.Language != FormatStyle::LK_Cpp) {
     nextToken();
     return false;
   }
-  const FormatToken* Previous = getPreviousToken();
+  const FormatToken *Previous = getPreviousToken();
   if (Previous &&
       (Previous->isOneOf(tok::identifier, tok::kw_operator, tok::kw_new,
                          tok::kw_delete) ||
        Previous->closesScope() || Previous->isSimpleTypeSpecifier())) {
     nextToken();
     return false;
   }
   assert(FormatTok->is(tok::l_square));

 void UnwrappedLineParser::parseAccessSpecifier() {
   nextToken();
   // Understand Qt's slots.
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
     nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
   if (FormatTok->Tok.is(tok::colon))
     nextToken();
+  Line->accessLevelMarker.back() = 0;
   addUnwrappedLine();
+  addUnwrappedLine();
+  Line->accessLevelMarker.back() = 1;
 }
 
 bool UnwrappedLineParser::parseEnum() {
   // Won't be 'enum' for NS_ENUMs.
   if (FormatTok->Tok.is(tok::kw_enum))
     nextToken();
 
   // In TypeScript, "enum" can also be used as property name, e.g. in interface
while (!eof()) {
       nextToken();
     }
   }
   if (FormatTok->Tok.is(tok::l_brace)) {
     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::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
void UnwrappedLineParser::addUnwrappedLine() {
   if (Line->Tokens.empty())
     return;
   DEBUG({
     if (CurrentLines == &Lines)
       printDebugInfo(*Line);
   });
   CurrentLines->push_back(std::move(*Line));
   Line->Tokens.clear();
+  Line->accessLevelMarker = CurrentLines->back().accessLevelMarker;
   if (CurrentLines == &Lines && !PreprocessorDirectives.empty()) {
     CurrentLines->append(
         std::make_move_iterator(PreprocessorDirectives.begin()),
         std::make_move_iterator(PreprocessorDirectives.end()));
     PreprocessorDirectives.clear();
   }
 }
 

unittests/Format/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_clang_unittest(FormatTests
   CleanupTest.cpp
   FormatTest.cpp
+  FormatTestAccess.cpp
   FormatTestJava.cpp
   FormatTestJS.cpp
   FormatTestProto.cpp
   FormatTestSelective.cpp
   SortImportsTestJS.cpp
   SortIncludesTest.cpp
   )
 
 target_link_libraries(FormatTests
   clangBasic
   clangFormat
   clangFrontend
   clangRewrite
   clangToolingCore
   )

unittests/Format/FormatTestAccess.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 FormatTestAccess : public ::testing::Test {
+protected:
+  std::string format(llvm::StringRef Code, bool normalFormat) {
+    const FormatStyle &Style = getGoogleStyleWithColumns(normalFormat);
+    DEBUG(llvm::errs() << "---\n");
+    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));
+    DEBUG(llvm::errs() << "\n" << *Result << "\n\n");
+    return *Result;
+  }
+
+  FormatStyle getGoogleStyleWithColumns(bool normalFormat) {
+    FormatStyle Style = getLLVMStyle();
+    Style.AccessModifierOffset = normalFormat ? 0 : -2;
+    Style.AccessModifierStandardIndent = normalFormat;
+    return Style;
+  }
+
+  void verifyFormat(llvm::StringRef Code, bool normalFormat) {
+    EXPECT_EQ(Code.str(), format(test::messUp(Code), normalFormat));
+  }
+};
+
+TEST_F(FormatTestAccess, NoChangeWithoutAccess) {
+  verifyFormat("class A {\n"
+               "  int a1;\n"
+               "};",
+               false);
+  verifyFormat("class B {\n"
+               "  int b1;\n"
+               "};",
+               true);
+}
+
+TEST_F(FormatTestAccess, ChangeWithAccess) {
+  verifyFormat("class C {\n"
+               "  int c1;\n"
+               "public:\n"
+               "  int c2;\n"
+               "};",
+               false);
+  verifyFormat("class D {\n"
+               "  int d1;\n"
+               "  public:\n"
+               "    int d2;\n"
+               "};",
+               true);
+}
+
+TEST_F(FormatTestAccess, MultipleAccessLevels) {
+  verifyFormat("class E {\n"
+               "  int e1;\n"
+               "public:\n"
+               "  int e2;\n"
+               "private:\n"
+               "  int e3;\n"
+               "protected:\n"
+               "  int e4;\n"
+               "};",
+               false);
+  verifyFormat("class F {\n"
+               "  int f1;\n"
+               "  public:\n"
+               "    int f2;\n"
+               "  private:\n"
+               "    int f3;\n"
+               "  protected:\n"
+               "    int f4;\n"
+               "};",
+               true);
+}
+
+TEST_F(FormatTestAccess, 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"
+               "};",
+               false);
+  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"
+               "};",
+               true);
+}
+
+} // end namespace
+} // end namespace format
+} // end namespace clang