From: Krasimir Georgiev <krasimir@google.com>
Date: Thu, 2 Mar 2017 09:54:44 +0000 (+0000)
Subject: [clang-format] Use number of unwrapped lines for short namespace
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=77b000e5f923dc575e614259c3506ed5bc855297;p=clang

[clang-format] Use number of unwrapped lines for short namespace

Summary:
This patch makes the namespace comment fixer use the number of unwrapped lines
that a namespace spans to detect it that namespace is short, thus not needing
end comments to be added.
This is needed to ensure clang-format is idempotent. Previously, a short namespace
was detected by the original source code lines. This has the effect of requiring two
runs for this example:
```
namespace { class A; }
```
after first run:
```
namespace {
class A;
}
```
after second run:
```
namespace {
class A;
} // namespace
```

Reviewers: djasper

Reviewed By: djasper

Subscribers: cfe-commits, klimek

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@296736 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Format/NamespaceEndCommentsFixer.cpp b/lib/Format/NamespaceEndCommentsFixer.cpp
index 495c34db75..578dcf7b67 100644
--- a/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -23,7 +23,7 @@ namespace clang {
 namespace format {
 
 namespace {
-// The maximal number of lines that a short namespace spans.
+// The maximal number of unwrapped lines that a short namespace spans.
 // Short namespaces don't need an end comment.
 static const int kShortNamespaceMaxLines = 1;
 
@@ -60,14 +60,6 @@ std::string computeEndCommentText(StringRef NamespaceName, bool AddNewline) {
   return text;
 }
 
-bool isShort(const FormatToken *NamespaceTok, const FormatToken *RBraceTok,
-             const SourceManager &SourceMgr) {
-  int StartLine =
-      SourceMgr.getSpellingLineNumber(NamespaceTok->Tok.getLocation());
-  int EndLine = SourceMgr.getSpellingLineNumber(RBraceTok->Tok.getLocation());
-  return EndLine - StartLine + 1 <= kShortNamespaceMaxLines;
-}
-
 bool hasEndComment(const FormatToken *RBraceTok) {
   return RBraceTok->Next && RBraceTok->Next->is(tok::comment);
 }
@@ -151,7 +143,8 @@ tooling::Replacements NamespaceEndCommentsFixer::analyze(
     const std::string EndCommentText =
         computeEndCommentText(NamespaceName, AddNewline);
     if (!hasEndComment(RBraceTok)) {
-      if (!isShort(NamespaceTok, RBraceTok, SourceMgr))
+      bool isShort = I - StartLineIndex <= kShortNamespaceMaxLines + 1;
+      if (!isShort)
         addEndComment(RBraceTok, EndCommentText, SourceMgr, &Fixes);
       continue;
     }
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 30cc749624..7ab0c752d1 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -185,7 +185,7 @@ TEST_F(FormatTest, RemovesEmptyLines) {
   EXPECT_EQ("namespace N {\n"
             "\n"
             "int i;\n"
-            "}  // namespace N",
+            "}",
             format("namespace N {\n"
                    "\n"
                    "int    i;\n"
@@ -281,8 +281,7 @@ TEST_F(FormatTest, RemovesEmptyLines) {
                    "}", LLVMWithNoNamespaceFix));
   EXPECT_EQ("namespace {\n"
             "int i;\n"
-            "\n"
-            "} // namespace",
+            "}",
             format("namespace {\n"
                    "int i;\n"
                    "\n"
@@ -5460,7 +5459,7 @@ TEST_F(FormatTest, IncorrectCodeMissingSemicolon) {
   EXPECT_EQ("namespace N {\n"
             "void f() {}\n"
             "void g()\n"
-            "}",
+            "} // namespace N",
             format("namespace N  { void f( ) { } void g( ) }"));
 }
 
@@ -6140,8 +6139,8 @@ TEST_F(FormatTest, FormatStarDependingOnContext) {
                "  void f() {}\n"
                "  int *a;\n"
                "};\n"
-               "}\n"
-               "}");
+               "} // namespace b\n"
+               "} // namespace a");
 }
 
 TEST_F(FormatTest, SpecialTokensAtEndOfLine) {
@@ -7934,7 +7933,7 @@ TEST_F(FormatTest, StroustrupBraceBreaking) {
                "struct B {\n"
                "  int x;\n"
                "};\n"
-               "}\n",
+               "} // namespace a\n",
                StroustrupBraceStyle);
 
   verifyFormat("void foo()\n"
diff --git a/unittests/Format/NamespaceEndCommentsFixerTest.cpp b/unittests/Format/NamespaceEndCommentsFixerTest.cpp
index c24894e7d4..28212f8524 100644
--- a/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ b/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -47,87 +47,123 @@ protected:
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
   EXPECT_EQ("namespace {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace",
             fixNamespaceEndComments("namespace {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("namespace {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace\n",
             fixNamespaceEndComments("namespace {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}\n"));
   EXPECT_EQ("namespace A {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace A",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("inline namespace A {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace A",
             fixNamespaceEndComments("inline namespace A {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("namespace ::A {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace ::A",
             fixNamespaceEndComments("namespace ::A {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("namespace ::A::B {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace ::A::B",
             fixNamespaceEndComments("namespace ::A::B {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("namespace /**/::/**/A/**/::/**/B/**/ {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace ::A::B",
             fixNamespaceEndComments("namespace /**/::/**/A/**/::/**/B/**/ {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}"));
   EXPECT_EQ("namespace A {\n"
             "namespace B {\n"
             "  int i;\n"
+            "}\n"
+            "}// namespace A",
+            fixNamespaceEndComments("namespace A {\n"
+                                    "namespace B {\n"
+                                    "  int i;\n"
+                                    "}\n"
+                                    "}"));
+  EXPECT_EQ("namespace A {\n"
+            "namespace B {\n"
+            "  int i;\n"
+            "  int j;\n"
             "}// namespace B\n"
             "}// namespace A",
             fixNamespaceEndComments("namespace A {\n"
                                     "namespace B {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "}\n"
                                     "}"));
   EXPECT_EQ("namespace A {\n"
             "  int a;\n"
+            "  int b;\n"
             "}// namespace A\n"
             "namespace B {\n"
             "  int b;\n"
+            "  int a;\n"
             "}// namespace B",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int a;\n"
+                                    "  int b;\n"
                                     "}\n"
                                     "namespace B {\n"
                                     "  int b;\n"
+                                    "  int a;\n"
                                     "}"));
   EXPECT_EQ("namespace A {\n"
             "  int a1;\n"
+            "  int a2;\n"
             "}// namespace A\n"
             "namespace A {\n"
             "  int a2;\n"
+            "  int a1;\n"
             "}// namespace A",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int a1;\n"
+                                    "  int a2;\n"
                                     "}\n"
                                     "namespace A {\n"
                                     "  int a2;\n"
+                                    "  int a1;\n"
                                     "}"));
   EXPECT_EQ("namespace A {\n"
             "  int a;\n"
+            "  int b;\n"
             "}// namespace A\n"
             "// comment about b\n"
             "int b;",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int a;\n"
+                                    "  int b;\n"
                                     "}\n"
                                     "// comment about b\n"
                                     "int b;"));
@@ -136,7 +172,7 @@ TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
             "namespace B {\n"
             "namespace C {\n"
             "namespace D {\n"
-            "}// namespace D\n"
+            "}\n"
             "}// namespace C\n"
             "}// namespace B\n"
             "}// namespace A",
@@ -153,36 +189,44 @@ TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
 TEST_F(NamespaceEndCommentsFixerTest, AddsNewlineIfNeeded) {
   EXPECT_EQ("namespace A {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace A\n"
-            " int j;",
+            " int k;",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int i;\n"
-                                    "} int j;"));
+                                    "  int j;\n"
+                                    "} int k;"));
   EXPECT_EQ("namespace {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace\n"
-            " int j;",
+            " int k;",
             fixNamespaceEndComments("namespace {\n"
                                     "  int i;\n"
-                                    "} int j;"));
+                                    "  int j;\n"
+                                    "} int k;"));
   EXPECT_EQ("namespace A {\n"
             "  int i;\n"
+            "  int j;\n"
             "}// namespace A\n"
             " namespace B {\n"
             "  int j;\n"
+            "  int k;\n"
             "}// namespace B",
             fixNamespaceEndComments("namespace A {\n"
                                     "  int i;\n"
+                                    "  int j;\n"
                                     "} namespace B {\n"
                                     "  int j;\n"
+                                    "  int k;\n"
                                     "}"));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddEndCommentForShortNamespace) {
   EXPECT_EQ("namespace {}", fixNamespaceEndComments("namespace {}"));
   EXPECT_EQ("namespace A {}", fixNamespaceEndComments("namespace A {}"));
-  EXPECT_EQ("namespace A { int i; }",
-            fixNamespaceEndComments("namespace A { int i; }"));
+  EXPECT_EQ("namespace A { a }",
+            fixNamespaceEndComments("namespace A { a }"));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, DoesNotAddCommentAfterUnaffectedRBrace) {