ioeric created this revision.
ioeric added reviewers: djasper, klimek.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Make clang-format cleaner remove redundant commas/colons in constructor 
initializer list.

http://reviews.llvm.org/D19804

Files:
  lib/Format/Format.cpp
  unittests/Format/CleanupTest.cpp

Index: unittests/Format/CleanupTest.cpp
===================================================================
--- unittests/Format/CleanupTest.cpp
+++ unittests/Format/CleanupTest.cpp
@@ -113,6 +113,116 @@
   EXPECT_EQ(Expected, Result);
 }
 
+TEST_F(CleanupTest, CtorInitializationSimpleRedundantComma) {
+  std::string Code = "class A {\nA() : , {} };";
+  std::string Expected = "class A {\nA()  {} };";
+  std::vector<tooling::Range> Ranges;
+  Ranges.push_back(tooling::Range(17, 0));
+  Ranges.push_back(tooling::Range(19, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  Code = "class A {\nA() : x(1), {} };";
+  Expected = "class A {\nA() : x(1) {} };";
+  Ranges.clear();
+  Ranges.push_back(tooling::Range(23, 0));
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, CtorInitializationBracesInParens) {
+  std::string Code = "class A {\nA() : x({1}),, {} };";
+  std::string Expected = "class A {\nA() : x({1}) {} };";
+  std::vector<tooling::Range> Ranges;
+  Ranges.push_back(tooling::Range(24, 0));
+  Ranges.push_back(tooling::Range(26, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, RedundantCommaNotInAffectedRanges) {
+  std::string Code =
+      "class A {\nA() : x({1}), /* comment */, { int x = 0; } };";
+  std::string Expected =
+      "class A {\nA() : x({1}), /* comment */, { int x = 0; } };";
+  // Set the affected range to be "int x = 0", which does not intercept the
+  // constructor initialization list.
+  std::vector<tooling::Range> Ranges(1, tooling::Range(42, 9));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  Code = "class A {\nA() : x(1), {} };";
+  Expected = "class A {\nA() : x(1), {} };";
+  // No range. Fixer should do nothing.
+  Ranges.clear();
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, CtorInitializationCommentAroundCommas) {
+  // Remove redundant commas and comment between them.
+  std::string Code = "class A {\nA() : x({1}), /* comment */, {} };";
+  std::string Expected = "class A {\nA() : x({1}) {} };";
+  std::vector<tooling::Range> Ranges;
+  Ranges.push_back(tooling::Range(25, 0));
+  Ranges.push_back(tooling::Range(40, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  // Remove trailing comma and comment.
+  Code = "class A {\nA() : x({1}), // comment\n{} };";
+  Expected = "class A {\nA() : x({1})\n{} };";
+  Ranges = std::vector<tooling::Range>(1, tooling::Range(25, 0));
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  // Remove trailing comma, but leave the comment.
+  Code = "class A {\nA() : x({1}), // comment\n , y(1),{} };";
+  Expected = "class A {\nA() : x({1}), // comment\n  y(1){} };";
+  Ranges = std::vector<tooling::Range>(1, tooling::Range(38, 0));
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  // Remove trailing comma and the comment before it.
+  Code = "class A {\nA() : x({1}), \n/* comment */, y(1),{} };";
+  Expected = "class A {\nA() : x({1}), \n y(1){} };";
+  Ranges = std::vector<tooling::Range>(1, tooling::Range(40, 0));
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+
+  // Remove trailing comma and the comment after it.
+  Code = "class A {\nA() : , // comment\n y(1),{} };";
+  Expected = "class A {\nA() : \n y(1){} };";
+  Ranges = std::vector<tooling::Range>(1, tooling::Range(17, 0));
+  Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, SkipImbalancedParentheses) {
+  std::string Code = "class A {\nA() : x((),, {} };";
+  std::string Expected = "class A {\nA() : x((),, {} };";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, CtorInitializerInNamespace) {
+  std::string Code = "namespace A {\n"
+                     "namespace B {\n" // missing r_brace
+                     "} // namespace A\n\n"
+                     "namespace C {\n"
+                     "class A { A() : x(0),, {} };\n"
+                     "inline namespace E { namespace { } }\n"
+                     "}";
+  std::string Expected = "namespace A {\n"
+                         "\n\n\nnamespace C {\n"
+                         "class A { A() : x(0) {} };\n   \n"
+                         "}";
+  std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1816,6 +1816,11 @@
 
     checkEmptyNamespace(AnnotatedLines);
 
+    for (auto &Line : AnnotatedLines) {
+      if (Line->Affected)
+        checkConstructorInitList(*Line);
+    }
+
     return generateFixes();
   }
 
@@ -1904,6 +1909,84 @@
     return true;
   }
 
+  // Looks for and removes redundant colon/commas in the constructor initializer
+  // list.
+  void checkConstructorInitList(AnnotatedLine &Line) {
+    FormatToken *Tok = Line.First;
+    while (Tok && Tok->Type != TT_CtorInitializerColon) {
+      Tok = Tok->Next;
+    }
+    if (!Tok)
+      return;
+
+    assert(Tok->is(tok::colon) && Tok->Type == TT_CtorInitializerColon);
+    FormatToken *CtorColonTok = Tok;
+    FormatToken *LastTokenNotDeleted = Tok;
+    Tok = Tok->Next;
+    // This vector stores comments between the last token not deleted and the
+    // current token.
+    SmallVector<FormatToken *, 1> Comments;
+    // True if the initializer list is empty, i.e. the intializer colon is
+    // redundant.
+    bool IsListEmpty = true;
+    while (Tok) {
+      switch (Tok->Tok.getKind()) {
+      case tok::comma:
+        if (LastTokenNotDeleted->isOneOf(tok::comma, tok::colon)) {
+          deleteToken(Tok);
+          // If there is a new line before the deleted comma, the comment may
+          // belong to the previous token.
+          if (!Tok->HasUnescapedNewline)
+            for (auto *Comment : Comments)
+              deleteToken(Comment);
+        }
+        break;
+      case tok::l_paren:
+        // We need to skip a pair of parentheses here because it is possible
+        // that "(..., { ... })" appears in the initialization list, and we do
+        // not want to delete the comma before '{' inside the parentheses.
+        if (!Tok->MatchingParen)
+          return; // FIXME: error handling.
+        Tok = Tok->MatchingParen;
+        LastTokenNotDeleted = Tok->Previous;
+        break;
+      case tok::l_brace:
+        // Reach the end of the initializer list; delete the comma at the end of
+        // the list.
+        if (LastTokenNotDeleted->is(tok::comma)) {
+          deleteToken(LastTokenNotDeleted);
+          for (auto *Comment : Comments)
+            deleteToken(Comment);
+        }
+        break;
+      case tok::comment:
+        // If the last deleted token is followed by a comment "//...", then we
+        // delete the comment as well.
+        if (Tok->Previous &&
+            DeletedTokens.find(Tok->Previous) != DeletedTokens.end() &&
+            Tok->TokenText.startswith("//"))
+          deleteToken(Tok);
+        else
+          Comments.push_back(Tok);
+        break;
+      default:
+        IsListEmpty = false;
+        break;
+      }
+      if (Tok->isNot(tok::comment)) {
+        Comments.clear();
+      }
+      if (DeletedTokens.find(Tok) == DeletedTokens.end() &&
+          Tok->isNot(tok::comment))
+        LastTokenNotDeleted = Tok;
+
+      Tok = Tok->Next;
+    }
+
+    if (IsListEmpty)
+      deleteToken(CtorColonTok);
+  }
+
   // Delete the given token.
   inline void deleteToken(FormatToken *Tok) {
     if (Tok)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to