[clang-format] [PR51640] - New AfterEnum brace wrapping changes have cause C# behaviour to change

LLVM 13.0.0-rc2 shows change of behaviour in enum and interface BraceWrapping (likely before we simply didn't wrap)  but may be related to {D99840}

Logged as https://bugs.llvm.org/show_bug.cgi?id=51640

This change ensure AfterEnum works for

`internal|public|protected|private enum A {`  in the same way as it works for `enum A {` in C++

A similar issue was also observed with `interface` in C#

Reviewed By: krasimir, owenpan

Differential Revision: https://reviews.llvm.org/D108810

(cherry picked from commit ed367b9dff10ee1df9ac1984eb2ad7544da7ab06)

4 files changed, 264 insertions, 8 deletions

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 11dc661abc24..86c9ac4aa364 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -3604,6 +3604,16 @@ static bool isAllmanLambdaBrace(const FormatToken &Tok) {
           !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
 }
 
+// Returns the first token on the line that is not a comment.
+static const FormatToken *getFirstNonComment(const AnnotatedLine &Line) {
+  const FormatToken *Next = Line.First;
+  if (!Next)
+    return Next;
+  if (Next->is(tok::comment))
+    Next = Next->getNextNonComment();
+  return Next;
+}
+
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
                                      const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
@@ -3785,12 +3795,34 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
   if (Right.is(TT_InlineASMBrace))
     return Right.HasUnescapedNewline;
 
-  if (isAllmanBrace(Left) || isAllmanBrace(Right))
-    return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
-           (Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
-            Style.BraceWrapping.AfterEnum) ||
-           (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
+  if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
+    auto FirstNonComment = getFirstNonComment(Line);
+    bool AccessSpecifier =
+        FirstNonComment &&
+        FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,
+                                 tok::kw_private, tok::kw_protected);
+
+    if (Style.BraceWrapping.AfterEnum) {
+      if (Line.startsWith(tok::kw_enum) ||
+          Line.startsWith(tok::kw_typedef, tok::kw_enum))
+        return true;
+      // Ensure BraceWrapping for `public enum A {`.
+      if (AccessSpecifier && FirstNonComment->Next &&
+          FirstNonComment->Next->is(tok::kw_enum))
+        return true;
+    }
+
+    // Ensure BraceWrapping for `public interface A {`.
+    if (Style.BraceWrapping.AfterClass &&
+        ((AccessSpecifier && FirstNonComment->Next &&
+          FirstNonComment->Next->is(Keywords.kw_interface)) ||
+         Line.startsWith(Keywords.kw_interface)))
+      return true;
+
+    return (Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
            (Line.startsWith(tok::kw_struct) && Style.BraceWrapping.AfterStruct);
+  }
+
   if (Left.is(TT_ObjCBlockLBrace) &&
       Style.AllowShortBlocksOnASingleLine == FormatStyle::SBS_Never)
     return true;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 673986d16af2..8487875064aa 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2532,6 +2532,8 @@ bool UnwrappedLineParser::parseEnum() {
   if (FormatTok->Tok.is(tok::kw_enum))
     nextToken();
 
+  const FormatToken &InitialToken = *FormatTok;
+
   // In TypeScript, "enum" can also be used as property name, e.g. in interface
   // declarations. An "enum" keyword followed by a colon would be a syntax
   // error and thus assume it is just an identifier.
@@ -2578,7 +2580,8 @@ bool UnwrappedLineParser::parseEnum() {
     return true;
   }
 
-  if (!Style.AllowShortEnumsOnASingleLine)
+  if (!Style.AllowShortEnumsOnASingleLine &&
+      ShouldBreakBeforeBrace(Style, InitialToken))
     addUnwrappedLine();
   // Parse enum body.
   nextToken();
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c575f643b5a1..61a06294c8f9 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -2451,6 +2451,14 @@ TEST_F(FormatTest, ShortEnums) {
   Style.AllowShortEnumsOnASingleLine = true;
   verifyFormat("enum { A, B, C } ShortEnum1, ShortEnum2;", Style);
   Style.AllowShortEnumsOnASingleLine = false;
+  verifyFormat("enum {\n"
+               "  A,\n"
+               "  B,\n"
+               "  C\n"
+               "} ShortEnum1, ShortEnum2;",
+               Style);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = true;
   verifyFormat("enum\n"
                "{\n"
                "  A,\n"
@@ -22205,8 +22213,7 @@ TEST_F(FormatTest, IndentAccessModifiers) {
                Style);
   // Enumerations are not records and should be unaffected.
   Style.AllowShortEnumsOnASingleLine = false;
-  verifyFormat("enum class E\n"
-               "{\n"
+  verifyFormat("enum class E {\n"
                "  A,\n"
                "  B\n"
                "};\n",
diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp
index 3c990339cf74..b7ea8d3672a2 100644
--- a/clang/unittests/Format/FormatTestCSharp.cpp
+++ b/clang/unittests/Format/FormatTestCSharp.cpp
@@ -402,6 +402,7 @@ TEST_F(FormatTestCSharp, CSharpRegions) {
 }
 
 TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
+  // AfterEnum is true by default.
   verifyFormat("public enum var\n"
                "{\n"
                "    none,\n"
@@ -1100,5 +1101,218 @@ class A {
                getGoogleStyle(FormatStyle::LK_Cpp));
 }
 
+TEST_F(FormatTestCSharp, CSharpAfterEnum) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterEnum = false;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum {\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("internal enum MyEnum {\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("public enum MyEnum {\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("protected enum MyEnum {\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("private enum MyEnum {\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+
+  Style.BraceWrapping.AfterEnum = true;
+  Style.AllowShortEnumsOnASingleLine = false;
+
+  verifyFormat("enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("internal enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("public enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("protected enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("private enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("/* Foo */ private enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+  verifyFormat("/* Foo */ /* Bar */ private enum MyEnum\n"
+               "{\n"
+               "  Foo,\n"
+               "  Bar,\n"
+               "}",
+               Style);
+}
+
+TEST_F(FormatTestCSharp, CSharpAfterClass) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterClass = false;
+
+  verifyFormat("class MyClass {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("internal class MyClass {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("public class MyClass {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("protected class MyClass {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("private class MyClass {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+
+  verifyFormat("interface Interface {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("internal interface Interface {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("public interface Interface {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("protected interface Interface {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("private interface Interface {\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+
+  Style.BraceWrapping.AfterClass = true;
+
+  verifyFormat("class MyClass\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("internal class MyClass\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("public class MyClass\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("protected class MyClass\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("private class MyClass\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+
+  verifyFormat("interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("internal interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("public interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("protected interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("private interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("/* Foo */ private interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+  verifyFormat("/* Foo */ /* Bar */ private interface MyInterface\n"
+               "{\n"
+               "  int a;\n"
+               "  int b;\n"
+               "}",
+               Style);
+}
+
 } // namespace format
 } // end namespace clang