From 87f8a624e795c3810f76316b7afb62986976970b Mon Sep 17 00:00:00 2001 From: Marek Kurdej Date: Wed, 27 Sep 2017 07:51:51 +0000 Subject: [PATCH] [clang-format] Fix FixNamespaceComments when BraceWrapping AfterNamespace is true. Summary: NamespaceEndCommentsFixer did not fix namespace comments when the brace opening the namespace was not on the same line as the "namespace" keyword. It occurs in Allman, GNU and Linux styles and whenever BraceWrapping.AfterNamespace is true. Before: ```lang=cpp namespace a { void f(); void g(); } ``` After: ```lang=cpp namespace a { void f(); void g(); } // namespace a ``` Reviewers: krasimir Reviewed By: krasimir Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D37904 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314279 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/NamespaceEndCommentsFixer.cpp | 6 ++++++ unittests/Format/FormatTest.cpp | 23 ++++++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/Format/NamespaceEndCommentsFixer.cpp b/lib/Format/NamespaceEndCommentsFixer.cpp index 85b70b8c0a..c660843dca 100644 --- a/lib/Format/NamespaceEndCommentsFixer.cpp +++ b/lib/Format/NamespaceEndCommentsFixer.cpp @@ -118,6 +118,12 @@ getNamespaceToken(const AnnotatedLine *line, return nullptr; assert(StartLineIndex < AnnotatedLines.size()); const FormatToken *NamespaceTok = AnnotatedLines[StartLineIndex]->First; + if (NamespaceTok->is(tok::l_brace)) { + // "namespace" keyword can be on the line preceding '{', e.g. in styles + // where BraceWrapping.AfterNamespace is true. + if (StartLineIndex > 0) + NamespaceTok = AnnotatedLines[StartLineIndex - 1]->First; + } // Detect "(inline)? namespace" in the beginning of a line. if (NamespaceTok->is(tok::kw_inline)) NamespaceTok = NamespaceTok->getNextNonComment(); diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 2b3d571da7..8ef11cb3dd 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -1413,7 +1413,7 @@ TEST_F(FormatTest, FormatsTypedefEnum) { verifyFormat("typedef enum {} EmptyEnum;"); verifyFormat("typedef enum { A, B, C } ShortEnum;"); verifyFormat("typedef enum {\n" - " ZERO = 0,\n" + " ZERO = 0,\n" " ONE = 1,\n" " TWO = 2,\n" " THREE = 3\n" @@ -1425,7 +1425,7 @@ TEST_F(FormatTest, FormatsTypedefEnum) { verifyFormat("typedef enum { A, B, C } ShortEnum;"); verifyFormat("typedef enum\n" "{\n" - " ZERO = 0,\n" + " ZERO = 0,\n" " ONE = 1,\n" " TWO = 2,\n" " THREE = 3\n" @@ -9323,7 +9323,7 @@ TEST_F(FormatTest, LinuxBraceBreaking) { "struct B {\n" " int x;\n" "};\n" - "}\n", + "} // namespace a\n", LinuxBraceStyle); verifyFormat("enum X {\n" " Y = 0,\n" @@ -9453,6 +9453,19 @@ TEST_F(FormatTest, StroustrupBraceBreaking) { TEST_F(FormatTest, AllmanBraceBreaking) { FormatStyle AllmanBraceStyle = getLLVMStyle(); AllmanBraceStyle.BreakBeforeBraces = FormatStyle::BS_Allman; + + EXPECT_EQ("namespace a\n" + "{\n" + "void f();\n" + "void g();\n" + "} // namespace a\n", + format("namespace a\n" + "{\n" + "void f();\n" + "void g();\n" + "}\n", + AllmanBraceStyle)); + verifyFormat("namespace a\n" "{\n" "class A\n" @@ -9471,7 +9484,7 @@ TEST_F(FormatTest, AllmanBraceBreaking) { "{\n" " int x;\n" "};\n" - "}", + "} // namespace a", AllmanBraceStyle); verifyFormat("void f()\n" @@ -9677,7 +9690,7 @@ TEST_F(FormatTest, GNUBraceBreaking) { " }\n" " void g() { return; }\n" "}\n" - "}", + "} // namespace a", GNUBraceStyle); verifyFormat("void f()\n" -- 2.40.0