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 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) 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 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 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 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 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(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(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(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(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 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 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