Skip to content

Commit

Permalink
[clang-format] [PR51640] - New AfterEnum brace wrapping changes have …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@mydeveloperday
mydeveloperday committed Aug 27, 2021
1 parent 2930c83 commit ed367b9
Show file tree
Hide file tree
Showing 2 changed files with 253 additions and 6 deletions.
42 changes: 37 additions & 5 deletions clang/lib/Format/TokenAnnotator.cpp
View file
Expand Up @@ -3597,6 +3597,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;
Expand Down Expand Up @@ -3783,12 +3793,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;
Expand Down
217 changes: 216 additions & 1 deletion clang/unittests/Format/FormatTestCSharp.cpp
View file
Expand Up @@ -402,7 +402,9 @@ TEST_F(FormatTestCSharp, CSharpRegions) {
}

TEST_F(FormatTestCSharp, CSharpKeyWordEscaping) {
verifyFormat("public enum var {\n"
// AfterEnum is true by default.
verifyFormat("public enum var\n"
"{\n"
" none,\n"
" @string,\n"
" bool,\n"
Expand Down Expand Up @@ -1099,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

0 comments on commit ed367b9

Please sign in to comment.