From ed367b9dff10ee1df9ac1984eb2ad7544da7ab06 Mon Sep 17 00:00:00 2001 From: mydeveloperday Date: Fri, 27 Aug 2021 19:10:45 +0100 Subject: [PATCH] [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 --- clang/lib/Format/TokenAnnotator.cpp | 42 +++++- clang/unittests/Format/FormatTestCSharp.cpp | 217 +++++++++++++++++++++++++++- 2 files changed, 253 insertions(+), 6 deletions(-) diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 8506eb7..044a96e 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -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; @@ -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; diff --git a/clang/unittests/Format/FormatTestCSharp.cpp b/clang/unittests/Format/FormatTestCSharp.cpp index 8cd29e5..b7ea8d3 100644 --- a/clang/unittests/Format/FormatTestCSharp.cpp +++ b/clang/unittests/Format/FormatTestCSharp.cpp @@ -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" @@ -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 -- 2.7.4